ActiveMQ
  1. ActiveMQ
  2. AMQ-3211

JMSXUserId Can be spoofed by client

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 5.4.2
    • Fix Version/s: 5.5.0
    • Component/s: Broker
    • Labels:
      None

      Description

      It seems the JMSXUserId can be spoofed by client contrary to what http://activemq.apache.org/jmsxuserid.html says.

      My test setup is populateJMSXUserID="true set in a single broker, a JAAS config org.apache.activemq.jaas.TextFileCertificateLoginModule and using mutual auth SSL (i.e., ?needClientAuth=true for transportConnector setup), and a single consumer and producer based on small modifications of the ConsumerTool and ProducerTool examples in the 5.4.2 distro. See attached the changes to the distro package to demonstrate the bug. Just do
      1. run apache-activemq-5.4.2/bin/activemq-admin start
      2. in apache-activemq-5.4.2/example run ant consumer -Durl=ssl://localhost:61617 -Dmax=3 -Dverbose=true
      3. in another shell in apache-activemq-5.4.2/example run ant producer -Durl=ssl://localhost:61617 -Dmax=3 -Dverbose=true
      4. look at the output of the consumer for the properties printed after each received message (the producer spoofs only on even numbered messages)

      When the client does not set the property, then i get the properly authenticated DN as JMSXUserID using message.getStringProperty("JMSXUserID"). However, when the client sets it, i get the value set by the client. The only difference i notice is that in the former case, message.getPropertyNames() does not return JMSXUserID whereas in the spoofed case it does.

      i wonder whether in the context of https://issues.apache.org/jira/browse/QPID-943 or https://issues.apache.org/jira/browse/AMQ-2840 (which interestingly doesn't list JMSXUserID as supported in a comment even though it is?) something got messed up?

      1. JMSXUserID-bug.diff
        42 kB
        Michael Steiner
      2. JMSXUserID-bug.conf-src.tar.bz2
        34 kB
        Michael Steiner

        Issue Links

          Activity

          Hide
          Gary Tully added a comment -

          updated the doco, https://cwiki.apache.org/confluence/display/ACTIVEMQ/JMSXUserID

          thanks for the feedback

          Show
          Gary Tully added a comment - updated the doco, https://cwiki.apache.org/confluence/display/ACTIVEMQ/JMSXUserID thanks for the feedback
          Hide
          Michael Steiner added a comment -

          i kindof assumed that i wouldn't be allowed to change but as i saw an edit button and could sign up, i thought i should give it try instead of offloading everything to you

          Anyway, what i would have added is along the lines of below: the disclaimer of the second paragraph certainly needs some editing regarding version references – not sure how you handle that – but i thought it would be good to document the vulnerability. Arguably, one could also mention the getPropertyNames() test i outlined above as temporary workaround for older versions but i'm not 100% sure whether it secure, so i omitted that.

          ``If you allow anonymous access, you MUST also add the useAuthenticatedPrincipalForJMSXUserID property of the broker element and set it to true. Otherwise, anonymous clients can spoof identities. Note, though, that for SSL certificate based authentication, e.g., when using TextFileCertificateLoginModule JAAS module, this will change the semantics of the broker-provided JMSXUserID. Instead of returning the DN of the certificate, it will provide the name the DN is mapped to by the JAAS module.

          Also note that versions up to (and including) 5.4.2 are vulnerable to spoofing. A fix is included in 5.5-SNAPSHOT > March 12th, 2011.''

          Show
          Michael Steiner added a comment - i kindof assumed that i wouldn't be allowed to change but as i saw an edit button and could sign up, i thought i should give it try instead of offloading everything to you Anyway, what i would have added is along the lines of below: the disclaimer of the second paragraph certainly needs some editing regarding version references – not sure how you handle that – but i thought it would be good to document the vulnerability. Arguably, one could also mention the getPropertyNames() test i outlined above as temporary workaround for older versions but i'm not 100% sure whether it secure, so i omitted that. ``If you allow anonymous access, you MUST also add the useAuthenticatedPrincipalForJMSXUserID property of the broker element and set it to true. Otherwise, anonymous clients can spoof identities. Note, though, that for SSL certificate based authentication, e.g., when using TextFileCertificateLoginModule JAAS module, this will change the semantics of the broker-provided JMSXUserID. Instead of returning the DN of the certificate, it will provide the name the DN is mapped to by the JAAS module. Also note that versions up to (and including) 5.4.2 are vulnerable to spoofing. A fix is included in 5.5-SNAPSHOT > March 12th, 2011.''
          Hide
          Gary Tully added a comment -

          yeah, confluence is restricted to committers,
          can you add (via comment or diff/patch) any doco updates to this jira and I will update confluence.

          Show
          Gary Tully added a comment - yeah, confluence is restricted to committers, can you add (via comment or diff/patch) any doco updates to this jira and I will update confluence.
          Hide
          Michael Steiner added a comment -

          Thanks! Indeed, that field forces now the guest ids of JAAS into JMSXUserID. Incidentally, it also makes JMSXUserID now return mapped identities rather than X509 DNs for the SSL case.

          BTW: Given the spoofing potential without that option (and the bug in <5.5), i tried to update http://activemq.apache.org/jmsxuserid.html correspondingly. But i guess just signing up to confluence is not enough to update these pages?

          Show
          Michael Steiner added a comment - Thanks! Indeed, that field forces now the guest ids of JAAS into JMSXUserID. Incidentally, it also makes JMSXUserID now return mapped identities rather than X509 DNs for the SSL case. BTW: Given the spoofing potential without that option (and the bug in <5.5), i tried to update http://activemq.apache.org/jmsxuserid.html correspondingly. But i guess just signing up to confluence is not enough to update these pages?
          Hide
          Gary Tully added a comment - - edited

          there is an additional broker attribute: useAuthenticatedPrincipalForJMSXUserID which will ensure that the authenticated principal is placed in the JMSXUserId, such that it is explicitly set or overridden in the authenticated case.

          Show
          Gary Tully added a comment - - edited there is an additional broker attribute: useAuthenticatedPrincipalForJMSXUserID which will ensure that the authenticated principal is placed in the JMSXUserId, such that it is explicitly set or overridden in the authenticated case.
          Hide
          Michael Steiner added a comment -

          > Can you validate the next 5.5-SNAPSHOT to ensure the fix captures your use case.

          Thanks for the quick turn-around!!

          I did not find a new built SNAPSHOT version but i saw changes in the
          source and so built the version from svn. While a few tests failed,
          (e.g., org.apache.activemq.network.DuplexNetworkMBeanTest and
          org.apache.activemq.bugs.AMQ2021Test), i could successfully run my
          tests and confirm that with the change JMSXUserID could not be spoofed
          anymore (at least not the way i did in the past .

          > A client supplied JMSXUserID value is only used as a last resort. I
          > think it is better from an interop and backward compatibility
          > perspective to not explicitly disallow setting this property.

          I'm not sure whether there is really a good use case for having clients
          providing it, at least my security mindset would make opt for erring
          on the safe side. However, some of the the scenarios mentioned in
          https://issues.apache.org/jira/browse/QPID-943 might need that
          (although the original request still required validation at the broker
          side which would be fail-safe). However, I see at least two potentially
          problematic cases:

          • whether application code would see correct or potentially spoofed
            fields would depend on a config.
          • what happens if the config must allow unauthenticated clients?

          The former is maybe more a stylic/robustness issue, i.e., multiple
          lines of defense. The latter though is a clear security issue. And
          after a quick test with non-x509 based JAAS config and

          org.apache.activemq.jaas.GuestLoginModule sufficient
          debug=true
          org.apache.activemq.jaas.guest.user="guest"
          org.apache.activemq.jaas.guest.group="guests";

          it seems anonymous guest result in no JMXSUserID (ie.d., above
          guest.user/group seem to be ignored as far as JMSXUserID is
          concerned) and i can still spoof ids.

          So ideally, not allowing clients from setting it at all would be
          best. Alternatives, i see (in decreasing orders of desirability )

          (a) provide an option which allows passing through only validated
          JMSXUserID (or to be on the fail-safe side, an option which
          would have to be explicitly provided to allow a pass-through)

          (b) Garantuee that an anonymous user has always a well-identified
          broker-provided JMSXUserID value

          (c) provide a way for the code to check where the value come from.

          (Arguably, all together might even be better

          Regarding (c), there is actually a way by which it can be safely
          determined that the field is not spoofed: if
          message.getPropertyNames() does not include JMSXUserID but
          .getStringProperty("JMSXUserID") is returned, it will not have been
          set by JMS client code. Not sure, though, whether one couldn't modify
          the JMS client library to change that behaviour and it might also
          provide false alerts if the client has set the value but the broker
          has overridden/corrected it.

          Show
          Michael Steiner added a comment - > Can you validate the next 5.5-SNAPSHOT to ensure the fix captures your use case. Thanks for the quick turn-around!! I did not find a new built SNAPSHOT version but i saw changes in the source and so built the version from svn. While a few tests failed, (e.g., org.apache.activemq.network.DuplexNetworkMBeanTest and org.apache.activemq.bugs.AMQ2021Test), i could successfully run my tests and confirm that with the change JMSXUserID could not be spoofed anymore (at least not the way i did in the past . > A client supplied JMSXUserID value is only used as a last resort. I > think it is better from an interop and backward compatibility > perspective to not explicitly disallow setting this property. I'm not sure whether there is really a good use case for having clients providing it, at least my security mindset would make opt for erring on the safe side. However, some of the the scenarios mentioned in https://issues.apache.org/jira/browse/QPID-943 might need that (although the original request still required validation at the broker side which would be fail-safe). However, I see at least two potentially problematic cases: whether application code would see correct or potentially spoofed fields would depend on a config. what happens if the config must allow unauthenticated clients? The former is maybe more a stylic/robustness issue, i.e., multiple lines of defense. The latter though is a clear security issue. And after a quick test with non-x509 based JAAS config and org.apache.activemq.jaas.GuestLoginModule sufficient debug=true org.apache.activemq.jaas.guest.user="guest" org.apache.activemq.jaas.guest.group="guests"; it seems anonymous guest result in no JMXSUserID (ie.d., above guest.user/group seem to be ignored as far as JMSXUserID is concerned) and i can still spoof ids. So ideally, not allowing clients from setting it at all would be best. Alternatives, i see (in decreasing orders of desirability ) (a) provide an option which allows passing through only validated JMSXUserID (or to be on the fail-safe side, an option which would have to be explicitly provided to allow a pass-through) (b) Garantuee that an anonymous user has always a well-identified broker-provided JMSXUserID value (c) provide a way for the code to check where the value come from. (Arguably, all together might even be better Regarding (c), there is actually a way by which it can be safely determined that the field is not spoofed: if message.getPropertyNames() does not include JMSXUserID but .getStringProperty("JMSXUserID") is returned, it will not have been set by JMS client code. Not sure, though, whether one couldn't modify the JMS client library to change that behaviour and it might also provide false alerts if the client has set the value but the broker has overridden/corrected it.
          Hide
          Gary Tully added a comment -

          resolved in http://svn.apache.org/viewvc?rev=1080212&view=rev
          Can you validate the next 5.5-SNAPSHOT to ensure the fix captures your use case.
          A client supplied JMSXUserID value is only used as a last resort. I think it is better from an interop and backward compatibility perspective to not explicitly disallow setting this property.

          Show
          Gary Tully added a comment - resolved in http://svn.apache.org/viewvc?rev=1080212&view=rev Can you validate the next 5.5-SNAPSHOT to ensure the fix captures your use case. A client supplied JMSXUserID value is only used as a last resort. I think it is better from an interop and backward compatibility perspective to not explicitly disallow setting this property.
          Hide
          Gary Tully added a comment -

          This seems broken to me. ActiveMQ does allow setting the JMSXUserId property but it should only be used in the absence of any other setting of user credentials.
          Currently, the call to get JMSXUserId looks first at the existing properties and then at the internal userId attribute. That logic should be reversed.

          Typically, users should not set the JMSXUserId value, but if they do, it should be ignored if there is an alternative policy in place, ie: when BrokerService.populateJMSXUserID is specified and either the user id is set on the ActiveMQConnectionFactory or found through Authentication.

          Show
          Gary Tully added a comment - This seems broken to me. ActiveMQ does allow setting the JMSXUserId property but it should only be used in the absence of any other setting of user credentials. Currently, the call to get JMSXUserId looks first at the existing properties and then at the internal userId attribute. That logic should be reversed. Typically, users should not set the JMSXUserId value, but if they do, it should be ignored if there is an alternative policy in place, ie: when BrokerService.populateJMSXUserID is specified and either the user id is set on the ActiveMQConnectionFactory or found through Authentication.
          Hide
          Michael Steiner added a comment -

          The diff from the standard apache-activemq-5.4.2 distribution which demonstrates the bug

          Show
          Michael Steiner added a comment - The diff from the standard apache-activemq-5.4.2 distribution which demonstrates the bug
          Hide
          Michael Steiner added a comment -

          The files from apache-activemq-5.4.2/conf and apache-activemq-5.4.2/examples which include all changes i've done to demonstrate problem (see for comments with MSTEINER in it ...)

          Show
          Michael Steiner added a comment - The files from apache-activemq-5.4.2/conf and apache-activemq-5.4.2/examples which include all changes i've done to demonstrate problem (see for comments with MSTEINER in it ...)

            People

            • Assignee:
              Gary Tully
              Reporter:
              Michael Steiner
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development