[
https://issues.apache.org/jira/browse/AMQ-4249?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Timothy Bish resolved AMQ-4249.
-------------------------------
Resolution: Fixed
Fix Version/s: 5.8.0
Nice investigation, patch applied to trunk.
> Race condition in PropertiesLoginModule
> ---------------------------------------
>
> Key: AMQ-4249
> URL: https://issues.apache.org/jira/browse/AMQ-4249
> Project: ActiveMQ
> Issue Type: Bug
> Components: Broker
> Affects Versions: 5.5.1, 5.6.0, 5.7.0, 5.8.0
> Environment: ActiveMQ 5.5.1 with JAAS authentication via
> PropertiesLoginModule configured, Spring JMS 3.0.5, 64-bit, Intel quad core
> CPU (i7 950), Windows 7
> Reporter: Tarjei Skorgenes
> Priority: Minor
> Labels: Authentication, JAAS, Security
> Fix For: 5.8.0
>
> Attachments: AMQ-4249-proposed-fix.diff, AMQ-4249-testcase.diff
>
>
> h1. Problem
> While setting up a connection pool towards ActiveMq 5.5.1 using Bitronix
> 2.1.3 I've been having some issues related to authentication and
> authorization of the JMS connections.
> When doing a clean restart of the JMS-clients JVM the connection pool has
> been unable to connect successfully with one or more of the 14 connections
> it's been set up to use.
> The error messages I've been getting has usually been one of the following
> two:
> # User system is not authorized to read from:
> ActiveMQ.Advisory.TempQueue,ActiveMQ.Advisory.TempTopic
> # User name or password is invalid.
> The broker has been set up using a very simplistic configuration:
> {code:xml}
> <?xml version="1.0" encoding="UTF-8"?>
> <beans xmlns="http://www.springframework.org/schema/beans";
> xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
> xmlns:context="http://www.springframework.org/schema/context";
> xsi:schemaLocation="http://activemq.apache.org/schema/core
> http://activemq.apache.org/schema/core/activemq-core-5.5.0.xsd
> http://www.springframework.org/schema/beans
> http://www.springframework.org/schema/beans/spring-beans.xsd
> http://www.springframework.org/schema/context
> http://www.springframework.org/schema/context/spring-context.xsd";>
> <context:property-placeholder />
> <broker brokerId="localhost"
> xmlns="http://activemq.apache.org/schema/core";
> dataDirectory="${datadir}" start="false">
> <managementContext>
> <managementContext connectorHost="0.0.0.0"
> connectorPort="14522" createConnector="true" />
> </managementContext>
> <plugins>
> <jaasAuthenticationPlugin
> configuration="activemq-domain"
> discoverLoginConfig="true" />
> <authorizationPlugin>
> <map>
> <authorizationMap>
> <authorizationEntries>
> <authorizationEntry
> queue=">" read="admins"
> write="admins"
> admin="admins" />
> <authorizationEntry
> topic=">" read="admins"
> write="admins"
> admin="admins" />
> <authorizationEntry
> topic="ActiveMQ.Advisory.>"
>
> read="guests,users" write="guests,users" admin="guests,users" />
> </authorizationEntries>
> </authorizationMap>
> </map>
> </authorizationPlugin>
> </plugins>
> <transportConnectors>
> <transportConnector id="openwire"
> uri="tcp://0.0.0.0:61616?trace=true" />
> </transportConnectors>
> </broker>
> </beans>
> {code}
> The JAAS-configuration has been verified to match username and password used
> by the client when connecting (username = system):
> h4. login.config
> {code}
> activemq-domain {
> org.apache.activemq.jaas.PropertiesLoginModule required
> debug=false
> org.apache.activemq.jaas.properties.user="users.properties"
> org.apache.activemq.jaas.properties.group="groups.properties";
> };
> {code}
> h4.users.properties
> {code}
> system=manager
> user=password
> guest=password
> {code}
> h4. groups.properties
> {code}
> admins=system
> users=user
> guests=guest
> {code}
> h1. Cause
> After debugging the problem it seems as if the problem is caused by a race
> condition introduced in PropertiesLoginModule in revision
> [1086219|https://fisheye6.atlassian.com/changelog/activemq?showid=1086219]
> (AMQ-3244). When the reload-support was added the users- and groups-fields
> were changed into static fields. But no additional synchronization was
> introduced, thereby introducing a race condition when several threads are
> entering the initialize- and commit-methods are the same time.
> The following section of the initialize-method in PropertiesLoginModule
> contains the first part of the race condition. Please note the unsynchronized
> modification of both the users- and groups static fields:
> {code:java}
> if (reload || users == null || uf.lastModified() > usersReloadTime) {
> if (debug) {
> LOG.debug("Reloading users from " + uf.getAbsolutePath());
> }
> try {
> users = new Properties(); // XXX Here be dragons
> 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);
> }
> }
> 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 (debug) {
> LOG.debug("Reloading groups from " + gf.getAbsolutePath());
> }
> try {
> groups = new Properties(); // XXX Here be dragons
> 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);
> }
> }
> {code}
> The next part comes in the login-method when the users password is retrieved:
> {code:java}
> String password = users.getProperty(user);
> {code}
> The final part of the puzzle occurs in the commit-method:
> {code:java}
> for (Enumeration<?> enumeration = groups.keys();
> enumeration.hasMoreElements();) {
> String name = (String)enumeration.nextElement();
> String[] userList = ((String)groups.getProperty(name) +
> "").split(",");
> for (int i = 0; i < userList.length; i++) {
> if (user.equals(userList[i])) {
> principals.add(new GroupPrincipal(name));
> break;
> }
> }
> }
> {code}
> The retrieval of the user password will fail if invoked by a thread
> immediately after a different thread has assigned an empty Properties-object
> to the users field in the initialize-method.
> Similarly population of the GroupPrincipals into the Subject in the
> commit-method will silently fail if executed by a thread immediately after a
> different thread has assigned an empty Properties-object to the groups-field
> in the initialize-method.
> h1. Proposed solution
> I've created a testcase that reproduces the problem and an additional patch
> that introduces a wrapper around the Properties-objects for the users- and
> groups-fields.
> The testcase and the proposed solution is available via
> https://github.com/lothor/activemq.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
