diff --git a/activemq-jaas/src/main/java/org/apache/activemq/jaas/PrincipalProperties.java b/activemq-jaas/src/main/java/org/apache/activemq/jaas/PrincipalProperties.java new file mode 100644 index 0000000000..601d0cccac --- /dev/null +++ b/activemq-jaas/src/main/java/org/apache/activemq/jaas/PrincipalProperties.java @@ -0,0 +1,67 @@ +/** + * 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.activemq.jaas; + +import java.io.File; +import java.io.FileInputStream; +import java.io.FileNotFoundException; +import java.io.IOException; +import java.util.Map; +import java.util.Properties; +import java.util.Set; + +import org.slf4j.Logger; + +class PrincipalProperties { + private final Properties principals; + private final long reloadTime; + + PrincipalProperties(final String type, final File source, final Logger log) { + Properties props = new Properties(); + long reloadTime = 0; + try { + load(source, props); + reloadTime = System.currentTimeMillis(); + } catch (IOException ioe) { + log.warn("Unable to load " + type + " properties file " + source); + } + this.reloadTime = reloadTime; + this.principals = props; + } + + @SuppressWarnings({ "unchecked", "rawtypes" }) + Set> entries() { + return (Set) principals.entrySet(); + } + + String getProperty(String name) { + return principals.getProperty(name); + } + + long getReloadTime() { + return reloadTime; + } + + private void load(final File source, Properties props) throws FileNotFoundException, IOException { + FileInputStream in = new FileInputStream(source); + try { + props.load(in); + } finally { + in.close(); + } + } +} diff --git a/activemq-jaas/src/main/java/org/apache/activemq/jaas/PropertiesLoginModule.java b/activemq-jaas/src/main/java/org/apache/activemq/jaas/PropertiesLoginModule.java index ece59d9226..cc0d84702a 100644 --- a/activemq-jaas/src/main/java/org/apache/activemq/jaas/PropertiesLoginModule.java +++ b/activemq-jaas/src/main/java/org/apache/activemq/jaas/PropertiesLoginModule.java @@ -19,10 +19,8 @@ package org.apache.activemq.jaas; import java.io.File; import java.io.IOException; import java.security.Principal; -import java.util.Enumeration; import java.util.HashSet; import java.util.Map; -import java.util.Properties; import java.util.Set; import javax.security.auth.Subject; @@ -50,14 +48,10 @@ public class PropertiesLoginModule implements LoginModule { private boolean debug; private boolean reload = false; - private static String usersFile; - private static String groupsFile; - private static Properties users; - private static Properties groups; - private static long usersReloadTime = 0; - private static long groupsReloadTime = 0; + private static volatile PrincipalProperties users; + private static volatile PrincipalProperties groups; private String user; - private Set principals = new HashSet(); + private final Set principals = new HashSet(); private File baseDir; private boolean loginSucceeded; @@ -67,49 +61,33 @@ public class PropertiesLoginModule implements LoginModule { this.callbackHandler = callbackHandler; loginSucceeded = false; - debug = "true".equalsIgnoreCase((String)options.get("debug")); + debug = "true".equalsIgnoreCase((String) options.get("debug")); if (options.get("reload") != null) { - reload = "true".equalsIgnoreCase((String)options.get("reload")); + reload = "true".equalsIgnoreCase((String) options.get("reload")); } if (options.get("baseDir") != null) { - baseDir = new File((String)options.get("baseDir")); + baseDir = new File((String) options.get("baseDir")); } setBaseDir(); - usersFile = (String) options.get(USER_FILE) + ""; + String usersFile = (String) options.get(USER_FILE) + ""; File uf = baseDir != null ? new File(baseDir, usersFile) : new File(usersFile); - if (reload || users == null || uf.lastModified() > usersReloadTime) { + if (reload || users == null || uf.lastModified() > users.getReloadTime()) { if (debug) { LOG.debug("Reloading users from " + uf.getAbsolutePath()); } - try { - users = new Properties(); - java.io.FileInputStream in = new java.io.FileInputStream(uf); - users.load(in); - in.close(); - usersReloadTime = System.currentTimeMillis(); - } catch (IOException ioe) { - LOG.warn("Unable to load user properties file " + uf); - } + users = new PrincipalProperties("user", uf, LOG); } - groupsFile = (String) options.get(GROUP_FILE) + ""; + String groupsFile = (String) options.get(GROUP_FILE) + ""; File gf = baseDir != null ? new File(baseDir, groupsFile) : new File(groupsFile); - if (reload || groups == null || gf.lastModified() > groupsReloadTime) { + if (reload || groups == null || gf.lastModified() > groups.getReloadTime()) { if (debug) { LOG.debug("Reloading groups from " + gf.getAbsolutePath()); } - try { - groups = new Properties(); - java.io.FileInputStream in = new java.io.FileInputStream(gf); - groups.load(in); - in.close(); - groupsReloadTime = System.currentTimeMillis(); - } catch (IOException ioe) { - LOG.warn("Unable to load group properties file " + gf); - } + groups = new PrincipalProperties("group", gf, LOG); } } @@ -137,8 +115,8 @@ public class PropertiesLoginModule implements LoginModule { } catch (UnsupportedCallbackException uce) { throw new LoginException(uce.getMessage() + " not available to obtain information from user"); } - user = ((NameCallback)callbacks[0]).getName(); - char[] tmpPassword = ((PasswordCallback)callbacks[1]).getPassword(); + user = ((NameCallback) callbacks[0]).getName(); + char[] tmpPassword = ((PasswordCallback) callbacks[1]).getPassword(); if (tmpPassword == null) { tmpPassword = new char[0]; } @@ -167,9 +145,9 @@ public class PropertiesLoginModule implements LoginModule { if (result) { principals.add(new UserPrincipal(user)); - for (Enumeration enumeration = groups.keys(); enumeration.hasMoreElements();) { - String name = (String)enumeration.nextElement(); - String[] userList = ((String)groups.getProperty(name) + "").split(","); + for (Map.Entry entry : groups.entries()) { + String name = entry.getKey(); + String[] userList = entry.getValue().split(","); for (int i = 0; i < userList.length; i++) { if (user.equals(userList[i])) { principals.add(new GroupPrincipal(name)); @@ -215,4 +193,12 @@ public class PropertiesLoginModule implements LoginModule { user = null; loginSucceeded = false; } + + /** + * For test-usage only. + */ + static void resetUsersAndGroupsCache() { + users = null; + groups = null; + } } diff --git a/activemq-jaas/src/test/java/org/apache/activemq/jaas/PropertiesLoginModuleRaceConditionTest.java b/activemq-jaas/src/test/java/org/apache/activemq/jaas/PropertiesLoginModuleRaceConditionTest.java new file mode 100644 index 0000000000..5e38671a18 --- /dev/null +++ b/activemq-jaas/src/test/java/org/apache/activemq/jaas/PropertiesLoginModuleRaceConditionTest.java @@ -0,0 +1,191 @@ +/** + * 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.activemq.jaas; + +import static org.junit.Assert.assertTrue; + +import java.io.File; +import java.io.FileNotFoundException; +import java.io.FileOutputStream; +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; +import java.util.Properties; +import java.util.concurrent.ArrayBlockingQueue; +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; + +import javax.security.auth.Subject; +import javax.security.auth.callback.CallbackHandler; + +import org.junit.After; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ErrorCollector; +import org.junit.rules.TemporaryFolder; +import org.junit.rules.TestName; + +public class PropertiesLoginModuleRaceConditionTest { + + private static final String GROUPS_FILE = "groups.properties"; + private static final String USERS_FILE = "users.properties"; + private static final String USERNAME = "first"; + private static final String PASSWORD = "secret"; + + @Rule + public final ErrorCollector e = new ErrorCollector(); + + @Rule + public final TemporaryFolder temp = new TemporaryFolder(); + + @Rule + public final TestName name = new TestName(); + + private Map options; + private BlockingQueue errors; + private ExecutorService pool; + private CallbackHandler callback; + + private static class LoginTester implements Runnable { + private final CountDownLatch finished; + private final BlockingQueue errors; + private final Map options; + private final CountDownLatch start; + private final CallbackHandler callback; + + LoginTester(CountDownLatch start, CountDownLatch finished, + BlockingQueue errors, Map options, + CallbackHandler callbackHandler) { + this.finished = finished; + this.errors = errors; + this.options = options; + this.start = start; + this.callback = callbackHandler; + } + + @Override + public void run() { + try { + start.await(); + + Subject subject = new Subject(); + PropertiesLoginModule module = new PropertiesLoginModule(); + module.initialize(subject, callback, new HashMap(), + options); + module.login(); + module.commit(); + } catch (Exception e) { + errors.offer(e); + } finally { + finished.countDown(); + } + } + } + + @Before + public void before() throws FileNotFoundException, IOException { + createUsers(); + createGroups(); + + options = new HashMap(); + options.put("reload", "true"); // Used to simplify reproduction of the + // race condition + options.put("org.apache.activemq.jaas.properties.user", USERS_FILE); + options.put("org.apache.activemq.jaas.properties.group", GROUPS_FILE); + options.put("baseDir", temp.getRoot().getAbsolutePath()); + + errors = new ArrayBlockingQueue(processorCount()); + pool = Executors.newFixedThreadPool(processorCount()); + callback = new JassCredentialCallbackHandler(USERNAME, PASSWORD); + } + + @After + public void after() throws InterruptedException { + pool.shutdown(); + assertTrue(pool.awaitTermination(500, TimeUnit.SECONDS)); + PropertiesLoginModule.resetUsersAndGroupsCache(); + } + + @Test + public void raceConditionInUsersAndGroupsLoading() throws InterruptedException, FileNotFoundException, IOException { + + // Brute force approach to increase the likelihood of the race condition occurring + for (int i = 0; i < 25000; i++) { + final CountDownLatch start = new CountDownLatch(1); + final CountDownLatch finished = new CountDownLatch(processorCount()); + prepareLoginThreads(start, finished); + + // Releases every login thread simultaneously to increase our chances of + // encountering the race condition + start.countDown(); + + finished.await(); + if (isRaceConditionDetected()) { + e.addError(new AssertionError("At least one race condition in PropertiesLoginModule " + + "has been encountered. Please examine the " + + "following stack traces for more details:")); + for (Exception exception : errors) { + e.addError(exception); + } + return; + } + } + } + + private boolean isRaceConditionDetected() { + return errors.size() > 0; + } + + private void prepareLoginThreads(final CountDownLatch start, final CountDownLatch finished) { + for (int processor = 1; processor <= processorCount() * 2; processor++) { + pool.submit(new LoginTester(start, finished, errors, options, callback)); + } + } + + private int processorCount() { + return Runtime.getRuntime().availableProcessors(); + } + + private void store(Properties from, File to) throws FileNotFoundException, IOException { + FileOutputStream output = new FileOutputStream(to); + try { + from.store(output, "Generated by " + name.getMethodName()); + } finally { + output.close(); + } + } + + private void createGroups() throws FileNotFoundException, IOException { + Properties groups = new Properties(); + for (int i = 0; i < 100; i++) { + groups.put("group" + i, "first,second,third"); + } + store(groups, temp.newFile(GROUPS_FILE)); + } + + private void createUsers() throws FileNotFoundException, IOException { + Properties users = new Properties(); + users.put(USERNAME, PASSWORD); + users.put("second", PASSWORD); + users.put("third", PASSWORD); + store(users, temp.newFile(USERS_FILE)); + } +}