qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gordon Sim" <g...@redhat.com>
Subject Re: Review Request 18968: [C++ broker] userId is not passed to ACL when DIGEST-MD5 is used while creating link
Date Mon, 10 Mar 2014 16:23:49 GMT

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


Perhaps the cleanest thing is just to move any ACl check on the connection to the handling
of open(). That avoids different paths for different mechanisms which seems error prone, and
also any special tricks for determining whether username is yet available (it *must* be available
by open if one is set).

- Gordon Sim


On March 10, 2014, 2:20 p.m., Pavel Moravec wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18968/
> -----------------------------------------------------------
> 
> (Updated March 10, 2014, 2:20 p.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 1575923 
> 
> Diff: https://reviews.apache.org/r/18968/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Pavel Moravec
> 
>


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