diff --git a/java/org/apache/catalina/realm/MessageDigestCredentialHandler.java b/java/org/apache/catalina/realm/MessageDigestCredentialHandler.java index b5ab367b8479..13cc432e2380 100644 --- a/java/org/apache/catalina/realm/MessageDigestCredentialHandler.java +++ b/java/org/apache/catalina/realm/MessageDigestCredentialHandler.java @@ -135,7 +135,7 @@ public boolean matches(String inputCredentials, String storedCredentials) { if (getAlgorithm() == null) { // No digests, compare directly - return ConstantTime.equals(inputCredentials, storedCredentials, false); + return ConstantTime.equals(storedCredentials, inputCredentials, false); } else { // Some directories and databases prefix the password with the hash // type. The string is in a format compatible with Base64.encode not @@ -148,7 +148,7 @@ public boolean matches(String inputCredentials, String storedCredentials) { inputCredentials.getBytes(StandardCharsets.ISO_8859_1)); String base64UserDigest = Base64.getEncoder().encodeToString(userDigest); - return ConstantTime.equals(base64UserDigest, base64ServerDigest, false); + return ConstantTime.equals(base64ServerDigest, base64UserDigest, false); } else if (storedCredentials.startsWith("{SSHA}")) { // "{SSHA}" // Need to convert the salt to bytes to apply it to the user's @@ -179,7 +179,7 @@ public boolean matches(String inputCredentials, String storedCredentials) { byte[] userDigestBytes = ConcurrentMessageDigest.digest(getAlgorithm(), inputCredentials.getBytes(StandardCharsets.ISO_8859_1), serverSaltBytes); - return ConstantTime.equals(userDigestBytes, serverDigestBytes); + return ConstantTime.equals(serverDigestBytes, userDigestBytes); } else if (storedCredentials.indexOf('$') > -1) { return matchesSaltIterationsEncoded(inputCredentials, storedCredentials); } else { diff --git a/java/org/apache/coyote/ajp/AjpProcessor.java b/java/org/apache/coyote/ajp/AjpProcessor.java index c27c41b285ad..e197eeb0df68 100644 --- a/java/org/apache/coyote/ajp/AjpProcessor.java +++ b/java/org/apache/coyote/ajp/AjpProcessor.java @@ -825,7 +825,7 @@ private void prepareRequest() { requestHeaderMessage.getBytes(tmpMB); if (secret != null && !secret.isEmpty()) { secretPresentInRequest = true; - if (!ConstantTime.equals(tmpMB.getByteChunk(), secret)) { + if (!ConstantTime.equals(secret, tmpMB.getByteChunk())) { response.setStatus(403); setErrorState(ErrorState.CLOSE_CLEAN, null); } diff --git a/java/org/apache/tomcat/util/security/ConstantTime.java b/java/org/apache/tomcat/util/security/ConstantTime.java index 992cfdf82554..41711fb41d54 100644 --- a/java/org/apache/tomcat/util/security/ConstantTime.java +++ b/java/org/apache/tomcat/util/security/ConstantTime.java @@ -121,6 +121,48 @@ public static boolean equals(final ByteChunk bc, final String s) { return result == 0; } + /** + * Implements String / ByteChunk equality which always compares all characters, without stopping early if any + * characters do not match. + *

+ * Note: This implementation was adapted from {@link MessageDigest#isEqual} which we assume is as + * optimizer-defeating as possible. + * + * @param s The string to compare. + * @param bc The ByteChunk to compare. + * + * @return true if the strings are equal to each other, false otherwise. + */ + public static boolean equals(final String s, final ByteChunk bc) { + if (s == null && bc == null) { + return true; + } + if (s == null || bc == null) { + return false; + } + + final int len1 = s.length(); + final int len2 = bc.getLength(); + + byte[] bytes = bc.getBytes(); + + if (len2 == 0) { + return len1 == 0; + } + + int result = 0; + result |= len1 - len2; + + // time-constant comparison + for (int i = 0; i < len1; i++) { + // If i >= len2, index2 is 0; otherwise, i. + final int index2 = ((i - len2) >>> 31) * i; + char c = s.charAt(i); + byte b = bytes[bc.getStart() + index2]; + result |= c ^ (b & 0xFF); + } + return result == 0; + } /** * Implements byte-array equality which always compares all bytes in the array, without stopping early if any bytes