Uploaded image for project: 'Qpid JMS'
  1. Qpid JMS
  2. QPIDJMS-169

SASL Plain Mechanism should be enforcing UTF-8 encoding

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.8.0
    • Fix Version/s: 0.9.0
    • Component/s: qpid-jms-client
    • Labels:
      None

      Description

      When encoding the user / pass in the SASL plain mechanism we aren't enforcing UTF-8 encoding which is the required encoding.

        Activity

        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 98ba842408afad3275966b0eb8041501e54d86c5 in qpid-jms's branch refs/heads/master from Timothy Bish
        [ https://git-wip-us.apache.org/repos/asf?p=qpid-jms.git;h=98ba842 ]

        QPIDJMS-169 Enforce user / pass encoded as UTF-0

        Show
        jira-bot ASF subversion and git services added a comment - Commit 98ba842408afad3275966b0eb8041501e54d86c5 in qpid-jms's branch refs/heads/master from Timothy Bish [ https://git-wip-us.apache.org/repos/asf?p=qpid-jms.git;h=98ba842 ] QPIDJMS-169 Enforce user / pass encoded as UTF-0
        Hide
        k-wall Keith Wall added a comment - - edited

        Hi Tim,

        I am not sure about this change. The old implementation was platform dependent and certainly wrong, but SASL requires that usernames/passwords are sasl-preped[1] which defines a character mapping. My reading of the RFCs is that this applies to SASL PLAIN too.

        org.apache.qpid.jms.sasl.AbstractScramSHAMechanism#saslPrep is a noddy implementation which side steps most of the requirements by restricting the usernames/passwords to ASCII. I think org.apache.qpid.jms.sasl.PlainMechanism should at least do the same, or we should switch to a real saslprep algorithm, but the latter should be coordinated across all the components.

        [1] https://www.ietf.org/rfc/rfc4013.txt

        Show
        k-wall Keith Wall added a comment - - edited Hi Tim, I am not sure about this change. The old implementation was platform dependent and certainly wrong, but SASL requires that usernames/passwords are sasl-preped [1] which defines a character mapping. My reading of the RFCs is that this applies to SASL PLAIN too. org.apache.qpid.jms.sasl.AbstractScramSHAMechanism#saslPrep is a noddy implementation which side steps most of the requirements by restricting the usernames/passwords to ASCII. I think org.apache.qpid.jms.sasl.PlainMechanism should at least do the same, or we should switch to a real saslprep algorithm, but the latter should be coordinated across all the components. [1] https://www.ietf.org/rfc/rfc4013.txt
        Hide
        gemmellr Robbie Gemmell added a comment -

        The newer RFCs seem to recommend saslprep rather than require it (probably since the earlier RFCs defining the mech are older than sasl-prep, except the SCRAM ones, which perhaps have more need for it since they use a more structured content), with not doing prep mentioned as one approach during discussion that different components can behave differently.

        The JVMs built in PLAIN sasl client code doesnt seem to do any prep and just encodes the input directly as UTF8, which appears to be what our other bits are also doing currently. Regardless, this change really just ensures the existing behaviour is fixed rather than floating about if a non-UTF8 default is in place, so I think it seems reasonable. Any further changes seem like something additional JIRAs would handle, and would likely need to be coordinated across all the components as you say.

        Show
        gemmellr Robbie Gemmell added a comment - The newer RFCs seem to recommend saslprep rather than require it (probably since the earlier RFCs defining the mech are older than sasl-prep, except the SCRAM ones, which perhaps have more need for it since they use a more structured content), with not doing prep mentioned as one approach during discussion that different components can behave differently. The JVMs built in PLAIN sasl client code doesnt seem to do any prep and just encodes the input directly as UTF8, which appears to be what our other bits are also doing currently. Regardless, this change really just ensures the existing behaviour is fixed rather than floating about if a non-UTF8 default is in place, so I think it seems reasonable. Any further changes seem like something additional JIRAs would handle, and would likely need to be coordinated across all the components as you say.
        Hide
        tabish121 Timothy Bish added a comment -

        Thanks Robbie. I had re-read the SASL PLAIN RFC last night just to be sure that I'd read the requirements right and as Robbie pointed out it is not mandated in the RFC but the UTF8 bit is so fixing that was my main priority for this JIRA. I hadn't had time to chase down other implementations to see if any did a prep for plain but it seems Robbie did some legwork there.

        For now I think this change is correct and at least gets us in line with the basic requirements. We of course are happy to have contributions so if someone want to jump on the SaslPrep implementation that'd be a nice add.

        Show
        tabish121 Timothy Bish added a comment - Thanks Robbie. I had re-read the SASL PLAIN RFC last night just to be sure that I'd read the requirements right and as Robbie pointed out it is not mandated in the RFC but the UTF8 bit is so fixing that was my main priority for this JIRA. I hadn't had time to chase down other implementations to see if any did a prep for plain but it seems Robbie did some legwork there. For now I think this change is correct and at least gets us in line with the basic requirements. We of course are happy to have contributions so if someone want to jump on the SaslPrep implementation that'd be a nice add.
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 3c19c6af36c0fd8a07f0245148bcd136021a590a in qpid-jms's branch refs/heads/master from Timothy Bish
        [ https://git-wip-us.apache.org/repos/asf?p=qpid-jms.git;h=3c19c6a ]

        QPIDJMS-169 Use the StandardCharsets value for UTF8

        Show
        jira-bot ASF subversion and git services added a comment - Commit 3c19c6af36c0fd8a07f0245148bcd136021a590a in qpid-jms's branch refs/heads/master from Timothy Bish [ https://git-wip-us.apache.org/repos/asf?p=qpid-jms.git;h=3c19c6a ] QPIDJMS-169 Use the StandardCharsets value for UTF8
        Hide
        k-wall Keith Wall added a comment -

        Robbie/Tim,

        Thanks for the replies, I withdraw the concern. I hadn't realised the saslprep for the older mechanisms was a retrospective recommendation suggested in RFC 4013.

        Keith.

        Show
        k-wall Keith Wall added a comment - Robbie/Tim, Thanks for the replies, I withdraw the concern. I hadn't realised the saslprep for the older mechanisms was a retrospective recommendation suggested in RFC 4013. Keith.

          People

          • Assignee:
            tabish121 Timothy Bish
            Reporter:
            tabish121 Timothy Bish
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development