activemq-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Timothy Bish (JIRA)" <j...@apache.org>
Subject [jira] [Resolved] (AMQ-4249) Race condition in PropertiesLoginModule
Date Mon, 14 Jan 2013 21:52:16 GMT

     [ 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

Mime
View raw message