You are viewing a plain text version of this content. The canonical link for it is here.
Posted to proton@qpid.apache.org by Andrew Stitcher <as...@redhat.com> on 2015/02/24 21:48:22 UTC
Proposed SASL changes (API and functional)
As many of you know I've been working on implementing a SASL AMQP
protocol layer that does more than PLAIN and ANONYMOUS for proton-c.
I'm currently in at a point where the work is reasonably functional
(with some gaps)
I've put together a fairly comprehensive account of this work on the
Apache wiki: https://cwiki.apache.org/confluence/x/B5cWAw
If you are at all interested please go and look at the proposal and
comment on it there.
You can see my actual code changes in my github proton repo:
https://github.com/astitcher/qpid-proton/commits/sasl-work
[This is my working branch, so not all the changes make a lot of sense,
just pay attention to the tip of the branch]
In a short while when people have had enough time to absorb the proposal
and comment I will post a code review of the actual code changes. As
there are substantial API changes I'd like to get this in for 0.9
because we were intending to stabilise the API at this point.
Thanks.
Andrew
Re: Proposed SASL changes (API and functional)
Posted by Rafael Schloming <rh...@alum.mit.edu>.
This is from Andrew's wiki comment. Sorry to paste it back to the list, but
I'm having some difficulty commenting there:
>
> 1. Setters with no getters
> Philosophically I don't agree that you need to make all properties
> read/write. I see no particular reason to make these properties readable
> since they will never change once they are set, or in the case of the
> password should actually not be accessible once set (because the
> implementation *should* be scrubbing the bytes from memory after use).
> In my view if the application needs the value again it already has it.
> In the case of the read-only property authenticated user I definitely
> think that needs to be read only.
> Having said that, I don't feel that strongly about getters for the
> client username and hostname.
>
> There's actually an important point here worth noting. With reactive use,
I don't think it's true, pragmatically speaking, that the application has
the value again when it needs it. When your primary means of operating on a
connection is through handling events, the only state you have easy access
to is what is provided by those events. Taking your suggestion, if I wanted
to do something simple like log a debug statement from an event handler and
include the hostname and/or username of the connection in that info, my
only recourse would be to malloc some sort of ancillary struct and attach
it to the connection and fill it in with the very same hostname that the
connection is holding internally, or alternatively access some sort of
global state that holds a map from the connection back to that same
information. If your point is that this is possible then of course that's
true, but it doesn't seem at all reasonable.
>
> 1. inconsistency with existing use of "remote" in API
> I take your point - I'm happy to remove "remote" from the API name -
> would "connected" be all right? pn_transport_set_hostname() just
> doesn't seem specific enough to me - it might just as well be telling the
> transport what *our* hostname is.
> 2. Redundancy of pn_connection_set_hostname() and
> pn_transport_set_<something>_hostname()
> Yes these are definitely redundant, and I would need to deprecate the
> connection version and set it from the transport when binding them together
> - good catch.
> The transport version must be primary, as (in principle at least, if
> not in the current implementation) you don't need the connection object
> until you have authenticated the transport and authentication and
> encryption may to know need the hostname you are connecting to. I think it
> would have to be an error to rebind (on reconnect) a connection with a
> different hostname than the transport hostname.
>
> This isn't consistent with how the connection and transport are actually
used. With the reactive API, the user creates the connection endpoint first
and configures it with all the relevant details that it cares about. The
transport isn't created at all until the client actually opens the
connection (which could be somewhere completely different from where it
configures the connection). It's also important to note that the user
doesn't actually create the transport at all. The default handlers do this
when the PN_CONNECTION_LOCAL_OPEN event occurs. The users don't even need
to be aware that a transport object exists at all if they don't care to
customize it. This is a nice property and would be difficult to maintain if
you start pushing connection level configuration like hostname into the
transport.
I think if you flip things around it actually makes more sense. As a server
you are going to have a transport prior to having a connection, and in this
case you want to access the hostname-that-your-remote-peer desires for
vhost purposes, but it makes no sense to actually set it. As a client, a
transport is pretty much useless until it is bound to a connection, as you
can't safely do much more than send the protocol header, so the natural
sequence is to create the connection first and set the hostname you want to
connect to, and not worry about the Transport.
>
> 1. Having a separate set_user/set_password
> That would make sense. However from this conversation I'm wondering
> actually if we should more carefully distinguish the client and server
> ends. And so have a client only API to set user/password and a server only
> API to extract the authenticated username.
>
> So in conclusion how about:
>
> - Changing pn_transport_set_remote_hostname() →
> pn_transport_set_connect_hostname() (connect/connected/connection?)
> - Adding pn_transport_get_connect_hostname()
> (connect/connected/connection?)
> - Deprecating pn_connection_set/get_hostname() in favour of
> pn_transport_set/get_connect_hostname()
> Actually changing the pn_transport_bind() code would be required too.
> - Removing pn_transport_set_user_password() and pn_transport_get_user()
> - Replacing them with pn_transport_set_client_user(),
> pn_set_client_password(), pn_transport_get_client_user() and
> pn_transport_get_authenticated_user().
>
> It might make sense to follow a similar pattern as the hostname here, i.e.
in the client scenario you configure the connection with
pn_connection_get/set_user() prior to the existence of the transport, and
in the server scenario the transport simply provides
pn_transport_remote_user(...).
> You'll note I've added in the getters that I don't think are necessary,
> but that you do (hostname and client_user), but I've maintained the
> password property as write only and the authenticated user as read only.
>
I wasn't suggesting that you need to provide an accessor for the password,
I was suggesting that if you don't provide one, you shouldn't call the
"setter" pn_transport_set_password(...) since that will violate
expectations by not having a corresponding pn_transport_get_password(...).
--Rafael
Re: Proposed SASL changes (API and functional)
Posted by Rafael Schloming <rh...@alum.mit.edu>.
This is from Andrew's wiki comment. Sorry to paste it back to the list, but
I'm having some difficulty commenting there:
>
> 1. Setters with no getters
> Philosophically I don't agree that you need to make all properties
> read/write. I see no particular reason to make these properties readable
> since they will never change once they are set, or in the case of the
> password should actually not be accessible once set (because the
> implementation *should* be scrubbing the bytes from memory after use).
> In my view if the application needs the value again it already has it.
> In the case of the read-only property authenticated user I definitely
> think that needs to be read only.
> Having said that, I don't feel that strongly about getters for the
> client username and hostname.
>
> There's actually an important point here worth noting. With reactive use,
I don't think it's true, pragmatically speaking, that the application has
the value again when it needs it. When your primary means of operating on a
connection is through handling events, the only state you have easy access
to is what is provided by those events. Taking your suggestion, if I wanted
to do something simple like log a debug statement from an event handler and
include the hostname and/or username of the connection in that info, my
only recourse would be to malloc some sort of ancillary struct and attach
it to the connection and fill it in with the very same hostname that the
connection is holding internally, or alternatively access some sort of
global state that holds a map from the connection back to that same
information. If your point is that this is possible then of course that's
true, but it doesn't seem at all reasonable.
>
> 1. inconsistency with existing use of "remote" in API
> I take your point - I'm happy to remove "remote" from the API name -
> would "connected" be all right? pn_transport_set_hostname() just
> doesn't seem specific enough to me - it might just as well be telling the
> transport what *our* hostname is.
> 2. Redundancy of pn_connection_set_hostname() and
> pn_transport_set_<something>_hostname()
> Yes these are definitely redundant, and I would need to deprecate the
> connection version and set it from the transport when binding them together
> - good catch.
> The transport version must be primary, as (in principle at least, if
> not in the current implementation) you don't need the connection object
> until you have authenticated the transport and authentication and
> encryption may to know need the hostname you are connecting to. I think it
> would have to be an error to rebind (on reconnect) a connection with a
> different hostname than the transport hostname.
>
> This isn't consistent with how the connection and transport are actually
used. With the reactive API, the user creates the connection endpoint first
and configures it with all the relevant details that it cares about. The
transport isn't created at all until the client actually opens the
connection (which could be somewhere completely different from where it
configures the connection). It's also important to note that the user
doesn't actually create the transport at all. The default handlers do this
when the PN_CONNECTION_LOCAL_OPEN event occurs. The users don't even need
to be aware that a transport object exists at all if they don't care to
customize it. This is a nice property and would be difficult to maintain if
you start pushing connection level configuration like hostname into the
transport.
I think if you flip things around it actually makes more sense. As a server
you are going to have a transport prior to having a connection, and in this
case you want to access the hostname-that-your-remote-peer desires for
vhost purposes, but it makes no sense to actually set it. As a client, a
transport is pretty much useless until it is bound to a connection, as you
can't safely do much more than send the protocol header, so the natural
sequence is to create the connection first and set the hostname you want to
connect to, and not worry about the Transport.
>
> 1. Having a separate set_user/set_password
> That would make sense. However from this conversation I'm wondering
> actually if we should more carefully distinguish the client and server
> ends. And so have a client only API to set user/password and a server only
> API to extract the authenticated username.
>
> So in conclusion how about:
>
> - Changing pn_transport_set_remote_hostname() →
> pn_transport_set_connect_hostname() (connect/connected/connection?)
> - Adding pn_transport_get_connect_hostname()
> (connect/connected/connection?)
> - Deprecating pn_connection_set/get_hostname() in favour of
> pn_transport_set/get_connect_hostname()
> Actually changing the pn_transport_bind() code would be required too.
> - Removing pn_transport_set_user_password() and pn_transport_get_user()
> - Replacing them with pn_transport_set_client_user(),
> pn_set_client_password(), pn_transport_get_client_user() and
> pn_transport_get_authenticated_user().
>
> It might make sense to follow a similar pattern as the hostname here, i.e.
in the client scenario you configure the connection with
pn_connection_get/set_user() prior to the existence of the transport, and
in the server scenario the transport simply provides
pn_transport_remote_user(...).
> You'll note I've added in the getters that I don't think are necessary,
> but that you do (hostname and client_user), but I've maintained the
> password property as write only and the authenticated user as read only.
>
I wasn't suggesting that you need to provide an accessor for the password,
I was suggesting that if you don't provide one, you shouldn't call the
"setter" pn_transport_set_password(...) since that will violate
expectations by not having a corresponding pn_transport_get_password(...).
--Rafael
Re: Proposed SASL changes (API and functional)
Posted by Alan Conway <ac...@redhat.com>.
On Wed, 2015-02-25 at 13:45 -0500, Andrew Stitcher wrote:
> On Tue, 2015-02-24 at 15:48 -0500, Andrew Stitcher wrote:
> > ...
> > If you are at all interested please go and look at the proposal and
> > comment on it there.
>
> Thank you very much to Alan and Jakub for commenting on my proposal.
>
> The reason I asked people to comment over on the wiki is that it is very
> hard to find a discussion like this related to a specific proposal after
> some time has elapsed if it is in email, whereas actually attached to
> the proposal on the wiki keeps all the relevant comments together.
>
> If it is ok with them I will copy the comments over there:
> Alan, Jakub?
Copy away
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
For additional commands, e-mail: users-help@qpid.apache.org
Re: Proposed SASL changes (API and functional)
Posted by Alan Conway <ac...@redhat.com>.
On Wed, 2015-02-25 at 13:45 -0500, Andrew Stitcher wrote:
> On Tue, 2015-02-24 at 15:48 -0500, Andrew Stitcher wrote:
> > ...
> > If you are at all interested please go and look at the proposal and
> > comment on it there.
>
> Thank you very much to Alan and Jakub for commenting on my proposal.
>
> The reason I asked people to comment over on the wiki is that it is very
> hard to find a discussion like this related to a specific proposal after
> some time has elapsed if it is in email, whereas actually attached to
> the proposal on the wiki keeps all the relevant comments together.
>
> If it is ok with them I will copy the comments over there:
> Alan, Jakub?
Copy away
Re: Proposed SASL changes (API and functional)
Posted by Jakub Scholz <ja...@scholz.cz>.
On Wed, Feb 25, 2015 at 7:45 PM, Andrew Stitcher <as...@redhat.com>
wrote:
> If it is ok with them I will copy the comments over there:
> Alan, Jakub?
>
Sorry, I missed the wiki part. Feel free to copy my comment there if you
want.
Re: Proposed SASL changes (API and functional)
Posted by Andrew Stitcher <as...@redhat.com>.
On Tue, 2015-02-24 at 15:48 -0500, Andrew Stitcher wrote:
> ...
> If you are at all interested please go and look at the proposal and
> comment on it there.
Thank you very much to Alan and Jakub for commenting on my proposal.
The reason I asked people to comment over on the wiki is that it is very
hard to find a discussion like this related to a specific proposal after
some time has elapsed if it is in email, whereas actually attached to
the proposal on the wiki keeps all the relevant comments together.
If it is ok with them I will copy the comments over there:
Alan, Jakub?
Thanks
Andrew
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
For additional commands, e-mail: users-help@qpid.apache.org
Re: Proposed SASL changes (API and functional)
Posted by Andrew Stitcher <as...@redhat.com>.
On Wed, 2015-02-25 at 10:46 -0500, Alan Conway wrote:
> ...
> One ignorant question: Qpid has a min/max "Security Strength Factor" for
> encryption rather than a binary enable/disable. Is that relevant here?
(Hardly an ignorant question!) You make a very good point, and this
design may indeed be a little simplistic - largely because I've not
implemented the encryption side yet!
1. I doubt that max ssf is all that useful in practice.
2. Effectively pn_transport_require_encryption() is the same as setting
min ssf >1, but is simpler to understand! An alternative might be
pn_transport_require_ssf(int) however that isn't as clear and it's not
obvious how to choose the ssf value. Perhaps the '1' should be
configurable differently.
Some input from those who did the similar work in qpidd might be useful.
Just some random wittering.
Andrew
Re: Proposed SASL changes (API and functional)
Posted by Andrew Stitcher <as...@redhat.com>.
On Wed, 2015-02-25 at 10:46 -0500, Alan Conway wrote:
> ...
> One ignorant question: Qpid has a min/max "Security Strength Factor" for
> encryption rather than a binary enable/disable. Is that relevant here?
(Hardly an ignorant question!) You make a very good point, and this
design may indeed be a little simplistic - largely because I've not
implemented the encryption side yet!
1. I doubt that max ssf is all that useful in practice.
2. Effectively pn_transport_require_encryption() is the same as setting
min ssf >1, but is simpler to understand! An alternative might be
pn_transport_require_ssf(int) however that isn't as clear and it's not
obvious how to choose the ssf value. Perhaps the '1' should be
configurable differently.
Some input from those who did the similar work in qpidd might be useful.
Just some random wittering.
Andrew
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
For additional commands, e-mail: users-help@qpid.apache.org
Re: Proposed SASL changes (API and functional)
Posted by Alan Conway <ac...@redhat.com>.
On Tue, 2015-02-24 at 15:48 -0500, Andrew Stitcher wrote:
> As many of you know I've been working on implementing a SASL AMQP
> protocol layer that does more than PLAIN and ANONYMOUS for proton-c.
>
> I'm currently in at a point where the work is reasonably functional
> (with some gaps)
>
> I've put together a fairly comprehensive account of this work on the
> Apache wiki: https://cwiki.apache.org/confluence/x/B5cWAw
>
> If you are at all interested please go and look at the proposal and
> comment on it there.
>
> You can see my actual code changes in my github proton repo:
> https://github.com/astitcher/qpid-proton/commits/sasl-work
>
> [This is my working branch, so not all the changes make a lot of sense,
> just pay attention to the tip of the branch]
>
> In a short while when people have had enough time to absorb the proposal
> and comment I will post a code review of the actual code changes. As
> there are substantial API changes I'd like to get this in for 0.9
> because we were intending to stabilise the API at this point.
This looks very good to me.
One ignorant question: Qpid has a min/max "Security Strength Factor" for
encryption rather than a binary enable/disable. Is that relevant here?
Cheers,
Alan.
Re: Proposed SASL changes (API and functional)
Posted by Andrew Stitcher <as...@redhat.com>.
On Tue, 2015-02-24 at 15:48 -0500, Andrew Stitcher wrote:
> ...
> If you are at all interested please go and look at the proposal and
> comment on it there.
Thank you very much to Alan and Jakub for commenting on my proposal.
The reason I asked people to comment over on the wiki is that it is very
hard to find a discussion like this related to a specific proposal after
some time has elapsed if it is in email, whereas actually attached to
the proposal on the wiki keeps all the relevant comments together.
If it is ok with them I will copy the comments over there:
Alan, Jakub?
Thanks
Andrew
Re: Proposed SASL changes (API and functional)
Posted by Robbie Gemmell <ro...@gmail.com>.
On 27 February 2015 at 11:56, Robbie Gemmell <ro...@gmail.com> wrote:
> On 26 February 2015 at 17:52, Andrew Stitcher <as...@redhat.com> wrote:
>> On Thu, 2015-02-26 at 12:28 +0000, Robbie Gemmell wrote:
>>> ...
>>> I'm going to post my comments here and on the wiki, as I dont think
>>> many (except maybe you) will actually see them on the wiki ;)
>>
>> Thank you for the excellent feedback. I'm going to answer on the wiki -
>> as it'll save me from cutting and pasting.
>>
>> [I did try to add the lists as watchers, but that doesn't seem to be
>> possible in any obvious way]
>>
>> wiki link:
>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=51812103&focusedCommentId=51812468#comment-51812468
>>
>> Andrew
>>
>>
>
> Replied on the Wiki, but also including below for anyone not clicking
> these links ;)
>
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=51812103&focusedCommentId=51812571#comment-51812571
>
>
>
> 3. Do you really mean to have only ANONYMOUS for 0.9 for Proton-J?
> That would render it fairly useless if the existing transport/sasl API
> is removed since that would prevent people using any existing
> implementation they already have (ANONYMOUS, PLAIN, CRAM-MD5 thus far
> for the JMS client, with EXTERNAL sitting on the TODO list).
>
>
> 4. I do still think there is a change (by having to handle the SASL
> previously, you would always know the authentication result before the
> connection ever happened, and you had to call done on the server to
> say it was, whereas now you get a transport event in either case), but
> I take your point that you could adjust to waiting for the successful
> connection open state/event to be reached and both approaches would
> then appear to work the same from that point.
>
>
> 5. I'd say the positive list of mechs to use is more straight forward
> in many scenarios, and the exclusion list could work better in some
> others. Offering both options would let people do whatever they want.
> - The exclusion list might be something you used initially to e.g
> prevent use of ANONYMOUS, but you have that scenario covered by
> pn_transport_require_auth already.
> - After that, it is presumably to be used for things like disabling
> mechs that are either not considered secure enough (e.g PLAIN, because
> you are requiring everying uses SCRAM-SHA-<foo> today because its
> Friday), or are installed but not configured/usable (e.g GSSAPI as you
> mentioned). Between those, you would need to know the name of every
> mechanism that could ever be present, and then exclude them all in
> case they should be installed (possibly later). Or, you might better
> get what you want by being able to specify only the mechs you want to
> be enabled and have already installed as a result.
> - On what to do to do if a positively-requested mechanism isnt
> available, as I said we could posibly give the choice of of throwing
> to indicate the requirement couldn't be met or potentially making that
> optional, because if you positively-specify more than one you probabyl
> dont care which is used too much.
>
>
> 6. I saw the deprecation, but I wasnt aware the default had already
> been changed for allow skip, was that ever mentioned anywhere, e.g a
> JIRA raised to cover a significant change in behaviour? I think its
> the wrong decision personally, and code using the engine should have
> to opt-in to permitting non-SASL connections (and ANONYMOUS). If the
> rest of the current SASL API is going away and the default remains
> changed, then it seems allow skip should probably just be removed too,
> since its very likely the only people using it will be configuring the
> new behaviour and there is a new API for anyone wanting to configure
> the historical default behaviour when they realise (this will
> presumably be release-noted?) it becomes necessary to secure it with
> the new release.
Replied on the wiki once more.
https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=51812103&focusedCommentId=51812858#comment-51812858
..but again for anyone not reading there here is the reply (forgive
the wiki text):
{quote}
5. The exclusion list is not meant to be used to exclude ANONYMOUS in
any way, as ANONYMOUS won't even be offered if authentication is
required.
{quote}
I realise, I was just using that as an example of one of reasons I
might use an exclude list, and highlighted that it wouldnt be required
in that case because of the alternative method.
That said, ANONYMOUS presumably will be offered if the 'force
anonymous' method is also in use?
{quote}
I think having both exclusion and inclusion in the API is the worst
option, because then the coder has to understand how the two lists
interact (and in the past exactly this sort of issue has been a
security risk). As it stands, with the cyrus sasl code you do have
both options, just at different levels, you can include mechs by using
the cyrus sasl configuration files, and exclude mechs using the API.
I think the idea of forcing a specific single mech has some merit - a
generalisation of the proposed "force ANONYMOUS" option - it still
leaves open the question of interaction with the exclusion list
though.
{quote}
I dont think we need both either, just the positive/inlusion list, but
I also dont see it needing to be that confusing if we did, with the
simplest case just being to make specifying them mutually exclusive.
I think if you generalise the forcing then it simply becomes a
'positive/inclusion' list that can take 1 or more entries, which
removes any need for wondering about interaction with an exclusion
lists or specific methods for forcing ANONYMOUS etc (which itself
already has interesting interaction issues with 'requires auth' and
the exclusion list, since someone might put ANONYMOUS in there even if
that isnt whats intended).
{quote}
6. I don't (didn't?) think the change of default is that significant
as all the server code in the proton tree previously allowed (or even
required) anonymous , in that code even though SASL was required the
ANONYMOUS mech was specified. All that the new default has done is to
remove a level of unnecessary processing. Of course the situation in
the Java tree might be different.
{quote}
We may be discussing things from different viewpoints. I dont care
about the server code in the proton tree, since we can change that to
do whatever we like based on what the engine does. That code is not
the only use of the engine.
{quote}
In any case, I find it hard to believe that someone writing a
production secured server wouldn't think about security enough to know
whether they wanted authenticated and/or encrypted connections and
specify them as such.
{quote}
That you believe it to be so certain to be specified is more reason in
my mind to default to the previous behaviour.
{quote}
I will emphasize that the old "skip SASL" option was not in itself
related to authentication or not as the ANONYMOUS SASL mech was still
allowed. The new code unifies the default across the layers and allows
you to skip an unnecessary layer - This change should definitely be
release noted.
{quote}
As above, it might have been allowed in *our* code, but that is not
the only code to consider.
{quote}
Your point about just removing pn_sasl_allow_skip() is noted (and I
agree), given my previous reasoning for pn_sasl_client() and
pn_sasl_server() it should go (and it will force anyone using it to
understand the change in default too)
{quote}
Unfortunately the only people likely to be using it will be the people
who wouldnt really need to understand the change in default.
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
For additional commands, e-mail: users-help@qpid.apache.org
Re: Proposed SASL changes (API and functional)
Posted by Robbie Gemmell <ro...@gmail.com>.
On 27 February 2015 at 11:56, Robbie Gemmell <ro...@gmail.com> wrote:
> On 26 February 2015 at 17:52, Andrew Stitcher <as...@redhat.com> wrote:
>> On Thu, 2015-02-26 at 12:28 +0000, Robbie Gemmell wrote:
>>> ...
>>> I'm going to post my comments here and on the wiki, as I dont think
>>> many (except maybe you) will actually see them on the wiki ;)
>>
>> Thank you for the excellent feedback. I'm going to answer on the wiki -
>> as it'll save me from cutting and pasting.
>>
>> [I did try to add the lists as watchers, but that doesn't seem to be
>> possible in any obvious way]
>>
>> wiki link:
>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=51812103&focusedCommentId=51812468#comment-51812468
>>
>> Andrew
>>
>>
>
> Replied on the Wiki, but also including below for anyone not clicking
> these links ;)
>
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=51812103&focusedCommentId=51812571#comment-51812571
>
>
>
> 3. Do you really mean to have only ANONYMOUS for 0.9 for Proton-J?
> That would render it fairly useless if the existing transport/sasl API
> is removed since that would prevent people using any existing
> implementation they already have (ANONYMOUS, PLAIN, CRAM-MD5 thus far
> for the JMS client, with EXTERNAL sitting on the TODO list).
>
>
> 4. I do still think there is a change (by having to handle the SASL
> previously, you would always know the authentication result before the
> connection ever happened, and you had to call done on the server to
> say it was, whereas now you get a transport event in either case), but
> I take your point that you could adjust to waiting for the successful
> connection open state/event to be reached and both approaches would
> then appear to work the same from that point.
>
>
> 5. I'd say the positive list of mechs to use is more straight forward
> in many scenarios, and the exclusion list could work better in some
> others. Offering both options would let people do whatever they want.
> - The exclusion list might be something you used initially to e.g
> prevent use of ANONYMOUS, but you have that scenario covered by
> pn_transport_require_auth already.
> - After that, it is presumably to be used for things like disabling
> mechs that are either not considered secure enough (e.g PLAIN, because
> you are requiring everying uses SCRAM-SHA-<foo> today because its
> Friday), or are installed but not configured/usable (e.g GSSAPI as you
> mentioned). Between those, you would need to know the name of every
> mechanism that could ever be present, and then exclude them all in
> case they should be installed (possibly later). Or, you might better
> get what you want by being able to specify only the mechs you want to
> be enabled and have already installed as a result.
> - On what to do to do if a positively-requested mechanism isnt
> available, as I said we could posibly give the choice of of throwing
> to indicate the requirement couldn't be met or potentially making that
> optional, because if you positively-specify more than one you probabyl
> dont care which is used too much.
>
>
> 6. I saw the deprecation, but I wasnt aware the default had already
> been changed for allow skip, was that ever mentioned anywhere, e.g a
> JIRA raised to cover a significant change in behaviour? I think its
> the wrong decision personally, and code using the engine should have
> to opt-in to permitting non-SASL connections (and ANONYMOUS). If the
> rest of the current SASL API is going away and the default remains
> changed, then it seems allow skip should probably just be removed too,
> since its very likely the only people using it will be configuring the
> new behaviour and there is a new API for anyone wanting to configure
> the historical default behaviour when they realise (this will
> presumably be release-noted?) it becomes necessary to secure it with
> the new release.
Replied on the wiki once more.
https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=51812103&focusedCommentId=51812858#comment-51812858
..but again for anyone not reading there here is the reply (forgive
the wiki text):
{quote}
5. The exclusion list is not meant to be used to exclude ANONYMOUS in
any way, as ANONYMOUS won't even be offered if authentication is
required.
{quote}
I realise, I was just using that as an example of one of reasons I
might use an exclude list, and highlighted that it wouldnt be required
in that case because of the alternative method.
That said, ANONYMOUS presumably will be offered if the 'force
anonymous' method is also in use?
{quote}
I think having both exclusion and inclusion in the API is the worst
option, because then the coder has to understand how the two lists
interact (and in the past exactly this sort of issue has been a
security risk). As it stands, with the cyrus sasl code you do have
both options, just at different levels, you can include mechs by using
the cyrus sasl configuration files, and exclude mechs using the API.
I think the idea of forcing a specific single mech has some merit - a
generalisation of the proposed "force ANONYMOUS" option - it still
leaves open the question of interaction with the exclusion list
though.
{quote}
I dont think we need both either, just the positive/inlusion list, but
I also dont see it needing to be that confusing if we did, with the
simplest case just being to make specifying them mutually exclusive.
I think if you generalise the forcing then it simply becomes a
'positive/inclusion' list that can take 1 or more entries, which
removes any need for wondering about interaction with an exclusion
lists or specific methods for forcing ANONYMOUS etc (which itself
already has interesting interaction issues with 'requires auth' and
the exclusion list, since someone might put ANONYMOUS in there even if
that isnt whats intended).
{quote}
6. I don't (didn't?) think the change of default is that significant
as all the server code in the proton tree previously allowed (or even
required) anonymous , in that code even though SASL was required the
ANONYMOUS mech was specified. All that the new default has done is to
remove a level of unnecessary processing. Of course the situation in
the Java tree might be different.
{quote}
We may be discussing things from different viewpoints. I dont care
about the server code in the proton tree, since we can change that to
do whatever we like based on what the engine does. That code is not
the only use of the engine.
{quote}
In any case, I find it hard to believe that someone writing a
production secured server wouldn't think about security enough to know
whether they wanted authenticated and/or encrypted connections and
specify them as such.
{quote}
That you believe it to be so certain to be specified is more reason in
my mind to default to the previous behaviour.
{quote}
I will emphasize that the old "skip SASL" option was not in itself
related to authentication or not as the ANONYMOUS SASL mech was still
allowed. The new code unifies the default across the layers and allows
you to skip an unnecessary layer - This change should definitely be
release noted.
{quote}
As above, it might have been allowed in *our* code, but that is not
the only code to consider.
{quote}
Your point about just removing pn_sasl_allow_skip() is noted (and I
agree), given my previous reasoning for pn_sasl_client() and
pn_sasl_server() it should go (and it will force anyone using it to
understand the change in default too)
{quote}
Unfortunately the only people likely to be using it will be the people
who wouldnt really need to understand the change in default.
Re: Proposed SASL changes (API and functional)
Posted by Robbie Gemmell <ro...@gmail.com>.
On 26 February 2015 at 17:52, Andrew Stitcher <as...@redhat.com> wrote:
> On Thu, 2015-02-26 at 12:28 +0000, Robbie Gemmell wrote:
>> ...
>> I'm going to post my comments here and on the wiki, as I dont think
>> many (except maybe you) will actually see them on the wiki ;)
>
> Thank you for the excellent feedback. I'm going to answer on the wiki -
> as it'll save me from cutting and pasting.
>
> [I did try to add the lists as watchers, but that doesn't seem to be
> possible in any obvious way]
>
> wiki link:
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=51812103&focusedCommentId=51812468#comment-51812468
>
> Andrew
>
>
Replied on the Wiki, but also including below for anyone not clicking
these links ;)
https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=51812103&focusedCommentId=51812571#comment-51812571
3. Do you really mean to have only ANONYMOUS for 0.9 for Proton-J?
That would render it fairly useless if the existing transport/sasl API
is removed since that would prevent people using any existing
implementation they already have (ANONYMOUS, PLAIN, CRAM-MD5 thus far
for the JMS client, with EXTERNAL sitting on the TODO list).
4. I do still think there is a change (by having to handle the SASL
previously, you would always know the authentication result before the
connection ever happened, and you had to call done on the server to
say it was, whereas now you get a transport event in either case), but
I take your point that you could adjust to waiting for the successful
connection open state/event to be reached and both approaches would
then appear to work the same from that point.
5. I'd say the positive list of mechs to use is more straight forward
in many scenarios, and the exclusion list could work better in some
others. Offering both options would let people do whatever they want.
- The exclusion list might be something you used initially to e.g
prevent use of ANONYMOUS, but you have that scenario covered by
pn_transport_require_auth already.
- After that, it is presumably to be used for things like disabling
mechs that are either not considered secure enough (e.g PLAIN, because
you are requiring everying uses SCRAM-SHA-<foo> today because its
Friday), or are installed but not configured/usable (e.g GSSAPI as you
mentioned). Between those, you would need to know the name of every
mechanism that could ever be present, and then exclude them all in
case they should be installed (possibly later). Or, you might better
get what you want by being able to specify only the mechs you want to
be enabled and have already installed as a result.
- On what to do to do if a positively-requested mechanism isnt
available, as I said we could posibly give the choice of of throwing
to indicate the requirement couldn't be met or potentially making that
optional, because if you positively-specify more than one you probabyl
dont care which is used too much.
6. I saw the deprecation, but I wasnt aware the default had already
been changed for allow skip, was that ever mentioned anywhere, e.g a
JIRA raised to cover a significant change in behaviour? I think its
the wrong decision personally, and code using the engine should have
to opt-in to permitting non-SASL connections (and ANONYMOUS). If the
rest of the current SASL API is going away and the default remains
changed, then it seems allow skip should probably just be removed too,
since its very likely the only people using it will be configuring the
new behaviour and there is a new API for anyone wanting to configure
the historical default behaviour when they realise (this will
presumably be release-noted?) it becomes necessary to secure it with
the new release.
Re: Proposed SASL changes (API and functional)
Posted by Robbie Gemmell <ro...@gmail.com>.
On 26 February 2015 at 17:52, Andrew Stitcher <as...@redhat.com> wrote:
> On Thu, 2015-02-26 at 12:28 +0000, Robbie Gemmell wrote:
>> ...
>> I'm going to post my comments here and on the wiki, as I dont think
>> many (except maybe you) will actually see them on the wiki ;)
>
> Thank you for the excellent feedback. I'm going to answer on the wiki -
> as it'll save me from cutting and pasting.
>
> [I did try to add the lists as watchers, but that doesn't seem to be
> possible in any obvious way]
>
> wiki link:
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=51812103&focusedCommentId=51812468#comment-51812468
>
> Andrew
>
>
Replied on the Wiki, but also including below for anyone not clicking
these links ;)
https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=51812103&focusedCommentId=51812571#comment-51812571
3. Do you really mean to have only ANONYMOUS for 0.9 for Proton-J?
That would render it fairly useless if the existing transport/sasl API
is removed since that would prevent people using any existing
implementation they already have (ANONYMOUS, PLAIN, CRAM-MD5 thus far
for the JMS client, with EXTERNAL sitting on the TODO list).
4. I do still think there is a change (by having to handle the SASL
previously, you would always know the authentication result before the
connection ever happened, and you had to call done on the server to
say it was, whereas now you get a transport event in either case), but
I take your point that you could adjust to waiting for the successful
connection open state/event to be reached and both approaches would
then appear to work the same from that point.
5. I'd say the positive list of mechs to use is more straight forward
in many scenarios, and the exclusion list could work better in some
others. Offering both options would let people do whatever they want.
- The exclusion list might be something you used initially to e.g
prevent use of ANONYMOUS, but you have that scenario covered by
pn_transport_require_auth already.
- After that, it is presumably to be used for things like disabling
mechs that are either not considered secure enough (e.g PLAIN, because
you are requiring everying uses SCRAM-SHA-<foo> today because its
Friday), or are installed but not configured/usable (e.g GSSAPI as you
mentioned). Between those, you would need to know the name of every
mechanism that could ever be present, and then exclude them all in
case they should be installed (possibly later). Or, you might better
get what you want by being able to specify only the mechs you want to
be enabled and have already installed as a result.
- On what to do to do if a positively-requested mechanism isnt
available, as I said we could posibly give the choice of of throwing
to indicate the requirement couldn't be met or potentially making that
optional, because if you positively-specify more than one you probabyl
dont care which is used too much.
6. I saw the deprecation, but I wasnt aware the default had already
been changed for allow skip, was that ever mentioned anywhere, e.g a
JIRA raised to cover a significant change in behaviour? I think its
the wrong decision personally, and code using the engine should have
to opt-in to permitting non-SASL connections (and ANONYMOUS). If the
rest of the current SASL API is going away and the default remains
changed, then it seems allow skip should probably just be removed too,
since its very likely the only people using it will be configuring the
new behaviour and there is a new API for anyone wanting to configure
the historical default behaviour when they realise (this will
presumably be release-noted?) it becomes necessary to secure it with
the new release.
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
For additional commands, e-mail: users-help@qpid.apache.org
Re: Proposed SASL changes (API and functional)
Posted by Andrew Stitcher <as...@redhat.com>.
On Thu, 2015-02-26 at 12:28 +0000, Robbie Gemmell wrote:
> ...
> I'm going to post my comments here and on the wiki, as I dont think
> many (except maybe you) will actually see them on the wiki ;)
Thank you for the excellent feedback. I'm going to answer on the wiki -
as it'll save me from cutting and pasting.
[I did try to add the lists as watchers, but that doesn't seem to be
possible in any obvious way]
wiki link:
https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=51812103&focusedCommentId=51812468#comment-51812468
Andrew
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
For additional commands, e-mail: users-help@qpid.apache.org
Re: Proposed SASL changes (API and functional)
Posted by Andrew Stitcher <as...@redhat.com>.
On Thu, 2015-02-26 at 12:28 +0000, Robbie Gemmell wrote:
> ...
> I'm going to post my comments here and on the wiki, as I dont think
> many (except maybe you) will actually see them on the wiki ;)
Thank you for the excellent feedback. I'm going to answer on the wiki -
as it'll save me from cutting and pasting.
[I did try to add the lists as watchers, but that doesn't seem to be
possible in any obvious way]
wiki link:
https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=51812103&focusedCommentId=51812468#comment-51812468
Andrew
Re: Proposed SASL changes (API and functional)
Posted by Robbie Gemmell <ro...@gmail.com>.
On 25 February 2015 at 18:40, Andrew Stitcher <as...@redhat.com> wrote:
> On Wed, 2015-02-25 at 10:27 +0100, Jakub Scholz wrote:
>> ...
>> But I find this part a bit dangerous:
>> "Classically in protocols where SASL was not optional the way to avoid
>> double authentication was to use the EXTERNAL SASL mechanism. With AMQP,
>> SASL is optional, so if SSL is used for client authentication the SASL
>> layer could be entirely omitted and so using EXTERNAL is not necessary."
>>
>
> This is really just a statement about how AMQP 1.0 works - if you like -
> it is an aside praising the good protocol design sense of the standard's
> authors (you know who you are!).
>
>> I understand the idea and I would even agree that this is the proper way
>> how to do it in the long term. But I'm not sure whether all brokers support
>> this concept. For example, I'm not sure whether you can configure the Qpid
>> C++ broker in a way to accept AMQP 1.0 connections with SSL Client
>> Authentication without SASL EXTERNAL while at the same time accepting AMQP
>> 0-10 connections only with SASL EXTERNAL. Therefore I would be afraid that
>> allowing SSL Client Authentication only without SASL could cause some
>> serious incompatibilities - I think both should be possible / supported.
>
> And both are supported.
>
> The qpidd 0-10 support is not going to change. The qpidd 1.0 support is
> on a different code path so there is little bleed over in functionality.
>
> The proton server code can auto detect which protocol layers the client
> is using, and subject to it being an allowed protocol configuration,
> authenticate it.
>
> Other AMQP 1.0 implementations may not support leaving out the SASL
> layer and so you can certainly always tell the client to use it (even if
> it adds no useful functionality as in the ANONYMOUS and EXTERNAL cases).
>
> So as far as the current plans for proton go if you require SSL client
> authentication it will happen whether or not a SASL layer is there.
>
> As EXTERNAL and better SSL integration with the transport code is not
> yet implemented there may be something significant I've missed in this
> analysis, in which case it's all subject to change!
>
> I hope that helps.
>
I'm going to post my comments here and on the wiki, as I dont think
many (except maybe you) will actually see them on the wiki ;)
Assuming I've read the email thread correctly, you do plan on
implementing EXTERNAL so that clients can be authenticated using SSL
client auth + EXTERNAL. I think that is a good idea, as even though it
can be ommitted not all brokers or clients will want to support doing
so. There may also be cases where implementations handle the SSL by
themselves and only want to do SASL via proton (albeit by
reading/writing bytes currently). Possibly less so on the C side, but
almost everyone seems to do that with Proton-J. Mentioning Proton-J,
are there any plans there?
The completion of the SASL stage only being indicated by an event,
combined with removal of the old read/write sasl bytes methods, seems
to end any prospect of not using the events if you want SASL (at
least, not without moving to intercepting the raw SASL frames before
the transport). Does this mark the end of the old non-events API?
The method for excluding certain server mechanisms seems a bit odd to
me. A method to configure the mechanisms to be used feels like it
might be a better fit (either ignoring mechanism that arent installed,
or throwing to indicate they arent present, or making that a choice),
what do you think?
How does pn_transport_require_auth interact with old 'allow skip'
functionality that let transports with SASL configured permit non-SASL
connections? The two have different defaults, as skipping wasnt
allowed previously (hence the old method being added), whereas
apparently it now would be by default. That itself feels a little
wrong to me, I think people should have to opt-in to that behaviour as
they did previously.
You mention removing deprecated APIs in the form of
pn_sasl_client/pn_sasl_server. I think its worth mentioning these were
only to become deprecated in 0.9.
Robbie
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
For additional commands, e-mail: users-help@qpid.apache.org
Re: Proposed SASL changes (API and functional)
Posted by Andrew Stitcher <as...@redhat.com>.
On Tue, 2015-03-03 at 15:28 +0000, Gordon Sim wrote:
> On 03/03/2015 01:50 AM, Andrew Stitcher wrote:
> > On Mon, 2015-03-02 at 18:41 +0000, Gordon Sim wrote:
> >> In fact the c++ broker doesn't use an AMQP 1.0 style layer for SSL at
> >> all - i.e. it does not recognise the special AMQP 1.0 TLS header sent in
> >> the clear prior to TLS handshaking as described in 5.2 of the AMQP spec.
> >> The qpid::messaging c++ client doesn't send one either. Both use the
> >> 'alternative establishment' as described by 5.2.1 (though for a
> >> different reason than the one suggested there). So yet another point of
> >> possible interoperability issues.
> >
> > FYI: Currently Proton-C does not support the "AMQP 1.0" style SSL header
> > either to send or receive (they are recognised for error message
> > purposes currently)
>
> Thanks for the clarification! I was planning to investigate that, since
> I knew that ssl 'works' between proton-c and qpidd.
>
> > - this is a piece of work I have scheduled post the
> > SASL integration.
>
> We probably want to retain some way of using the 'alternative
> establishment' as well, in order to not lose interop.
Here my focus is on the server end autodetection, so better interop is
achieved by coping with both SSL alternatives.
I think that initially, the correct default for the client should be to
use the AMQP 1.0 type header for the default port (5672) and the
"alternative establishment" for other ports (5761 or other).
Andrew
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
For additional commands, e-mail: users-help@qpid.apache.org
Re: Proposed SASL changes (API and functional)
Posted by Gordon Sim <gs...@redhat.com>.
On 03/03/2015 01:50 AM, Andrew Stitcher wrote:
> On Mon, 2015-03-02 at 18:41 +0000, Gordon Sim wrote:
>> In fact the c++ broker doesn't use an AMQP 1.0 style layer for SSL at
>> all - i.e. it does not recognise the special AMQP 1.0 TLS header sent in
>> the clear prior to TLS handshaking as described in 5.2 of the AMQP spec.
>> The qpid::messaging c++ client doesn't send one either. Both use the
>> 'alternative establishment' as described by 5.2.1 (though for a
>> different reason than the one suggested there). So yet another point of
>> possible interoperability issues.
>
> FYI: Currently Proton-C does not support the "AMQP 1.0" style SSL header
> either to send or receive (they are recognised for error message
> purposes currently)
Thanks for the clarification! I was planning to investigate that, since
I knew that ssl 'works' between proton-c and qpidd.
> - this is a piece of work I have scheduled post the
> SASL integration.
We probably want to retain some way of using the 'alternative
establishment' as well, in order to not lose interop.
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
For additional commands, e-mail: users-help@qpid.apache.org
Re: Proposed SASL changes (API and functional)
Posted by Andrew Stitcher <as...@redhat.com>.
On Mon, 2015-03-02 at 18:41 +0000, Gordon Sim wrote:
> On 03/02/2015 06:06 PM, Jakub Scholz wrote:
> > That's not a problem. My concern here were not really any future changes
> > introduced into these components with this change. The point is, that
> > whatever client is written based on Proton 0.9 later this year, it should
> > work with the Qpid C++ broker from today. And whatever broker is written
> > based on Proton 0.9 should work with the qpid::messaging API from today.
>
> Sorry, I was skimming the thread and latched on to Andrew's response
> without properly digesting your initial post.
>
> I agree with your point and indeed you are correct that at present the
> Qpid c++ broker requires a SASL layer with EXTERNAL in order to
> authenticate a client by the SSL certificate it supplies.
>
> In fact the c++ broker doesn't use an AMQP 1.0 style layer for SSL at
> all - i.e. it does not recognise the special AMQP 1.0 TLS header sent in
> the clear prior to TLS handshaking as described in 5.2 of the AMQP spec.
> The qpid::messaging c++ client doesn't send one either. Both use the
> 'alternative establishment' as described by 5.2.1 (though for a
> different reason than the one suggested there). So yet another point of
> possible interoperability issues.
FYI: Currently Proton-C does not support the "AMQP 1.0" style SSL header
either to send or receive (they are recognised for error message
purposes currently) - this is a piece of work I have scheduled post the
SASL integration.
Andrew
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
For additional commands, e-mail: users-help@qpid.apache.org
Re: Proposed SASL changes (API and functional)
Posted by Gordon Sim <gs...@redhat.com>.
On 03/02/2015 06:06 PM, Jakub Scholz wrote:
> That's not a problem. My concern here were not really any future changes
> introduced into these components with this change. The point is, that
> whatever client is written based on Proton 0.9 later this year, it should
> work with the Qpid C++ broker from today. And whatever broker is written
> based on Proton 0.9 should work with the qpid::messaging API from today.
Sorry, I was skimming the thread and latched on to Andrew's response
without properly digesting your initial post.
I agree with your point and indeed you are correct that at present the
Qpid c++ broker requires a SASL layer with EXTERNAL in order to
authenticate a client by the SSL certificate it supplies.
In fact the c++ broker doesn't use an AMQP 1.0 style layer for SSL at
all - i.e. it does not recognise the special AMQP 1.0 TLS header sent in
the clear prior to TLS handshaking as described in 5.2 of the AMQP spec.
The qpid::messaging c++ client doesn't send one either. Both use the
'alternative establishment' as described by 5.2.1 (though for a
different reason than the one suggested there). So yet another point of
possible interoperability issues.
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
For additional commands, e-mail: users-help@qpid.apache.org
Re: Proposed SASL changes (API and functional)
Posted by Jakub Scholz <ja...@scholz.cz>.
That's not a problem. My concern here were not really any future changes
introduced into these components with this change. The point is, that
whatever client is written based on Proton 0.9 later this year, it should
work with the Qpid C++ broker from today. And whatever broker is written
based on Proton 0.9 should work with the qpid::messaging API from today.
Jakub
On Mon, Mar 2, 2015 at 6:43 PM, Gordon Sim <gs...@redhat.com> wrote:
> On 02/25/2015 06:40 PM, Andrew Stitcher wrote:
>
>> On Wed, 2015-02-25 at 10:27 +0100, Jakub Scholz wrote:
>>
>>> I understand the idea and I would even agree that this is the proper way
>>> how to do it in the long term. But I'm not sure whether all brokers
>>> support
>>> this concept. For example, I'm not sure whether you can configure the
>>> Qpid
>>> C++ broker in a way to accept AMQP 1.0 connections with SSL Client
>>> Authentication without SASL EXTERNAL while at the same time accepting
>>> AMQP
>>> 0-10 connections only with SASL EXTERNAL. Therefore I would be afraid
>>> that
>>> allowing SSL Client Authentication only without SASL could cause some
>>> serious incompatibilities - I think both should be possible / supported.
>>>
>>
>> And both are supported.
>>
>> The qpidd 0-10 support is not going to change. The qpidd 1.0 support is
>> on a different code path so there is little bleed over in functionality.
>>
>
> Also, for the sake of clarity, the qpidd 1.0 SASL support does not use
> proton anyway at present (neither does the qpid::messaging c++ client), so
> this doesn't change the 1.0 path either.
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
> For additional commands, e-mail: users-help@qpid.apache.org
>
>
Re: Proposed SASL changes (API and functional)
Posted by Gordon Sim <gs...@redhat.com>.
On 02/25/2015 06:40 PM, Andrew Stitcher wrote:
> On Wed, 2015-02-25 at 10:27 +0100, Jakub Scholz wrote:
>> I understand the idea and I would even agree that this is the proper way
>> how to do it in the long term. But I'm not sure whether all brokers support
>> this concept. For example, I'm not sure whether you can configure the Qpid
>> C++ broker in a way to accept AMQP 1.0 connections with SSL Client
>> Authentication without SASL EXTERNAL while at the same time accepting AMQP
>> 0-10 connections only with SASL EXTERNAL. Therefore I would be afraid that
>> allowing SSL Client Authentication only without SASL could cause some
>> serious incompatibilities - I think both should be possible / supported.
>
> And both are supported.
>
> The qpidd 0-10 support is not going to change. The qpidd 1.0 support is
> on a different code path so there is little bleed over in functionality.
Also, for the sake of clarity, the qpidd 1.0 SASL support does not use
proton anyway at present (neither does the qpid::messaging c++ client),
so this doesn't change the 1.0 path either.
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
For additional commands, e-mail: users-help@qpid.apache.org
Re: Proposed SASL changes (API and functional)
Posted by Robbie Gemmell <ro...@gmail.com>.
On 25 February 2015 at 18:40, Andrew Stitcher <as...@redhat.com> wrote:
> On Wed, 2015-02-25 at 10:27 +0100, Jakub Scholz wrote:
>> ...
>> But I find this part a bit dangerous:
>> "Classically in protocols where SASL was not optional the way to avoid
>> double authentication was to use the EXTERNAL SASL mechanism. With AMQP,
>> SASL is optional, so if SSL is used for client authentication the SASL
>> layer could be entirely omitted and so using EXTERNAL is not necessary."
>>
>
> This is really just a statement about how AMQP 1.0 works - if you like -
> it is an aside praising the good protocol design sense of the standard's
> authors (you know who you are!).
>
>> I understand the idea and I would even agree that this is the proper way
>> how to do it in the long term. But I'm not sure whether all brokers support
>> this concept. For example, I'm not sure whether you can configure the Qpid
>> C++ broker in a way to accept AMQP 1.0 connections with SSL Client
>> Authentication without SASL EXTERNAL while at the same time accepting AMQP
>> 0-10 connections only with SASL EXTERNAL. Therefore I would be afraid that
>> allowing SSL Client Authentication only without SASL could cause some
>> serious incompatibilities - I think both should be possible / supported.
>
> And both are supported.
>
> The qpidd 0-10 support is not going to change. The qpidd 1.0 support is
> on a different code path so there is little bleed over in functionality.
>
> The proton server code can auto detect which protocol layers the client
> is using, and subject to it being an allowed protocol configuration,
> authenticate it.
>
> Other AMQP 1.0 implementations may not support leaving out the SASL
> layer and so you can certainly always tell the client to use it (even if
> it adds no useful functionality as in the ANONYMOUS and EXTERNAL cases).
>
> So as far as the current plans for proton go if you require SSL client
> authentication it will happen whether or not a SASL layer is there.
>
> As EXTERNAL and better SSL integration with the transport code is not
> yet implemented there may be something significant I've missed in this
> analysis, in which case it's all subject to change!
>
> I hope that helps.
>
I'm going to post my comments here and on the wiki, as I dont think
many (except maybe you) will actually see them on the wiki ;)
Assuming I've read the email thread correctly, you do plan on
implementing EXTERNAL so that clients can be authenticated using SSL
client auth + EXTERNAL. I think that is a good idea, as even though it
can be ommitted not all brokers or clients will want to support doing
so. There may also be cases where implementations handle the SSL by
themselves and only want to do SASL via proton (albeit by
reading/writing bytes currently). Possibly less so on the C side, but
almost everyone seems to do that with Proton-J. Mentioning Proton-J,
are there any plans there?
The completion of the SASL stage only being indicated by an event,
combined with removal of the old read/write sasl bytes methods, seems
to end any prospect of not using the events if you want SASL (at
least, not without moving to intercepting the raw SASL frames before
the transport). Does this mark the end of the old non-events API?
The method for excluding certain server mechanisms seems a bit odd to
me. A method to configure the mechanisms to be used feels like it
might be a better fit (either ignoring mechanism that arent installed,
or throwing to indicate they arent present, or making that a choice),
what do you think?
How does pn_transport_require_auth interact with old 'allow skip'
functionality that let transports with SASL configured permit non-SASL
connections? The two have different defaults, as skipping wasnt
allowed previously (hence the old method being added), whereas
apparently it now would be by default. That itself feels a little
wrong to me, I think people should have to opt-in to that behaviour as
they did previously.
You mention removing deprecated APIs in the form of
pn_sasl_client/pn_sasl_server. I think its worth mentioning these were
only to become deprecated in 0.9.
Robbie
Re: Proposed SASL changes (API and functional)
Posted by Andrew Stitcher <as...@redhat.com>.
On Wed, 2015-02-25 at 10:27 +0100, Jakub Scholz wrote:
> ...
> But I find this part a bit dangerous:
> "Classically in protocols where SASL was not optional the way to avoid
> double authentication was to use the EXTERNAL SASL mechanism. With AMQP,
> SASL is optional, so if SSL is used for client authentication the SASL
> layer could be entirely omitted and so using EXTERNAL is not necessary."
>
This is really just a statement about how AMQP 1.0 works - if you like -
it is an aside praising the good protocol design sense of the standard's
authors (you know who you are!).
> I understand the idea and I would even agree that this is the proper way
> how to do it in the long term. But I'm not sure whether all brokers support
> this concept. For example, I'm not sure whether you can configure the Qpid
> C++ broker in a way to accept AMQP 1.0 connections with SSL Client
> Authentication without SASL EXTERNAL while at the same time accepting AMQP
> 0-10 connections only with SASL EXTERNAL. Therefore I would be afraid that
> allowing SSL Client Authentication only without SASL could cause some
> serious incompatibilities - I think both should be possible / supported.
And both are supported.
The qpidd 0-10 support is not going to change. The qpidd 1.0 support is
on a different code path so there is little bleed over in functionality.
The proton server code can auto detect which protocol layers the client
is using, and subject to it being an allowed protocol configuration,
authenticate it.
Other AMQP 1.0 implementations may not support leaving out the SASL
layer and so you can certainly always tell the client to use it (even if
it adds no useful functionality as in the ANONYMOUS and EXTERNAL cases).
So as far as the current plans for proton go if you require SSL client
authentication it will happen whether or not a SASL layer is there.
As EXTERNAL and better SSL integration with the transport code is not
yet implemented there may be something significant I've missed in this
analysis, in which case it's all subject to change!
I hope that helps.
Andrew
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
For additional commands, e-mail: users-help@qpid.apache.org
Re: Proposed SASL changes (API and functional)
Posted by Andrew Stitcher <as...@redhat.com>.
On Wed, 2015-02-25 at 10:27 +0100, Jakub Scholz wrote:
> ...
> But I find this part a bit dangerous:
> "Classically in protocols where SASL was not optional the way to avoid
> double authentication was to use the EXTERNAL SASL mechanism. With AMQP,
> SASL is optional, so if SSL is used for client authentication the SASL
> layer could be entirely omitted and so using EXTERNAL is not necessary."
>
This is really just a statement about how AMQP 1.0 works - if you like -
it is an aside praising the good protocol design sense of the standard's
authors (you know who you are!).
> I understand the idea and I would even agree that this is the proper way
> how to do it in the long term. But I'm not sure whether all brokers support
> this concept. For example, I'm not sure whether you can configure the Qpid
> C++ broker in a way to accept AMQP 1.0 connections with SSL Client
> Authentication without SASL EXTERNAL while at the same time accepting AMQP
> 0-10 connections only with SASL EXTERNAL. Therefore I would be afraid that
> allowing SSL Client Authentication only without SASL could cause some
> serious incompatibilities - I think both should be possible / supported.
And both are supported.
The qpidd 0-10 support is not going to change. The qpidd 1.0 support is
on a different code path so there is little bleed over in functionality.
The proton server code can auto detect which protocol layers the client
is using, and subject to it being an allowed protocol configuration,
authenticate it.
Other AMQP 1.0 implementations may not support leaving out the SASL
layer and so you can certainly always tell the client to use it (even if
it adds no useful functionality as in the ANONYMOUS and EXTERNAL cases).
So as far as the current plans for proton go if you require SSL client
authentication it will happen whether or not a SASL layer is there.
As EXTERNAL and better SSL integration with the transport code is not
yet implemented there may be something significant I've missed in this
analysis, in which case it's all subject to change!
I hope that helps.
Andrew
Re: Proposed SASL changes (API and functional)
Posted by Jakub Scholz <ja...@scholz.cz>.
Hi Andrew,
I'm definitely not a Proton expert, so please excuse me if I missed
something.
But I find this part a bit dangerous:
"Classically in protocols where SASL was not optional the way to avoid
double authentication was to use the EXTERNAL SASL mechanism. With AMQP,
SASL is optional, so if SSL is used for client authentication the SASL
layer could be entirely omitted and so using EXTERNAL is not necessary."
I understand the idea and I would even agree that this is the proper way
how to do it in the long term. But I'm not sure whether all brokers support
this concept. For example, I'm not sure whether you can configure the Qpid
C++ broker in a way to accept AMQP 1.0 connections with SSL Client
Authentication without SASL EXTERNAL while at the same time accepting AMQP
0-10 connections only with SASL EXTERNAL. Therefore I would be afraid that
allowing SSL Client Authentication only without SASL could cause some
serious incompatibilities - I think both should be possible / supported.
Regards
Jakub
On Tue, Feb 24, 2015 at 9:48 PM, Andrew Stitcher <as...@redhat.com>
wrote:
> As many of you know I've been working on implementing a SASL AMQP
> protocol layer that does more than PLAIN and ANONYMOUS for proton-c.
>
> I'm currently in at a point where the work is reasonably functional
> (with some gaps)
>
> I've put together a fairly comprehensive account of this work on the
> Apache wiki: https://cwiki.apache.org/confluence/x/B5cWAw
>
> If you are at all interested please go and look at the proposal and
> comment on it there.
>
> You can see my actual code changes in my github proton repo:
> https://github.com/astitcher/qpid-proton/commits/sasl-work
>
> [This is my working branch, so not all the changes make a lot of sense,
> just pay attention to the tip of the branch]
>
> In a short while when people have had enough time to absorb the proposal
> and comment I will post a code review of the actual code changes. As
> there are substantial API changes I'd like to get this in for 0.9
> because we were intending to stabilise the API at this point.
>
> Thanks.
>
> Andrew
>
>
Re: Proposed SASL changes (API and functional)
Posted by Andrew Stitcher <as...@redhat.com>.
On Thu, 2015-02-26 at 09:45 -0500, Rafael Schloming wrote:
> ...
I've cut and pasted this into the wiki and will reply there.
wiki link:
https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=51812103&focusedCommentId=51812475#comment-51812475
Andrew
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
For additional commands, e-mail: users-help@qpid.apache.org
Re: Proposed SASL changes (API and functional)
Posted by Rafael Schloming <rh...@alum.mit.edu>.
On Tue, Feb 24, 2015 at 3:48 PM, Andrew Stitcher <as...@redhat.com>
wrote:
> As many of you know I've been working on implementing a SASL AMQP
> protocol layer that does more than PLAIN and ANONYMOUS for proton-c.
>
> I'm currently in at a point where the work is reasonably functional
> (with some gaps)
>
> I've put together a fairly comprehensive account of this work on the
> Apache wiki: https://cwiki.apache.org/confluence/x/B5cWAw
>
> If you are at all interested please go and look at the proposal and
> comment on it there.
>
> You can see my actual code changes in my github proton repo:
> https://github.com/astitcher/qpid-proton/commits/sasl-work
>
> [This is my working branch, so not all the changes make a lot of sense,
> just pay attention to the tip of the branch]
>
> In a short while when people have had enough time to absorb the proposal
> and comment I will post a code review of the actual code changes. As
> there are substantial API changes I'd like to get this in for 0.9
> because we were intending to stabilise the API at this point.
>
This seems like a sensible direction in general. I'm definitely interested
in seeing some example usages, but it looks like that's on your list
already.
A couple random comments on the API changes.
I noticed in your description of the API you have added
pn_transport_set_remote_hostname. It's a bit odd to have the setter but no
getter. Secondly there is a terminology conflict here. In general the API
uses the convention that remote_blah refers to information supplied by the
remote peer. You are not using it that way, you are using it to communicate
expected information about the remote peer, and that is actually local
information, not remote information. It really doesn't ever make sense to
have a public set_remote_blah anywhere given the API conventions since the
only place you are ever going to set those fields is with information
supplied to you over the wire by the remote peer.
Also, I'm not convinced you actually need this at all since AMQP defines
the connection hostname and there is already a setter for this. The value
that goes into the hostname frame of a connection is defined to be the
expected/desired hostname that the client will authenticate against (think
vhosts). It is also required to be the same as the one specified in the
sasl-init frame if present. That means pn_connection_set_hostname and
pn_transport_set_remote_hostname as you have defined it are effectively
aliases which is a confusing state of affairs on its own even without the
general conventions around what the remote prefix means.
Another getter/setter asymmetry is the the get_user which doesn't have a
corresponding set_user directly, but does have an indirect one through
set_user_password.
I know the getter/setter stuff is a bit niggling, but beyond being nice to
follow the general conventions, it's actually quite awkward when it comes
to the bindings. In general get/set_foo is mapped into a property, e.g.
obj.foo/obj.foo = blah, and this breaks down if you start having set_foo
without get_foo (write only attributes are kind of ugly/unexpected).
I'd suggest you have get/set_user with set_user being independent of
setting the password. That will map more naturally into an ordinary "user"
property in the bindings, and the setter can return an error code for a
server transport if you want it to be read-only. I also think this is nicer
in general since I expect there are cases where you may want to supply the
password in a different place than you configure the user, and with them
both bundled into the same call, that gets awkward.
--Rafael
Re: Proposed SASL changes (API and functional)
Posted by Alan Conway <ac...@redhat.com>.
On Tue, 2015-02-24 at 15:48 -0500, Andrew Stitcher wrote:
> As many of you know I've been working on implementing a SASL AMQP
> protocol layer that does more than PLAIN and ANONYMOUS for proton-c.
>
> I'm currently in at a point where the work is reasonably functional
> (with some gaps)
>
> I've put together a fairly comprehensive account of this work on the
> Apache wiki: https://cwiki.apache.org/confluence/x/B5cWAw
>
> If you are at all interested please go and look at the proposal and
> comment on it there.
>
> You can see my actual code changes in my github proton repo:
> https://github.com/astitcher/qpid-proton/commits/sasl-work
>
> [This is my working branch, so not all the changes make a lot of sense,
> just pay attention to the tip of the branch]
>
> In a short while when people have had enough time to absorb the proposal
> and comment I will post a code review of the actual code changes. As
> there are substantial API changes I'd like to get this in for 0.9
> because we were intending to stabilise the API at this point.
This looks very good to me.
One ignorant question: Qpid has a min/max "Security Strength Factor" for
encryption rather than a binary enable/disable. Is that relevant here?
Cheers,
Alan.
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
For additional commands, e-mail: users-help@qpid.apache.org
Re: Proposed SASL changes (API and functional)
Posted by Ken Giusti <kg...@redhat.com>.
----- Original Message -----
> From: "Gordon Sim" <gs...@redhat.com>
> To: users@qpid.apache.org
> Cc: proton@qpid.apache.org
> Sent: Monday, March 2, 2015 2:30:28 PM
> Subject: Re: Proposed SASL changes (API and functional)
>
> On 02/24/2015 08:48 PM, Andrew Stitcher wrote:
> > In a short while when people have had enough time to absorb the proposal
> > and comment I will post a code review of the actual code changes. As
> > there are substantial API changes I'd like to get this in for 0.9
> > because we were intending to stabilise the API at this point.
>
> To me it seems a bit late in the cycle for 0.9 for changes of this
> magnitude and I'd prefer to see it included during 0.10 with any
> accompanying changes to ssl.
>
+1 to that. While I like where these API changes are headed, I don't think we should hold 0.9 up. I'd prefer cutting 0.9 without these changes. Put the API changes on trunk once 0.9 is out and give us some more time to 'kick the tires'.
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
For additional commands, e-mail: users-help@qpid.apache.org
Re: Proposed SASL changes (API and functional)
Posted by Robbie Gemmell <ro...@gmail.com>.
On 2 March 2015 at 19:30, Gordon Sim <gs...@redhat.com> wrote:
> On 02/24/2015 08:48 PM, Andrew Stitcher wrote:
>>
>> In a short while when people have had enough time to absorb the proposal
>> and comment I will post a code review of the actual code changes. As
>> there are substantial API changes I'd like to get this in for 0.9
>> because we were intending to stabilise the API at this point.
>
>
> To me it seems a bit late in the cycle for 0.9 for changes of this magnitude
> and I'd prefer to see it included during 0.10 with any accompanying changes
> to ssl.
I've been starting to think that way too. Shipping 0.9 now without the
major SASL changes, then turning 0.10 around much more quickly with
those and anything else finished in a similar timeframe seems like a
nicer prospect to me. Its already > 2 months behind schedule afterall
and there are dependent releases (0.32) already long underway.
Robbie
Re: Proposed SASL changes (API and functional)
Posted by Ken Giusti <kg...@redhat.com>.
----- Original Message -----
> From: "Gordon Sim" <gs...@redhat.com>
> To: users@qpid.apache.org
> Cc: proton@qpid.apache.org
> Sent: Monday, March 2, 2015 2:30:28 PM
> Subject: Re: Proposed SASL changes (API and functional)
>
> On 02/24/2015 08:48 PM, Andrew Stitcher wrote:
> > In a short while when people have had enough time to absorb the proposal
> > and comment I will post a code review of the actual code changes. As
> > there are substantial API changes I'd like to get this in for 0.9
> > because we were intending to stabilise the API at this point.
>
> To me it seems a bit late in the cycle for 0.9 for changes of this
> magnitude and I'd prefer to see it included during 0.10 with any
> accompanying changes to ssl.
>
+1 to that. While I like where these API changes are headed, I don't think we should hold 0.9 up. I'd prefer cutting 0.9 without these changes. Put the API changes on trunk once 0.9 is out and give us some more time to 'kick the tires'.
-K
Re: Proposed SASL changes (API and functional)
Posted by Robbie Gemmell <ro...@gmail.com>.
On 2 March 2015 at 19:30, Gordon Sim <gs...@redhat.com> wrote:
> On 02/24/2015 08:48 PM, Andrew Stitcher wrote:
>>
>> In a short while when people have had enough time to absorb the proposal
>> and comment I will post a code review of the actual code changes. As
>> there are substantial API changes I'd like to get this in for 0.9
>> because we were intending to stabilise the API at this point.
>
>
> To me it seems a bit late in the cycle for 0.9 for changes of this magnitude
> and I'd prefer to see it included during 0.10 with any accompanying changes
> to ssl.
I've been starting to think that way too. Shipping 0.9 now without the
major SASL changes, then turning 0.10 around much more quickly with
those and anything else finished in a similar timeframe seems like a
nicer prospect to me. Its already > 2 months behind schedule afterall
and there are dependent releases (0.32) already long underway.
Robbie
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
For additional commands, e-mail: users-help@qpid.apache.org
Re: Proposed SASL changes (API and functional)
Posted by Gordon Sim <gs...@redhat.com>.
On 02/24/2015 08:48 PM, Andrew Stitcher wrote:
> In a short while when people have had enough time to absorb the proposal
> and comment I will post a code review of the actual code changes. As
> there are substantial API changes I'd like to get this in for 0.9
> because we were intending to stabilise the API at this point.
To me it seems a bit late in the cycle for 0.9 for changes of this
magnitude and I'd prefer to see it included during 0.10 with any
accompanying changes to ssl.
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
For additional commands, e-mail: users-help@qpid.apache.org
Re: Proposed SASL changes (API and functional)
Posted by Gordon Sim <gs...@redhat.com>.
On 02/24/2015 08:48 PM, Andrew Stitcher wrote:
> In a short while when people have had enough time to absorb the proposal
> and comment I will post a code review of the actual code changes. As
> there are substantial API changes I'd like to get this in for 0.9
> because we were intending to stabilise the API at this point.
To me it seems a bit late in the cycle for 0.9 for changes of this
magnitude and I'd prefer to see it included during 0.10 with any
accompanying changes to ssl.
Re: Proposed SASL changes (API and functional)
Posted by Jakub Scholz <ja...@scholz.cz>.
Hi Andrew,
I'm definitely not a Proton expert, so please excuse me if I missed
something.
But I find this part a bit dangerous:
"Classically in protocols where SASL was not optional the way to avoid
double authentication was to use the EXTERNAL SASL mechanism. With AMQP,
SASL is optional, so if SSL is used for client authentication the SASL
layer could be entirely omitted and so using EXTERNAL is not necessary."
I understand the idea and I would even agree that this is the proper way
how to do it in the long term. But I'm not sure whether all brokers support
this concept. For example, I'm not sure whether you can configure the Qpid
C++ broker in a way to accept AMQP 1.0 connections with SSL Client
Authentication without SASL EXTERNAL while at the same time accepting AMQP
0-10 connections only with SASL EXTERNAL. Therefore I would be afraid that
allowing SSL Client Authentication only without SASL could cause some
serious incompatibilities - I think both should be possible / supported.
Regards
Jakub
On Tue, Feb 24, 2015 at 9:48 PM, Andrew Stitcher <as...@redhat.com>
wrote:
> As many of you know I've been working on implementing a SASL AMQP
> protocol layer that does more than PLAIN and ANONYMOUS for proton-c.
>
> I'm currently in at a point where the work is reasonably functional
> (with some gaps)
>
> I've put together a fairly comprehensive account of this work on the
> Apache wiki: https://cwiki.apache.org/confluence/x/B5cWAw
>
> If you are at all interested please go and look at the proposal and
> comment on it there.
>
> You can see my actual code changes in my github proton repo:
> https://github.com/astitcher/qpid-proton/commits/sasl-work
>
> [This is my working branch, so not all the changes make a lot of sense,
> just pay attention to the tip of the branch]
>
> In a short while when people have had enough time to absorb the proposal
> and comment I will post a code review of the actual code changes. As
> there are substantial API changes I'd like to get this in for 0.9
> because we were intending to stabilise the API at this point.
>
> Thanks.
>
> Andrew
>
>