This is an automated email from the ASF dual-hosted git repository. markt-asf pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 6771e06aee1e6da82b36d528a3f6a9969c682748 Author: Mark Thomas <[email protected]> AuthorDate: Thu Apr 23 17:56:48 2026 +0100 Switch AJP secret comparison to a constant time algorithm. # Conflicts: # webapps/docs/changelog.xml --- .../realm/DigestCredentialHandlerBase.java | 40 ++---- java/org/apache/coyote/ajp/AjpProcessor.java | 3 +- .../apache/tomcat/util/security/ConstantTime.java | 142 +++++++++++++++++++++ webapps/docs/changelog.xml | 3 + 4 files changed, 156 insertions(+), 32 deletions(-) diff --git a/java/org/apache/catalina/realm/DigestCredentialHandlerBase.java b/java/org/apache/catalina/realm/DigestCredentialHandlerBase.java index eda0aa816a..a3edbeb547 100644 --- a/java/org/apache/catalina/realm/DigestCredentialHandlerBase.java +++ b/java/org/apache/catalina/realm/DigestCredentialHandlerBase.java @@ -25,6 +25,7 @@ import org.apache.catalina.CredentialHandler; import org.apache.juli.logging.Log; import org.apache.tomcat.util.buf.HexUtils; import org.apache.tomcat.util.res.StringManager; +import org.apache.tomcat.util.security.ConstantTime; /** * Base implementation for the Tomcat provided {@link CredentialHandler}s. @@ -286,38 +287,12 @@ public abstract class DigestCredentialHandlerBase implements CredentialHandler { * here is only guaranteed to work with plain ASCII characters. * * @return <code>true</code> if the strings are equal to each other, <code>false</code> otherwise. + * + * @deprecated Use {@link ConstantTime#equals(String, String, boolean)}. This method will be removed in Tomcat 12. */ + @Deprecated public static boolean equals(final String s1, final String s2, final boolean ignoreCase) { - if (s1 == s2) { - return true; - } - if (s1 == null || s2 == null) { - return false; - } - - final int len1 = s1.length(); - final int len2 = s2.length(); - - 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 c1 = s1.charAt(i); - char c2 = s2.charAt(index2); - if (ignoreCase) { - c1 = Character.toLowerCase(c1); - c2 = Character.toLowerCase(c2); - } - result |= c1 ^ c2; - } - return result == 0; + return ConstantTime.equals(s1, s2, ignoreCase); } /** @@ -333,8 +308,11 @@ public abstract class DigestCredentialHandlerBase implements CredentialHandler { * @param b2 The second array to compare. * * @return <code>true</code> if the arrays are equal to each other, <code>false</code> otherwise. + * + * @deprecated Use {@link ConstantTime#equals(byte[], byte[])}. This method will be removed in Tomcat 12. */ + @Deprecated public static boolean equals(final byte[] b1, final byte[] b2) { - return MessageDigest.isEqual(b1, b2); + return ConstantTime.equals(b1, b2); } } diff --git a/java/org/apache/coyote/ajp/AjpProcessor.java b/java/org/apache/coyote/ajp/AjpProcessor.java index d7fb2d433d..3eaddec9a9 100644 --- a/java/org/apache/coyote/ajp/AjpProcessor.java +++ b/java/org/apache/coyote/ajp/AjpProcessor.java @@ -55,6 +55,7 @@ import org.apache.tomcat.util.net.ApplicationBufferHandler; import org.apache.tomcat.util.net.SSLSupport; import org.apache.tomcat.util.net.SocketWrapperBase; import org.apache.tomcat.util.res.StringManager; +import org.apache.tomcat.util.security.ConstantTime; /** * AJP Processor implementation. @@ -818,7 +819,7 @@ public class AjpProcessor extends AbstractProcessor { requestHeaderMessage.getBytes(tmpMB); if (secret != null && !secret.isEmpty()) { secretPresentInRequest = true; - if (!tmpMB.equals(secret)) { + if (!ConstantTime.equals(tmpMB.getByteChunk(), secret)) { 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 new file mode 100644 index 0000000000..9a9d16b365 --- /dev/null +++ b/java/org/apache/tomcat/util/security/ConstantTime.java @@ -0,0 +1,142 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.tomcat.util.security; + +import java.security.MessageDigest; + +import org.apache.tomcat.util.buf.ByteChunk; + +/** + * Utility class for methods that, for security reasons, need to run in - as far as practical - constant time. + */ +public class ConstantTime { + + private ConstantTime() { + // Hide default constructor for this utility class + } + + + /** + * Implements String equality which always compares all characters in the string, without stopping early if any + * characters do not match. + * <p> + * <i>Note:</i> This implementation was adapted from {@link MessageDigest#isEqual} which we assume is as + * optimizer-defeating as possible. + * + * @param s1 The first string to compare. + * @param s2 The second string to compare. + * @param ignoreCase <code>true</code> if the strings should be compared without regard to case. Note that "true" + * here is only guaranteed to work with plain ASCII characters. + * + * @return <code>true</code> if the strings are equal to each other, <code>false</code> otherwise. + */ + public static boolean equals(final String s1, final String s2, final boolean ignoreCase) { + if (s1 == s2) { + return true; + } + if (s1 == null || s2 == null) { + return false; + } + + final int len1 = s1.length(); + final int len2 = s2.length(); + + 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 c1 = s1.charAt(i); + char c2 = s2.charAt(index2); + if (ignoreCase) { + c1 = Character.toLowerCase(c1); + c2 = Character.toLowerCase(c2); + } + result |= c1 ^ c2; + } + return result == 0; + } + + + /** + * Implements ByteChunk / String equality which always compares all characters, without stopping early if any + * characters do not match. + * <p> + * <i>Note:</i> This implementation was adapted from {@link MessageDigest#isEqual} which we assume is as + * optimizer-defeating as possible. + * + * @param bc The ByteChunk to compare. + * @param s The string to compare. + * + * @return <code>true</code> if the strings are equal to each other, <code>false</code> otherwise. + */ + public static boolean equals(final ByteChunk bc, final String s) { + if (bc == null && s == null) { + return true; + } + if (bc == null || s == null) { + return false; + } + + final int len1 = bc.getLength(); + final int len2 = s.length(); + + byte[] bytes = bc.getBytes(); + + if (len2 == 0) { + return len1 == 0; + } + + boolean result = true; + 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; + byte b = bytes[i]; + char c = s.charAt(index2); + result &= (b == c); + } + return result; + } + + + /** + * Implements byte-array equality which always compares all bytes in the array, without stopping early if any bytes + * do not match. + * <p> + * <i>Note:</i> Implementation note: this method delegates to {@link MessageDigest#isEqual} under the assumption + * that it provides a constant-time comparison of the bytes in the arrays. Java 7+ has such an implementation, but + * neither the Javadoc nor any specification requires it. Therefore, Tomcat should continue to use <i>this</i> + * method internally in case the JDK implementation changes so this method can be re-implemented properly. + * + * @param b1 The first array to compare. + * @param b2 The second array to compare. + * + * @return <code>true</code> if the arrays are equal to each other, <code>false</code> otherwise. + */ + public static boolean equals(final byte[] b1, final byte[] b2) { + return MessageDigest.isEqual(b1, b2); + } +} diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 2cd61b305f..3fb4423032 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -338,6 +338,9 @@ connection level window update to individual HTTP/2 streams. Based on <pr>996</pr> by Mike Tingey Jr. (markt) </fix> + <fix> + Switch AJP secret comparison to a constant time algorithm. (markt) + </fix> </changelog> </subsection> <subsection name="Jasper"> --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
