Authorization Not Checked within Transacted JMS Sessions

[tmjkeeney]

I have a JMS security configuration problem with JBoss 3.2.3 where I am using straight JMS to write a message to a Queue from a Session Bean. I have configured the Session Bean to use the JVM IL and am explicitly managing transactions within the JMS Session. Since I am not providing login credentials during the JMS ConnectionFactory/Queue lookups, the username is set to the unauthenticatedIdentity = guest. The guest user is not assigned any roles in jbossmq-state.xml, and the JaasSecurityManager confirms this in the TRACE log. The problem is that the guest user has free access to write messages to the Queue even though that Queue is configured with explicit security roles. I noticed that when I disabled transactions in the JMS Session, the guest fails authorization on the Queue and is not able to write messages. This is the expected behavior.

I believe I traced the problem to a particular method in the SpySession class:

 //called by a MessageProducer object which needs to publish a message
 void sendMessage( SpyMessage m )
 throws JMSException
 {
 if ( closed )
 {
 throw new IllegalStateException( "The session is closed" );
 }

 // Make sure the message has the correct client id
 m.header.producerClientId = connection.getClientID();

 if ( transacted )
 {
 connection.spyXAResourceManager.addMessage( currentTransactionId, m.myClone() );
 }
 else
 {
 connection.sendToServer( m );
 }

 }

If transacted == false, then you can follow the call stack to ServerSecurityInterceptor.addMessage(), which contains the code to authorize a message write. However, if transacted == true, I do not see how ServerSecurityInterceptor.addMessage() is invoked. This confirms my observation that a transacted Session will permit an authenticated user to write to any Queue. This seems like a major security flaw.

I would like to believe that I have missed some configuration item. Note that I also tried the above scenario, switching local transactions thru java:/ConnectionFactory for global transactions thru java:/JmsXA. The security leak is still there.

Please advise me as to how I may prevent an authenticated, but not authorized, user (whether guest or someone else) from writing to a Queue within a transaction.

Thanks,

--Tom

[adrian.brock]

That is horrible.

Here's the fix (org.jboss.mq.security.ServerSecurityInterceptor) add the method

 // Check that a transaction request is authorized to send the messages
 public void transact(ConnectionToken dc, TransactionRequest t) throws JMSException
 {
 if (t.messages != null)
 {
 // Optimization for common case
 if (t.messages.length == 1)
 {
 String dest = ((SpyDestination) t.messages[0].getJMSDestination()).getName();
 if (authorizeWrite(dc, dest) == false)
 throw new JMSSecurityException("Connection not authorized to addMessages to destination: " + dest);
 }
 else if (t.messages.length > 0)
 {
 HashSet destinations = new HashSet();
 for (int i = 0; i < t.messages.length; ++i)
 destinations.add(((SpyDestination) t.messages.getJMSDestination()).getName());

 for (Iterator i = destinations.iterator(); i.hasNext();)
 {
 String destinationName = (String) i.next();
 if (authorizeWrite(dc, destinationName) == false)
 if (!authorizeWrite(dc, destinationName))
 throw new JMSSecurityException("Connection not authorized to addMessages to destination: " + destinationName);
 }
 }
 }
 super.transact(dc, t);
 }

Thanks for the bug report.

[tmjkeeney]

I tested the fix with a single message in the transaction, and access was denied as expected for users without authorization. Thank you sir! I noticed two minor issues with the fix though.

1) It appears that Nukes interpreted the array index [ i ] as an italics style in the code that adds the destination names to the Set.

2) There is a redundant authorization check for transactions with > 1 message.

I believe the final code should be (see bold-faced lines for corrections):

 // Check that a transaction request is authorized to send the messages
 public void transact(ConnectionToken dc, TransactionRequest t) throws JMSException
 {
 if (t.messages != null)
 {
 // Optimization for common case
 if (t.messages.length == 1)
 {
 String dest = ((SpyDestination) t.messages[0].getJMSDestination()).getName();
 if (authorizeWrite(dc, dest) == false)
 throw new JMSSecurityException("Connection not authorized to addMessages to destination: " + dest);
 }
 else if (t.messages.length > 0)
 {
 HashSet destinations = new HashSet();
 for (int i = 0; i < t.messages.length; ++i)
 destinations.add(((SpyDestination) t.messages[ i ].getJMSDestination()).getName());

 for (Iterator i = destinations.iterator(); i.hasNext();)
 {
 String destinationName = (String) i.next();
 if (authorizeWrite(dc, destinationName) == false)
 throw new JMSSecurityException("Connection not authorized to addMessages to destination: " + destinationName);
 }
 }
 }
 super.transact(dc, t);
 }

[adrian.brock]

Thanks that was a cut and paste error.