qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "mick goulish" <m...@redhat.com>
Subject Re: Review Request 18968: [C++ broker] userId is not passed to ACL when DIGEST-MD5 is used while creating link
Date Tue, 11 Mar 2014 12:59:03 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18968/#review36778
-----------------------------------------------------------

Ship it!


I can't think of any reason why this check should not be moved to this point.   ( And it looks
like there is an excellent reason why it *must* be moved here... )

- mick goulish


On March 11, 2014, 8:06 a.m., Pavel Moravec wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18968/
> -----------------------------------------------------------
> 
> (Updated March 11, 2014, 8:06 a.m.)
> 
> 
> 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 1576205 
> 
> Diff: https://reviews.apache.org/r/18968/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Pavel Moravec
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message