You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by lq...@apache.org on 2016/09/30 14:40:48 UTC
svn commit: r1762919 - in /qpid/java/trunk/broker-core/src:
main/java/org/apache/qpid/server/security/auth/database/
main/java/org/apache/qpid/server/security/auth/manager/
test/java/org/apache/qpid/server/security/auth/database/
Author: lquack
Date: Fri Sep 30 14:40:48 2016
New Revision: 1762919
URL: http://svn.apache.org/viewvc?rev=1762919&view=rev
Log:
QPID-7414: [Java Broker] Reject username and passwords with colons for AbstractPasswordFilePrincipalDatabase
Added:
qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/security/auth/database/AbstractPasswordFilePrincipalDatabaseTest.java
Modified:
qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/security/auth/database/AbstractPasswordFilePrincipalDatabase.java
qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/PrincipalDatabaseAuthenticationManager.java
qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/security/auth/database/Base64MD5PasswordFilePrincipalDatabaseTest.java
qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/security/auth/database/PlainPasswordFilePrincipalDatabaseTest.java
Modified: qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/security/auth/database/AbstractPasswordFilePrincipalDatabase.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/security/auth/database/AbstractPasswordFilePrincipalDatabase.java?rev=1762919&r1=1762918&r2=1762919&view=diff
==============================================================================
--- qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/security/auth/database/AbstractPasswordFilePrincipalDatabase.java (original)
+++ qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/security/auth/database/AbstractPasswordFilePrincipalDatabase.java Fri Sep 30 14:40:48 2016
@@ -164,6 +164,13 @@ public abstract class AbstractPasswordFi
{
throw new AccountNotFoundException(principal.getName());
}
+ for (char c : password)
+ {
+ if (c == ':')
+ {
+ throw new IllegalArgumentException("Illegal character in password");
+ }
+ }
char[] orig = user.getPassword();
_userUpdate.lock();
@@ -379,6 +386,17 @@ public abstract class AbstractPasswordFi
{
return false;
}
+ if (principal.getName().contains(":"))
+ {
+ throw new IllegalArgumentException("Username must not contain colons (\":\").");
+ }
+ for (char c : password)
+ {
+ if (c == ':')
+ {
+ throw new IllegalArgumentException("Illegal character in password");
+ }
+ }
U user = createUserFromPassword(principal, password);
Modified: qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/PrincipalDatabaseAuthenticationManager.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/PrincipalDatabaseAuthenticationManager.java?rev=1762919&r1=1762918&r2=1762919&view=diff
==============================================================================
--- qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/PrincipalDatabaseAuthenticationManager.java (original)
+++ qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/PrincipalDatabaseAuthenticationManager.java Fri Sep 30 14:40:48 2016
@@ -339,10 +339,18 @@ public abstract class PrincipalDatabaseA
Principal p = new UsernamePrincipal(username, this);
PrincipalAdapter principalAdapter = new PrincipalAdapter(p);
principalAdapter.create(); // for a duplicate user DuplicateNameException should be thrown
- boolean created = getPrincipalDatabase().createPrincipal(p, password.toCharArray());
- if (!created)
+ try
+ {
+ boolean created = getPrincipalDatabase().createPrincipal(p, password.toCharArray());
+ if (!created)
+ {
+ throw new IllegalArgumentException("User '" + username + "' was not added into principal database");
+ }
+ }
+ catch (RuntimeException e)
{
- throw new IllegalArgumentException("User '" + username + "' was not added into principal database");
+ principalAdapter.deleteAsync();
+ throw e;
}
_userMap.put(p, principalAdapter);
return Futures.immediateFuture((C)principalAdapter);
@@ -467,13 +475,13 @@ public abstract class PrincipalDatabaseA
{
String userName = _user.getName();
deleteUserFromDatabase(userName);
- deleted();
- setState(State.DELETED);
}
catch (AccountNotFoundException e)
{
- LOGGER.warn("Failed to delete user " + _user, e);
+ // pass
}
+ deleted();
+ setState(State.DELETED);
return Futures.immediateFuture(null);
}
}
Added: qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/security/auth/database/AbstractPasswordFilePrincipalDatabaseTest.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/security/auth/database/AbstractPasswordFilePrincipalDatabaseTest.java?rev=1762919&view=auto
==============================================================================
--- qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/security/auth/database/AbstractPasswordFilePrincipalDatabaseTest.java (added)
+++ qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/security/auth/database/AbstractPasswordFilePrincipalDatabaseTest.java Fri Sep 30 14:40:48 2016
@@ -0,0 +1,169 @@
+/*
+ * 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.database;
+
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.security.Principal;
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.qpid.server.security.auth.UsernamePrincipal;
+import org.apache.qpid.test.utils.QpidTestCase;
+
+public abstract class AbstractPasswordFilePrincipalDatabaseTest extends QpidTestCase
+{
+ protected static final String TEST_COMMENT = "# Test Comment";
+ protected static final String TEST_USERNAME = "testUser";
+ protected static final String TEST_PASSWORD = "testPassword";
+ protected static final char[] TEST_PASSWORD_CHARS = TEST_PASSWORD.toCharArray();
+
+ private List<File> _testPwdFiles = new ArrayList<File>();
+ private Principal _principal = new UsernamePrincipal(TEST_USERNAME, null);
+
+ protected abstract AbstractPasswordFilePrincipalDatabase getDatabase();
+
+ @Override
+ public void tearDown() throws Exception
+ {
+ try
+ {
+ //clean up any additional files and their backups
+ for (File f : _testPwdFiles)
+ {
+ File oldPwdFile = new File(f.getAbsolutePath() + ".old");
+ if (oldPwdFile.exists())
+ {
+ oldPwdFile.delete();
+ }
+
+ f.delete();
+ }
+ }
+ finally
+ {
+ super.tearDown();
+ }
+ }
+
+ protected File createPasswordFile(int commentLines, int users)
+ {
+ try
+ {
+ File testFile = File.createTempFile(getTestName(), "tmp");
+ testFile.deleteOnExit();
+
+ BufferedWriter writer = new BufferedWriter(new FileWriter(testFile));
+
+ for (int i = 0; i < commentLines; i++)
+ {
+ writer.write(TEST_COMMENT);
+ writer.newLine();
+ }
+
+ for (int i = 0; i < users; i++)
+ {
+ writer.write(TEST_USERNAME + i + ":Password");
+ writer.newLine();
+ }
+
+ writer.flush();
+ writer.close();
+
+ _testPwdFiles.add(testFile);
+
+ return testFile;
+
+ }
+ catch (IOException e)
+ {
+ fail("Unable to create test password file." + e.getMessage());
+ }
+
+ return null;
+ }
+
+
+ protected void loadPasswordFile(File file)
+ {
+ try
+ {
+ getDatabase().open(file);
+ }
+ catch (IOException e)
+ {
+ fail("Password File was not created." + e.getMessage());
+ }
+ }
+
+
+ public void testRejectUsernameWithColon() throws Exception
+ {
+ String usernameWithColon = "user:name";
+ Principal principal = new UsernamePrincipal(usernameWithColon, null);
+
+ File testFile = createPasswordFile(0, 0);
+ loadPasswordFile(testFile);
+
+ try
+ {
+ getDatabase().createPrincipal(principal, TEST_PASSWORD_CHARS);
+ fail("Username with colon should be rejected");
+ }
+ catch (IllegalArgumentException e)
+ {
+ // pass
+ }
+ }
+
+ public void testRejectPasswordWithColon() throws Exception
+ {
+ String username = "username";
+ String passwordWithColon = "pass:word";
+ Principal principal = new UsernamePrincipal(username, null);
+
+ File testFile = createPasswordFile(0, 0);
+ loadPasswordFile(testFile);
+
+ try
+ {
+ getDatabase().createPrincipal(principal, passwordWithColon.toCharArray());
+ fail("Password with colon should be rejected");
+ }
+ catch (IllegalArgumentException e)
+ {
+ // pass
+ }
+
+ getDatabase().createPrincipal(_principal, TEST_PASSWORD_CHARS);
+ try
+ {
+ getDatabase().updatePassword(_principal, passwordWithColon.toCharArray());
+ fail("Password with colon should be rejected");
+ }
+ catch (IllegalArgumentException e)
+ {
+ // pass
+ }
+ }
+
+}
Modified: qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/security/auth/database/Base64MD5PasswordFilePrincipalDatabaseTest.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/security/auth/database/Base64MD5PasswordFilePrincipalDatabaseTest.java?rev=1762919&r1=1762918&r2=1762919&view=diff
==============================================================================
--- qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/security/auth/database/Base64MD5PasswordFilePrincipalDatabaseTest.java (original)
+++ qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/security/auth/database/Base64MD5PasswordFilePrincipalDatabaseTest.java Fri Sep 30 14:40:48 2016
@@ -21,7 +21,6 @@
package org.apache.qpid.server.security.auth.database;
import org.apache.qpid.server.security.auth.UsernamePrincipal;
-import org.apache.qpid.test.utils.QpidTestCase;
import javax.security.auth.callback.PasswordCallback;
import javax.security.auth.login.AccountNotFoundException;
@@ -40,12 +39,9 @@ import java.util.Arrays;
import java.util.List;
import java.util.regex.Pattern;
-public class Base64MD5PasswordFilePrincipalDatabaseTest extends QpidTestCase
+public class Base64MD5PasswordFilePrincipalDatabaseTest extends AbstractPasswordFilePrincipalDatabaseTest
{
- private static final String TEST_COMMENT = "# Test Comment";
-
- private static final String USERNAME = "testUser";
private static final String PASSWORD = "guest";
private static final String PASSWORD_B64MD5HASHED = "CE4DQ6BIb/BVMN9scFyLtA==";
private static char[] PASSWORD_MD5_CHARS;
@@ -69,6 +65,7 @@ public class Base64MD5PasswordFilePrinci
public void setUp() throws Exception
{
+ super.setUp();
_database = new Base64MD5PasswordFilePrincipalDatabase(null);
_pwdFile = File.createTempFile(this.getClass().getName(), "pwd");
_pwdFile.deleteOnExit();
@@ -78,75 +75,27 @@ public class Base64MD5PasswordFilePrinci
public void tearDown() throws Exception
{
- //clean up the created default password file and any backup
- File oldPwdFile = new File(_pwdFile.getAbsolutePath() + ".old");
- if(oldPwdFile.exists())
- {
- oldPwdFile.delete();
- }
-
- _pwdFile.delete();
-
- //clean up any additional files and their backups
- for(File f : _testPwdFiles)
- {
- oldPwdFile = new File(f.getAbsolutePath() + ".old");
- if(oldPwdFile.exists())
- {
- oldPwdFile.delete();
- }
-
- f.delete();
- }
- }
-
- private File createPasswordFile(int commentLines, int users)
- {
try
{
- File testFile = File.createTempFile("Base64MD5PDPDTest","tmp");
- testFile.deleteOnExit();
-
- BufferedWriter writer = new BufferedWriter(new FileWriter(testFile));
-
- for (int i = 0; i < commentLines; i++)
+ //clean up the created default password file and any backup
+ File oldPwdFile = new File(_pwdFile.getAbsolutePath() + ".old");
+ if (oldPwdFile.exists())
{
- writer.write(TEST_COMMENT);
- writer.newLine();
- }
-
- for (int i = 0; i < users; i++)
- {
- writer.write(USERNAME + i + ":Password");
- writer.newLine();
+ oldPwdFile.delete();
}
- writer.flush();
- writer.close();
-
- _testPwdFiles.add(testFile);
-
- return testFile;
-
+ _pwdFile.delete();
}
- catch (IOException e)
+ finally
{
- fail("Unable to create test password file." + e.getMessage());
+ super.tearDown();
}
-
- return null;
}
- private void loadPasswordFile(File file)
+ @Override
+ protected AbstractPasswordFilePrincipalDatabase getDatabase()
{
- try
- {
- _database.open(file);
- }
- catch (IOException e)
- {
- fail("Password File was not created." + e.getMessage());
- }
+ return _database;
}
/** **** Test Methods ************** */
@@ -162,7 +111,7 @@ public class Base64MD5PasswordFilePrinci
{
public String getName()
{
- return USERNAME;
+ return TEST_USERNAME;
}
};
@@ -192,7 +141,7 @@ public class Base64MD5PasswordFilePrinci
}
assertTrue("Password returned was incorrect.", Arrays.equals(PASSWORD_MD5_CHARS, callback.getPassword()));
- assertNotNull("Created User was not saved", _database.getUser(USERNAME));
+ assertNotNull("Created User was not saved", _database.getUser(TEST_USERNAME));
assertFalse("Duplicate user created.", _database.createPrincipal(principal, PASSWORD.toCharArray()));
}
@@ -253,7 +202,7 @@ public class Base64MD5PasswordFilePrinci
loadPasswordFile(testFile);
- Principal user = _database.getUser(USERNAME + "0");
+ Principal user = _database.getUser(TEST_USERNAME + "0");
assertNotNull("Generated user not present.", user);
try
@@ -287,7 +236,7 @@ public class Base64MD5PasswordFilePrinci
//pass
}
- assertNull("Deleted user still present.", _database.getUser(USERNAME + "0"));
+ assertNull("Deleted user still present.", _database.getUser(TEST_USERNAME + "0"));
}
public void testGetUsers()
@@ -315,7 +264,7 @@ public class Base64MD5PasswordFilePrinci
String name = principal.getName();
- int id = Integer.parseInt(name.substring(USERNAME.length()));
+ int id = Integer.parseInt(name.substring(TEST_USERNAME.length()));
assertFalse("Duplicated username retrieve", verify[id]);
verify[id] = true;
@@ -334,7 +283,7 @@ public class Base64MD5PasswordFilePrinci
loadPasswordFile(testFile);
- Principal testUser = _database.getUser(USERNAME + "0");
+ Principal testUser = _database.getUser(TEST_USERNAME + "0");
assertNotNull(testUser);
@@ -365,7 +314,7 @@ public class Base64MD5PasswordFilePrinci
assertEquals("User line not complete '" + userLine + "'", 2, result.length);
- assertEquals("Username not correct,", USERNAME + "0", result[0]);
+ assertEquals("Username not correct,", TEST_USERNAME + "0", result[0]);
assertEquals("New Password not correct,", NEW_PASSWORD_HASH, result[1]);
assertFalse("File has more content", reader.ready());
Modified: qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/security/auth/database/PlainPasswordFilePrincipalDatabaseTest.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/security/auth/database/PlainPasswordFilePrincipalDatabaseTest.java?rev=1762919&r1=1762918&r2=1762919&view=diff
==============================================================================
--- qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/security/auth/database/PlainPasswordFilePrincipalDatabaseTest.java (original)
+++ qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/security/auth/database/PlainPasswordFilePrincipalDatabaseTest.java Fri Sep 30 14:40:48 2016
@@ -41,41 +41,29 @@ import java.util.ArrayList;
import java.util.List;
import java.util.regex.Pattern;
-public class PlainPasswordFilePrincipalDatabaseTest extends QpidTestCase
+public class PlainPasswordFilePrincipalDatabaseTest extends AbstractPasswordFilePrincipalDatabaseTest
{
- private static final String TEST_COMMENT = "# Test Comment";
- private static final String TEST_PASSWORD = "testPassword";
- private static final char[] TEST_PASSWORD_CHARS = TEST_PASSWORD.toCharArray();
- private static final String TEST_USERNAME = "testUser";
-
+
private Principal _principal = new UsernamePrincipal(TEST_USERNAME, null);
private PlainPasswordFilePrincipalDatabase _database;
private List<File> _testPwdFiles = new ArrayList<File>();
public void setUp() throws Exception
{
+ super.setUp();
final AuthenticationProvider mockAuthenticationProvider = mock(AuthenticationProvider.class);
when(mockAuthenticationProvider.getContextValue(Integer.class, AbstractScramAuthenticationManager.QPID_AUTHMANAGER_SCRAM_ITERATION_COUNT)).thenReturn(4096);
_database = new PlainPasswordFilePrincipalDatabase(mockAuthenticationProvider);
_testPwdFiles.clear();
}
- public void tearDown() throws Exception
+ @Override
+ protected AbstractPasswordFilePrincipalDatabase getDatabase()
{
- //clean up any additional files and their backups
- for(File f : _testPwdFiles)
- {
- File oldPwdFile = new File(f.getAbsolutePath() + ".old");
- if(oldPwdFile.exists())
- {
- oldPwdFile.delete();
- }
-
- f.delete();
- }
+ return _database;
}
-
+
// ******* Test Methods ********** //
public void testCreatePrincipal()
@@ -326,17 +314,22 @@ public class PlainPasswordFilePrincipalD
testFile.delete();
}
- private void createUserPrincipal() throws IOException
+ public void testCreateUserPrincipal() throws IOException
+ {
+ Principal newPrincipal = createUserPrincipal();
+ assertNotNull(newPrincipal);
+ assertEquals(_principal.getName(), newPrincipal.getName());
+ }
+
+ private Principal createUserPrincipal()
{
File testFile = createPasswordFile(0, 0);
loadPasswordFile(testFile);
-
+
_database.createPrincipal(_principal, TEST_PASSWORD_CHARS);
- Principal newPrincipal = _database.getUser(TEST_USERNAME);
- assertNotNull(newPrincipal);
- assertEquals(_principal.getName(), newPrincipal.getName());
+ return _database.getUser(TEST_USERNAME);
}
-
+
public void testVerifyPassword() throws IOException, AccountNotFoundException
{
createUserPrincipal();
@@ -363,59 +356,4 @@ public class PlainPasswordFilePrincipalD
assertFalse(_database.verifyPassword(TEST_USERNAME, TEST_PASSWORD_CHARS));
assertTrue(_database.verifyPassword(TEST_USERNAME, newPwd));
}
-
-
-
- // *********** Utility Methods ******** //
-
- private File createPasswordFile(int commentLines, int users)
- {
- try
- {
- File testFile = File.createTempFile(this.getClass().getName(),"tmp");
- testFile.deleteOnExit();
-
- BufferedWriter writer = new BufferedWriter(new FileWriter(testFile));
-
- for (int i = 0; i < commentLines; i++)
- {
- writer.write(TEST_COMMENT);
- writer.newLine();
- }
-
- for (int i = 0; i < users; i++)
- {
- writer.write(TEST_USERNAME + i + ":" + TEST_PASSWORD);
- writer.newLine();
- }
-
- writer.flush();
- writer.close();
-
- _testPwdFiles.add(testFile);
-
- return testFile;
-
- }
- catch (IOException e)
- {
- fail("Unable to create test password file." + e.getMessage());
- }
-
- return null;
- }
-
- private void loadPasswordFile(File file)
- {
- try
- {
- _database.open(file);
- }
- catch (IOException e)
- {
- fail("Password File was not created." + e.getMessage());
- }
- }
-
-
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org