This is an automated email from the ASF dual-hosted git repository.
markt-asf pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/10.1.x by this push:
new a102a2a157 Switch AJP secret comparison to a constant time algorithm.
a102a2a157 is described below
commit a102a2a157868ca51d83eaf5a119ccd9976a113e
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 b766a4ee6d..ead435ca57 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.
@@ -836,7 +837,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 3deec51653..b451df6f2a 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -231,6 +231,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="WebSocket">
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
