diff --git a/jetty-jaspi/src/main/java/org/eclipse/jetty/security/jaspi/modules/DigestAuthModule.java b/jetty-jaspi/src/main/java/org/eclipse/jetty/security/jaspi/modules/DigestAuthModule.java index 7c9138a4d5a..1b9b6f6b72f 100644 --- a/jetty-jaspi/src/main/java/org/eclipse/jetty/security/jaspi/modules/DigestAuthModule.java +++ b/jetty-jaspi/src/main/java/org/eclipse/jetty/security/jaspi/modules/DigestAuthModule.java @@ -336,7 +336,7 @@ public class DigestAuthModule extends BaseAuthModule byte[] digest = md.digest(); // check digest - return (TypeUtil.toString(digest, 16).equalsIgnoreCase(response)); + return stringEquals(TypeUtil.toString(digest, 16).toLowerCase(), response == null ? null : response.toLowerCase()); } catch (Exception e) { @@ -351,6 +351,5 @@ public class DigestAuthModule extends BaseAuthModule { return username + "," + response; } - } } diff --git a/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/DigestAuthenticator.java b/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/DigestAuthenticator.java index e81544cf805..160238767eb 100644 --- a/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/DigestAuthenticator.java +++ b/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/DigestAuthenticator.java @@ -383,7 +383,7 @@ public class DigestAuthenticator extends LoginAuthenticator byte[] digest = md.digest(); // check digest - return (TypeUtil.toString(digest, 16).equalsIgnoreCase(response)); + return stringEquals(TypeUtil.toString(digest, 16).toLowerCase(), response == null ? null : response.toLowerCase()); } catch (Exception e) { diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/security/Credential.java b/jetty-util/src/main/java/org/eclipse/jetty/util/security/Credential.java index 6fe84042366..8b825a37a93 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/security/Credential.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/security/Credential.java @@ -26,10 +26,7 @@ import java.util.ServiceLoader; import org.eclipse.jetty.util.TypeUtil; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; -import org.eclipse.jetty.util.security.Credential.Crypt; -import org.eclipse.jetty.util.security.Credential.MD5; -/* ------------------------------------------------------------ */ /** * Credentials. The Credential class represents an abstract mechanism for checking authentication credentials. A credential instance either represents a secret, * or some data that could only be derived from knowing the secret. @@ -40,18 +37,13 @@ import org.eclipse.jetty.util.security.Credential.MD5; * This class includes an implementation for unix Crypt an MD5 digest. * * @see Password - * */ public abstract class Credential implements Serializable { - + private static final long serialVersionUID = -7760551052768181572L; + private static final Logger LOG = Log.getLogger(Credential.class); private static final ServiceLoader CREDENTIAL_PROVIDER_LOADER = ServiceLoader.load(CredentialProvider.class); - private static final Logger LOG = Log.getLogger(Credential.class); - - private static final long serialVersionUID = -7760551052768181572L; - - /* ------------------------------------------------------------ */ /** * Check a credential * @@ -62,7 +54,6 @@ public abstract class Credential implements Serializable */ public abstract boolean check(Object credentials); - /* ------------------------------------------------------------ */ /** * Get a credential from a String. If the credential String starts with a known Credential type (eg "CRYPT:" or "MD5:" ) then a Credential of that type is * returned. Otherwise, it tries to find a credential provider whose prefix matches with the start of the credential String. Else the credential is assumed @@ -94,15 +85,51 @@ public abstract class Credential implements Serializable return new Password(credential); } - /* ------------------------------------------------------------ */ + /** + *

Utility method that replaces String.equals() to avoid timing attacks.

+ * + * @param s1 the first string to compare + * @param s2 the second string to compare + * @return whether the two strings are equal + */ + protected static boolean stringEquals(String s1, String s2) + { + if (s1 == s2) + return true; + if (s1 == null || s2 == null || s1.length() != s2.length()) + return false; + boolean result = false; + for (int i = 0; i < s1.length(); i++) + result |= s1.charAt(i) == s2.charAt(i); + return result; + } + + /** + *

Utility method that replaces Arrays.equals() to avoid timing attacks.

+ * + * @param b1 the first byte array to compare + * @param b2 the second byte array to compare + * @return whether the two byte arrays are equal + */ + protected static boolean byteEquals(byte[] b1, byte[] b2) + { + if (b1 == b2) + return true; + if (b1 == null || b2 == null || b1.length != b2.length) + return false; + boolean result = false; + for (int i = 0; i < b1.length; i++) + result |= b1[i] == b2[i]; + return result; + } + /** * Unix Crypt Credentials */ public static class Crypt extends Credential { private static final long serialVersionUID = -2027792997664744210L; - - public static final String __TYPE = "CRYPT:"; + private static final String __TYPE = "CRYPT:"; private final String _cooked; @@ -118,9 +145,7 @@ public abstract class Credential implements Serializable credentials = new String((char[])credentials); if (!(credentials instanceof String) && !(credentials instanceof Password)) LOG.warn("Can't check " + credentials.getClass() + " against CRYPT"); - - String passwd = credentials.toString(); - return _cooked.equals(UnixCrypt.crypt(passwd,_cooked)); + return stringEquals(_cooked, UnixCrypt.crypt(credentials.toString(),_cooked)); } @Override @@ -128,59 +153,49 @@ public abstract class Credential implements Serializable { if (!(credential instanceof Crypt)) return false; - Crypt c = (Crypt)credential; - - return _cooked.equals(c._cooked); + return stringEquals(_cooked, c._cooked); } public static String crypt(String user, String pw) { - return "CRYPT:" + UnixCrypt.crypt(pw,user); + return __TYPE + UnixCrypt.crypt(pw, user); } } - /* ------------------------------------------------------------ */ /** * MD5 Credentials */ public static class MD5 extends Credential { private static final long serialVersionUID = 5533846540822684240L; - - public static final String __TYPE = "MD5:"; - - public static final Object __md5Lock = new Object(); - + private static final String __TYPE = "MD5:"; + private static final Object __md5Lock = new Object(); private static MessageDigest __md; private final byte[] _digest; - /* ------------------------------------------------------------ */ MD5(String digest) { - digest = digest.startsWith(__TYPE)?digest.substring(__TYPE.length()):digest; - _digest = TypeUtil.parseBytes(digest,16); + digest = digest.startsWith(__TYPE) ? digest.substring(__TYPE.length()) : digest; + _digest = TypeUtil.parseBytes(digest, 16); } - /* ------------------------------------------------------------ */ public byte[] getDigest() { return _digest; } - /* ------------------------------------------------------------ */ @Override public boolean check(Object credentials) { try { - byte[] digest = null; - if (credentials instanceof char[]) credentials = new String((char[])credentials); if (credentials instanceof Password || credentials instanceof String) { + byte[] digest; synchronized (__md5Lock) { if (__md == null) @@ -189,16 +204,11 @@ public abstract class Credential implements Serializable __md.update(credentials.toString().getBytes(StandardCharsets.ISO_8859_1)); digest = __md.digest(); } - if (digest == null || digest.length != _digest.length) - return false; - boolean digestMismatch = false; - for (int i = 0; i < digest.length; i++) - digestMismatch |= (digest[i] != _digest[i]); - return !digestMismatch; + return byteEquals(_digest, digest); } else if (credentials instanceof MD5) { - return equals((MD5)credentials); + return equals(credentials); } else if (credentials instanceof Credential) { @@ -223,20 +233,10 @@ public abstract class Credential implements Serializable public boolean equals(Object obj) { if (obj instanceof MD5) - { - MD5 md5 = (MD5)obj; - if (_digest.length != md5._digest.length) - return false; - boolean digestMismatch = false; - for (int i = 0; i < _digest.length; i++) - digestMismatch |= (_digest[i] != md5._digest[i]); - return !digestMismatch; - } - + return byteEquals(_digest, ((MD5)obj)._digest); return false; } - /* ------------------------------------------------------------ */ public static String digest(String password) { try @@ -262,7 +262,7 @@ public abstract class Credential implements Serializable digest = __md.digest(); } - return __TYPE + TypeUtil.toString(digest,16); + return __TYPE + TypeUtil.toString(digest, 16); } catch (Exception e) { diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/security/Password.java b/jetty-util/src/main/java/org/eclipse/jetty/util/security/Password.java index 6f1a88981b5..48391e9ebe2 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/security/Password.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/security/Password.java @@ -20,7 +20,6 @@ package org.eclipse.jetty.util.security; import java.io.IOException; import java.nio.charset.StandardCharsets; -import java.util.Arrays; import java.util.Locale; import org.eclipse.jetty.util.log.Log; @@ -96,15 +95,20 @@ public class Password extends Credential @Override public boolean check(Object credentials) { - if (this == credentials) return true; + if (this == credentials) + return true; - if (credentials instanceof Password) return credentials.equals(_pw); + if (credentials instanceof Password) + return credentials.equals(_pw); - if (credentials instanceof String) return credentials.equals(_pw); + if (credentials instanceof String) + return stringEquals(_pw, (String)credentials); - if (credentials instanceof char[]) return Arrays.equals(_pw.toCharArray(), (char[]) credentials); + if (credentials instanceof char[]) + return stringEquals(_pw, new String((char[])credentials)); - if (credentials instanceof Credential) return ((Credential) credentials).check(_pw); + if (credentials instanceof Credential) + return ((Credential)credentials).check(_pw); return false; } @@ -120,14 +124,10 @@ public class Password extends Credential return false; if (o instanceof Password) - { - Password p = (Password) o; - //noinspection StringEquality - return p._pw == _pw || (null != _pw && _pw.equals(p._pw)); - } + return stringEquals(_pw, ((Password)o)._pw); if (o instanceof String) - return o.equals(_pw); + return stringEquals(_pw, (String)o); return false; } @@ -175,7 +175,6 @@ public class Password extends Credential } return buf.toString(); - } /* ------------------------------------------------------------ */