-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18968/
-----------------------------------------------------------
Review request for qpid, Gordon Sim and mick goulish.
Bugs: QPID-5621
https://issues.apache.org/jira/browse/QPID-5621
Repository: qpid
Description
-------
Root cause of the problem: ACL for links is checked after getting
connection.startOk AMQP method. While DIGEST-MD5 (and other auth.methods)
provide userId later on - during connection.secureOk AMQP method.
So the ACL check for the SASL mechanisms relying on challenge & response should
be postponed until ConnectionHandler::Handler::secureOk method.
I have two issues with the patch:
1) How to identify SASL methods relying on challenge & response? I used
"((body.getMechanism()=="ANONYMOUS")||(body.getMechanism()=="PLAIN"))" test
there but dont like the explicit SASL mechs comparison..
(And I am not even 100% sure the list of mechanisms is correct - I just *guess*
SSL or GSSAPI sends challenge and response as well.
2) Can a user have empty username? If so, then in the test:
if ((connection.getUserId()!="") && (connection.isFederationLink()))
the first condition will never match - while the condition is necessary as
usually SASL authentication requires several challenge+response exchanges, i.e.
several connection.secureOk methods received, while only the latest one has
userId finally set.
Diffs
-----
/trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.cpp 1575923
Diff: https://reviews.apache.org/r/18968/diff/
Testing
-------
Thanks,
Pavel Moravec
