Skip to content

Commit 1620a65

Browse files
committed
DefaultUserInfo empty password hash (elastic#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 9fe8c5d commit 1620a65

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.ClearRealmCacheRequest;
4241
import org.elasticsearch.xpack.core.security.action.realm.ClearRealmCacheResponse;
@@ -83,19 +82,13 @@ public class NativeUsersStore {
8382

8483
private final Settings settings;
8584
private final Client client;
86-
private final ReservedUserInfo disabledDefaultUserInfo;
87-
private final ReservedUserInfo enabledDefaultUserInfo;
8885

8986
private final SecurityIndexManager securityIndex;
9087

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

10194
/**
@@ -540,9 +533,10 @@ public void onResponse(GetResponse getResponse) {
540533
} else if (enabled == null) {
541534
listener.onFailure(new IllegalStateException("enabled must not be null!"));
542535
} else if (password.isEmpty()) {
543-
listener.onResponse((enabled ? enabledDefaultUserInfo : disabledDefaultUserInfo).deepClone());
536+
listener.onResponse(enabled ? ReservedUserInfo.defaultEnabledUserInfo()
537+
: ReservedUserInfo.defaultDisabledUserInfo());
544538
} else {
545-
listener.onResponse(new ReservedUserInfo(password.toCharArray(), enabled, false));
539+
listener.onResponse(new ReservedUserInfo(password.toCharArray(), enabled));
546540
}
547541
} else {
548542
listener.onResponse(null);
@@ -596,7 +590,7 @@ public void onResponse(SearchResponse searchResponse) {
596590
listener.onFailure(new IllegalStateException("enabled must not be null!"));
597591
return;
598592
} else {
599-
userInfos.put(username, new ReservedUserInfo(password.toCharArray(), enabled, false));
593+
userInfos.put(username, new ReservedUserInfo(password.toCharArray(), enabled));
600594
}
601595
}
602596
listener.onResponse(userInfos);
@@ -683,22 +677,32 @@ static final class ReservedUserInfo {
683677

684678
public final char[] passwordHash;
685679
public final boolean enabled;
686-
public final boolean hasEmptyPassword;
687680
private final Hasher hasher;
688681

689-
ReservedUserInfo(char[] passwordHash, boolean enabled, boolean hasEmptyPassword) {
682+
ReservedUserInfo(char[] passwordHash, boolean enabled) {
690683
this.passwordHash = passwordHash;
691684
this.enabled = enabled;
692-
this.hasEmptyPassword = hasEmptyPassword;
693685
this.hasher = Hasher.resolveFromHash(this.passwordHash);
694686
}
695687

696688
ReservedUserInfo deepClone() {
697-
return new ReservedUserInfo(Arrays.copyOf(passwordHash, passwordHash.length), enabled, hasEmptyPassword);
689+
return new ReservedUserInfo(Arrays.copyOf(passwordHash, passwordHash.length), enabled);
690+
}
691+
692+
boolean hasEmptyPassword() {
693+
return passwordHash.length == 0;
698694
}
699695

700696
boolean verifyPassword(SecureString data) {
701697
return hasher.verify(data, this.passwordHash);
702698
}
699+
700+
static ReservedUserInfo defaultEnabledUserInfo() {
701+
return new ReservedUserInfo(new char[0], true);
702+
}
703+
704+
static ReservedUserInfo defaultDisabledUserInfo() {
705+
return new ReservedUserInfo(new char[0], false);
706+
}
703707
}
704708
}

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
@@ -66,9 +66,6 @@ public class ReservedRealm extends CachingUsernamePasswordRealm {
6666
private final boolean realmEnabled;
6767
private final boolean anonymousEnabled;
6868
private final SecurityIndexManager securityIndex;
69-
private final Hasher reservedRealmHasher;
70-
private final ReservedUserInfo disabledDefaultUserInfo;
71-
private final ReservedUserInfo enabledDefaultUserInfo;
7269

7370
private final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(logger.getName());
7471

@@ -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
}
@@ -249,7 +241,7 @@ private ReservedUserInfo getDefaultUserInfo(String username) {
249241
if (ElasticUser.NAME.equals(username)) {
250242
return bootstrapUserInfo.deepClone();
251243
} else {
252-
return enabledDefaultUserInfo.deepClone();
244+
return ReservedUserInfo.defaultEnabledUserInfo();
253245
}
254246
}
255247

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
@@ -128,7 +128,7 @@ public void testBlankPasswordInIndexImpliesDefaultPassword() throws Exception {
128128
actionRespond(GetRequest.class, new GetResponse(result));
129129

130130
final NativeUsersStore.ReservedUserInfo userInfo = future.get();
131-
assertThat(userInfo.hasEmptyPassword, equalTo(true));
131+
assertThat(userInfo.hasEmptyPassword(), equalTo(true));
132132
assertThat(userInfo.enabled, equalTo(true));
133133
assertTrue(Hasher.verifyHash(new SecureString("".toCharArray()), userInfo.passwordHash));
134134
}
@@ -190,7 +190,7 @@ public void testVerifyNonExistentUser() throws Exception {
190190
false,
191191
null,
192192
Collections.emptyMap(),
193-
Collections.emptyMap());
193+
Collections.emptyMap());
194194

195195
actionRespond(GetRequest.class, new GetResponse(getResult));
196196

@@ -201,6 +201,17 @@ public void testVerifyNonExistentUser() throws Exception {
201201
assertThat(result.getMessage(), nullValue());
202202
}
203203

204+
public void testDefaultReservedUserInfoPasswordEmpty() {
205+
NativeUsersStore.ReservedUserInfo disabledUserInfo = NativeUsersStore.ReservedUserInfo.defaultDisabledUserInfo();
206+
NativeUsersStore.ReservedUserInfo enabledUserInfo = NativeUsersStore.ReservedUserInfo.defaultEnabledUserInfo();
207+
NativeUsersStore.ReservedUserInfo constructedUserInfo =
208+
new NativeUsersStore.ReservedUserInfo(Hasher.PBKDF2.hash(new SecureString(randomAlphaOfLength(14))), randomBoolean());
209+
210+
assertThat(disabledUserInfo.hasEmptyPassword(), equalTo(true));
211+
assertThat(enabledUserInfo.hasEmptyPassword(), equalTo(true));
212+
assertThat(constructedUserInfo.hasEmptyPassword(), equalTo(false));
213+
}
214+
204215
private <ARequest extends ActionRequest, AResponse extends ActionResponse> ARequest actionRespond(Class<ARequest> requestClass,
205216
AResponse response) {
206217
Tuple<ARequest, ActionListener<?>> tuple = findRequest(requestClass);
@@ -209,11 +220,11 @@ private <ARequest extends ActionRequest, AResponse extends ActionResponse> ARequ
209220
}
210221

211222
private <ARequest extends ActionRequest> Tuple<ARequest, ActionListener<?>> findRequest(
212-
Class<ARequest> requestClass) {
223+
Class<ARequest> requestClass) {
213224
return this.requests.stream()
214-
.filter(t -> requestClass.isInstance(t.v1()))
215-
.map(t -> new Tuple<ARequest, ActionListener<?>>(requestClass.cast(t.v1()), t.v2()))
216-
.findFirst().orElseThrow(() -> new RuntimeException("Cannot find request of type " + requestClass));
225+
.filter(t -> requestClass.isInstance(t.v1()))
226+
.map(t -> new Tuple<ARequest, ActionListener<?>>(requestClass.cast(t.v1()), t.v2()))
227+
.findFirst().orElseThrow(() -> new RuntimeException("Cannot find request of type " + requestClass));
217228
}
218229

219230
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
@@ -143,7 +143,7 @@ private void verifySuccessfulAuthentication(boolean enabled) throws Exception {
143143
when(securityIndex.indexExists()).thenReturn(true);
144144
doAnswer((i) -> {
145145
ActionListener callback = (ActionListener) i.getArguments()[1];
146-
callback.onResponse(new ReservedUserInfo(hasher.hash(newPassword), enabled, false));
146+
callback.onResponse(new ReservedUserInfo(hasher.hash(newPassword), enabled));
147147
return null;
148148
}).when(usersStore).getReservedUserInfo(eq(principal), any(ActionListener.class));
149149

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

@@ -317,7 +317,7 @@ public void testFailedAuthentication() throws Exception {
317317
// Mocked users store is initiated with default hashing algorithm
318318
final Hasher hasher = Hasher.resolve("bcrypt");
319319
char[] hash = hasher.hash(password);
320-
ReservedUserInfo userInfo = new ReservedUserInfo(hash, true, false);
320+
ReservedUserInfo userInfo = new ReservedUserInfo(hash, true);
321321
mockGetAllReservedUserInfo(usersStore, Collections.singletonMap("elastic", userInfo));
322322
final ReservedRealm reservedRealm = new ReservedRealm(mock(Environment.class), Settings.EMPTY, usersStore,
323323
new AnonymousUser(Settings.EMPTY), securityIndex, threadPool);
@@ -382,7 +382,7 @@ public void testBootstrapElasticPasswordFailsOnceElasticUserExists() throws Exce
382382
doAnswer((i) -> {
383383
ActionListener callback = (ActionListener) i.getArguments()[1];
384384
char[] hash = hasher.hash(password);
385-
ReservedUserInfo userInfo = new ReservedUserInfo(hash, true, false);
385+
ReservedUserInfo userInfo = new ReservedUserInfo(hash, true);
386386
callback.onResponse(userInfo);
387387
return null;
388388
}).when(usersStore).getReservedUserInfo(eq("elastic"), any(ActionListener.class));

0 commit comments

Comments
 (0)