Comment 2 for bug 949874

Revision history for this message
Vsevolod Novikov (nnseva) wrote :

IMHO there is a misunderstanding in the txAMQP code concerned to the authentication algorithm.

As I can see in the spec 0-8, there is a definition of the AMQP-specific SASL PLAIN authentication there:

<field name="response" type="longstr">
security response data
<doc>
A block of opaque data passed to the security mechanism. The contents of this data are defined by the SASL security mechanism. For the PLAIN security mechanism this is defined as a field table holding two fields, LOGIN and PASSWORD.
</doc>
<assert check="notnull"/>
</field>

The next version of the spec avoids this explanation:

<field name="response" domain="longstr" label="security response data">
<doc>
A block of opaque data passed to the security mechanism. The contents of this data are defined by the SASL security mechanism.
</doc>
<assert check="notnull"/>
</field>

The modern RabbitMQ server supports old-style (0-8) PLAIN mechanism in all specs supported, renaming it to AMQPLAIN.

So, for compatibility reason, IMHO the mechanism parameter should be checked against 'PLAIN' and 'AMQPLAIN' values, instead of protocol version comparison in the client library, as made in the applied patch.

I should note also, that the modern version of the RabbitMQ broker supports specs till 0-9-1, so the initial spec for RabbitMQ has been fixed to 0-9 also (the 0-9-1 causes problems looking like problems of the txAMQP itself)

All changes, including 2 new test cases for authentication (FIXIT: check against other brokers), are in the applied patch.

TODO: test cases for different specs supported by the broker required IMHO.