This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 11.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 5090bdfa2d82c15fc044ee06f7d91aa5d666033e Author: opencode <[email protected]> AuthorDate: Fri Oct 16 14:08:26 2026 +0200 Require at least one iteration when computing message digests ConcurrentMessageDigest.digest(String, int, byte[]...) documents the iterations argument as the number of times the digest is applied, but the value was not checked. A count of zero or less still produced the digest of the input, because the first round runs unconditionally and the additional rounds only run when the count is greater than one. Callers that passed such a count silently received a result that did not match the documented semantics. The method now throws IllegalArgumentException when fewer than one iteration is requested, which is consistent with how the JDK validates the iteration count of PBEKeySpec. Stored credentials used by the message digest credential handler carry their own iteration count. A stored count below one was previously accepted and silently verified as a single iteration. Such credentials are now reported as invalid, matching the existing handling of other malformed stored credentials, so verification returns false instead of using an iteration count the digest computation rejects. Add regression tests covering the rejected counts, repeated iteration semantics and stored credentials with an invalid iteration count. --- .../realm/DigestCredentialHandlerBase.java | 6 ++ .../util/security/ConcurrentMessageDigest.java | 7 +++ .../tomcat/util/security/LocalStrings.properties | 1 + .../realm/TestMessageDigestCredentialHandler.java | 23 +++++++ .../util/security/TestConcurrentMessageDigest.java | 70 ++++++++++++++++++---- 5 files changed, 96 insertions(+), 11 deletions(-) diff --git a/java/org/apache/catalina/realm/DigestCredentialHandlerBase.java b/java/org/apache/catalina/realm/DigestCredentialHandlerBase.java index 6c4b216e64..ced174e5ce 100644 --- a/java/org/apache/catalina/realm/DigestCredentialHandlerBase.java +++ b/java/org/apache/catalina/realm/DigestCredentialHandlerBase.java @@ -198,6 +198,12 @@ public abstract class DigestCredentialHandlerBase implements CredentialHandler { return false; } + if (iterations < 1) { + // Stored credentials must specify at least one iteration + logInvalidStoredCredentials(storedCredentials); + return false; + } + String storedHexEncoded = storedCredentials.substring(sep2 + 1); byte[] salt; try { diff --git a/java/org/apache/tomcat/util/security/ConcurrentMessageDigest.java b/java/org/apache/tomcat/util/security/ConcurrentMessageDigest.java index ec785a2246..06e0bd4600 100644 --- a/java/org/apache/tomcat/util/security/ConcurrentMessageDigest.java +++ b/java/org/apache/tomcat/util/security/ConcurrentMessageDigest.java @@ -117,9 +117,16 @@ public class ConcurrentMessageDigest { * @param iterations the number of times to apply the digest * @param input the byte arrays to digest * @return the digest + * + * @throws IllegalArgumentException if iterations is less than one */ public static byte[] digest(String algorithm, int iterations, byte[]... input) { + if (iterations < 1) { + throw new IllegalArgumentException( + sm.getString("concurrentMessageDigest.invalidIterations", Integer.toString(iterations))); + } + Queue<MessageDigest> queue = queues.get(algorithm); if (queue == null) { throw new IllegalStateException(sm.getString("concurrentMessageDigest.noDigest")); diff --git a/java/org/apache/tomcat/util/security/LocalStrings.properties b/java/org/apache/tomcat/util/security/LocalStrings.properties index 310e51c13b..6b9a406e91 100644 --- a/java/org/apache/tomcat/util/security/LocalStrings.properties +++ b/java/org/apache/tomcat/util/security/LocalStrings.properties @@ -16,4 +16,5 @@ # Do not edit this file directly. # To edit translations see: https://tomcat.apache.org/getinvolved.html#Translations +concurrentMessageDigest.invalidIterations=At least one iteration is required. [{0}] concurrentMessageDigest.noDigest=Digest algorithm not initialized or unavailable diff --git a/test/org/apache/catalina/realm/TestMessageDigestCredentialHandler.java b/test/org/apache/catalina/realm/TestMessageDigestCredentialHandler.java index 2020da3e7d..5b79d71f63 100644 --- a/test/org/apache/catalina/realm/TestMessageDigestCredentialHandler.java +++ b/test/org/apache/catalina/realm/TestMessageDigestCredentialHandler.java @@ -48,6 +48,29 @@ public class TestMessageDigestCredentialHandler { } } + /* + * A stored credential that specifies fewer than one iteration is invalid. It must not be treated as valid and it + * must not trigger an exception during verification. + */ + @Test + public void testZeroIterationsStoredCredential() throws Exception { + MessageDigestCredentialHandler mdch = new MessageDigestCredentialHandler(); + mdch.setAlgorithm("SHA-512"); + mdch.setIterations(1); + mdch.setSaltLength(8); + String storedCredential = mdch.mutate(PWD); + + int sep1 = storedCredential.indexOf('$'); + int sep2 = storedCredential.indexOf('$', sep1 + 1); + Assert.assertTrue(sep1 > 0 && sep2 > sep1); + String tamperedCredential = storedCredential.substring(0, sep1 + 1) + "0" + storedCredential.substring(sep2); + + MessageDigestCredentialHandler verifier = new MessageDigestCredentialHandler(); + verifier.setAlgorithm("SHA-512"); + Assert.assertFalse(verifier.matches(PWD, tamperedCredential)); + } + + private void doTest(String digest, int saltLength, int iterations) throws NoSuchAlgorithmException { MessageDigestCredentialHandler mdch = new MessageDigestCredentialHandler(); MessageDigestCredentialHandler verifier = new MessageDigestCredentialHandler(); diff --git a/test/org/apache/tomcat/util/security/TestConcurrentMessageDigest.java b/test/org/apache/tomcat/util/security/TestConcurrentMessageDigest.java index 2ce4c136c9..b92c749b5b 100644 --- a/test/org/apache/tomcat/util/security/TestConcurrentMessageDigest.java +++ b/test/org/apache/tomcat/util/security/TestConcurrentMessageDigest.java @@ -1,21 +1,22 @@ /* - * 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 + * 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. + * 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 org.junit.Assert; import org.junit.Test; public class TestConcurrentMessageDigest { @@ -37,4 +38,51 @@ public class TestConcurrentMessageDigest { ConcurrentMessageDigest.init(SHA256); ConcurrentMessageDigest.digest(SHA256, new byte[8]); } + + + /* + * A zero iteration count is not a valid number of times to apply the digest. It must fail rather than silently + * applying the digest once. + */ + @Test + public void testZeroIterations() throws Exception { + ConcurrentMessageDigest.init(SHA256); + + Assert.assertThrows(IllegalArgumentException.class, + () -> ConcurrentMessageDigest.digest(SHA256, 0, new byte[8])); + } + + + /* + * A negative iteration count is not a valid number of times to apply the digest. It must fail rather than + * silently applying the digest once. + */ + @Test + public void testNegativeIterations() throws Exception { + ConcurrentMessageDigest.init(SHA256); + + Assert.assertThrows(IllegalArgumentException.class, + () -> ConcurrentMessageDigest.digest(SHA256, -5, new byte[8])); + } + + + /* + * Multiple iterations must repeatedly digest the result of the previous iteration. + */ + @Test + public void testMultipleIterations() throws Exception { + ConcurrentMessageDigest.init(SHA256); + + byte[] input = "input".getBytes("UTF-8"); + + byte[] twice = ConcurrentMessageDigest.digest(SHA256, 2, input); + byte[] expected = ConcurrentMessageDigest.digest(SHA256, ConcurrentMessageDigest.digest(SHA256, input)); + + Assert.assertArrayEquals(expected, twice); + + byte[] threeTimes = ConcurrentMessageDigest.digest(SHA256, 3, input); + expected = ConcurrentMessageDigest.digest(SHA256, expected); + + Assert.assertArrayEquals(expected, threeTimes); + } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
