From 313d4a947481d6044db792455b4d6ae21f92dd25 Mon Sep 17 00:00:00 2001 From: Stephen Crawford Date: Wed, 22 Feb 2023 15:41:55 -0500 Subject: [PATCH] Flatten response times Signed-off-by: Stephen Crawford --- .../InternalAuthenticationBackend.java | 38 +++-- .../auth/InternalAuthBackendTests.java | 145 ++++++++++++++++++ 2 files changed, 174 insertions(+), 9 deletions(-) create mode 100644 src/test/java/org/opensearch/security/auth/InternalAuthBackendTests.java diff --git a/src/main/java/org/opensearch/security/auth/internal/InternalAuthenticationBackend.java b/src/main/java/org/opensearch/security/auth/internal/InternalAuthenticationBackend.java index 2546cbae46..adc49c8735 100644 --- a/src/main/java/org/opensearch/security/auth/internal/InternalAuthenticationBackend.java +++ b/src/main/java/org/opensearch/security/auth/internal/InternalAuthenticationBackend.java @@ -75,7 +75,7 @@ public boolean exists(User user) { if(securityRoles != null) { user.addSecurityRoles(securityRoles); } - + user.addAttributes(attributeMap); return true; } @@ -83,20 +83,38 @@ public boolean exists(User user) { return false; } + /** + * A helper function used to verify that both invalid and valid usernames have a hashing check during testing. + * @param hash A string hash of the stored user's password. + * @param array A char array of the provided password + * @return Whether the hash matches the provided password + */ + public boolean passwordMatchesHash(String hash, char[] array) { + return OpenBSDBCrypt.checkPassword(hash, array); + } + @Override public User authenticate(final AuthCredentials credentials) { + boolean userExists; + if (internalUsersModel == null) { throw new OpenSearchSecurityException("Internal authentication backend not configured. May be OpenSearch is not initialized."); } + final byte[] password; + String hash; if(!internalUsersModel.exists(credentials.getUsername())) { - throw new OpenSearchSecurityException(credentials.getUsername() + " not found"); + userExists = false; + password = credentials.getPassword(); + hash = "$2y$12$NmKhjNssNgSIj8iXT7SYxeXvMA1E95a9tCt4cySY9FrQ4fB18xEc2"; // Ensure the same cryptographic complexity for users not found and invalid password + } else { + userExists = true; + password = credentials.getPassword(); + hash = internalUsersModel.getHash(credentials.getUsername()); } - final byte[] password = credentials.getPassword(); - - if(password == null || password.length == 0) { + if (password == null || password.length == 0) { throw new OpenSearchSecurityException("empty passwords not supported"); } @@ -108,7 +126,7 @@ public User authenticate(final AuthCredentials credentials) { Arrays.fill(password, (byte)0); try { - if (OpenBSDBCrypt.checkPassword(internalUsersModel.getHash(credentials.getUsername()), array)) { + if (passwordMatchesHash(hash, array) && userExists) { final List roles = internalUsersModel.getBackenRoles(credentials.getUsername()); final Map customAttributes = internalUsersModel.getAttributes(credentials.getUsername()); if(customAttributes != null) { @@ -116,16 +134,18 @@ public User authenticate(final AuthCredentials credentials) { credentials.addAttribute("attr.internal."+attributeName.getKey(), attributeName.getValue()); } } - + final User user = new User(credentials.getUsername(), roles, credentials); - + final List securityRoles = internalUsersModel.getSecurityRoles(credentials.getUsername()); if(securityRoles != null) { user.addSecurityRoles(securityRoles); } - return user; } else { + if (!userExists) { + throw new OpenSearchSecurityException(credentials.getUsername() + " not found"); + } throw new OpenSearchSecurityException("password does not match"); } } finally { diff --git a/src/test/java/org/opensearch/security/auth/InternalAuthBackendTests.java b/src/test/java/org/opensearch/security/auth/InternalAuthBackendTests.java new file mode 100644 index 0000000000..3821be7038 --- /dev/null +++ b/src/test/java/org/opensearch/security/auth/InternalAuthBackendTests.java @@ -0,0 +1,145 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.auth; + +import java.nio.ByteBuffer; +import java.nio.CharBuffer; +import java.nio.charset.StandardCharsets; +import java.util.Arrays; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mockito; + +import org.opensearch.OpenSearchSecurityException; +import org.opensearch.security.auth.internal.InternalAuthenticationBackend; +import org.opensearch.security.securityconf.InternalUsersModel; +import org.opensearch.security.user.AuthCredentials; + +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.mockito.internal.verification.VerificationModeFactory.times; + +public class InternalAuthBackendTests { + + private InternalUsersModel internalUsersModel; + + private InternalAuthenticationBackend internalAuthenticationBackend; + + @Before + public void internalAuthBackendTestsSetup() { + internalAuthenticationBackend = spy(new InternalAuthenticationBackend()); + internalUsersModel = mock(InternalUsersModel.class); + internalAuthenticationBackend.onInternalUsersModelChanged(internalUsersModel); + } + + private char[] createArrayFromPasswordBytes(byte[] password) { + ByteBuffer wrap = ByteBuffer.wrap(password); + CharBuffer buf = StandardCharsets.UTF_8.decode(wrap); + char[] array = new char[buf.limit()]; + buf.get(array); + Arrays.fill(password, (byte)0); + return array; + } + + @Test + public void testHashActionWithValidUserValidPassword() { + + // Make authentication info for valid username with valid password + final String validPassword = "admin"; + final byte[] validPasswordBytes = validPassword.getBytes(); + + final AuthCredentials validUsernameAuth = new AuthCredentials("admin", validPasswordBytes); + + final String hash = "$2y$12$NmKhjNssNgSIj8iXT7SYxeXvMA1E95a9tCt4cySY9FrQ4fB18xEc2"; + + char[] array = createArrayFromPasswordBytes(validPasswordBytes); + + + when(internalUsersModel.getHash(validUsernameAuth.getUsername())).thenReturn(hash); + when(internalUsersModel.exists(validUsernameAuth.getUsername())).thenReturn(true); + doReturn(true).when(internalAuthenticationBackend).passwordMatchesHash(Mockito.any(String.class), Mockito.any(char[].class)); + + //Act + internalAuthenticationBackend.authenticate(validUsernameAuth); + + verify(internalAuthenticationBackend, times(1)).passwordMatchesHash(hash, array); + verify(internalUsersModel, times(1)).getBackenRoles(validUsernameAuth.getUsername()); + } + + @Test + public void testHashActionWithValidUserInvalidPassword() { + + // Make authentication info for valid with bad password + final String gibberishPassword = "ajdhflkasdjfaklsdf"; + final byte[] gibberishPasswordBytes = gibberishPassword.getBytes(); + final AuthCredentials validUsernameAuth = new AuthCredentials("admin", gibberishPasswordBytes); + + final String hash = "$2y$12$NmKhjNssNgSIj8iXT7SYxeXvMA1E95a9tCt4cySY9FrQ4fB18xEc2"; + + char[] array = createArrayFromPasswordBytes(gibberishPasswordBytes); + + when(internalUsersModel.getHash("admin")).thenReturn(hash); + when(internalUsersModel.exists("admin")).thenReturn(true); + + OpenSearchSecurityException ex = Assert.assertThrows(OpenSearchSecurityException.class, + () -> internalAuthenticationBackend.authenticate(validUsernameAuth)); + assert(ex.getMessage().contains("password does not match")); + verify(internalAuthenticationBackend, times(1)).passwordMatchesHash(hash, array); + } + + @Test + public void testHashActionWithInvalidUserValidPassword() { + + // Make authentication info for valid and invalid usernames both with bad passwords + final String validPassword = "admin"; + final byte[] validPasswordBytes = validPassword.getBytes(); + final AuthCredentials invalidUsernameAuth = new AuthCredentials("ertyuiykgjjfguyifdghc", validPasswordBytes); + + final String hash = "$2y$12$NmKhjNssNgSIj8iXT7SYxeXvMA1E95a9tCt4cySY9FrQ4fB18xEc2"; + + char[] array = createArrayFromPasswordBytes(validPasswordBytes); + + when(internalUsersModel.exists("ertyuiykgjjfguyifdghc")).thenReturn(false); + when(internalAuthenticationBackend.passwordMatchesHash(hash, array)).thenReturn(true); //Say that the password is correct + + OpenSearchSecurityException ex = Assert.assertThrows(OpenSearchSecurityException.class, + () -> internalAuthenticationBackend.authenticate(invalidUsernameAuth)); + assert(ex.getMessage().contains("not found")); + verify(internalAuthenticationBackend, times(1)).passwordMatchesHash(hash, array); + } + + @Test + public void testHashActionWithInvalidUserInvalidPassword() { + + // Make authentication info for valid and invalid usernames both with bad passwords + final String gibberishPassword = "ajdhflkasdjfaklsdf"; + final byte[] gibberishPasswordBytes = gibberishPassword.getBytes(); + final AuthCredentials invalidUsernameAuth = new AuthCredentials("ertyuiykgjjfguyifdghc", gibberishPasswordBytes); + + final String hash = "$2y$12$NmKhjNssNgSIj8iXT7SYxeXvMA1E95a9tCt4cySY9FrQ4fB18xEc2"; + + char[] array = createArrayFromPasswordBytes(gibberishPasswordBytes); + + when(internalUsersModel.exists("ertyuiykgjjfguyifdghc")).thenReturn(false); + + + OpenSearchSecurityException ex = Assert.assertThrows(OpenSearchSecurityException.class, + () -> internalAuthenticationBackend.authenticate(invalidUsernameAuth)); + verify(internalAuthenticationBackend, times(1)).passwordMatchesHash(hash, array); + assert(ex.getMessage().contains("not found")); + } +}