Stomp protocol does not correctly check authentication (security hole)

• Key: AMQ-1272
• Type: Bug
• Status: Reopened
• Priority: Major
• Assignee: Unassigned
• Reporter: Tom Samplonius
• Component/s: Broker
• Affects Version/s: 5.0.0
• Fix Version/s: 5.4.0

Description

ActiveMQ does not correctly validate the username and password of Stomp clients. A security exception is generated, but ignored, leaving the client connected, and with full and unrestricted access to ActiveMQ.

Further description, and a partial patch:

http://www.nabble.com/Getting-Stomp-support-to-a-usable-state...-tf3858629s2354.html#a11060452

BTW, while the patch in the above post, is crude, however, leaving unauthenticated users connected with full-access makes ActiveMQ and Stomp pretty unusable. So please apply the path, rather than do nothing.

Comments

[James Strachan]

added an affected version

[Tom Samplonius]

I have tested the patch that Pieter posted, and it doesn't make any changed.

It doesn't seem like this should be a hard problem to fix. I'd fix it, but I'm wish I understand the interaction between the various protocol classes and the core.

[Pieter]

I am the author of the patch mentioned. It worked for me (the patch was against apache-activemq-4.2-20070510.230653-54 snapshot). Without the patch applied, SecurityException's were already visible in the debug log, however the were silently dropped (there is a try/catch block somewhere in the connection handler that handles this. It then creates an ExceptionResponse which is passed to the handler in onStompConnect, where the response isn't checked anymore). If these were not visbile, I guess something else is wrong with the authentication setup.

I'm using the simple authenticator btw, but this shouldn't matter.

[Tom Samplonius]

I have re-tested the patch. I applied the patch (more or less) to activemq-5.0-20070621

The behaviour is a bit strange. A Stomp producer simply blocks (I'm using Net::Stomp). A Stomp consumer does too. Very odd. At least unauthenticated Stomp clients can't steal messages.

I'm going to try to attach my patch (context diff).

[Tom Samplonius]

Patch to Stomp ProtocolConverter.java

This is just an adapted from Pieter's patch, but to activemq-5.0-20070621, as a context diff.

[Tom Samplonius]

Here is the config that I'm using:

<plugins>
<simpleAuthenticationPlugin>
<users>
<authenticationUser username="system" password="manager" groups="producers,consumers,admins" />
<authenticationUser username="frontend" password="manager" groups="producers" />
<authenticationUser username="backend" password="manager" groups="consumers" />
</users>
</simpleAuthenticationPlugin>
<authorizationPlugin>
<map>
<authorizationMap>
<authorizationEntries>
<authorizationEntry queue=">" read="backend" write="frontend" admin="admins" />
</authorizationEntries>
</authorizationMap>
</map>
</authorizationPlugin>
</plugins>

[Tom Samplonius]

Just some further clarification. activemq-5.0-20070621 with the patch has the following behavior:

1. If a Stomp client uses the wrong password and/or wrong user name, it will just block forever when trying to send to a destination.

2. If a Stomp client uses a matching username and password, but that user does not have permission to write to the queue (as in the above nonsensical auth configuration, where none of the groups match), ActiveMQ will just eat the message and log an exception for each message:

ERROR Service - Async error occurred: java.lang.SecurityException: User system is not authorized to write to: queue://foo
java.lang.SecurityException: User system is not authorized to write to: queue://foo

So the client is never disconnected, and never is sent an error. It is a step in the right direction.

I'm using the PHP Stomp client, that might explain the difference in the behaviour with incorrecht auth details. I don't have access to the setup right now, I will report the exact behaviour when I do. I guess it's possible that the Net::Stomp code expects a Stomp ERROR frame, which isn't sent, or a CONNECTED frame (which isn't sent either). Perhaps it's waiting for these frames, I don't know the code. I think in my case the socket was disconnected, but I'm not sure. I will investigate.

[Tom Samplonius]

Well, the Perl Net::Stomp client definitely notices if the socket goes away, because if I kill ActiveMQ, my client reports an immediate error:

Error reading command: at /usr/local/lib/perl5/site_perl/5.8.8/Net/Stomp/Frame.pm line 37, <GEN0> line 25076.

This is expected though. When the receive_frame() method is called, Net::Stomp does a blocking read on a socket. If that socket is closed, the read will return with an error code. And then the Net::Stomp kicks out an "Error reading command" exception.

I have also retested this with a 5.0 snapshot from July 12, with the stomp.diff patch that I posed above. The behavior is the same. A Stomp client will just block during login. I can see in netstat that there is a one 61613 socket open as well, so it is not definitely not being closed by ActiveMQ.

[Dejan Bosanac]

I've refactored a ProtocolConverter, StompTest and added a few tests to make all this work.

Here's a list of things that has been done:

• added xbean resources to set up a test broker with configured authentication
• modified existing tests to connect with system/manager user so that they have right privileges
• modified protocolconverter.onStompConnect to send "error" frame on connect attempt with wrong credentials
• modified protocolConverter.createResponseHandler to send "error" frame on security exception on any other command
• added test cases to assert proper behavior on connect, send and subscribe

[Hiram Chirino]

Applied Dejan's patch.

[Donald Woods]

This security fix also needs to be applied to the 4.1 branch, for users like Geronimo who have not upgraded to AMQ 5.1.0 yet.

[Donald Woods]

Patch created against branches/activemq-4.1 Rev647819 (current as of 20080825)

[Gary Tully]

patch requested against 4.1.x - fix is on on trunk.