From da3aacf1075a12327e45b34d23d6852954c59dbd Mon Sep 17 00:00:00 2001 From: c-a-m Date: Tue, 9 Sep 2014 07:53:22 -0600 Subject: [PATCH] Passwords: SecuredString to lock down and clear password usage. SecuredString encapsulates handling of passwords and clearing them when done. This change includes changing everywhere passwords are used. After authentication the authentication service will clear the token - which will clear the password. This avoids using any passwords in String objects. This also adds commentary to BCrypt to show how it changed from the original external resource. It moves utility methods to CharArrays. Original commit: elastic/x-pack-elasticsearch@d0ffbae5c82155381a4146146efc25b7b520818d --- .../shield/authc/AuthenticationToken.java | 2 + .../authc/InternalAuthenticationService.java | 37 ++-- .../authc/esusers/FileUserPasswdStore.java | 3 +- .../authc/esusers/tool/ESUsersTool.java | 17 +- .../ActiveDirectoryConnectionFactory.java | 6 +- .../authc/ldap/LdapConnectionFactory.java | 4 +- .../shield/authc/ldap/LdapRealm.java | 2 - .../ldap/StandardLdapConnectionFactory.java | 6 +- .../shield/authc/support/BCrypt.java | 23 ++- .../authc/support/CachingUserPasswdStore.java | 12 +- .../support/CachingUsernamePasswordRealm.java | 4 +- .../shield/authc/support/CharArrays.java | 45 +++++ .../shield/authc/support/Hasher.java | 32 ++-- .../shield/authc/support/SecuredString.java | 158 ++++++++++++++++++ .../shield/authc/support/UserPasswdStore.java | 4 +- .../authc/support/UsernamePasswordToken.java | 45 +++-- .../shield/authc/system/SystemRealm.java | 5 + .../audit/logfile/LoggingAuditTrailTests.java | 5 + .../authc/esusers/ESUsersRealmTests.java | 14 +- .../esusers/FileUserPasswdStoreTests.java | 7 +- .../authc/esusers/tool/ESUsersToolTests.java | 21 +-- .../ldap/ActiveDirectoryFactoryTests.java | 5 +- .../authc/ldap/LdapConnectionTests.java | 17 +- .../shield/authc/ldap/LdapRealmTest.java | 29 ++-- .../CachingUsernamePasswordRealmTests.java | 6 +- .../shield/authc/support/HasherTests.java | 4 +- .../authc/support/SecuredStringTests.java | 97 +++++++++++ .../support/UsernamePasswordTokenTests.java | 8 +- .../shield/test/ShieldIntegrationTest.java | 9 +- .../transport/ssl/SslIntegrationTests.java | 6 +- .../transport/ssl/SslRequireAuthTests.java | 2 +- .../elasticsearch/test/ShieldRestTests.java | 7 +- 32 files changed, 496 insertions(+), 146 deletions(-) create mode 100644 src/main/java/org/elasticsearch/shield/authc/support/CharArrays.java create mode 100644 src/main/java/org/elasticsearch/shield/authc/support/SecuredString.java create mode 100644 src/test/java/org/elasticsearch/shield/authc/support/SecuredStringTests.java diff --git a/src/main/java/org/elasticsearch/shield/authc/AuthenticationToken.java b/src/main/java/org/elasticsearch/shield/authc/AuthenticationToken.java index ce7339e0ae1..48b92a9c80a 100644 --- a/src/main/java/org/elasticsearch/shield/authc/AuthenticationToken.java +++ b/src/main/java/org/elasticsearch/shield/authc/AuthenticationToken.java @@ -13,4 +13,6 @@ public interface AuthenticationToken { String principal(); Object credentials(); + + void clearCredentials(); } diff --git a/src/main/java/org/elasticsearch/shield/authc/InternalAuthenticationService.java b/src/main/java/org/elasticsearch/shield/authc/InternalAuthenticationService.java index d9bf28c82a6..78c9f1cc96c 100644 --- a/src/main/java/org/elasticsearch/shield/authc/InternalAuthenticationService.java +++ b/src/main/java/org/elasticsearch/shield/authc/InternalAuthenticationService.java @@ -97,24 +97,29 @@ public class InternalAuthenticationService extends AbstractComponent implements @SuppressWarnings("unchecked") public User authenticate(String action, TransportMessage message, AuthenticationToken token) throws AuthenticationException { assert token != null : "cannot authenticate null tokens"; - User user = (User) message.getContext().get(USER_CTX_KEY); - if (user != null) { - return user; - } - for (Realm realm : realms) { - if (realm.supports(token)) { - user = realm.authenticate(token); - if (user != null) { - message.putInContext(USER_CTX_KEY, user); - return user; - } else if (auditTrail != null) { - auditTrail.authenticationFailed(realm.type(), token, action, message); + try { + User user = (User) message.getContext().get(USER_CTX_KEY); + if (user != null) { + return user; + } + for (Realm realm : realms) { + if (realm.supports(token)) { + user = realm.authenticate(token); + if (user != null) { + message.putInContext(USER_CTX_KEY, user); + return user; + } else if (auditTrail != null) { + auditTrail.authenticationFailed(realm.type(), token, action, message); + } } } + if (auditTrail != null) { + auditTrail.authenticationFailed(token, action, message); + } + throw new AuthenticationException("Unable to authenticate user for request"); + } finally { + token.clearCredentials(); } - if (auditTrail != null) { - auditTrail.authenticationFailed(token, action, message); - } - throw new AuthenticationException("Unable to authenticate user for request"); + } } diff --git a/src/main/java/org/elasticsearch/shield/authc/esusers/FileUserPasswdStore.java b/src/main/java/org/elasticsearch/shield/authc/esusers/FileUserPasswdStore.java index 897f2e2bbfd..0e146de2a64 100644 --- a/src/main/java/org/elasticsearch/shield/authc/esusers/FileUserPasswdStore.java +++ b/src/main/java/org/elasticsearch/shield/authc/esusers/FileUserPasswdStore.java @@ -15,6 +15,7 @@ import org.elasticsearch.common.logging.ESLogger; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import org.elasticsearch.shield.authc.support.Hasher; +import org.elasticsearch.shield.authc.support.SecuredString; import org.elasticsearch.shield.authc.support.UserPasswdStore; import org.elasticsearch.shield.plugin.ShieldPlugin; import org.elasticsearch.watcher.FileChangesListener; @@ -61,7 +62,7 @@ public class FileUserPasswdStore extends AbstractComponent implements UserPasswd } @Override - public boolean verifyPassword(String username, char[] password) { + public boolean verifyPassword(String username, SecuredString password) { if (esUsers == null) { return false; } diff --git a/src/main/java/org/elasticsearch/shield/authc/esusers/tool/ESUsersTool.java b/src/main/java/org/elasticsearch/shield/authc/esusers/tool/ESUsersTool.java index 64698981da5..e1d5a852c31 100644 --- a/src/main/java/org/elasticsearch/shield/authc/esusers/tool/ESUsersTool.java +++ b/src/main/java/org/elasticsearch/shield/authc/esusers/tool/ESUsersTool.java @@ -11,12 +11,16 @@ import org.elasticsearch.common.cli.CliTool; import org.elasticsearch.common.cli.CliToolConfig; import org.elasticsearch.common.cli.Terminal; import org.elasticsearch.common.cli.commons.CommandLine; -import org.elasticsearch.common.collect.*; +import org.elasticsearch.common.collect.Lists; +import org.elasticsearch.common.collect.Maps; +import org.elasticsearch.common.collect.ObjectArrays; +import org.elasticsearch.common.collect.Sets; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import org.elasticsearch.shield.authc.esusers.FileUserPasswdStore; import org.elasticsearch.shield.authc.esusers.FileUserRolesStore; import org.elasticsearch.shield.authc.support.Hasher; +import org.elasticsearch.shield.authc.support.SecuredString; import java.nio.file.Files; import java.nio.file.Path; @@ -97,14 +101,14 @@ public class ESUsersTool extends CliTool { String rolesCsv = cli.getOptionValue("roles"); String[] roles = (rolesCsv != null) ? rolesCsv.split(",") : Strings.EMPTY_ARRAY; - return new Useradd(terminal, username, password, roles); + return new Useradd(terminal, username, new SecuredString(password), roles); } final String username; - final char[] passwd; + final SecuredString passwd; final String[] roles; - Useradd(Terminal terminal, String username, char[] passwd, String... roles) { + Useradd(Terminal terminal, String username, SecuredString passwd, String... roles) { super(terminal); this.username = username; this.passwd = passwd; @@ -212,12 +216,13 @@ public class ESUsersTool extends CliTool { } final String username; - final char[] passwd; + final SecuredString passwd; Passwd(Terminal terminal, String username, char[] passwd) { super(terminal); this.username = username; - this.passwd = passwd; + this.passwd = new SecuredString(passwd); + Arrays.fill(passwd, (char) 0); } @Override diff --git a/src/main/java/org/elasticsearch/shield/authc/ldap/ActiveDirectoryConnectionFactory.java b/src/main/java/org/elasticsearch/shield/authc/ldap/ActiveDirectoryConnectionFactory.java index a3c3956170d..fd46d89148e 100644 --- a/src/main/java/org/elasticsearch/shield/authc/ldap/ActiveDirectoryConnectionFactory.java +++ b/src/main/java/org/elasticsearch/shield/authc/ldap/ActiveDirectoryConnectionFactory.java @@ -11,6 +11,7 @@ import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.shield.ShieldException; +import org.elasticsearch.shield.authc.support.SecuredString; import javax.naming.Context; import javax.naming.NamingEnumeration; @@ -63,13 +64,12 @@ public class ActiveDirectoryConnectionFactory extends AbstractComponent implemen * @return An authenticated */ @Override - public LdapConnection bind(String userName, char[] password) { + public LdapConnection bind(String userName, SecuredString password) { String userPrincipal = userName + "@" + this.domainName; - Hashtable ldapEnv = new Hashtable<>(this.sharedLdapEnv); ldapEnv.put(Context.SECURITY_AUTHENTICATION, "simple"); ldapEnv.put(Context.SECURITY_PRINCIPAL, userPrincipal); - ldapEnv.put(Context.SECURITY_CREDENTIALS, password); + ldapEnv.put(Context.SECURITY_CREDENTIALS, password.internalChars()); try { DirContext ctx = new InitialDirContext(ldapEnv); diff --git a/src/main/java/org/elasticsearch/shield/authc/ldap/LdapConnectionFactory.java b/src/main/java/org/elasticsearch/shield/authc/ldap/LdapConnectionFactory.java index db84c77d000..cf3cb9a71aa 100644 --- a/src/main/java/org/elasticsearch/shield/authc/ldap/LdapConnectionFactory.java +++ b/src/main/java/org/elasticsearch/shield/authc/ldap/LdapConnectionFactory.java @@ -5,6 +5,8 @@ */ package org.elasticsearch.shield.authc.ldap; +import org.elasticsearch.shield.authc.support.SecuredString; + /** * This factory holds settings needed for authenticating to LDAP and creating LdapConnections. * Each created LdapConnection needs to be closed or else connections will pill up consuming resources. @@ -24,6 +26,6 @@ public interface LdapConnectionFactory { * Password authenticated bind * @param user name of the user to authenticate the connection with. */ - LdapConnection bind(String user, char[] password) ; + LdapConnection bind(String user, SecuredString password) ; } diff --git a/src/main/java/org/elasticsearch/shield/authc/ldap/LdapRealm.java b/src/main/java/org/elasticsearch/shield/authc/ldap/LdapRealm.java index 997b22ee6f4..0ccdcd7a117 100644 --- a/src/main/java/org/elasticsearch/shield/authc/ldap/LdapRealm.java +++ b/src/main/java/org/elasticsearch/shield/authc/ldap/LdapRealm.java @@ -16,7 +16,6 @@ import org.elasticsearch.shield.authc.support.CachingUsernamePasswordRealm; import org.elasticsearch.shield.authc.support.UsernamePasswordToken; import org.elasticsearch.transport.TransportMessage; -import java.util.Arrays; import java.util.List; import java.util.Set; @@ -62,7 +61,6 @@ public class LdapRealm extends CachingUsernamePasswordRealm implements Realm groupDNs = session.getGroups(); Set roles = roleMapper.mapRoles(groupDNs); User.Simple user = new User.Simple(token.principal(), roles.toArray(new String[roles.size()])); - Arrays.fill(token.credentials(), '\0'); return user; } catch (ShieldException e){ logger.info("Authentication Failed for user [{}]", e, token.principal()); diff --git a/src/main/java/org/elasticsearch/shield/authc/ldap/StandardLdapConnectionFactory.java b/src/main/java/org/elasticsearch/shield/authc/ldap/StandardLdapConnectionFactory.java index 54ac8416849..adda8b27fd7 100644 --- a/src/main/java/org/elasticsearch/shield/authc/ldap/StandardLdapConnectionFactory.java +++ b/src/main/java/org/elasticsearch/shield/authc/ldap/StandardLdapConnectionFactory.java @@ -11,6 +11,7 @@ import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.shield.ShieldException; +import org.elasticsearch.shield.authc.support.SecuredString; import javax.naming.Context; import javax.naming.NamingException; @@ -68,11 +69,11 @@ public class StandardLdapConnectionFactory extends AbstractComponent implements * @return authenticated exception */ @Override - public LdapConnection bind(String username, char[] password) { + public LdapConnection bind(String username, SecuredString password) { //SASL, MD5, etc. all options here stink, we really need to go over ssl + simple authentication Hashtable ldapEnv = new Hashtable<>(this.sharedLdapEnv); ldapEnv.put(Context.SECURITY_AUTHENTICATION, "simple"); - ldapEnv.put(Context.SECURITY_CREDENTIALS, password); + ldapEnv.put(Context.SECURITY_CREDENTIALS, password.internalChars()); for (String template : userDnTemplates) { String dn = buildDnFromTemplate(username, template); @@ -87,6 +88,7 @@ public class StandardLdapConnectionFactory extends AbstractComponent implements logger.warn("Failed ldap authentication with user template [{}], dn [{}]", template, dn); } } + throw new LdapException("Failed ldap authentication"); } diff --git a/src/main/java/org/elasticsearch/shield/authc/support/BCrypt.java b/src/main/java/org/elasticsearch/shield/authc/support/BCrypt.java index ce34f5d9848..abbe2e37a5d 100644 --- a/src/main/java/org/elasticsearch/shield/authc/support/BCrypt.java +++ b/src/main/java/org/elasticsearch/shield/authc/support/BCrypt.java @@ -14,7 +14,6 @@ package org.elasticsearch.shield.authc.support; // ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF // OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. -import java.io.UnsupportedEncodingException; import java.security.SecureRandom; /** @@ -638,13 +637,16 @@ public class BCrypt { } /** - * Hash a password using the OpenBSD bcrypt scheme + * Hash a password using the OpenBSD bcrypt scheme. + * + * Modified from the original to take a SecuredString instead of the original + * * @param password the password to hash * @param salt the salt to hash with (perhaps generated * using BCrypt.gensalt) * @return the hashed password */ - public static String hashpw(String password, String salt) { + public static String hashpw(SecuredString password, String salt) { BCrypt B; String real_salt; byte passwordb[], saltb[], hashed[]; @@ -669,12 +671,19 @@ public class BCrypt { rounds = Integer.parseInt(salt.substring(off, off + 2)); real_salt = salt.substring(off + 3, off + 25); - try { + + /* original code before introducing SecuredString + try { passwordb = (password + (minor >= 'a' ? "\000" : "")).getBytes("UTF-8"); } catch (UnsupportedEncodingException uee) { throw new AssertionError("UTF-8 is not supported"); } + */ + + // the next line is the SecuredString replacement for the above commented-out section + passwordb = ( minor >= 'a' ? password.concat("\000"): password ).utf8Bytes(); + saltb = decode_base64(real_salt, BCRYPT_SALT_LEN); B = new BCrypt(); @@ -740,12 +749,14 @@ public class BCrypt { /** * Check that a plaintext password matches a previously hashed - * one + * one. + * + * Modified from the original to take a SecuredString plaintext * @param plaintext the plaintext password to verify * @param hashed the previously-hashed password * @return true if the passwords match, false otherwise */ - public static boolean checkpw(String plaintext, String hashed) { + public static boolean checkpw(SecuredString plaintext, String hashed) { return hashed.compareTo(hashpw(plaintext, hashed)) == 0; } } diff --git a/src/main/java/org/elasticsearch/shield/authc/support/CachingUserPasswdStore.java b/src/main/java/org/elasticsearch/shield/authc/support/CachingUserPasswdStore.java index 61b4f177d95..5c628498e30 100644 --- a/src/main/java/org/elasticsearch/shield/authc/support/CachingUserPasswdStore.java +++ b/src/main/java/org/elasticsearch/shield/authc/support/CachingUserPasswdStore.java @@ -61,7 +61,7 @@ public abstract class CachingUserPasswdStore extends AbstractComponent implement } @Override - public final boolean verifyPassword(final String username, final char[] password) { + public final boolean verifyPassword(final String username, final SecuredString password) { if (cache == null) { return doVerifyPassword(username, password); } @@ -80,7 +80,7 @@ public abstract class CachingUserPasswdStore extends AbstractComponent implement * Verifies the given password. Both the given username, and if the username is verified, then the * given password. This method is used when the caching is disabled. */ - protected abstract boolean doVerifyPassword(String username, char[] password); + protected abstract boolean doVerifyPassword(String username, SecuredString password); protected abstract PasswordHash passwordHash(String username); @@ -92,8 +92,8 @@ public abstract class CachingUserPasswdStore extends AbstractComponent implement } @Override - public final void store(String username, char[] key) { - doStore(username, key); + public final void store(String username, SecuredString password) { + doStore(username, password); expire(username); } @@ -103,7 +103,7 @@ public abstract class CachingUserPasswdStore extends AbstractComponent implement expire(username); } - protected abstract void doStore(String username, char[] password); + protected abstract void doStore(String username, SecuredString password); protected abstract void doRemove(String username); } @@ -113,7 +113,7 @@ public abstract class CachingUserPasswdStore extends AbstractComponent implement */ static interface PasswordHash { - boolean verify(char[] password); + boolean verify(SecuredString password); } } diff --git a/src/main/java/org/elasticsearch/shield/authc/support/CachingUsernamePasswordRealm.java b/src/main/java/org/elasticsearch/shield/authc/support/CachingUsernamePasswordRealm.java index 3c7600ea6f0..cea974625f9 100644 --- a/src/main/java/org/elasticsearch/shield/authc/support/CachingUsernamePasswordRealm.java +++ b/src/main/java/org/elasticsearch/shield/authc/support/CachingUsernamePasswordRealm.java @@ -116,12 +116,12 @@ public abstract class CachingUsernamePasswordRealm extends AbstractComponent imp public static class UserWithHash { User user; char[] hash; - public UserWithHash(User user, char[] password){ + public UserWithHash(User user, SecuredString password){ this.user = user; this.hash = Hasher.HTPASSWD.hash(password); } - public boolean verify(char[] password){ + public boolean verify(SecuredString password){ return Hasher.HTPASSWD.verify(password, hash); } } diff --git a/src/main/java/org/elasticsearch/shield/authc/support/CharArrays.java b/src/main/java/org/elasticsearch/shield/authc/support/CharArrays.java new file mode 100644 index 00000000000..6f9b5a02f29 --- /dev/null +++ b/src/main/java/org/elasticsearch/shield/authc/support/CharArrays.java @@ -0,0 +1,45 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.shield.authc.support; + +import org.elasticsearch.common.base.Charsets; + +import java.nio.ByteBuffer; +import java.nio.CharBuffer; +import java.util.Arrays; + +/** + * Helper class similar to Arrays to handle conversions for Char arrays + */ +public class CharArrays { + static char[] utf8BytesToChars(byte[] utf8Bytes) { + ByteBuffer byteBuffer = ByteBuffer.wrap(utf8Bytes); + CharBuffer charBuffer = Charsets.UTF_8.decode(byteBuffer); + char[] chars = Arrays.copyOfRange(charBuffer.array(), charBuffer.position(), charBuffer.limit()); + byteBuffer.clear(); + charBuffer.clear(); + return chars; + } + /** + * Like String.indexOf for for an array of chars + */ + static int indexOf(char[] array, char ch){ + for (int i = 0; (i < array.length); i++) { + if (array[i] == ch) { + return i; + } + } + return -1; + } + + public static byte[] toUtf8Bytes(char[] chars) { + CharBuffer charBuffer = CharBuffer.wrap(chars); + ByteBuffer byteBuffer = Charsets.UTF_8.encode(charBuffer); + byte[] bytes = Arrays.copyOfRange(byteBuffer.array(), byteBuffer.position(), byteBuffer.limit()); + Arrays.fill(byteBuffer.array(), (byte) 0); // clear sensitive data + return bytes; + } +} diff --git a/src/main/java/org/elasticsearch/shield/authc/support/Hasher.java b/src/main/java/org/elasticsearch/shield/authc/support/Hasher.java index 340fbee6b35..19066bcf21f 100644 --- a/src/main/java/org/elasticsearch/shield/authc/support/Hasher.java +++ b/src/main/java/org/elasticsearch/shield/authc/support/Hasher.java @@ -10,12 +10,8 @@ import org.apache.commons.codec.digest.Crypt; import org.apache.commons.codec.digest.DigestUtils; import org.apache.commons.codec.digest.Md5Crypt; import org.elasticsearch.ElasticsearchIllegalArgumentException; -import org.elasticsearch.common.base.Charsets; import org.elasticsearch.common.os.OsUtils; -import java.nio.ByteBuffer; -import java.nio.CharBuffer; -import java.util.Arrays; import java.util.Locale; /** @@ -30,25 +26,25 @@ public enum Hasher { HTPASSWD() { @Override - public char[] hash(char[] text) { + public char[] hash(SecuredString text) { String salt = org.elasticsearch.shield.authc.support.BCrypt.gensalt(); - return BCrypt.hashpw(new String(text), salt).toCharArray(); + return BCrypt.hashpw(text, salt).toCharArray(); } @Override - public boolean verify(char[] text, char[] hash) { + public boolean verify(SecuredString text, char[] hash) { String hashStr = new String(hash); if (hashStr.startsWith(BCRYPT_PREFIX_Y)) { hashStr = BCRYPT_PREFIX + hashStr.substring(BCRYPT_PREFIX_Y.length()); } if (hashStr.startsWith(BCRYPT_PREFIX)) { - return BCrypt.checkpw(new String(text), hashStr); + return BCrypt.checkpw(text, hashStr); } if (hashStr.startsWith(PLAIN_PREFIX)) { hashStr = hashStr.substring(PLAIN_PREFIX.length()); - return hashStr.compareTo(new String(text)) == 0; + return text.equals(hashStr); } - byte[] textBytes = toBytes(text); + byte[] textBytes = CharArrays.toUtf8Bytes(text.internalChars()); if (hashStr.startsWith(APR1_PREFIX)) { return hashStr.compareTo(Md5Crypt.apr1Crypt(textBytes, hashStr)) == 0; } @@ -57,8 +53,8 @@ public enum Hasher { return hashStr.substring(SHA1_PREFIX.length()).compareTo(passwd64) == 0; } return CRYPT_SUPPORTED ? - hashStr.compareTo(Crypt.crypt(textBytes, hashStr)) == 0: // crypt algo - hashStr.compareTo(new String(text)) == 0; // plain text + hashStr.compareTo(Crypt.crypt(textBytes, hashStr)) == 0 : // crypt algo + text.equals(hashStr); // plain text } }; @@ -88,16 +84,8 @@ public enum Hasher { return hasher; } - public abstract char[] hash(char[] data); + public abstract char[] hash(SecuredString data); - public abstract boolean verify(char[] data, char[] hash); - - private static byte[] toBytes(char[] chars) { - CharBuffer charBuffer = CharBuffer.wrap(chars); - ByteBuffer byteBuffer = Charsets.UTF_8.encode(charBuffer); - byte[] bytes = Arrays.copyOfRange(byteBuffer.array(), byteBuffer.position(), byteBuffer.limit()); - Arrays.fill(byteBuffer.array(), (byte) 0); // clear sensitive data - return bytes; - } + public abstract boolean verify(SecuredString data, char[] hash); } diff --git a/src/main/java/org/elasticsearch/shield/authc/support/SecuredString.java b/src/main/java/org/elasticsearch/shield/authc/support/SecuredString.java new file mode 100644 index 00000000000..f78c53239ef --- /dev/null +++ b/src/main/java/org/elasticsearch/shield/authc/support/SecuredString.java @@ -0,0 +1,158 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.shield.authc.support; + +import org.elasticsearch.ElasticsearchException; + +import java.nio.CharBuffer; +import java.util.Arrays; + +/** + * This is not a string but a CharSequence that can be cleared of its memory. Important for handling passwords. + * + * @NotThreadSafe There is a chance that the chars could be cleared while doing operations on the chars. + * + * TODO: dot net's SecureString implementation does some obfuscation of the password to prevent gleaming passwords + * from memory dumps. (this is hard as dot net uses windows system crypto. Thats probably the reason java still doesn't have it) + */ +public class SecuredString implements CharSequence { + private final char[] chars; + private boolean cleared = false; + + /** + * Note: the passed in chars are not duplicated, but used directly for performance/optimization. DO NOT + * modify or clear the chars after it has been passed into this constructor. + * @param chars + */ + public SecuredString(char[] chars){ + this.chars = new char[chars.length]; + System.arraycopy(chars, 0, this.chars, 0, chars.length); + } + + /** + * This constructor is used internally for the concatenate method. It DOES duplicate the passed in array, unlike + * the public constructor + */ + private SecuredString(char[] chars, int start, int end){ + this.chars = new char[end - start]; + System.arraycopy(chars, start, this.chars, 0, this.chars.length); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null) return false; + + if (o instanceof SecuredString) { + SecuredString that = (SecuredString) o; + + if (cleared != that.cleared) return false; + if (!Arrays.equals(chars, that.chars)) return false; + + return true; + } + else if (o instanceof CharSequence) { + CharSequence that = (CharSequence) o; + if (cleared) return false; + if (chars.length != that.length()) return false; + + for(int i=0; i < chars.length; i++){ + if (chars[i] != that.charAt(i)) { + return false; + } + } + return true; + } + return false; + } + + @Override + public int hashCode() { + int result = Arrays.hashCode(chars); + result = 31 * result + (cleared ? 1 : 0); + return result; + } + + /** + * Note: This is a dangerous call that exists for performance/optimization + * DO NOT modify the array returned by this method. To clear the array call SecureString.clear(). + * @return the internal characters that MUST NOT be cleared manually + */ + public char[] internalChars(){ + throwIfCleared(); + return chars; + } + + /** + * @return utf8 encoded bytes + */ + public byte[] utf8Bytes(){ + throwIfCleared(); + return CharArrays.toUtf8Bytes(chars); + } + + @Override + public int length() { + throwIfCleared(); + return chars.length; + } + + @Override + public char charAt(int index) { + throwIfCleared(); + return chars[index]; + } + + @Override + public SecuredString subSequence(int start, int end) { + throwIfCleared(); + return new SecuredString(this.chars, start, end); + } + + /** + * Manually clear the underlying array holding the characters + */ + public void clear(){ + cleared = true; + Arrays.fill(chars, (char) 0); + } + + @Override + public void finalize() throws Throwable{ + clear(); + super.finalize(); + } + + public int indexOf(char toFind) { + for(int i=0; i message, UsernamePasswordToken defaultToken) { String authStr = message.getHeader(BASIC_AUTH_HEADER); if (authStr == null) { @@ -71,12 +79,15 @@ public class UsernamePasswordToken implements AuthenticationToken { throw new AuthenticationException("Invalid basic authentication header value"); } - String userpasswd = new String(Base64.decodeBase64(matcher.group(1)), Charsets.UTF_8); - int i = userpasswd.indexOf(':'); + char[] userpasswd = CharArrays.utf8BytesToChars(Base64.decodeBase64(matcher.group(1))); + int i = CharArrays.indexOf(userpasswd, ':'); if (i < 0) { throw new AuthenticationException("Invalid basic authentication header value"); } - return new UsernamePasswordToken(userpasswd.substring(0, i), userpasswd.substring(i+1).toCharArray()); + + return new UsernamePasswordToken( + new String(Arrays.copyOfRange(userpasswd, 0, i)), + new SecuredString(Arrays.copyOfRange(userpasswd, i + 1, userpasswd.length))); } public static UsernamePasswordToken extractToken(RestRequest request, UsernamePasswordToken defaultToken) { @@ -90,21 +101,27 @@ public class UsernamePasswordToken implements AuthenticationToken { throw new AuthenticationException("Invalid basic authentication header value"); } - String userpasswd = new String(Base64.decodeBase64(matcher.group(1)), Charsets.UTF_8); - int i = userpasswd.indexOf(':'); + char[] userpasswd = CharArrays.utf8BytesToChars(Base64.decodeBase64(matcher.group(1))); + int i = CharArrays.indexOf(userpasswd, ':'); if (i < 0) { throw new AuthenticationException("Invalid basic authentication header value"); } - return new UsernamePasswordToken(userpasswd.substring(0, i), userpasswd.substring(i+1).toCharArray()); + + return new UsernamePasswordToken( + new String(Arrays.copyOfRange(userpasswd, 0, i)), + new SecuredString(Arrays.copyOfRange(userpasswd, i + 1, userpasswd.length))); } public static void putTokenHeader(TransportRequest request, UsernamePasswordToken token) { request.putHeader("Authorization", basicAuthHeaderValue(token.username, token.password)); } - public static String basicAuthHeaderValue(String username, char[] passwd) { - String basicToken = username + ":" + new String(passwd); - basicToken = new String(Base64.encodeBase64(basicToken.getBytes(Charsets.UTF_8)), Charsets.UTF_8); + public static String basicAuthHeaderValue(String username, SecuredString passwd) { + CharBuffer chars = CharBuffer.allocate(username.length() + passwd.length() + 1); + chars.put(username).put(':').put(passwd.internalChars()); + + //TODO we still have passwords in Strings in headers + String basicToken = new String(Base64.encodeBase64(CharArrays.toUtf8Bytes(chars.array())), Charsets.UTF_8); return "Basic " + basicToken; } } diff --git a/src/main/java/org/elasticsearch/shield/authc/system/SystemRealm.java b/src/main/java/org/elasticsearch/shield/authc/system/SystemRealm.java index 3c7024e1139..847bf48d7c4 100644 --- a/src/main/java/org/elasticsearch/shield/authc/system/SystemRealm.java +++ b/src/main/java/org/elasticsearch/shield/authc/system/SystemRealm.java @@ -28,6 +28,11 @@ public class SystemRealm implements Realm { public Object credentials() { return null; } + + @Override + public void clearCredentials() { + + } }; @Override diff --git a/src/test/java/org/elasticsearch/shield/audit/logfile/LoggingAuditTrailTests.java b/src/test/java/org/elasticsearch/shield/audit/logfile/LoggingAuditTrailTests.java index be0a5100c83..73547d36a75 100644 --- a/src/test/java/org/elasticsearch/shield/audit/logfile/LoggingAuditTrailTests.java +++ b/src/test/java/org/elasticsearch/shield/audit/logfile/LoggingAuditTrailTests.java @@ -155,5 +155,10 @@ public class LoggingAuditTrailTests extends ElasticsearchTestCase { fail("it's not allowed to print the credentials of the auth token"); return null; } + + @Override + public void clearCredentials() { + + } } } diff --git a/src/test/java/org/elasticsearch/shield/authc/esusers/ESUsersRealmTests.java b/src/test/java/org/elasticsearch/shield/authc/esusers/ESUsersRealmTests.java index cfe32c71406..30caa896bd3 100644 --- a/src/test/java/org/elasticsearch/shield/authc/esusers/ESUsersRealmTests.java +++ b/src/test/java/org/elasticsearch/shield/authc/esusers/ESUsersRealmTests.java @@ -21,9 +21,7 @@ import org.elasticsearch.rest.RestChannel; import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.shield.User; -import org.elasticsearch.shield.authc.support.UserPasswdStore; -import org.elasticsearch.shield.authc.support.UserRolesStore; -import org.elasticsearch.shield.authc.support.UsernamePasswordToken; +import org.elasticsearch.shield.authc.support.*; import org.elasticsearch.test.ElasticsearchTestCase; import org.elasticsearch.transport.TransportRequest; import org.hamcrest.Matchers; @@ -62,7 +60,7 @@ public class ESUsersRealmTests extends ElasticsearchTestCase { MockUserPasswdStore userPasswdStore = new MockUserPasswdStore("user1", "test123"); MockUserRolesStore userRolesStore = new MockUserRolesStore("user1", "role1", "role2"); ESUsersRealm realm = new ESUsersRealm(settings, userPasswdStore, userRolesStore, restController); - User user = realm.authenticate(new UsernamePasswordToken("user1", "test123".toCharArray())); + User user = realm.authenticate(new UsernamePasswordToken("user1", SecuredStringTests.build("test123"))); assertTrue(userPasswdStore.called); assertTrue(userRolesStore.called); assertThat(user, notNullValue()); @@ -80,13 +78,13 @@ public class ESUsersRealmTests extends ElasticsearchTestCase { ESUsersRealm realm = new ESUsersRealm(settings, userPasswdStore, userRolesStore, restController); TransportRequest request = new TransportRequest() {}; - UsernamePasswordToken.putTokenHeader(request, new UsernamePasswordToken("user1", "test123".toCharArray())); + UsernamePasswordToken.putTokenHeader(request, new UsernamePasswordToken("user1", SecuredStringTests.build("test123"))); UsernamePasswordToken token = realm.token(request); assertThat(token, notNullValue()); assertThat(token.principal(), equalTo("user1")); assertThat(token.credentials(), notNullValue()); - assertThat(new String(token.credentials()), equalTo("test123")); + assertThat(new String(token.credentials().internalChars()), equalTo("test123")); } @@ -102,10 +100,10 @@ public class ESUsersRealmTests extends ElasticsearchTestCase { } @Override - public boolean verifyPassword(String username, char[] password) { + public boolean verifyPassword(String username, SecuredString password) { called = true; assertThat(username, equalTo(this.username)); - assertThat(new String(password), equalTo(this.password)); + assertThat(new String(password.internalChars()), equalTo(this.password)); return true; } } diff --git a/src/test/java/org/elasticsearch/shield/authc/esusers/FileUserPasswdStoreTests.java b/src/test/java/org/elasticsearch/shield/authc/esusers/FileUserPasswdStoreTests.java index f29fbb8790b..21a30418e8a 100644 --- a/src/test/java/org/elasticsearch/shield/authc/esusers/FileUserPasswdStoreTests.java +++ b/src/test/java/org/elasticsearch/shield/authc/esusers/FileUserPasswdStoreTests.java @@ -10,6 +10,7 @@ import org.elasticsearch.common.settings.ImmutableSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import org.elasticsearch.shield.authc.support.Hasher; +import org.elasticsearch.shield.authc.support.SecuredStringTests; import org.elasticsearch.test.ElasticsearchTestCase; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.watcher.ResourceWatcherService; @@ -82,20 +83,20 @@ public class FileUserPasswdStoreTests extends ElasticsearchTestCase { } }); - assertTrue(store.verifyPassword("bcrypt", "test123".toCharArray())); + assertTrue(store.verifyPassword("bcrypt", SecuredStringTests.build("test123"))); watcherService.start(); try (BufferedWriter writer = Files.newBufferedWriter(tmp, Charsets.UTF_8, StandardOpenOption.APPEND)) { writer.newLine(); - writer.append("foobar:" + new String(Hasher.HTPASSWD.hash("barfoo".toCharArray()))); + writer.append("foobar:" + new String(Hasher.HTPASSWD.hash(SecuredStringTests.build("barfoo")))); } if (!latch.await(5, TimeUnit.SECONDS)) { fail("Waited too long for the updated file to be picked up"); } - assertTrue(store.verifyPassword("foobar", "barfoo".toCharArray())); + assertTrue(store.verifyPassword("foobar", SecuredStringTests.build("barfoo"))); } finally { if (watcherService != null) { diff --git a/src/test/java/org/elasticsearch/shield/authc/esusers/tool/ESUsersToolTests.java b/src/test/java/org/elasticsearch/shield/authc/esusers/tool/ESUsersToolTests.java index b58bace9f7e..f993a12eea1 100644 --- a/src/test/java/org/elasticsearch/shield/authc/esusers/tool/ESUsersToolTests.java +++ b/src/test/java/org/elasticsearch/shield/authc/esusers/tool/ESUsersToolTests.java @@ -16,6 +16,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import org.elasticsearch.shield.authc.esusers.FileUserRolesStore; import org.elasticsearch.shield.authc.support.Hasher; +import org.elasticsearch.shield.authc.support.SecuredStringTests; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; @@ -44,7 +45,7 @@ public class ESUsersToolTests extends CliToolTestCase { assertThat(command, instanceOf(ESUsersTool.Useradd.class)); ESUsersTool.Useradd cmd = (ESUsersTool.Useradd) command; assertThat(cmd.username, equalTo("username")); - assertThat(new String(cmd.passwd), equalTo("changeme")); + assertThat(new String(cmd.passwd.internalChars()), equalTo("changeme")); assertThat(cmd.roles, notNullValue()); assertThat(cmd.roles, arrayContaining("r1", "r2", "r3")); } @@ -69,7 +70,7 @@ public class ESUsersToolTests extends CliToolTestCase { assertThat(command, instanceOf(ESUsersTool.Useradd.class)); ESUsersTool.Useradd cmd = (ESUsersTool.Useradd) command; assertThat(cmd.username, equalTo("username")); - assertThat(new String(cmd.passwd), equalTo("changeme")); + assertThat(new String(cmd.passwd.internalChars()), equalTo("changeme")); assertThat(cmd.roles, notNullValue()); assertThat(cmd.roles.length, is(0)); } @@ -84,7 +85,7 @@ public class ESUsersToolTests extends CliToolTestCase { .put("shield.authc.esusers.files.users_roles", userRolesFile) .build(); - ESUsersTool.Useradd cmd = new ESUsersTool.Useradd(new MockTerminal(), "user1", "changeme".toCharArray(), "r1", "r2"); + ESUsersTool.Useradd cmd = new ESUsersTool.Useradd(new MockTerminal(), "user1", SecuredStringTests.build("changeme"), "r1", "r2"); CliTool.ExitStatus status = execute(cmd, settings); assertThat(status, is(CliTool.ExitStatus.OK)); @@ -97,7 +98,7 @@ public class ESUsersToolTests extends CliToolTestCase { String line = lines.get(0); assertThat(line, startsWith("user1:")); String hash = line.substring("user1:".length()); - assertThat(Hasher.HTPASSWD.verify("changeme".toCharArray(), hash.toCharArray()), is(true)); + assertThat(Hasher.HTPASSWD.verify(SecuredStringTests.build("changeme"), hash.toCharArray()), is(true)); assertFileExists(userRolesFile); lines = Files.readLines(userRolesFile, Charsets.UTF_8); @@ -115,7 +116,7 @@ public class ESUsersToolTests extends CliToolTestCase { .put("shield.authc.esusers.files.users_roles", userRolesFile) .build(); - ESUsersTool.Useradd cmd = new ESUsersTool.Useradd(new MockTerminal(), "user1", "changeme".toCharArray(), "r1", "r2"); + ESUsersTool.Useradd cmd = new ESUsersTool.Useradd(new MockTerminal(), "user1", SecuredStringTests.build("changeme"), "r1", "r2"); CliTool.ExitStatus status = execute(cmd, settings); assertThat(status, is(CliTool.ExitStatus.OK)); @@ -131,7 +132,7 @@ public class ESUsersToolTests extends CliToolTestCase { for (String line : lines) { if (line.startsWith("user1")) { String hash = line.substring("user1:".length()); - assertThat(Hasher.HTPASSWD.verify("changeme".toCharArray(), hash.toCharArray()), is(true)); + assertThat(Hasher.HTPASSWD.verify(SecuredStringTests.build("changeme"), hash.toCharArray()), is(true)); } } @@ -150,7 +151,7 @@ public class ESUsersToolTests extends CliToolTestCase { .put("shield.authc.esusers.files.users_roles", userRolesFile) .build(); - ESUsersTool.Useradd cmd = new ESUsersTool.Useradd(new MockTerminal(), "user1", "changeme".toCharArray(), "r1", "r2"); + ESUsersTool.Useradd cmd = new ESUsersTool.Useradd(new MockTerminal(), "user1", SecuredStringTests.build("changeme"), "r1", "r2"); CliTool.ExitStatus status = execute(cmd, settings); assertThat(status, is(CliTool.ExitStatus.CODE_ERROR)); @@ -246,7 +247,7 @@ public class ESUsersToolTests extends CliToolTestCase { assertThat(command, instanceOf(ESUsersTool.Passwd.class)); ESUsersTool.Passwd cmd = (ESUsersTool.Passwd) command; assertThat(cmd.username, equalTo("user1")); - assertThat(new String(cmd.passwd), equalTo("changeme")); + assertThat(new String(cmd.passwd.internalChars()), equalTo("changeme")); } @Test @@ -273,7 +274,7 @@ public class ESUsersToolTests extends CliToolTestCase { assertThat(command, instanceOf(ESUsersTool.Passwd.class)); ESUsersTool.Passwd cmd = (ESUsersTool.Passwd) command; assertThat(cmd.username, equalTo("user1")); - assertThat(new String(cmd.passwd), equalTo("changeme")); + assertThat(new String(cmd.passwd.internalChars()), equalTo("changeme")); assertThat(secretRequested.get(), is(true)); } @@ -295,7 +296,7 @@ public class ESUsersToolTests extends CliToolTestCase { String line = lines.get(0); assertThat(line, startsWith("user1:")); String hash = line.substring("user1:".length()); - assertThat(Hasher.HTPASSWD.verify("changeme".toCharArray(), hash.toCharArray()), is(true)); + assertThat(Hasher.HTPASSWD.verify(SecuredStringTests.build("changeme"), hash.toCharArray()), is(true)); } @Test diff --git a/src/test/java/org/elasticsearch/shield/authc/ldap/ActiveDirectoryFactoryTests.java b/src/test/java/org/elasticsearch/shield/authc/ldap/ActiveDirectoryFactoryTests.java index 54ccf35a357..dd50fd70831 100644 --- a/src/test/java/org/elasticsearch/shield/authc/ldap/ActiveDirectoryFactoryTests.java +++ b/src/test/java/org/elasticsearch/shield/authc/ldap/ActiveDirectoryFactoryTests.java @@ -7,6 +7,7 @@ package org.elasticsearch.shield.authc.ldap; import org.elasticsearch.common.settings.ImmutableSettings; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.shield.authc.support.SecuredStringTests; import org.elasticsearch.test.ElasticsearchTestCase; import org.junit.Ignore; import org.junit.Test; @@ -29,7 +30,7 @@ public class ActiveDirectoryFactoryTests extends ElasticsearchTestCase { String userName = "ironman"; - LdapConnection ldap = connectionFactory.bind(userName, PASSWORD.toCharArray()); + LdapConnection ldap = connectionFactory.bind(userName, SecuredStringTests.build(PASSWORD)); String userDN = ldap.getAuthenticatedUserDn(); //System.out.println("userPassword check:"+ldap.checkPassword(userDn, userPass)); @@ -48,7 +49,7 @@ public class ActiveDirectoryFactoryTests extends ElasticsearchTestCase { LdapTest.buildLdapSettings(AD_LDAP_URL, userTemplate, groupSearchBase, isSubTreeSearch)); String user = "Tony Stark"; - LdapConnection ldap = connectionFactory.bind(user, PASSWORD.toCharArray()); + LdapConnection ldap = connectionFactory.bind(user, SecuredStringTests.build(PASSWORD)); List groups = ldap.getGroupsFromUserAttrs(ldap.getAuthenticatedUserDn()); List groups2 = ldap.getGroupsFromSearch(ldap.getAuthenticatedUserDn()); diff --git a/src/test/java/org/elasticsearch/shield/authc/ldap/LdapConnectionTests.java b/src/test/java/org/elasticsearch/shield/authc/ldap/LdapConnectionTests.java index db14cd33e12..55fc6316c36 100644 --- a/src/test/java/org/elasticsearch/shield/authc/ldap/LdapConnectionTests.java +++ b/src/test/java/org/elasticsearch/shield/authc/ldap/LdapConnectionTests.java @@ -4,7 +4,12 @@ * you may not use this file except in compliance with the Elastic License. */ package org.elasticsearch.shield.authc.ldap; - +import org.elasticsearch.common.settings.ImmutableSettings; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.shield.authc.support.SecuredString; +import org.elasticsearch.shield.authc.support.SecuredStringTests; +import org.elasticsearch.test.ElasticsearchTestCase; +import org.junit.Rule; import org.junit.Test; import java.util.List; @@ -28,7 +33,7 @@ public class LdapConnectionTests extends LdapTest { buildLdapSettings(ldapUrls, userTemplates, groupSearchBase, isSubTreeSearch)); String user = "Horatio Hornblower"; - char[] userPass = "pass".toCharArray(); + SecuredString userPass = SecuredStringTests.build("pass"); LdapConnection ldap = connectionFactory.bind(user, userPass); Map attrs = ldap.getUserAttrs(ldap.getAuthenticatedUserDn()); @@ -51,8 +56,8 @@ public class LdapConnectionTests extends LdapTest { buildLdapSettings(ldapUrl, userTemplates, groupSearchBase, isSubTreeSearch)); String user = "Horatio Hornblower"; - char[] userPass = "pass".toCharArray(); - ldapFac.bind(user, userPass); + SecuredString userPass = SecuredStringTests.build("pass"); + LdapConnection ldap = ldapFac.bind(user, userPass); } @Test @@ -65,7 +70,7 @@ public class LdapConnectionTests extends LdapTest { buildLdapSettings(apacheDsRule.getUrl(), userTemplate, groupSearchBase, isSubTreeSearch)); String user = "Horatio Hornblower"; - char[] userPass = "pass".toCharArray(); + SecuredString userPass = SecuredStringTests.build("pass"); LdapConnection ldap = ldapFac.bind(user, userPass); List groups = ldap.getGroupsFromSearch(ldap.getAuthenticatedUserDn()); @@ -82,7 +87,7 @@ public class LdapConnectionTests extends LdapTest { buildLdapSettings(apacheDsRule.getUrl(), userTemplate, groupSearchBase, isSubTreeSearch)); String user = "Horatio Hornblower"; - LdapConnection ldap = ldapFac.bind(user, "pass".toCharArray()); + LdapConnection ldap = ldapFac.bind(user, SecuredStringTests.build("pass")); List groups = ldap.getGroupsFromSearch(ldap.getAuthenticatedUserDn()); System.out.println("groups:"+groups); diff --git a/src/test/java/org/elasticsearch/shield/authc/ldap/LdapRealmTest.java b/src/test/java/org/elasticsearch/shield/authc/ldap/LdapRealmTest.java index 20a7f5b9e16..7da5caa0487 100644 --- a/src/test/java/org/elasticsearch/shield/authc/ldap/LdapRealmTest.java +++ b/src/test/java/org/elasticsearch/shield/authc/ldap/LdapRealmTest.java @@ -10,6 +10,8 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import org.elasticsearch.rest.RestController; import org.elasticsearch.shield.User; +import org.elasticsearch.shield.authc.support.SecuredString; +import org.elasticsearch.shield.authc.support.SecuredStringTests; import org.elasticsearch.shield.authc.support.UsernamePasswordToken; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.watcher.ResourceWatcherService; @@ -53,7 +55,7 @@ public class LdapRealmTest extends LdapTest { StandardLdapConnectionFactory ldapFactory = new StandardLdapConnectionFactory(settings); LdapRealm ldap = new LdapRealm(buildNonCachingSettings(), ldapFactory, buildGroupAsRoleMapper(), restController); - User user = ldap.authenticate(new UsernamePasswordToken(VALID_USERNAME, PASSWORD.toCharArray())); + User user = ldap.authenticate(new UsernamePasswordToken(VALID_USERNAME, SecuredStringTests.build(PASSWORD))); assertThat( user, notNullValue()); assertThat(user.roles(), arrayContaining("HMS Victory")); } @@ -68,12 +70,11 @@ public class LdapRealmTest extends LdapTest { LdapRealm ldap = new LdapRealm(buildNonCachingSettings(), ldapFactory, buildGroupAsRoleMapper(), restController); - User user = ldap.authenticate(new UsernamePasswordToken(VALID_USERNAME, PASSWORD.toCharArray())); + User user = ldap.authenticate(new UsernamePasswordToken(VALID_USERNAME, SecuredStringTests.build(PASSWORD))); assertThat( user, notNullValue()); - assertThat( user.roles(), arrayContaining("HMS Victory")); + assertThat(user.roles(), arrayContaining("HMS Victory")); } - @Ignore //this is still failing. not sure why. @Test public void testAuthenticate_caching(){ String groupSearchBase = "o=sevenSeas"; @@ -84,11 +85,11 @@ public class LdapRealmTest extends LdapTest { ldapFactory = spy(ldapFactory); LdapRealm ldap = new LdapRealm( buildCachingSettings(), ldapFactory, buildGroupAsRoleMapper(), restController); - User user = ldap.authenticate( new UsernamePasswordToken(VALID_USERNAME, PASSWORD.toCharArray())); - user = ldap.authenticate( new UsernamePasswordToken(VALID_USERNAME, PASSWORD.toCharArray())); + User user = ldap.authenticate( new UsernamePasswordToken(VALID_USERNAME, SecuredStringTests.build(PASSWORD))); + user = ldap.authenticate( new UsernamePasswordToken(VALID_USERNAME, SecuredStringTests.build(PASSWORD))); //verify one and only one bind -> caching is working - verify(ldapFactory, times(1)).bind(anyString(), any(char[].class)); + verify(ldapFactory, times(1)).bind(anyString(), any(SecuredString.class)); } @Test @@ -101,11 +102,11 @@ public class LdapRealmTest extends LdapTest { ldapFactory = spy(ldapFactory); LdapRealm ldap = new LdapRealm( buildNonCachingSettings(), ldapFactory, buildGroupAsRoleMapper(), restController); - User user = ldap.authenticate( new UsernamePasswordToken(VALID_USERNAME, PASSWORD.toCharArray())); - user = ldap.authenticate( new UsernamePasswordToken(VALID_USERNAME, PASSWORD.toCharArray())); + User user = ldap.authenticate( new UsernamePasswordToken(VALID_USERNAME, SecuredStringTests.build(PASSWORD))); + user = ldap.authenticate( new UsernamePasswordToken(VALID_USERNAME, SecuredStringTests.build(PASSWORD))); //verify two and only two binds -> caching is disabled - verify(ldapFactory, times(2)).bind(anyString(), any(char[].class)); + verify(ldapFactory, times(2)).bind(anyString(), any(SecuredString.class)); } @Ignore @@ -119,7 +120,7 @@ public class LdapRealmTest extends LdapTest { LdapRealm ldap = new LdapRealm( buildNonCachingSettings(), ldapFactory, buildGroupAsRoleMapper(), restController); - User user = ldap.authenticate( new UsernamePasswordToken("george", "R))Tr0x".toCharArray())); + User user = ldap.authenticate( new UsernamePasswordToken("george", SecuredStringTests.build("R))Tr0x"))); assertThat( user, notNullValue()); assertThat( user.roles(), hasItemInArray("upchuckers")); @@ -136,7 +137,7 @@ public class LdapRealmTest extends LdapTest { ActiveDirectoryConnectionFactory ldapFactory = new ActiveDirectoryConnectionFactory( settings ); LdapRealm ldap = new LdapRealm( buildNonCachingSettings(), ldapFactory, buildGroupAsRoleMapper(), restController); - User user = ldap.authenticate( new UsernamePasswordToken("george", "R))Tr0x".toCharArray())); + User user = ldap.authenticate( new UsernamePasswordToken("george", SecuredStringTests.build("R))Tr0x"))); assertThat( user, notNullValue()); assertThat( user.roles(), hasItemInArray("upchuckers")); @@ -152,8 +153,8 @@ public class LdapRealmTest extends LdapTest { private Settings buildCachingSettings() { return ImmutableSettings.builder() - .put("shield.authc.ldap."+LdapRealm.CACHE_TTL, 1) - .put("shield.authc.ldap."+LdapRealm.CACHE_MAX_USERS, 10) + .put("shield.authc.ldap."+LdapRealm.CACHE_TTL, 100000000) + .put("shield.authc.ldap." + LdapRealm.CACHE_MAX_USERS, 10) .build(); } diff --git a/src/test/java/org/elasticsearch/shield/authc/support/CachingUsernamePasswordRealmTests.java b/src/test/java/org/elasticsearch/shield/authc/support/CachingUsernamePasswordRealmTests.java index 36801090a76..797f54d83df 100644 --- a/src/test/java/org/elasticsearch/shield/authc/support/CachingUsernamePasswordRealmTests.java +++ b/src/test/java/org/elasticsearch/shield/authc/support/CachingUsernamePasswordRealmTests.java @@ -32,7 +32,7 @@ public class CachingUsernamePasswordRealmTests extends ElasticsearchTestCase { @Test public void testCache(){ AlwaysAuthenticateCachingRealm realm = new AlwaysAuthenticateCachingRealm(); - char[] pass = "pass".toCharArray(); + SecuredString pass = SecuredStringTests.build("pass"); realm.authenticate(new UsernamePasswordToken("a", pass)); realm.authenticate(new UsernamePasswordToken("b", pass)); realm.authenticate(new UsernamePasswordToken("c", pass)); @@ -50,8 +50,8 @@ public class CachingUsernamePasswordRealmTests extends ElasticsearchTestCase { AlwaysAuthenticateCachingRealm realm = new AlwaysAuthenticateCachingRealm(); String user = "testUser"; - char[] pass1 = "pass".toCharArray(); - char[] pass2 = "password".toCharArray(); + SecuredString pass1 = SecuredStringTests.build("pass"); + SecuredString pass2 = SecuredStringTests.build("password"); realm.authenticate(new UsernamePasswordToken(user, pass1)); realm.authenticate(new UsernamePasswordToken(user, pass1)); diff --git a/src/test/java/org/elasticsearch/shield/authc/support/HasherTests.java b/src/test/java/org/elasticsearch/shield/authc/support/HasherTests.java index ff311055480..4a78b36910e 100644 --- a/src/test/java/org/elasticsearch/shield/authc/support/HasherTests.java +++ b/src/test/java/org/elasticsearch/shield/authc/support/HasherTests.java @@ -16,7 +16,7 @@ public class HasherTests extends ElasticsearchTestCase { @Test public void testHtpasswdToolGenerated() throws Exception { Hasher hasher = Hasher.HTPASSWD; - char[] passwd = "test123".toCharArray(); + SecuredString passwd = SecuredStringTests.build("test123"); assertTrue(hasher.verify(passwd, "$2a$05$zxnP0vdREMxnEpkLCDI2OuSaSk/QEKA2.A42iOpI6U2u.RLLOWm1e".toCharArray())); assertTrue(hasher.verify(passwd, "$2a$10$FMhmFjwU5.qxQ/BsEciS9OqcJVkFMgXMo4uH5CelOR1j4N9zIv67e".toCharArray())); assertTrue(hasher.verify(passwd, "$apr1$R3DdqiAZ$aljIkaIVPSarmDMlJUBBP.".toCharArray())); @@ -28,7 +28,7 @@ public class HasherTests extends ElasticsearchTestCase { @Test public void testHtpasswdSelfGenerated() throws Exception { Hasher hasher = Hasher.HTPASSWD; - char[] passwd = "test123".toCharArray(); + SecuredString passwd = SecuredStringTests.build("test123"); assertTrue(hasher.verify(passwd, hasher.hash(passwd))); } } diff --git a/src/test/java/org/elasticsearch/shield/authc/support/SecuredStringTests.java b/src/test/java/org/elasticsearch/shield/authc/support/SecuredStringTests.java new file mode 100644 index 00000000000..7229dab1690 --- /dev/null +++ b/src/test/java/org/elasticsearch/shield/authc/support/SecuredStringTests.java @@ -0,0 +1,97 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.shield.authc.support; + +import com.carrotsearch.ant.tasks.junit4.dependencies.com.google.common.base.Charsets; +import org.junit.Test; +import static org.junit.Assert.*; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.*; + +public class SecuredStringTests { + public static SecuredString build(String password){ + return new SecuredString(password.toCharArray()); + } + + @Test + public void testAccessAfterClear(){ + SecuredString password = new SecuredString("password".toCharArray()); + SecuredString password2 = new SecuredString("password".toCharArray()); + + password.clear(); + + try { + password.internalChars(); + fail(); + } catch(Exception e){} + + try { + password.length(); + fail(); + } catch(Exception e){} + + try { + password.charAt(0); + fail(); + } catch(Exception e){} + + try { + password.concat("_suffix"); + fail(); + } catch(Exception e){} + + assertNotEquals(password, password2); + } + + @Test + public void testEqualsHashCode(){ + SecuredString password = new SecuredString("password".toCharArray()); + SecuredString password2 = new SecuredString("password".toCharArray()); + + assertEquals(password, password2); + assertEquals(password.hashCode(), password2.hashCode()); + } + + @Test + public void testsEqualsCharSequence(){ + SecuredString password = new SecuredString("password".toCharArray()); + StringBuffer password2 = new StringBuffer("password"); + String password3 = "password"; + + assertEquals(password, password2); + assertEquals(password, password3); + } + + @Test + public void testConcat() { + SecuredString password = new SecuredString("password".toCharArray()); + SecuredString password2 = new SecuredString("password".toCharArray()); + + SecuredString password3 = password.concat(password2); + assertThat(password3.length(), equalTo(password.length() + password2.length())); + assertThat(password3.internalChars(), equalTo("passwordpassword".toCharArray())); + } + + @Test + public void testSubsequence(){ + SecuredString password = new SecuredString("password".toCharArray()); + SecuredString password2 = password.subSequence(4, 8); + SecuredString password3 = password.subSequence(0, 4); + + assertThat(password2.internalChars(), equalTo("word".toCharArray())); + assertThat(password3.internalChars(), equalTo("pass".toCharArray())); + assertThat("ensure original is unmodified", password.internalChars(), equalTo("password".toCharArray())); + } + + @Test + public void testUFT8(){ + String password = "эластичный поиск-弾性検索"; + SecuredString securePass = new SecuredString(password.toCharArray()); + byte[] utf8 = securePass.utf8Bytes(); + String password2 = new String(utf8, Charsets.UTF_8); + assertThat(password2, equalTo(password)); + } +} diff --git a/src/test/java/org/elasticsearch/shield/authc/support/UsernamePasswordTokenTests.java b/src/test/java/org/elasticsearch/shield/authc/support/UsernamePasswordTokenTests.java index 346f105fc08..84e17fd2b23 100644 --- a/src/test/java/org/elasticsearch/shield/authc/support/UsernamePasswordTokenTests.java +++ b/src/test/java/org/elasticsearch/shield/authc/support/UsernamePasswordTokenTests.java @@ -31,7 +31,7 @@ public class UsernamePasswordTokenTests extends ElasticsearchTestCase { @Test public void testPutToken() throws Exception { TransportRequest request = new TransportRequest() {}; - UsernamePasswordToken.putTokenHeader(request, new UsernamePasswordToken("user1", "test123".toCharArray())); + UsernamePasswordToken.putTokenHeader(request, new UsernamePasswordToken("user1", SecuredStringTests.build("test123"))); String header = request.getHeader(UsernamePasswordToken.BASIC_AUTH_HEADER); assertThat(header, notNullValue()); assertTrue(header.startsWith("Basic ")); @@ -53,7 +53,7 @@ public class UsernamePasswordTokenTests extends ElasticsearchTestCase { UsernamePasswordToken token = UsernamePasswordToken.extractToken(request, null); assertThat(token, notNullValue()); assertThat(token.principal(), equalTo("user1")); - assertThat(new String(token.credentials()), equalTo("test123")); + assertThat(new String(token.credentials().internalChars()), equalTo("test123")); } @Test @@ -87,8 +87,8 @@ public class UsernamePasswordTokenTests extends ElasticsearchTestCase { @Test public void testExtractTokenRest() throws Exception { RestRequest request = mock(RestRequest.class); - UsernamePasswordToken token = new UsernamePasswordToken("username", "changeme".toCharArray()); - when(request.header(UsernamePasswordToken.BASIC_AUTH_HEADER)).thenReturn(UsernamePasswordToken.basicAuthHeaderValue("username", "changeme".toCharArray())); + UsernamePasswordToken token = new UsernamePasswordToken("username", SecuredStringTests.build("changeme")); + when(request.header(UsernamePasswordToken.BASIC_AUTH_HEADER)).thenReturn(UsernamePasswordToken.basicAuthHeaderValue("username", SecuredStringTests.build("changeme"))); assertThat(UsernamePasswordToken.extractToken(request, null), equalTo(token)); } diff --git a/src/test/java/org/elasticsearch/shield/test/ShieldIntegrationTest.java b/src/test/java/org/elasticsearch/shield/test/ShieldIntegrationTest.java index bacba8a303c..578a2fedc23 100644 --- a/src/test/java/org/elasticsearch/shield/test/ShieldIntegrationTest.java +++ b/src/test/java/org/elasticsearch/shield/test/ShieldIntegrationTest.java @@ -16,6 +16,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.transport.InetSocketTransportAddress; import org.elasticsearch.common.transport.TransportAddress; import org.elasticsearch.plugins.PluginsService; +import org.elasticsearch.shield.authc.support.SecuredString; import org.elasticsearch.shield.plugin.ShieldPlugin; import org.elasticsearch.shield.transport.netty.NettySecuredTransport; import org.elasticsearch.test.ElasticsearchIntegrationTest; @@ -62,7 +63,7 @@ public abstract class ShieldIntegrationTest extends ElasticsearchIntegrationTest File folder = newFolder(); ImmutableSettings.Builder builder = ImmutableSettings.builder() - .put("request.headers.Authorization", basicAuthHeaderValue(getClientUsername(), getClientPassword().toCharArray())) + .put("request.headers.Authorization", basicAuthHeaderValue(getClientUsername(), getClientPassword())) .put("discovery.zen.ping.multicast.enabled", false) .put("discovery.type", "zen") .put("node.mode", "network") @@ -85,7 +86,7 @@ public abstract class ShieldIntegrationTest extends ElasticsearchIntegrationTest @Override protected Settings transportClientSettings() { return ImmutableSettings.builder() - .put("request.headers.Authorization", basicAuthHeaderValue(getClientUsername(), getClientPassword().toCharArray())) + .put("request.headers.Authorization", basicAuthHeaderValue(getClientUsername(), getClientPassword())) .put(TransportModule.TRANSPORT_TYPE_KEY, NettySecuredTransport.class.getName()) .put("plugins." + PluginsService.LOAD_PLUGIN_FROM_CLASSPATH, false) .put("node.mode", "network") @@ -118,8 +119,8 @@ public abstract class ShieldIntegrationTest extends ElasticsearchIntegrationTest return DEFAULT_USER_NAME; } - protected String getClientPassword() { - return DEFAULT_PASSWORD; + protected SecuredString getClientPassword() { + return new SecuredString(DEFAULT_PASSWORD.toCharArray()); } protected Settings getSSLSettingsForStore(String resourcePathToStore, String password) { diff --git a/src/test/java/org/elasticsearch/shield/transport/ssl/SslIntegrationTests.java b/src/test/java/org/elasticsearch/shield/transport/ssl/SslIntegrationTests.java index 928361956d3..6e28001c73d 100644 --- a/src/test/java/org/elasticsearch/shield/transport/ssl/SslIntegrationTests.java +++ b/src/test/java/org/elasticsearch/shield/transport/ssl/SslIntegrationTests.java @@ -97,7 +97,7 @@ public class SslIntegrationTests extends ShieldIntegrationTest { .put("name", "programmatic_node") .put("cluster.name", internalCluster().getClusterName()) - .put("request.headers.Authorization", basicAuthHeaderValue(getClientUsername(), getClientPassword().toCharArray())) + .put("request.headers.Authorization", basicAuthHeaderValue(getClientUsername(), getClientPassword())) .put(TransportModule.TRANSPORT_TYPE_KEY, NettySecuredTransport.class.getName()) .put("plugins." + PluginsService.LOAD_PLUGIN_FROM_CLASSPATH, false) @@ -122,7 +122,7 @@ public class SslIntegrationTests extends ShieldIntegrationTest { .put("discovery.type", "zen") .putArray("discovery.zen.ping.unicast.hosts", getUnicastHostAddress()) - .put("request.headers.Authorization", basicAuthHeaderValue(getClientUsername(), getClientPassword().toCharArray())) + .put("request.headers.Authorization", basicAuthHeaderValue(getClientUsername(), getClientPassword())) .put(TransportModule.TRANSPORT_TYPE_KEY, NettySecuredTransport.class.getName()) .put("plugins." + PluginsService.LOAD_PLUGIN_FROM_CLASSPATH, false) .put("shield.transport.n2n.ip_filter.file", writeFile(newFolder(), "ip_filter.yml", ShieldIntegrationTest.CONFIG_IPFILTER_ALLOW_ALL)) @@ -173,7 +173,7 @@ public class SslIntegrationTests extends ShieldIntegrationTest { String url = String.format(Locale.ROOT, "https://%s:%s/", InetAddresses.toUriString(inetSocketTransportAddress.address().getAddress()), inetSocketTransportAddress.address().getPort()); HttpURLConnection connection = (HttpURLConnection) new URL(url).openConnection(); - connection.setRequestProperty("Authorization", UsernamePasswordToken.basicAuthHeaderValue(DEFAULT_USER_NAME, DEFAULT_PASSWORD.toCharArray())); + connection.setRequestProperty("Authorization", UsernamePasswordToken.basicAuthHeaderValue(getClientUsername(), getClientPassword())); connection.connect(); assertThat(connection.getResponseCode(), is(200)); diff --git a/src/test/java/org/elasticsearch/shield/transport/ssl/SslRequireAuthTests.java b/src/test/java/org/elasticsearch/shield/transport/ssl/SslRequireAuthTests.java index da2e7eab1d8..188372da551 100644 --- a/src/test/java/org/elasticsearch/shield/transport/ssl/SslRequireAuthTests.java +++ b/src/test/java/org/elasticsearch/shield/transport/ssl/SslRequireAuthTests.java @@ -130,7 +130,7 @@ public class SslRequireAuthTests extends ShieldIntegrationTest { String url = String.format(Locale.ROOT, "https://%s:%s/", InetAddresses.toUriString(inetSocketTransportAddress.address().getAddress()), inetSocketTransportAddress.address().getPort()); HttpURLConnection connection = (HttpURLConnection) new URL(url).openConnection(); - connection.setRequestProperty("Authorization", UsernamePasswordToken.basicAuthHeaderValue(DEFAULT_USER_NAME, DEFAULT_PASSWORD.toCharArray())); + connection.setRequestProperty("Authorization", UsernamePasswordToken.basicAuthHeaderValue(getClientUsername(), getClientPassword())); connection.connect(); assertThat(connection.getResponseCode(), is(200)); diff --git a/src/test/java/org/elasticsearch/test/ShieldRestTests.java b/src/test/java/org/elasticsearch/test/ShieldRestTests.java index 822eb7fc5a6..098866dd3e8 100644 --- a/src/test/java/org/elasticsearch/test/ShieldRestTests.java +++ b/src/test/java/org/elasticsearch/test/ShieldRestTests.java @@ -14,6 +14,7 @@ import org.elasticsearch.common.os.OsUtils; import org.elasticsearch.common.settings.ImmutableSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.plugins.PluginsService; +import org.elasticsearch.shield.authc.support.SecuredStringTests; import org.elasticsearch.shield.key.InternalKeyService; import org.elasticsearch.shield.plugin.ShieldPlugin; import org.elasticsearch.shield.transport.netty.NettySecuredTransport; @@ -82,7 +83,7 @@ public class ShieldRestTests extends ElasticsearchRestTests { ImmutableSettings.Builder builder = ImmutableSettings.builder() .put(InternalKeyService.FILE_SETTING, keyFile) - .put("request.headers.Authorization", basicAuthHeaderValue(DEFAULT_USER_NAME, DEFAULT_PASSWORD.toCharArray())) + .put("request.headers.Authorization", basicAuthHeaderValue(DEFAULT_USER_NAME, SecuredStringTests.build(DEFAULT_PASSWORD))) .put("discovery.zen.ping.multicast.enabled", false) .put("discovery.type", "zen") .put("node.mode", "network") @@ -123,7 +124,7 @@ public class ShieldRestTests extends ElasticsearchRestTests { File folder = createFolder(); return ImmutableSettings.builder() - .put("request.headers.Authorization", basicAuthHeaderValue(DEFAULT_USER_NAME, DEFAULT_PASSWORD.toCharArray())) + .put("request.headers.Authorization", basicAuthHeaderValue(DEFAULT_USER_NAME, SecuredStringTests.build(DEFAULT_PASSWORD))) .put(TransportModule.TRANSPORT_TYPE_KEY, NettySecuredTransport.class.getName()) .put("plugins." + PluginsService.LOAD_PLUGIN_FROM_CLASSPATH, false) .put("node.mode", "network") @@ -154,7 +155,7 @@ public class ShieldRestTests extends ElasticsearchRestTests { @Override protected Settings restClientSettings() { return ImmutableSettings.builder() - .put("request.headers.Authorization", basicAuthHeaderValue(DEFAULT_USER_NAME, DEFAULT_PASSWORD.toCharArray())).build(); + .put("request.headers.Authorization", basicAuthHeaderValue(DEFAULT_USER_NAME, SecuredStringTests.build(DEFAULT_PASSWORD))).build(); } /* static helper methods for the global test class */