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 afe3bcf1adc6..54ad4f062a10 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 @@ -341,7 +341,7 @@ public boolean check(Object credentials) 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) { @@ -356,6 +356,5 @@ public String toString() { 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 a982f56aa6b4..450556701199 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 @@ -401,7 +401,7 @@ public boolean check(Object credentials) 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 9b855faf0926..77692d35c4ba 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 @@ -71,6 +71,44 @@ public static Credential getCredential(String credential) 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 */ @@ -93,8 +131,7 @@ public boolean check(Object credentials) 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)); } public static String crypt(String user, String pw) @@ -143,26 +180,18 @@ public boolean check(Object credentials) __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) { - MD5 md5 = (MD5) credentials; - 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; + MD5 md5 = (MD5)credentials; + return byteEquals(_digest, md5._digest); } else if (credentials instanceof Credential) { // Allow credential to attempt check - i.e. this'll work // for DigestAuthModule$Digest credentials - return ((Credential) credentials).check(this); + return ((Credential)credentials).check(this); } else { 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 81ff1b92bd03..fe7839bf4e0a 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 @@ 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 String toStarString() @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 boolean equals(Object o) 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 static String obfuscate(String s) } return buf.toString(); - } /* ------------------------------------------------------------ */