Author: markt
Date: Mon Dec 12 10:03:35 2016
New Revision: 1773756
URL: http://svn.apache.org/viewvc?rev=1773756&view=rev
Log:
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=60446
Handle the case where the stored user credential uses a different key length
than the length currently configured for the CredentialHandler.
Based on a patch by Niklas Holm.
Modified:
tomcat/trunk/java/org/apache/catalina/realm/DigestCredentialHandlerBase.java
tomcat/trunk/java/org/apache/catalina/realm/LocalStrings.properties
tomcat/trunk/java/org/apache/catalina/realm/MessageDigestCredentialHandler.java
tomcat/trunk/java/org/apache/catalina/realm/SecretKeyCredentialHandler.java
tomcat/trunk/test/org/apache/catalina/realm/TestMessageDigestCredentialHandler.java
tomcat/trunk/test/org/apache/catalina/realm/TestSecretKeyCredentialHandler.java
tomcat/trunk/webapps/docs/changelog.xml
Modified:
tomcat/trunk/java/org/apache/catalina/realm/DigestCredentialHandlerBase.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/realm/DigestCredentialHandlerBase.java?rev=1773756&r1=1773755&r2=1773756&view=diff
==============================================================================
---
tomcat/trunk/java/org/apache/catalina/realm/DigestCredentialHandlerBase.java
(original)
+++
tomcat/trunk/java/org/apache/catalina/realm/DigestCredentialHandlerBase.java
Mon Dec 12 10:03:35 2016
@@ -126,6 +126,13 @@ public abstract class DigestCredentialHa
String serverCredential = mutate(userCredential, salt, iterations);
+ // Failed to generate server credential from user credential. Points to
+ // a configuration issue. The root cause should have been logged in the
+ // mutate() method.
+ if (serverCredential == null) {
+ return null;
+ }
+
if (saltLength == 0 && iterations == 1) {
// Output the simple/old format for backwards compatibility
return serverCredential;
@@ -155,6 +162,13 @@ public abstract class DigestCredentialHa
protected boolean matchesSaltIterationsEncoded(String inputCredentials,
String storedCredentials) {
+ if (storedCredentials == null) {
+ // Stored credentials are invalid
+ // This may be expected if nested credential handlers are being
used
+ logInvalidStoredCredentials(storedCredentials);
+ return false;
+ }
+
int sep1 = storedCredentials.indexOf('$');
int sep2 = storedCredentials.indexOf('$', sep1 + 1);
@@ -178,7 +192,13 @@ public abstract class DigestCredentialHa
return false;
}
- String inputHexEncoded = mutate(inputCredentials, salt, iterations);
+ String inputHexEncoded = mutate(inputCredentials, salt, iterations,
+ HexUtils.fromHexString(storedHexEncoded).length * Byte.SIZE);
+ if (inputHexEncoded == null) {
+ // Failed to mutate user credentials. Automatic fail.
+ // Root cause should be logged by mutate()
+ return false;
+ }
return storedHexEncoded.equalsIgnoreCase(inputHexEncoded);
}
@@ -204,7 +224,8 @@ public abstract class DigestCredentialHa
/**
* Generates the equivalent stored credentials for the given input
- * credentials, salt and iterations.
+ * credentials, salt and iterations. If the algorithm requires a key
length,
+ * the default will be used.
*
* @param inputCredentials User provided credentials
* @param salt Salt, if any
@@ -214,10 +235,35 @@ public abstract class DigestCredentialHa
* stored credentials
*
* @return The equivalent stored credentials for the given input
- * credentials
+ * credentials or <code>null</code> if the generation fails
*/
protected abstract String mutate(String inputCredentials, byte[] salt, int
iterations);
+
+ /**
+ * Generates the equivalent stored credentials for the given input
+ * credentials, salt, iterations and key length. The default implementation
+ * calls ignores the key length and calls
+ * {@link #mutate(String, byte[], int)}. Sub-classes that use the key
length
+ * should override this method.
+ *
+ * @param inputCredentials User provided credentials
+ * @param salt Salt, if any
+ * @param iterations Number of iterations of the algorithm
associated
+ * with this CredentialHandler applied to the
+ * inputCredentials to generate the equivalent
+ * stored credentials
+ * @param keyLength Length of the produced digest in bits for
+ * implementations where it's applicable
+ *
+ * @return The equivalent stored credentials for the given input
+ * credentials or <code>null</code> if the generation fails
+ */
+ protected String mutate(String inputCredentials, byte[] salt, int
iterations, int keyLength) {
+ return mutate(inputCredentials, salt, iterations);
+ }
+
+
/**
* Set the algorithm used to convert input credentials to stored
* credentials.
Modified: tomcat/trunk/java/org/apache/catalina/realm/LocalStrings.properties
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/realm/LocalStrings.properties?rev=1773756&r1=1773755&r2=1773756&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/realm/LocalStrings.properties
(original)
+++ tomcat/trunk/java/org/apache/catalina/realm/LocalStrings.properties Mon Dec
12 10:03:35 2016
@@ -93,5 +93,6 @@ combinedRealm.realmStartFail=Failed to s
lockOutRealm.authLockedUser=An attempt was made to authenticate the locked
user "{0}"
lockOutRealm.removeWarning=User "{0}" was removed from the failed users cache
after {1} seconds to keep the cache size within the limit set
credentialHandler.invalidStoredCredential=The invalid stored credential string
[{0}] was provided by the Realm to match with the user provided credentials
+credentialHandler.unableToMutateUserCredential=Failed to mutate user provided
credentials. This typically means the CredentialHandler configuration is invalid
mdCredentialHandler.unknownEncoding=The encoding [{0}] is not supported so the
current setting of [{1}] will still be used
pbeCredentialHandler.invalidKeySpec=Unable to generate a password based key
\ No newline at end of file
Modified:
tomcat/trunk/java/org/apache/catalina/realm/MessageDigestCredentialHandler.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/realm/MessageDigestCredentialHandler.java?rev=1773756&r1=1773755&r2=1773756&view=diff
==============================================================================
---
tomcat/trunk/java/org/apache/catalina/realm/MessageDigestCredentialHandler.java
(original)
+++
tomcat/trunk/java/org/apache/catalina/realm/MessageDigestCredentialHandler.java
Mon Dec 12 10:03:35 2016
@@ -148,6 +148,11 @@ public class MessageDigestCredentialHand
} else {
// Hex hashes should be compared case-insensitively
String userDigest = mutate(inputCredentials, null, 1);
+ if (userDigest == null) {
+ // Failed to mutate user credentials. Automatic fail.
+ // Root cause should be logged by mutate()
+ return false;
+ }
return storedCredentials.equalsIgnoreCase(userDigest);
}
}
Modified:
tomcat/trunk/java/org/apache/catalina/realm/SecretKeyCredentialHandler.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/realm/SecretKeyCredentialHandler.java?rev=1773756&r1=1773755&r2=1773756&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/realm/SecretKeyCredentialHandler.java
(original)
+++ tomcat/trunk/java/org/apache/catalina/realm/SecretKeyCredentialHandler.java
Mon Dec 12 10:03:35 2016
@@ -76,12 +76,17 @@ public class SecretKeyCredentialHandler
@Override
protected String mutate(String inputCredentials, byte[] salt, int
iterations) {
- KeySpec spec = new PBEKeySpec(inputCredentials.toCharArray(), salt,
iterations, getKeyLength());
+ return mutate(inputCredentials, salt, iterations, getKeyLength());
+ }
+
+ @Override
+ protected String mutate(String inputCredentials, byte[] salt, int
iterations, int keyLength) {
try {
+ KeySpec spec = new PBEKeySpec(inputCredentials.toCharArray(),
salt, iterations, keyLength);
return
HexUtils.toHexString(secretKeyFactory.generateSecret(spec).getEncoded());
- } catch (InvalidKeySpecException e) {
- log.warn("pbeCredentialHandler.invalidKeySpec", e);
+ } catch (InvalidKeySpecException | IllegalArgumentException e) {
+ log.warn(sm.getString("pbeCredentialHandler.invalidKeySpec"), e);
return null;
}
}
Modified:
tomcat/trunk/test/org/apache/catalina/realm/TestMessageDigestCredentialHandler.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/realm/TestMessageDigestCredentialHandler.java?rev=1773756&r1=1773755&r2=1773756&view=diff
==============================================================================
---
tomcat/trunk/test/org/apache/catalina/realm/TestMessageDigestCredentialHandler.java
(original)
+++
tomcat/trunk/test/org/apache/catalina/realm/TestMessageDigestCredentialHandler.java
Mon Dec 12 10:03:35 2016
@@ -49,10 +49,12 @@ public class TestMessageDigestCredential
private void doTest(String digest, int saltLength, int iterations) throws
NoSuchAlgorithmException {
MessageDigestCredentialHandler mdch = new
MessageDigestCredentialHandler();
+ MessageDigestCredentialHandler verifier = new
MessageDigestCredentialHandler();
mdch.setAlgorithm(digest);
mdch.setIterations(iterations);
mdch.setSaltLength(saltLength);
+ verifier.setAlgorithm(digest);
String storedCredential = mdch.mutate(PWD);
- Assert.assertTrue(mdch.matches(PWD, storedCredential));
+ Assert.assertTrue(verifier.matches(PWD, storedCredential));
}
}
Modified:
tomcat/trunk/test/org/apache/catalina/realm/TestSecretKeyCredentialHandler.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/realm/TestSecretKeyCredentialHandler.java?rev=1773756&r1=1773755&r2=1773756&view=diff
==============================================================================
---
tomcat/trunk/test/org/apache/catalina/realm/TestSecretKeyCredentialHandler.java
(original)
+++
tomcat/trunk/test/org/apache/catalina/realm/TestSecretKeyCredentialHandler.java
Mon Dec 12 10:03:35 2016
@@ -23,29 +23,62 @@ import org.junit.Test;
public class TestSecretKeyCredentialHandler {
- private static final String[] ALGORITHMS =
- new String[] {"PBKDF2WithHmacSHA1", "PBEWithMD5AndDES"};
-
- private static final String PWD = "password";
+ private static final String[] ALGORITHMS = { "PBKDF2WithHmacSHA1",
"PBEWithMD5AndDES" };
+ private static final String[] PASSWORDS = { "password",
"$!&#%!%@$#@*^$%&%%#!!*%$%&#@!^" };
+ private static final int[] KEYLENGTHS = { 8, 111, 256 };
+ private static final int[] SALTLENGTHS = { 1, 7, 12, 20 };
+ private static final int[] ITERATIONS = { 1, 2111, 10000 };
@Test
public void testGeneral() throws Exception {
for (String digest : ALGORITHMS) {
- for (int saltLength = 1; saltLength < 20; saltLength++) {
- for (int iterations = 1; iterations < 10000; iterations +=
1000)
- doTest(digest, saltLength, iterations);
+ for (String password : PASSWORDS) {
+ for (int saltLength : SALTLENGTHS) {
+ for (int iterations : ITERATIONS) {
+ for (int keyLength : KEYLENGTHS) {
+ doTest(password, digest, saltLength, iterations,
keyLength, true);
+ }
+ }
+ }
}
}
}
- private void doTest(String digest, int saltLength, int iterations)
- throws NoSuchAlgorithmException {
+ @Test
+ public void testZeroSalt() throws NoSuchAlgorithmException {
+ doTest(PASSWORDS[0], ALGORITHMS[0], 0, ITERATIONS[0], KEYLENGTHS[0],
false);
+ }
+
+ @Test
+ public void testZeroIterations() throws NoSuchAlgorithmException {
+ doTest(PASSWORDS[0], ALGORITHMS[0], SALTLENGTHS[0], 0, KEYLENGTHS[0],
false);
+ }
+
+ @Test
+ public void testZeroKeyLength() throws NoSuchAlgorithmException {
+ doTest(PASSWORDS[0], ALGORITHMS[0], SALTLENGTHS[0], ITERATIONS[0], 0,
false);
+ }
+
+ private void doTest(String password, String digest, int saltLength, int
iterations,
+ int keyLength, boolean expectMatch) throws
NoSuchAlgorithmException {
SecretKeyCredentialHandler pbech = new SecretKeyCredentialHandler();
+ SecretKeyCredentialHandler verifier = new SecretKeyCredentialHandler();
pbech.setAlgorithm(digest);
pbech.setIterations(iterations);
pbech.setSaltLength(saltLength);
- String storedCredential = pbech.mutate(PWD);
- Assert.assertTrue("[" + digest + "] [" + saltLength + "] [" +
iterations + "] [" + PWD +
- "] [" + storedCredential +"]", pbech.matches(PWD,
storedCredential));
+ pbech.setKeyLength(keyLength);
+ verifier.setAlgorithm(digest);
+ String storedCredential = pbech.mutate(password);
+ if (expectMatch) {
+ Assert.assertTrue(
+ "[" + digest + "] [" + saltLength + "] [" + iterations +
"] [" + keyLength + "] ["
+ + password + "] [" + storedCredential + "]",
+ verifier.matches(password, storedCredential));
+ } else {
+ Assert.assertFalse(
+ "[" + digest + "] [" + saltLength + "] [" + iterations +
"] [" + keyLength + "] ["
+ + password + "] [" + storedCredential + "]",
+ verifier.matches(password, storedCredential));
+ }
}
}
Modified: tomcat/trunk/webapps/docs/changelog.xml
URL:
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1773756&r1=1773755&r2=1773756&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Mon Dec 12 10:03:35 2016
@@ -45,6 +45,15 @@
issues do not "pop up" wrt. others).
-->
<section name="Tomcat 9.0.0.M16 (markt)" rtext="in development">
+ <subsection name="Catalina">
+ <changelog>
+ <fix>
+ <bug>60446</bug>: Handle the case where the stored user credential uses
+ a different key length than the length currently configured for the
+ <code>CredentialHandler</code>. Based on a patch by Niklas Holm.
(markt)
+ </fix>
+ </changelog>
+ </subsection>
<subsection name="Coyote">
<changelog>
<fix>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
