You are viewing a plain text version of this content. The canonical link for it is here.
Posted to proton@qpid.apache.org by astitcher <gi...@git.apache.org> on 2015/04/09 09:28:59 UTC
[GitHub] qpid-proton pull request: PROTON-334: SASL Implementation for Prot...
GitHub user astitcher opened a pull request:
https://github.com/apache/qpid-proton/pull/17
PROTON-334: SASL Implementation for Proton-C using Cyrus SASL
This work Adds some new APIs to the transport and connection
objects to make a higher level abstraction for authentication.
This generally makes it much easier to use authentication.
It also vastly changes the Proton C API for SASL and deprecates
nearly all of the previous interface that allowed reading and
writing individual SASL frames.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/astitcher/qpid-proton PROTON-334
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/qpid-proton/pull/17.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #17
----
commit 4a09c6a17f865df10f53fa61c8d2bc88d4627bb0
Author: Andrew Stitcher <as...@apache.org>
Date: 2015-04-09T06:14:14Z
PROTON-334: SASL Implementation for Proton-C using Cyrus SASL
This work Adds some new APIs to the transport and connection
objects to make a higher level abstraction for authentication.
This generally makes it much easier to use authentication.
It also vastly changes the Proton C API for SASL and deprecates
nearly all of the previous interface that allowed reading and
writing individual SASL frames.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] qpid-proton pull request: PROTON-334: SASL Implementation for Prot...
Posted by astitcher <gi...@git.apache.org>.
Github user astitcher commented on the pull request:
https://github.com/apache/qpid-proton/pull/17#issuecomment-93824913
@dnwe I think I copied that CMake code directly from qpid. I'll look at doing something like you suggested - the CMake output does look clunky.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] qpid-proton pull request: PROTON-334: SASL Implementation for Prot...
Posted by dnwe <gi...@git.apache.org>.
Github user dnwe commented on the pull request:
https://github.com/apache/qpid-proton/pull/17#issuecomment-93675959
@astitcher minor issue, unrelated to the feature itself, but the CMake build doesn't currently report which sasl implementation is being used. I just see:
```
-- Looking for sasl_checkpass in sasl2
-- Looking for sasl_checkpass in sasl2 - found
-- Looking for include file sasl/sasl.h
-- Looking for include file sasl/sasl.h - found
-- Found SASL: 1
```
Could you perhaps add something along the lines of:
```
if (FOUND_SASL_LIB)
message(STATUS "Found SASL: ${FOUND_SASL_LIB}")
else()
message(STATUS "Found SASL: BUILT-IN")
endif(FOUND_SASL_LIB)
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] qpid-proton pull request: PROTON-334: SASL Implementation for Prot...
Posted by rhs <gi...@git.apache.org>.
Github user rhs commented on the pull request:
https://github.com/apache/qpid-proton/pull/17#issuecomment-94437344
Looks good to me, assuming all the tests pass and what not, I'm +1.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
Re: [GitHub] qpid-proton pull request: PROTON-334: SASL
Implementation for Prot...
Posted by Chuck Rolke <cr...@redhat.com>.
Impressions:
* Formalizing the Auth/Encryption grid (table on the wiki page) is a Good Thing.
* Getting rid of pn_sasl_mechanisms and pn_sasl_remote_mechanisms where clients directly read and write SASL protocol frames is a winner. The proposed interface improves how and what a client can do by limiting the choices the client may take. The proposed interface has clear and direct control and a lot less margin for error. I'm just thinking of a new user figuring out how to handle SASL binary data versus calling set_user and set_password. I mean, if you had set_user and set_password now and the proposal was to have the client provide binary SASL data then the proposal would _never_ be approved. This proposal really improves the connection establishment control flow.
Security default:
* Leave it unauthenticated and unencrypted by default. If a user can get his stuff off the ground easily he'll be more likely to persist in the long run.
Examples:
* Your Auth/Encryption grid on the wiki page covers six reasonable/possible combinations of auth/encryption. A set of examples that drive this grid would really help. That is, if I'm writing a server and I require both auth and encryption, how do I do it? How do I know I've properly locked down my connections? And from the client side, how do I respond properly to the server requirements? What will be my error paths if I can't meet the server requirements?
Bottom line:
* Ship it!
-Chuck
----- Original Message -----
> From: "Andrew Stitcher" <as...@redhat.com>
> To: proton@qpid.apache.org
> Sent: Wednesday, April 15, 2015 4:06:05 PM
> Subject: Re: [GitHub] qpid-proton pull request: PROTON-334: SASL Implementation for Prot...
>
> I've received no further feedback at all about this review request.
>
> Do people want me to create a reviewboard item instead?
>
> For reference, this proposed change incorporates all the discussion -
> although I haven't necessarily agreed with everyone else's points!
>
> For clarity the main issue that I haven't changed on is the default for
> requiring authorisation/encryption on the server side of a proton-c
> connection. I'd like to solicit some other opinion about this (other
> than Robbie who has made his opinion clear at this point)
>
> The proposed code defaults to allowing unauthorised and unencrypted
> incoming connections by default. This is for ease of initial use
> considerations. The opposing viewpoint is that this is insecure by
> default and it would be best to be secure by default.
>
> I'd note that the previous state is a little confused, in that
> unencrypted is allowed by default, and authentication may or may not be
> required depending.
>
> I'd be reasonably happy to do either easy to use by default or secure by
> default, but I'm dead set against having the authentication and
> encryption defaults be different.
>
> Andrew
>
> On Thu, 2015-04-09 at 07:31 +0000, astitcher wrote:
> > Github user astitcher commented on the pull request:
> >
> > https://github.com/apache/qpid-proton/pull/17#issuecomment-91137151
> >
> > See the wiki for more information and context:
> > https://cwiki.apache.org/confluence/x/B5cWAw
> >
> >
> > ---
> > If your project is set up for it, you can reply to this email and have your
> > reply appear on GitHub as well. If your project does not have this feature
> > enabled and wishes so, or if the feature is enabled but not working, please
> > contact infrastructure at infrastructure@apache.org or file a JIRA ticket
> > with INFRA.
> > ---
>
>
>
Re: [GitHub] qpid-proton pull request: PROTON-334: SASL
Implementation for Prot...
Posted by Andrew Stitcher <as...@redhat.com>.
I've received no further feedback at all about this review request.
Do people want me to create a reviewboard item instead?
For reference, this proposed change incorporates all the discussion -
although I haven't necessarily agreed with everyone else's points!
For clarity the main issue that I haven't changed on is the default for
requiring authorisation/encryption on the server side of a proton-c
connection. I'd like to solicit some other opinion about this (other
than Robbie who has made his opinion clear at this point)
The proposed code defaults to allowing unauthorised and unencrypted
incoming connections by default. This is for ease of initial use
considerations. The opposing viewpoint is that this is insecure by
default and it would be best to be secure by default.
I'd note that the previous state is a little confused, in that
unencrypted is allowed by default, and authentication may or may not be
required depending.
I'd be reasonably happy to do either easy to use by default or secure by
default, but I'm dead set against having the authentication and
encryption defaults be different.
Andrew
On Thu, 2015-04-09 at 07:31 +0000, astitcher wrote:
> Github user astitcher commented on the pull request:
>
> https://github.com/apache/qpid-proton/pull/17#issuecomment-91137151
>
> See the wiki for more information and context:
> https://cwiki.apache.org/confluence/x/B5cWAw
>
>
> ---
> If your project is set up for it, you can reply to this email and have your
> reply appear on GitHub as well. If your project does not have this feature
> enabled and wishes so, or if the feature is enabled but not working, please
> contact infrastructure at infrastructure@apache.org or file a JIRA ticket
> with INFRA.
> ---
[GitHub] qpid-proton pull request: PROTON-334: SASL Implementation for Prot...
Posted by astitcher <gi...@git.apache.org>.
Github user astitcher commented on the pull request:
https://github.com/apache/qpid-proton/pull/17#issuecomment-91137151
See the wiki for more information and context:
https://cwiki.apache.org/confluence/x/B5cWAw
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] qpid-proton pull request: PROTON-334: SASL Implementation for Prot...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/qpid-proton/pull/17
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---