Skip to content

Commit 0e0fb73

Browse files
authored
DefaultUserInfo empty password hash (#64018)
Use an empty char array in place of the hash of the empty string for defaultEnabledReservedUser, defaultDisabledReservedUser and bootstrapUserInfo ReservedUserInfo objects, when the password is empty. In these cases, the hasEmptyPassword property is set to true and so the actual password hash is not ever checked or verified in ReservedRealm#doAuthenticate so this change has no functional effect on existing logic. If anything, it is more consistent with the logic in NativeUsersStore#setReservedUserEnabled when the user document didn't already exist. Even if there is / will be code that attempts to verify the hash, this would still work as intended as Hasher#resolve would resolve the NOOP Hasher from the empty string and its verify method would compare the char arrays. The reason for this change is that when in FIPS 140 approved only mode, the security provider's implementation of PBKDF2 might forbid operations when the password is smaller than 112 bits so the existing implementation would cause elasticsearch to fail.
1 parent b1d55e2 commit 0e0fb73

File tree

4 files changed

+44
-37
lines changed

4 files changed

+44
-37
lines changed

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/esnative/NativeUsersStore.java

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@
3636
import org.elasticsearch.index.query.QueryBuilder;
3737
import org.elasticsearch.index.query.QueryBuilders;
3838
import org.elasticsearch.search.SearchHit;
39-
import org.elasticsearch.xpack.core.XPackSettings;
4039
import org.elasticsearch.xpack.core.security.ScrollHelper;
4140
import org.elasticsearch.xpack.core.security.action.realm.ClearRealmCacheAction;
4241
import org.elasticsearch.xpack.core.security.action.realm.ClearRealmCacheRequest;
@@ -82,19 +81,13 @@ public class NativeUsersStore {
8281

8382
private final Settings settings;
8483
private final Client client;
85-
private final ReservedUserInfo disabledDefaultUserInfo;
86-
private final ReservedUserInfo enabledDefaultUserInfo;
8784

8885
private final SecurityIndexManager securityIndex;
8986

9087
public NativeUsersStore(Settings settings, Client client, SecurityIndexManager securityIndex) {
9188
this.settings = settings;
9289
this.client = client;
9390
this.securityIndex = securityIndex;
94-
final char[] emptyPasswordHash = Hasher.resolve(XPackSettings.PASSWORD_HASHING_ALGORITHM.get(settings)).
95-
hash(new SecureString("".toCharArray()));
96-
this.disabledDefaultUserInfo = new ReservedUserInfo(emptyPasswordHash, false, true);
97-
this.enabledDefaultUserInfo = new ReservedUserInfo(emptyPasswordHash, true, true);
9891
}
9992

10093
/**
@@ -539,9 +532,10 @@ public void onResponse(GetResponse getResponse) {
539532
} else if (enabled == null) {
540533
listener.onFailure(new IllegalStateException("enabled must not be null!"));
541534
} else if (password.isEmpty()) {
542-
listener.onResponse((enabled ? enabledDefaultUserInfo : disabledDefaultUserInfo).deepClone());
535+
listener.onResponse(enabled ? ReservedUserInfo.defaultEnabledUserInfo()
536+
: ReservedUserInfo.defaultDisabledUserInfo());
543537
} else {
544-
listener.onResponse(new ReservedUserInfo(password.toCharArray(), enabled, false));
538+
listener.onResponse(new ReservedUserInfo(password.toCharArray(), enabled));
545539
}
546540
} else {
547541
listener.onResponse(null);
@@ -595,7 +589,7 @@ public void onResponse(SearchResponse searchResponse) {
595589
listener.onFailure(new IllegalStateException("enabled must not be null!"));
596590
return;
597591
} else {
598-
userInfos.put(username, new ReservedUserInfo(password.toCharArray(), enabled, false));
592+
userInfos.put(username, new ReservedUserInfo(password.toCharArray(), enabled));
599593
}
600594
}
601595
listener.onResponse(userInfos);
@@ -680,22 +674,32 @@ static final class ReservedUserInfo {
680674

681675
public final char[] passwordHash;
682676
public final boolean enabled;
683-
public final boolean hasEmptyPassword;
684677
private final Hasher hasher;
685678

686-
ReservedUserInfo(char[] passwordHash, boolean enabled, boolean hasEmptyPassword) {
679+
ReservedUserInfo(char[] passwordHash, boolean enabled) {
687680
this.passwordHash = passwordHash;
688681
this.enabled = enabled;
689-
this.hasEmptyPassword = hasEmptyPassword;
690682
this.hasher = Hasher.resolveFromHash(this.passwordHash);
691683
}
692684

693685
ReservedUserInfo deepClone() {
694-
return new ReservedUserInfo(Arrays.copyOf(passwordHash, passwordHash.length), enabled, hasEmptyPassword);
686+
return new ReservedUserInfo(Arrays.copyOf(passwordHash, passwordHash.length), enabled);
687+
}
688+
689+
boolean hasEmptyPassword() {
690+
return passwordHash.length == 0;
695691
}
696692

697693
boolean verifyPassword(SecureString data) {
698694
return hasher.verify(data, this.passwordHash);
699695
}
696+
697+
static ReservedUserInfo defaultEnabledUserInfo() {
698+
return new ReservedUserInfo(new char[0], true);
699+
}
700+
701+
static ReservedUserInfo defaultDisabledUserInfo() {
702+
return new ReservedUserInfo(new char[0], false);
703+
}
700704
}
701705
}

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/esnative/ReservedRealm.java

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,6 @@ public class ReservedRealm extends CachingUsernamePasswordRealm {
6262
private final boolean realmEnabled;
6363
private final boolean anonymousEnabled;
6464
private final SecurityIndexManager securityIndex;
65-
private final Hasher reservedRealmHasher;
66-
private final ReservedUserInfo disabledDefaultUserInfo;
67-
private final ReservedUserInfo enabledDefaultUserInfo;
6865

6966
private final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(logger.getName());
7067

@@ -80,13 +77,10 @@ public ReservedRealm(Environment env, Settings settings, NativeUsersStore native
8077
this.anonymousUser = anonymousUser;
8178
this.anonymousEnabled = AnonymousUser.isAnonymousEnabled(settings);
8279
this.securityIndex = securityIndex;
83-
this.reservedRealmHasher = Hasher.resolve(XPackSettings.PASSWORD_HASHING_ALGORITHM.get(settings));
84-
final char[] emptyPasswordHash = reservedRealmHasher.hash(new SecureString("".toCharArray()));
85-
disabledDefaultUserInfo = new ReservedUserInfo(emptyPasswordHash, false, true);
86-
enabledDefaultUserInfo = new ReservedUserInfo(emptyPasswordHash, true, true);
87-
final char[] hash = BOOTSTRAP_ELASTIC_PASSWORD.get(settings).length() == 0 ? emptyPasswordHash :
80+
final Hasher reservedRealmHasher = Hasher.resolve(XPackSettings.PASSWORD_HASHING_ALGORITHM.get(settings));
81+
final char[] hash = BOOTSTRAP_ELASTIC_PASSWORD.get(settings).length() == 0 ? new char[0] :
8882
reservedRealmHasher.hash(BOOTSTRAP_ELASTIC_PASSWORD.get(settings));
89-
bootstrapUserInfo = new ReservedUserInfo(hash, true, hash == emptyPasswordHash);
83+
bootstrapUserInfo = new ReservedUserInfo(hash, true);
9084
}
9185

9286
@Override
@@ -100,7 +94,7 @@ protected void doAuthenticate(UsernamePasswordToken token, ActionListener<Authen
10094
AuthenticationResult result;
10195
if (userInfo != null) {
10296
try {
103-
if (userInfo.hasEmptyPassword) {
97+
if (userInfo.hasEmptyPassword()) {
10498
result = AuthenticationResult.terminate("failed to authenticate user [" + token.principal() + "]", null);
10599
} else if (userInfo.verifyPassword(token.credentials())) {
106100
final User user = getUser(token.principal(), userInfo);
@@ -110,8 +104,6 @@ protected void doAuthenticate(UsernamePasswordToken token, ActionListener<Authen
110104
result = AuthenticationResult.terminate("failed to authenticate user [" + token.principal() + "]", null);
111105
}
112106
} finally {
113-
assert userInfo.passwordHash != disabledDefaultUserInfo.passwordHash : "default user info must be cloned";
114-
assert userInfo.passwordHash != enabledDefaultUserInfo.passwordHash : "default user info must be cloned";
115107
assert userInfo.passwordHash != bootstrapUserInfo.passwordHash : "bootstrap user info must be cloned";
116108
Arrays.fill(userInfo.passwordHash, (char) 0);
117109
}
@@ -246,7 +238,7 @@ private ReservedUserInfo getDefaultUserInfo(String username) {
246238
if (ElasticUser.NAME.equals(username)) {
247239
return bootstrapUserInfo.deepClone();
248240
} else {
249-
return enabledDefaultUserInfo.deepClone();
241+
return ReservedUserInfo.defaultEnabledUserInfo();
250242
}
251243
}
252244

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/NativeUsersStoreTests.java

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ public void testBlankPasswordInIndexImpliesDefaultPassword() throws Exception {
126126
actionRespond(GetRequest.class, new GetResponse(result));
127127

128128
final NativeUsersStore.ReservedUserInfo userInfo = future.get();
129-
assertThat(userInfo.hasEmptyPassword, equalTo(true));
129+
assertThat(userInfo.hasEmptyPassword(), equalTo(true));
130130
assertThat(userInfo.enabled, equalTo(true));
131131
assertTrue(Hasher.verifyHash(new SecureString("".toCharArray()), userInfo.passwordHash));
132132
}
@@ -187,7 +187,7 @@ public void testVerifyNonExistentUser() throws Exception {
187187
false,
188188
null,
189189
Collections.emptyMap(),
190-
Collections.emptyMap());
190+
Collections.emptyMap());
191191

192192
actionRespond(GetRequest.class, new GetResponse(getResult));
193193

@@ -198,6 +198,17 @@ public void testVerifyNonExistentUser() throws Exception {
198198
assertThat(result.getMessage(), nullValue());
199199
}
200200

201+
public void testDefaultReservedUserInfoPasswordEmpty() {
202+
NativeUsersStore.ReservedUserInfo disabledUserInfo = NativeUsersStore.ReservedUserInfo.defaultDisabledUserInfo();
203+
NativeUsersStore.ReservedUserInfo enabledUserInfo = NativeUsersStore.ReservedUserInfo.defaultEnabledUserInfo();
204+
NativeUsersStore.ReservedUserInfo constructedUserInfo =
205+
new NativeUsersStore.ReservedUserInfo(Hasher.PBKDF2.hash(new SecureString(randomAlphaOfLength(14))), randomBoolean());
206+
207+
assertThat(disabledUserInfo.hasEmptyPassword(), equalTo(true));
208+
assertThat(enabledUserInfo.hasEmptyPassword(), equalTo(true));
209+
assertThat(constructedUserInfo.hasEmptyPassword(), equalTo(false));
210+
}
211+
201212
private <ARequest extends ActionRequest, AResponse extends ActionResponse> ARequest actionRespond(Class<ARequest> requestClass,
202213
AResponse response) {
203214
Tuple<ARequest, ActionListener<?>> tuple = findRequest(requestClass);
@@ -206,11 +217,11 @@ private <ARequest extends ActionRequest, AResponse extends ActionResponse> ARequ
206217
}
207218

208219
private <ARequest extends ActionRequest> Tuple<ARequest, ActionListener<?>> findRequest(
209-
Class<ARequest> requestClass) {
220+
Class<ARequest> requestClass) {
210221
return this.requests.stream()
211-
.filter(t -> requestClass.isInstance(t.v1()))
212-
.map(t -> new Tuple<ARequest, ActionListener<?>>(requestClass.cast(t.v1()), t.v2()))
213-
.findFirst().orElseThrow(() -> new RuntimeException("Cannot find request of type " + requestClass));
222+
.filter(t -> requestClass.isInstance(t.v1()))
223+
.map(t -> new Tuple<ARequest, ActionListener<?>>(requestClass.cast(t.v1()), t.v2()))
224+
.findFirst().orElseThrow(() -> new RuntimeException("Cannot find request of type " + requestClass));
214225
}
215226

216227
private void respondToGetUserRequest(String username, SecureString password, String[] roles) throws IOException {

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/ReservedRealmTests.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ private void verifySuccessfulAuthentication(boolean enabled) throws Exception {
139139
when(securityIndex.indexExists()).thenReturn(true);
140140
doAnswer((i) -> {
141141
ActionListener callback = (ActionListener) i.getArguments()[1];
142-
callback.onResponse(new ReservedUserInfo(hasher.hash(newPassword), enabled, false));
142+
callback.onResponse(new ReservedUserInfo(hasher.hash(newPassword), enabled));
143143
return null;
144144
}).when(usersStore).getReservedUserInfo(eq(principal), any(ActionListener.class));
145145

@@ -151,7 +151,7 @@ private void verifySuccessfulAuthentication(boolean enabled) throws Exception {
151151
// the realm assumes it owns the hashed password so it fills it with 0's
152152
doAnswer((i) -> {
153153
ActionListener callback = (ActionListener) i.getArguments()[1];
154-
callback.onResponse(new ReservedUserInfo(hasher.hash(newPassword), true, false));
154+
callback.onResponse(new ReservedUserInfo(hasher.hash(newPassword), true));
155155
return null;
156156
}).when(usersStore).getReservedUserInfo(eq(principal), any(ActionListener.class));
157157

@@ -302,7 +302,7 @@ public void testFailedAuthentication() throws Exception {
302302
// Mocked users store is initiated with default hashing algorithm
303303
final Hasher hasher = Hasher.resolve("bcrypt");
304304
char[] hash = hasher.hash(password);
305-
ReservedUserInfo userInfo = new ReservedUserInfo(hash, true, false);
305+
ReservedUserInfo userInfo = new ReservedUserInfo(hash, true);
306306
mockGetAllReservedUserInfo(usersStore, Collections.singletonMap("elastic", userInfo));
307307
final ReservedRealm reservedRealm = new ReservedRealm(mock(Environment.class), Settings.EMPTY, usersStore,
308308
new AnonymousUser(Settings.EMPTY), securityIndex, threadPool);
@@ -367,7 +367,7 @@ public void testBootstrapElasticPasswordFailsOnceElasticUserExists() throws Exce
367367
doAnswer((i) -> {
368368
ActionListener callback = (ActionListener) i.getArguments()[1];
369369
char[] hash = hasher.hash(password);
370-
ReservedUserInfo userInfo = new ReservedUserInfo(hash, true, false);
370+
ReservedUserInfo userInfo = new ReservedUserInfo(hash, true);
371371
callback.onResponse(userInfo);
372372
return null;
373373
}).when(usersStore).getReservedUserInfo(eq("elastic"), any(ActionListener.class));

0 commit comments

Comments
 (0)