Author: robbie Date: Thu Mar 31 11:00:04 2011 New Revision: 1087250 URL: http://svn.apache.org/viewvc?rev=1087250&view=rev Log: QPID-3158 - Defect in the CRAM-MD5-HEX mechanism - CRAMMD5HexInitialiser fails to pad bytes in range 0A-0F with leading zero. Add testcase to test CRAM-MD5-HEX mechanism. Guard against nulls in SASL SaslServerFactory.getMechanismNames implementations to avoid dependency on mechanism registration order.
Applied patch from Keith Wall <[email protected]> Added: qpid/branches/0.5.x-dev/qpid/java/broker/src/test/java/org/apache/qpid/server/security/auth/sasl/CRAMMD5HexServerTest.java qpid/branches/0.5.x-dev/qpid/java/tools/src/main/java/org/apache/qpid/tools/QpidPasswordCheck.java Modified: qpid/branches/0.5.x-dev/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/sasl/amqplain/AmqPlainSaslServerFactory.java qpid/branches/0.5.x-dev/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/sasl/crammd5/CRAMMD5HexInitialiser.java qpid/branches/0.5.x-dev/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/sasl/plain/PlainSaslServerFactory.java qpid/branches/0.5.x-dev/qpid/java/client/src/main/java/org/apache/qpid/client/security/amqplain/AmqPlainSaslClientFactory.java qpid/branches/0.5.x-dev/qpid/java/tools/build.xml Modified: qpid/branches/0.5.x-dev/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/sasl/amqplain/AmqPlainSaslServerFactory.java URL: http://svn.apache.org/viewvc/qpid/branches/0.5.x-dev/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/sasl/amqplain/AmqPlainSaslServerFactory.java?rev=1087250&r1=1087249&r2=1087250&view=diff ============================================================================== --- qpid/branches/0.5.x-dev/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/sasl/amqplain/AmqPlainSaslServerFactory.java (original) +++ qpid/branches/0.5.x-dev/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/sasl/amqplain/AmqPlainSaslServerFactory.java Thu Mar 31 11:00:04 2011 @@ -45,9 +45,10 @@ public class AmqPlainSaslServerFactory i public String[] getMechanismNames(Map props) { - if (props.containsKey(Sasl.POLICY_NOPLAINTEXT) || - props.containsKey(Sasl.POLICY_NODICTIONARY) || - props.containsKey(Sasl.POLICY_NOACTIVE)) + if (props != null && + (props.containsKey(Sasl.POLICY_NOPLAINTEXT) || + props.containsKey(Sasl.POLICY_NODICTIONARY) || + props.containsKey(Sasl.POLICY_NOACTIVE))) { // returned array must be non null according to interface documentation return new String[0]; Modified: qpid/branches/0.5.x-dev/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/sasl/crammd5/CRAMMD5HexInitialiser.java URL: http://svn.apache.org/viewvc/qpid/branches/0.5.x-dev/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/sasl/crammd5/CRAMMD5HexInitialiser.java?rev=1087250&r1=1087249&r2=1087250&view=diff ============================================================================== --- qpid/branches/0.5.x-dev/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/sasl/crammd5/CRAMMD5HexInitialiser.java (original) +++ qpid/branches/0.5.x-dev/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/sasl/crammd5/CRAMMD5HexInitialiser.java Thu Mar 31 11:00:04 2011 @@ -70,7 +70,7 @@ public class CRAMMD5HexInitialiser exten for (char c : password) { //toHexString does not prepend 0 so we have to - if (((byte) c > -1) && (byte) c < 10) + if (((byte) c > -1) && (byte) c < 0x10) { sb.append(0); } Modified: qpid/branches/0.5.x-dev/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/sasl/plain/PlainSaslServerFactory.java URL: http://svn.apache.org/viewvc/qpid/branches/0.5.x-dev/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/sasl/plain/PlainSaslServerFactory.java?rev=1087250&r1=1087249&r2=1087250&view=diff ============================================================================== --- qpid/branches/0.5.x-dev/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/sasl/plain/PlainSaslServerFactory.java (original) +++ qpid/branches/0.5.x-dev/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/sasl/plain/PlainSaslServerFactory.java Thu Mar 31 11:00:04 2011 @@ -45,9 +45,10 @@ public class PlainSaslServerFactory impl public String[] getMechanismNames(Map props) { - if (props.containsKey(Sasl.POLICY_NOPLAINTEXT) || - props.containsKey(Sasl.POLICY_NODICTIONARY) || - props.containsKey(Sasl.POLICY_NOACTIVE)) + if (props != null && + (props.containsKey(Sasl.POLICY_NOPLAINTEXT) || + props.containsKey(Sasl.POLICY_NODICTIONARY) || + props.containsKey(Sasl.POLICY_NOACTIVE))) { // returned array must be non null according to interface documentation return new String[0]; Added: qpid/branches/0.5.x-dev/qpid/java/broker/src/test/java/org/apache/qpid/server/security/auth/sasl/CRAMMD5HexServerTest.java URL: http://svn.apache.org/viewvc/qpid/branches/0.5.x-dev/qpid/java/broker/src/test/java/org/apache/qpid/server/security/auth/sasl/CRAMMD5HexServerTest.java?rev=1087250&view=auto ============================================================================== --- qpid/branches/0.5.x-dev/qpid/java/broker/src/test/java/org/apache/qpid/server/security/auth/sasl/CRAMMD5HexServerTest.java (added) +++ qpid/branches/0.5.x-dev/qpid/java/broker/src/test/java/org/apache/qpid/server/security/auth/sasl/CRAMMD5HexServerTest.java Thu Mar 31 11:00:04 2011 @@ -0,0 +1,230 @@ +/* + * + * 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.qpid.server.security.auth.sasl; + +import java.io.File; +import java.io.IOException; +import java.security.MessageDigest; +import java.security.Principal; + +import javax.crypto.Mac; +import javax.crypto.spec.SecretKeySpec; +import javax.security.auth.login.AccountNotFoundException; +import javax.security.sasl.SaslException; +import javax.security.sasl.SaslServer; + +import junit.framework.TestCase; + +import org.apache.commons.codec.binary.Hex; +import org.apache.qpid.server.security.auth.database.Base64MD5PasswordFilePrincipalDatabase; +import org.apache.qpid.server.security.auth.sasl.crammd5.CRAMMD5HexInitialiser; +import org.apache.qpid.server.security.auth.sasl.crammd5.CRAMMD5HexSaslServer; +import org.apache.qpid.server.security.auth.sasl.crammd5.CRAMMD5HexServerFactory; + +/** + * Test for the CRAM-MD5-HEX SASL mechanism. + * + * This test case focuses on testing {@link CRAMMD5HexSaslServer} but also exercises + * collaborators {@link CRAMMD5HexInitialiser} and {@link Base64MD5PasswordFilePrincipalDatabase} + */ +public class CRAMMD5HexServerTest extends TestCase +{ + + private SaslServer _saslServer; // Class under test + private CRAMMD5HexServerFactory _saslFactory; + + @Override + protected void setUp() throws Exception + { + super.setUp(); + + CRAMMD5HexInitialiser _initializer = new CRAMMD5HexInitialiser(); + + //Use properties to create a PrincipalDatabase + Base64MD5PasswordFilePrincipalDatabase db = createTestPrincipalDatabase(); + assertEquals("Unexpected number of test users in the db", 2, db.getUsers().size()); + + _initializer.initialise(db); + + _saslFactory = new CRAMMD5HexServerFactory(); + + _saslServer = _saslFactory.createSaslServer(CRAMMD5HexSaslServer.MECHANISM, + "AMQP", + "localhost", + _initializer.getProperties(), + _initializer.getCallbackHandler()); + assertNotNull("Unable to create saslServer with mechanism type " + CRAMMD5HexSaslServer.MECHANISM, _saslServer); + + } + + public void testSuccessfulAuth() throws Exception + { + + final byte[] serverChallenge = _saslServer.evaluateResponse(new byte[0]); + + // Generate client response + final byte[] clientResponse = generateClientResponse("knownuser", "guest", serverChallenge); + + + byte[] nextServerChallenge = _saslServer.evaluateResponse(clientResponse); + assertTrue("Exchange must be flagged as complete after successful authentication", _saslServer.isComplete()); + assertNull("Next server challenge must be null after successful authentication", nextServerChallenge); + + } + + public void testKnownUserPresentsWrongPassword() throws Exception + { + byte[] serverChallenge = _saslServer.evaluateResponse(new byte[0]); + + + final byte[] clientResponse = generateClientResponse("knownuser", "wrong!", serverChallenge); + try + { + _saslServer.evaluateResponse(clientResponse); + fail("Exception not thrown"); + } + catch (SaslException se) + { + // PASS + } + assertFalse("Exchange must not be flagged as complete after unsuccessful authentication", _saslServer.isComplete()); + } + + public void testUnknownUser() throws Exception + { + final byte[] serverChallenge = _saslServer.evaluateResponse(new byte[0]); + + + final byte[] clientResponse = generateClientResponse("unknownuser", "guest", serverChallenge); + + try + { + _saslServer.evaluateResponse(clientResponse); + fail("Exception not thrown"); + } + catch (SaslException se) + { + assertExceptionHasUnderlyingAsCause(AccountNotFoundException.class, se); + // PASS + } + assertFalse("Exchange must not be flagged as complete after unsuccessful authentication", _saslServer.isComplete()); + } + + /** + * + * Demonstrates QPID-3158. A defect meant that users with some valid password were failing to + * authenticate when using the .NET 0-8 client (uses this SASL mechanism). + * It so happens that password "guest2" was one of the affected passwords. + * + * @throws Exception + */ + public void testSuccessfulAuthReproducingQpid3158() throws Exception + { + byte[] serverChallenge = _saslServer.evaluateResponse(new byte[0]); + + // Generate client response + byte[] resp = generateClientResponse("qpid3158user", "guest2", serverChallenge); + + byte[] nextServerChallenge = _saslServer.evaluateResponse(resp); + assertTrue("Exchange must be flagged as complete after successful authentication", _saslServer.isComplete()); + assertNull("Next server challenge must be null after successful authentication", nextServerChallenge); + } + + /** + * Since we don't have a CRAM-MD5-HEX implementation client implementation in Java, this method + * provides the implementation for first principals. + * + * @param userId user id + * @param clearTextPassword clear text password + * @param serverChallenge challenge from server + * + * @return challenge response + */ + private byte[] generateClientResponse(final String userId, final String clearTextPassword, final byte[] serverChallenge) throws Exception + { + byte[] digestedPasswordBytes = MessageDigest.getInstance("MD5").digest(clearTextPassword.getBytes()); + char[] hexEncodedDigestedPassword = Hex.encodeHex(digestedPasswordBytes); + byte[] hexEncodedDigestedPasswordBytes = new String(hexEncodedDigestedPassword).getBytes(); + + + Mac hmacMd5 = Mac.getInstance("HmacMD5"); + hmacMd5.init(new SecretKeySpec(hexEncodedDigestedPasswordBytes, "HmacMD5")); + final byte[] messageAuthenticationCode = hmacMd5.doFinal(serverChallenge); + + // Build client response + String responseAsString = userId + " " + new String(Hex.encodeHex(messageAuthenticationCode)); + byte[] resp = responseAsString.getBytes(); + return resp; + } + + /** + * Creates a test principal database. + * + * @return + * @throws IOException + */ + private Base64MD5PasswordFilePrincipalDatabase createTestPrincipalDatabase() throws IOException + { + Base64MD5PasswordFilePrincipalDatabase db = new Base64MD5PasswordFilePrincipalDatabase(); + File file = File.createTempFile("passwd", "db"); + file.deleteOnExit(); + db.setPasswordFile(file.getCanonicalPath()); + db.createPrincipal( createTestPrincipal("knownuser"), "guest".toCharArray()); + db.createPrincipal( createTestPrincipal("qpid3158user"), "guest2".toCharArray()); + return db; + } + + private Principal createTestPrincipal(final String name) + { + return new Principal() + { + + @Override + public String getName() + { + return name; + } + }; + } + + private void assertExceptionHasUnderlyingAsCause(final Class<? extends Throwable> expectedUnderlying, Throwable e) + { + assertNotNull(e); + int infiniteLoopGuard = 0; // Guard against loops in the cause chain + boolean foundExpectedUnderlying = false; + while (e.getCause() != null && infiniteLoopGuard++ < 10) + { + if (expectedUnderlying.equals(e.getCause().getClass())) + { + foundExpectedUnderlying = true; + break; + } + e = e.getCause(); + } + + if (!foundExpectedUnderlying) + { + fail("Not found expected underlying exception " + expectedUnderlying + " as underlying cause of " + e.getClass()); + } + } + +} Modified: qpid/branches/0.5.x-dev/qpid/java/client/src/main/java/org/apache/qpid/client/security/amqplain/AmqPlainSaslClientFactory.java URL: http://svn.apache.org/viewvc/qpid/branches/0.5.x-dev/qpid/java/client/src/main/java/org/apache/qpid/client/security/amqplain/AmqPlainSaslClientFactory.java?rev=1087250&r1=1087249&r2=1087250&view=diff ============================================================================== --- qpid/branches/0.5.x-dev/qpid/java/client/src/main/java/org/apache/qpid/client/security/amqplain/AmqPlainSaslClientFactory.java (original) +++ qpid/branches/0.5.x-dev/qpid/java/client/src/main/java/org/apache/qpid/client/security/amqplain/AmqPlainSaslClientFactory.java Thu Mar 31 11:00:04 2011 @@ -48,9 +48,10 @@ public class AmqPlainSaslClientFactory i public String[] getMechanismNames(Map props) { - if (props.containsKey(Sasl.POLICY_NOPLAINTEXT) || - props.containsKey(Sasl.POLICY_NODICTIONARY) || - props.containsKey(Sasl.POLICY_NOACTIVE)) + if (props != null && + (props.containsKey(Sasl.POLICY_NOPLAINTEXT) || + props.containsKey(Sasl.POLICY_NODICTIONARY) || + props.containsKey(Sasl.POLICY_NOACTIVE))) { // returned array must be non null according to interface documentation return new String[0]; Modified: qpid/branches/0.5.x-dev/qpid/java/tools/build.xml URL: http://svn.apache.org/viewvc/qpid/branches/0.5.x-dev/qpid/java/tools/build.xml?rev=1087250&r1=1087249&r2=1087250&view=diff ============================================================================== --- qpid/branches/0.5.x-dev/qpid/java/tools/build.xml (original) +++ qpid/branches/0.5.x-dev/qpid/java/tools/build.xml Thu Mar 31 11:00:04 2011 @@ -24,4 +24,14 @@ <import file="../module.xml"/> + <target name="jar" depends="jar.manifest,jar.nomanifest,jar.manifest.qpidpasswordcheck" description="create jars"/> + + <!-- Extra target to build QpidPasswordCheck as workaround QPID-3158 --> + <target name="jar.manifest.qpidpasswordcheck"> + <jar destfile="${build.lib}/${project.name}-passwordcheck-${project.version}.jar" basedir="${module.classes}" includes="**/QpidPasswordCheck.class"> + <manifest> + <attribute name="Main-Class" value="org.apache.qpid.tools.QpidPasswordCheck"/> + </manifest> + </jar> + </target> </project> Added: qpid/branches/0.5.x-dev/qpid/java/tools/src/main/java/org/apache/qpid/tools/QpidPasswordCheck.java URL: http://svn.apache.org/viewvc/qpid/branches/0.5.x-dev/qpid/java/tools/src/main/java/org/apache/qpid/tools/QpidPasswordCheck.java?rev=1087250&view=auto ============================================================================== --- qpid/branches/0.5.x-dev/qpid/java/tools/src/main/java/org/apache/qpid/tools/QpidPasswordCheck.java (added) +++ qpid/branches/0.5.x-dev/qpid/java/tools/src/main/java/org/apache/qpid/tools/QpidPasswordCheck.java Thu Mar 31 11:00:04 2011 @@ -0,0 +1,134 @@ +/* + * + * 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.qpid.tools; + +import java.io.Console; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.util.Arrays; + +/** + * Command line tool to determine if a password will be affected by QPID-3158. + */ +public class QpidPasswordCheck +{ + + public static void main(String[] argv) throws Exception + { + Console console = System.console(); + + if (console == null) { + System.err.println("Console not available. This utility must be run on the command line."); + System.exit(-1); + } + else + { + System.err.println("Defect QPID-3158 in the Qpid java broker (releases 2.6.0.8 and below) means that certain valid client " + + "passwords cause spurious authentication failures when used from the Qpid 0-8 .NET client with a broker " + + "using the Base64MD5 password database type."); + System.err.println(); + System.err.println("This utility accepts a candidate password and reports if the password will cause the defect."); + System.err.println(); + + final char[] password = console.readPassword("Enter candidate password: "); + final char[] repeatedPassword = console.readPassword("Enter candidate password again: "); + + if (!Arrays.equals(password,repeatedPassword)) + { + System.err.println("Sorry, those passwords do not match."); + System.exit(-1); + } + + + final byte[] hashedPassword = MessageDigest.getInstance("MD5").digest(new String(password).getBytes()); + + + final char[] hashedPasswordChars = new char[hashedPassword.length]; + + int index = 0; + for (byte c : hashedPassword) + { + hashedPasswordChars[index++] = (char) c; + } + + char[] brokenRepresentation = brokenToHex(hashedPasswordChars); + char[] correctRepresentation = fixedToHex(hashedPasswordChars); + + if (Arrays.equals(brokenRepresentation,correctRepresentation)) + { + System.err.println("Password is suitable for use."); + System.exit(0); + } + else + { + System.err.println("Sorry, that password is NOT suitable for use."); + System.exit(1); + } + + } + } + + + private static char[] brokenToHex(char[] password) + { + StringBuilder sb = new StringBuilder(); + for (char c : password) + { + //toHexString does not prepend 0 so we have to + if (((byte) c > -1) && (byte) c < 10 ) + { + sb.append(0); + } + + sb.append(Integer.toHexString(c & 0xFF)); + } + + //Extract the hex string as char[] + char[] hex = new char[sb.length()]; + + sb.getChars(0, sb.length(), hex, 0); + + return hex; + } + + private static char[] fixedToHex(char[] password) + { + StringBuilder sb = new StringBuilder(); + for (char c : password) + { + //toHexString does not prepend 0 so we have to + if (((byte) c > -1) && (byte) c < 0x10 ) + { + sb.append(0); + } + + sb.append(Integer.toHexString(c & 0xFF)); + } + + //Extract the hex string as char[] + char[] hex = new char[sb.length()]; + + sb.getChars(0, sb.length(), hex, 0); + + return hex; + } + +} --------------------------------------------------------------------- Apache Qpid - AMQP Messaging Implementation Project: http://qpid.apache.org Use/Interact: mailto:[email protected]
