You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by "Tarjei Skorgenes (JIRA)" <ji...@apache.org> on 2013/01/12 02:01:17 UTC
[jira] [Commented] (AMQ-4249) Race condition in
PropertiesLoginModule
[ https://issues.apache.org/jira/browse/AMQ-4249?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13551713#comment-13551713 ]
Tarjei Skorgenes commented on AMQ-4249:
---------------------------------------
I've verified that the problem seems to be present both in 5.5.1 and the current trunk (5.8-SNAPSHOT). Based on the SVN history I'm fairly certain it's present in 5.6 and 5.7 as well.
> 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
>
> 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 illustrates the problem and an additional patch that introduces a wrapper around the Properties-objects for the users- and groups-fields. The wrapper uses a read-write lock to synchronize access and modification of the properties-objects containing the users and groups.
> The testcase is available via https://github.com/lothor/activemq/commit/42f430c74a345729ca099eab2464d562ded62fbe and the proposed solution via https://github.com/lothor/activemq/commit/263cf14c796e993261351186084b6c3afd939afb.
--
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