You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Benjamin Hindman <be...@berkeley.edu> on 2013/09/24 08:11:56 UTC
Review Request 14310: Added SASL authentication support.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14310/
-----------------------------------------------------------
Review request for mesos, Ben Mahler and Vinod Kone.
Repository: mesos-git
Description
-------
Initial SASL based authentication support. I'll be adding more tests and cleaning up authentication error versus authentication failures. (I'll likely also be cleaning some stuff up as I deal with compilation issues with different libsasl2* versions.)
Diffs
-----
configure.ac 96ac7a81311e9f3765bd54e078368566a3cc33ca
include/mesos/mesos.proto 8f845cc9b7cd491622cb29a69f6909170510a664
src/Makefile.am 3eae9642b639e78f101e7fe580aa803beadbebda
src/messages/messages.proto 4d400c2d8f6ecb9f7cf8635b2a98341206bbb6c5
src/sasl/authenticatee.hpp PRE-CREATION
src/sasl/authenticator.hpp PRE-CREATION
src/sasl/auxprop.hpp PRE-CREATION
src/sasl/auxprop.cpp PRE-CREATION
src/tests/sasl_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/14310/diff/
Testing
-------
make check
Thanks,
Benjamin Hindman
Re: Review Request 14310: Added SASL authentication support.
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On Sept. 24, 2013, 8:45 p.m., Jan Schaumann wrote:
> > src/sasl/authenticatee.hpp, line 76
> > <https://reviews.apache.org/r/14310/diff/3/?file=356658#file356658line76>
> >
> > malloc may fail, should check if return was NULL
Thanks!
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14310/#review26365
-----------------------------------------------------------
On Sept. 24, 2013, 8:26 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14310/
> -----------------------------------------------------------
>
> (Updated Sept. 24, 2013, 8:26 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Initial SASL based authentication support. (I'll likely also be cleaning some stuff up as I deal with compilation issues with different libsasl2* versions.)
>
>
> Diffs
> -----
>
> configure.ac 96ac7a81311e9f3765bd54e078368566a3cc33ca
> include/mesos/mesos.proto 8f845cc9b7cd491622cb29a69f6909170510a664
> src/Makefile.am 3eae9642b639e78f101e7fe580aa803beadbebda
> src/messages/messages.proto 4d400c2d8f6ecb9f7cf8635b2a98341206bbb6c5
> src/sasl/authenticatee.hpp PRE-CREATION
> src/sasl/authenticator.hpp PRE-CREATION
> src/sasl/auxprop.hpp PRE-CREATION
> src/sasl/auxprop.cpp PRE-CREATION
> src/tests/sasl_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/14310/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 14310: Added SASL authentication support.
Posted by Jan Schaumann <js...@netmeister.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14310/#review26365
-----------------------------------------------------------
src/sasl/authenticatee.hpp
<https://reviews.apache.org/r/14310/#comment51469>
malloc may fail, should check if return was NULL
- Jan Schaumann
On Sept. 24, 2013, 8:26 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14310/
> -----------------------------------------------------------
>
> (Updated Sept. 24, 2013, 8:26 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Initial SASL based authentication support. (I'll likely also be cleaning some stuff up as I deal with compilation issues with different libsasl2* versions.)
>
>
> Diffs
> -----
>
> configure.ac 96ac7a81311e9f3765bd54e078368566a3cc33ca
> include/mesos/mesos.proto 8f845cc9b7cd491622cb29a69f6909170510a664
> src/Makefile.am 3eae9642b639e78f101e7fe580aa803beadbebda
> src/messages/messages.proto 4d400c2d8f6ecb9f7cf8635b2a98341206bbb6c5
> src/sasl/authenticatee.hpp PRE-CREATION
> src/sasl/authenticator.hpp PRE-CREATION
> src/sasl/auxprop.hpp PRE-CREATION
> src/sasl/auxprop.cpp PRE-CREATION
> src/tests/sasl_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/14310/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 14310: Added SASL authentication support.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14310/#review26366
-----------------------------------------------------------
src/messages/messages.proto
<https://reviews.apache.org/r/14310/#comment51470>
Required?
src/sasl/authenticatee.hpp
<https://reviews.apache.org/r/14310/#comment51471>
Set an 'initialized' flag so we don't block forever.
src/sasl/authenticatee.hpp
<https://reviews.apache.org/r/14310/#comment51472>
Make this (and the one above) a CHECK.
src/sasl/authenticator.hpp
<https://reviews.apache.org/r/14310/#comment51473>
Can kill this.
src/sasl/authenticator.hpp
<https://reviews.apache.org/r/14310/#comment51474>
Do we need to also handle 'pwcheck_method'?
- Benjamin Hindman
On Sept. 24, 2013, 8:26 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14310/
> -----------------------------------------------------------
>
> (Updated Sept. 24, 2013, 8:26 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Initial SASL based authentication support. (I'll likely also be cleaning some stuff up as I deal with compilation issues with different libsasl2* versions.)
>
>
> Diffs
> -----
>
> configure.ac 96ac7a81311e9f3765bd54e078368566a3cc33ca
> include/mesos/mesos.proto 8f845cc9b7cd491622cb29a69f6909170510a664
> src/Makefile.am 3eae9642b639e78f101e7fe580aa803beadbebda
> src/messages/messages.proto 4d400c2d8f6ecb9f7cf8635b2a98341206bbb6c5
> src/sasl/authenticatee.hpp PRE-CREATION
> src/sasl/authenticator.hpp PRE-CREATION
> src/sasl/auxprop.hpp PRE-CREATION
> src/sasl/auxprop.cpp PRE-CREATION
> src/tests/sasl_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/14310/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 14310: Added SASL authentication support.
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On Sept. 25, 2013, 7:09 a.m., Ben Mahler wrote:
> > src/sasl/authenticator.hpp, line 322
> > <https://reviews.apache.org/r/14310/diff/3/?file=356659#file356659line322>
> >
> > Why hard-code length when you could do:
> >
> > if (...) {
> > *result = "in-memory-auxprop";
> > } else if (...) {
> > *result = "CRAM-MD5";
> > }
> >
> > if (length != NULL) {
> > *length = strlen(*result);
> > }
> >
> > I couldn't find good documentation on this, can length be NULL? Should we add an else block to guard against unexpected option strings?
Cool thanks ... although, we don't want to set length unless we've set result. ;)
The documentation is in the header:
* len -- length of result (may be NULL)
> On Sept. 25, 2013, 7:09 a.m., Ben Mahler wrote:
> > src/sasl/authenticator.hpp, line 383
> > <https://reviews.apache.org/r/14310/diff/3/?file=356659#file356659line383>
> >
> > Is there a sasl constant for this?
> >
> > I see the following in sasl.h:
> >
> > #define SASL_AUX_PASSWORD "*userPassword" /* User Password (of authid) */
> >
> > The leading * is odd, and I see conversation around it. For example, they skip over it here:
> > http://marc.info/?l=cyrus-sasl&m=104072288229946&w=2
> >
> > Not sure how your
> >
> > I see you've used the newer SASL_AUX_PASSWORD_PROP below, should you use that constant here and skip the *?
Yup, s/userPassword/SASL_AUX_PASSWORD_PROP/.
> On Sept. 25, 2013, 7:09 a.m., Ben Mahler wrote:
> > src/sasl/auxprop.hpp, lines 68-73
> > <https://reviews.apache.org/r/14310/diff/3/?file=356660#file356660line68>
> >
> > What made you name the second argument as api?
> >
> > I see:
> >
> > /* default name for auxprop plug-in entry point is "sasl_auxprop_init"
> > * similar to sasl_server_plug_init model, except only returns one
> > * sasl_auxprop_plug_t structure;
> > */
> > typedef int sasl_auxprop_init_t(const sasl_utils_t *utils,
> > int max_version,
> > int *out_version,
> > sasl_auxprop_plug_t **plug,
> > const char *plugname);
> >
> > Referring to:
> >
> > /* plug-in entry point:
> > * utils -- utility callback functions
> > * plugname -- name of plug-in (may be NULL)
> > * max_version -- highest server plug version supported
> > * returns:
> > * out_version -- server plug-in version of result
> > * pluglist -- list of mechanism plug-ins
> > * plugcount -- number of mechanism plug-ins
> > * results:
> > * SASL_OK -- success
> > * SASL_NOMEM -- failure
> > * SASL_BADVERS -- max_version too small
> > * SASL_BADPARAM -- bad config string
> > * ...
> > */
> > typedef int sasl_server_plug_init_t(const sasl_utils_t *utils,
> > int max_version,
> > int *out_version,
> > sasl_server_plug_t **pluglist,
> > int *plugcount);
> >
> > I can see how max_version is effectively the api version, is that why you renamed it?
Yes.
> On Sept. 25, 2013, 7:09 a.m., Ben Mahler wrote:
> > src/sasl/auxprop.cpp, line 40
> > <https://reviews.apache.org/r/14310/diff/3/?file=356661#file356661line40>
> >
> > Sasl didn't make this const?!
Nope. :(
> On Sept. 25, 2013, 7:09 a.m., Ben Mahler wrote:
> > src/sasl/auxprop.cpp, line 59
> > <https://reviews.apache.org/r/14310/diff/3/?file=356661#file356661line59>
> >
> > You wanted me to remind you to explain this line.
Yup, thanks!
> On Sept. 25, 2013, 7:09 a.m., Ben Mahler wrote:
> > src/sasl/auxprop.cpp, lines 61-63
> > <https://reviews.apache.org/r/14310/diff/3/?file=356661#file356661line61>
> >
> > How about: CHECK(properties != NULL) << ...
How about: CHECK_NOTNULL(properties) << ...
;)
> On Sept. 25, 2013, 7:09 a.m., Ben Mahler wrote:
> > src/sasl/auxprop.cpp, lines 72-84
> > <https://reviews.apache.org/r/14310/diff/3/?file=356661#file356661line72>
> >
> > From your formatting here, it looks as though you wanted newlines?
> >
> > If not, then maybe we should add quotes around the values to make this clearer on a single line:
> >
> > e.g.
> > << " user: '" << user << "'"
I added quotes.
> On Sept. 25, 2013, 7:09 a.m., Ben Mahler wrote:
> > src/sasl/auxprop.cpp, line 88
> > <https://reviews.apache.org/r/14310/diff/3/?file=356661#file356661line88>
> >
> > It looks like you can keep property in the for loop scope:
> >
> > for (const propval* property = properties; property->name != NULL; ++property) {
> >
> > Did you avoid this because you wanted the for loop to fit on a single line?
Yup.
> On Sept. 25, 2013, 7:09 a.m., Ben Mahler wrote:
> > src/sasl/auxprop.cpp, lines 138-139
> > <https://reviews.apache.org/r/14310/diff/3/?file=356661#file356661line138>
> >
> > s/for subsequent/to the same name as previous/
Thanks!
> On Sept. 25, 2013, 7:09 a.m., Ben Mahler wrote:
> > src/sasl/auxprop.cpp, line 147
> > <https://reviews.apache.org/r/14310/diff/3/?file=356661#file356661line147>
> >
> > Did you mean -1?
> >
> > From the documentation:
> > * vallen -- length of value, if < 0 then strlen(value) will be used
Hmm, my header file says '<= 0'! Regardless, both say '< 0' so I'll switch to -1. Thanks!
> On Sept. 25, 2013, 7:09 a.m., Ben Mahler wrote:
> > src/sasl/authenticator.hpp, line 245
> > <https://reviews.apache.org/r/14310/diff/3/?file=356659#file356659line245>
> >
> > Can you add the sasl_errstring to the log message?
Done.
> On Sept. 25, 2013, 7:09 a.m., Ben Mahler wrote:
> > src/sasl/authenticator.hpp, lines 97-98
> > <https://reviews.apache.org/r/14310/diff/3/?file=356659#file356659line97>
> >
> > Append sasl_errstring as done elsewhere?
Done.
> On Sept. 25, 2013, 7:09 a.m., Ben Mahler wrote:
> > src/sasl/authenticatee.hpp, lines 259-261
> > <https://reviews.apache.org/r/14310/diff/3/?file=356658#file356658line259>
> >
> > Should we follow the SASL example here?
> >
> > We should not be sending when data/length is NULL/0 and SASL_OK is returned, looking at their sample client:
> >
> > https://github.com/winlibs/cyrus-sasl/blob/master/sample/sample-client.c#L818
I updated it to check for output != NULL and length > 0.
> On Sept. 25, 2013, 7:09 a.m., Ben Mahler wrote:
> > src/sasl/authenticatee.hpp, line 215
> > <https://reviews.apache.org/r/14310/diff/3/?file=356658#file356658line215>
> >
> > Reading the manpage for sasl_client_start, I only see SASL_CONTINUE as a posible return code. What made you check for SASL_OK?
The sample client.
> On Sept. 25, 2013, 7:09 a.m., Ben Mahler wrote:
> > src/sasl/authenticatee.hpp, line 59
> > <https://reviews.apache.org/r/14310/diff/3/?file=356658#file356658line59>
> >
> > Do you want to hide this in a namespace? Ditto for AuthenticatorProcess.
It's in the mesos::internal::sasl namespace which I think is okay for now.
> On Sept. 25, 2013, 7:09 a.m., Ben Mahler wrote:
> > src/sasl/authenticator.hpp, line 234
> > <https://reviews.apache.org/r/14310/diff/3/?file=356659#file356659line234>
> >
> > Exclamation warranted? ;)
> >
> > Ditto on these other log lines.
Downgraded my excitement.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14310/#review26367
-----------------------------------------------------------
On Sept. 24, 2013, 8:26 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14310/
> -----------------------------------------------------------
>
> (Updated Sept. 24, 2013, 8:26 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Initial SASL based authentication support. (I'll likely also be cleaning some stuff up as I deal with compilation issues with different libsasl2* versions.)
>
>
> Diffs
> -----
>
> configure.ac 96ac7a81311e9f3765bd54e078368566a3cc33ca
> include/mesos/mesos.proto 8f845cc9b7cd491622cb29a69f6909170510a664
> src/Makefile.am 3eae9642b639e78f101e7fe580aa803beadbebda
> src/messages/messages.proto 4d400c2d8f6ecb9f7cf8635b2a98341206bbb6c5
> src/sasl/authenticatee.hpp PRE-CREATION
> src/sasl/authenticator.hpp PRE-CREATION
> src/sasl/auxprop.hpp PRE-CREATION
> src/sasl/auxprop.cpp PRE-CREATION
> src/tests/sasl_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/14310/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 14310: Added SASL authentication support.
Posted by Benjamin Hindman <be...@berkeley.edu>.
Indeed, how sad.
On Wed, Sep 25, 2013 at 3:48 PM, Ben Mahler <be...@gmail.com>wrote:
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14310/
>
> On September 25th, 2013, 7:09 a.m. UTC, *Ben Mahler* wrote:
>
> src/sasl/auxprop.cpp<https://reviews.apache.org/r/14310/diff/3/?file=356661#file356661line61> (Diff
> revision 3)
>
> 61
>
> if (properties == NULL) {
>
> 62
>
> LOG(FATAL) << "Invalid auxiliary properties requested for lookup";
>
> 63
>
> }
>
> How about: CHECK(properties != NULL) << ...
>
> On September 25th, 2013, 10:30 p.m. UTC, *Benjamin Hindman* wrote:
>
> How about: CHECK_NOTNULL(properties) << ...
>
> ;)
>
> CHECK_NOTNULL() does not expose a stream ;)
>
> "Since this macro returns the given pointer, this is very useful in constructor initializer lists.
>
> struct S {
> S(Something* ptr) : ptr_(CHECK_NOTNULL(ptr)) {}
> Something* ptr_;
> };
> Note that you cannot use this macro as a C++ stream due to this feature. Please use CHECK_EQ described above to log a custom message before aborting the application."
>
>
> - Ben
>
> On September 24th, 2013, 8:26 p.m. UTC, Benjamin Hindman wrote:
> Review request for mesos, Ben Mahler and Vinod Kone.
> By Benjamin Hindman.
>
> *Updated Sept. 24, 2013, 8:26 p.m.*
> *Repository: * mesos-git
> Description
>
> Initial SASL based authentication support. (I'll likely also be cleaning some stuff up as I deal with compilation issues with different libsasl2* versions.)
>
> Testing
>
> make check
>
> Diffs
>
> - configure.ac (96ac7a81311e9f3765bd54e078368566a3cc33ca)
> - include/mesos/mesos.proto (8f845cc9b7cd491622cb29a69f6909170510a664)
> - src/Makefile.am (3eae9642b639e78f101e7fe580aa803beadbebda)
> - src/messages/messages.proto
> (4d400c2d8f6ecb9f7cf8635b2a98341206bbb6c5)
> - src/sasl/authenticatee.hpp (PRE-CREATION)
> - src/sasl/authenticator.hpp (PRE-CREATION)
> - src/sasl/auxprop.hpp (PRE-CREATION)
> - src/sasl/auxprop.cpp (PRE-CREATION)
> - src/tests/sasl_tests.cpp (PRE-CREATION)
>
> View Diff <https://reviews.apache.org/r/14310/diff/>
>
Re: Review Request 14310: Added SASL authentication support.
Posted by Ben Mahler <be...@gmail.com>.
> On Sept. 25, 2013, 7:09 a.m., Ben Mahler wrote:
> > src/sasl/auxprop.cpp, lines 61-63
> > <https://reviews.apache.org/r/14310/diff/3/?file=356661#file356661line61>
> >
> > How about: CHECK(properties != NULL) << ...
>
> Benjamin Hindman wrote:
> How about: CHECK_NOTNULL(properties) << ...
>
> ;)
CHECK_NOTNULL() does not expose a stream ;)
"Since this macro returns the given pointer, this is very useful in constructor initializer lists.
struct S {
S(Something* ptr) : ptr_(CHECK_NOTNULL(ptr)) {}
Something* ptr_;
};
Note that you cannot use this macro as a C++ stream due to this feature. Please use CHECK_EQ described above to log a custom message before aborting the application."
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14310/#review26367
-----------------------------------------------------------
On Sept. 24, 2013, 8:26 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14310/
> -----------------------------------------------------------
>
> (Updated Sept. 24, 2013, 8:26 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Initial SASL based authentication support. (I'll likely also be cleaning some stuff up as I deal with compilation issues with different libsasl2* versions.)
>
>
> Diffs
> -----
>
> configure.ac 96ac7a81311e9f3765bd54e078368566a3cc33ca
> include/mesos/mesos.proto 8f845cc9b7cd491622cb29a69f6909170510a664
> src/Makefile.am 3eae9642b639e78f101e7fe580aa803beadbebda
> src/messages/messages.proto 4d400c2d8f6ecb9f7cf8635b2a98341206bbb6c5
> src/sasl/authenticatee.hpp PRE-CREATION
> src/sasl/authenticator.hpp PRE-CREATION
> src/sasl/auxprop.hpp PRE-CREATION
> src/sasl/auxprop.cpp PRE-CREATION
> src/tests/sasl_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/14310/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 14310: Added SASL authentication support.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14310/#review26367
-----------------------------------------------------------
Ship it!
src/sasl/authenticatee.hpp
<https://reviews.apache.org/r/14310/#comment51480>
Do you want to hide this in a namespace? Ditto for AuthenticatorProcess.
src/sasl/authenticatee.hpp
<https://reviews.apache.org/r/14310/#comment51482>
Line this up with the comment on the previous line.
src/sasl/authenticatee.hpp
<https://reviews.apache.org/r/14310/#comment51484>
s/THe/The/
src/sasl/authenticatee.hpp
<https://reviews.apache.org/r/14310/#comment51483>
s/choosen/chosen
src/sasl/authenticatee.hpp
<https://reviews.apache.org/r/14310/#comment51485>
Reading the manpage for sasl_client_start, I only see SASL_CONTINUE as a posible return code. What made you check for SASL_OK?
src/sasl/authenticatee.hpp
<https://reviews.apache.org/r/14310/#comment51495>
Should we follow the SASL example here?
We should not be sending when data/length is NULL/0 and SASL_OK is returned, looking at their sample client:
https://github.com/winlibs/cyrus-sasl/blob/master/sample/sample-client.c#L818
src/sasl/authenticatee.hpp
<https://reviews.apache.org/r/14310/#comment51487>
This change mixes casts and static casts, stick to one?
src/sasl/authenticator.hpp
<https://reviews.apache.org/r/14310/#comment51499>
Append sasl_errstring as done elsewhere?
src/sasl/authenticator.hpp
<https://reviews.apache.org/r/14310/#comment51500>
Would be nice to comment on what NULL means for these as you've done above, ditto for the other calls if it's succinct :)
src/sasl/authenticator.hpp
<https://reviews.apache.org/r/14310/#comment51501>
What do you mean by this comment?
src/sasl/authenticator.hpp
<https://reviews.apache.org/r/14310/#comment51502>
Exclamation warranted? ;)
Ditto on these other log lines.
src/sasl/authenticator.hpp
<https://reviews.apache.org/r/14310/#comment51503>
Can you add the sasl_errstring to the log message?
src/sasl/authenticator.hpp
<https://reviews.apache.org/r/14310/#comment51504>
Seems the sasl_errstring would also be useful in this log message?
src/sasl/authenticator.hpp
<https://reviews.apache.org/r/14310/#comment51505>
Ditto on including sasl_errstring on these last two log messages.
src/sasl/authenticator.hpp
<https://reviews.apache.org/r/14310/#comment51506>
Why hard-code length when you could do:
if (...) {
*result = "in-memory-auxprop";
} else if (...) {
*result = "CRAM-MD5";
}
if (length != NULL) {
*length = strlen(*result);
}
I couldn't find good documentation on this, can length be NULL? Should we add an else block to guard against unexpected option strings?
src/sasl/authenticator.hpp
<https://reviews.apache.org/r/14310/#comment51507>
Is there a sasl constant for this?
I see the following in sasl.h:
#define SASL_AUX_PASSWORD "*userPassword" /* User Password (of authid) */
The leading * is odd, and I see conversation around it. For example, they skip over it here:
http://marc.info/?l=cyrus-sasl&m=104072288229946&w=2
Not sure how your
I see you've used the newer SASL_AUX_PASSWORD_PROP below, should you use that constant here and skip the *?
src/sasl/auxprop.hpp
<https://reviews.apache.org/r/14310/#comment51508>
What made you name the second argument as api?
I see:
/* default name for auxprop plug-in entry point is "sasl_auxprop_init"
* similar to sasl_server_plug_init model, except only returns one
* sasl_auxprop_plug_t structure;
*/
typedef int sasl_auxprop_init_t(const sasl_utils_t *utils,
int max_version,
int *out_version,
sasl_auxprop_plug_t **plug,
const char *plugname);
Referring to:
/* plug-in entry point:
* utils -- utility callback functions
* plugname -- name of plug-in (may be NULL)
* max_version -- highest server plug version supported
* returns:
* out_version -- server plug-in version of result
* pluglist -- list of mechanism plug-ins
* plugcount -- number of mechanism plug-ins
* results:
* SASL_OK -- success
* SASL_NOMEM -- failure
* SASL_BADVERS -- max_version too small
* SASL_BADPARAM -- bad config string
* ...
*/
typedef int sasl_server_plug_init_t(const sasl_utils_t *utils,
int max_version,
int *out_version,
sasl_server_plug_t **pluglist,
int *plugcount);
I can see how max_version is effectively the api version, is that why you renamed it?
src/sasl/auxprop.cpp
<https://reviews.apache.org/r/14310/#comment51509>
Sasl didn't make this const?!
src/sasl/auxprop.cpp
<https://reviews.apache.org/r/14310/#comment51479>
You wanted me to remind you to explain this line.
src/sasl/auxprop.cpp
<https://reviews.apache.org/r/14310/#comment51510>
How about: CHECK(properties != NULL) << ...
src/sasl/auxprop.cpp
<https://reviews.apache.org/r/14310/#comment51511>
From your formatting here, it looks as though you wanted newlines?
If not, then maybe we should add quotes around the values to make this clearer on a single line:
e.g.
<< " user: '" << user << "'"
src/sasl/auxprop.cpp
<https://reviews.apache.org/r/14310/#comment51512>
It looks like you can keep property in the for loop scope:
for (const propval* property = properties; property->name != NULL; ++property) {
Did you avoid this because you wanted the for loop to fit on a single line?
src/sasl/auxprop.cpp
<https://reviews.apache.org/r/14310/#comment51514>
s/for subsequent/to the same name as previous/
src/sasl/auxprop.cpp
<https://reviews.apache.org/r/14310/#comment51515>
Did you mean -1?
From the documentation:
* vallen -- length of value, if < 0 then strlen(value) will be used
- Ben Mahler
On Sept. 24, 2013, 8:26 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14310/
> -----------------------------------------------------------
>
> (Updated Sept. 24, 2013, 8:26 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Initial SASL based authentication support. (I'll likely also be cleaning some stuff up as I deal with compilation issues with different libsasl2* versions.)
>
>
> Diffs
> -----
>
> configure.ac 96ac7a81311e9f3765bd54e078368566a3cc33ca
> include/mesos/mesos.proto 8f845cc9b7cd491622cb29a69f6909170510a664
> src/Makefile.am 3eae9642b639e78f101e7fe580aa803beadbebda
> src/messages/messages.proto 4d400c2d8f6ecb9f7cf8635b2a98341206bbb6c5
> src/sasl/authenticatee.hpp PRE-CREATION
> src/sasl/authenticator.hpp PRE-CREATION
> src/sasl/auxprop.hpp PRE-CREATION
> src/sasl/auxprop.cpp PRE-CREATION
> src/tests/sasl_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/14310/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 14310: Added SASL authentication support.
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On Sept. 25, 2013, 5:32 p.m., Vinod Kone wrote:
> > configure.ac, line 516
> > <https://reviews.apache.org/r/14310/diff/3/?file=356654#file356654line516>
> >
> > I thought we needed couple other packages too. Can we check for them here?
I've added a TODO because properly checking is considerably more involved.
> On Sept. 25, 2013, 5:32 p.m., Vinod Kone wrote:
> > src/messages/messages.proto, line 344
> > <https://reviews.apache.org/r/14310/diff/3/?file=356657#file356657line344>
> >
> > Should the mechanisms be enums?
I think strings are okay. We have enough string manipulation techniques that makes this easy.
> On Sept. 25, 2013, 5:32 p.m., Vinod Kone wrote:
> > src/sasl/authenticatee.hpp, line 81
> > <https://reviews.apache.org/r/14310/diff/3/?file=356658#file356658line81>
> >
> > Why virtual?
http://stackoverflow.com/questions/461203/when-to-use-virtual-destructors
> On Sept. 25, 2013, 5:32 p.m., Vinod Kone wrote:
> > src/sasl/authenticatee.hpp, line 284
> > <https://reviews.apache.org/r/14310/diff/3/?file=356658#file356658line284>
> >
> > Can you add a comment here on why you set it to false instead of an error?
I put a comment on Authenticatee::authenticate instead!
> On Sept. 25, 2013, 5:32 p.m., Vinod Kone wrote:
> > src/sasl/authenticatee.hpp, line 52
> > <https://reviews.apache.org/r/14310/diff/3/?file=356658#file356658line52>
> >
> > Since want users to not re-use Authenticatee for multiple attempts, how about taking the 'pid' as part of constructor argument? That probably drives the point home better?
I thought about this a bit but I don't really like the idea of keeping the pid around after the first authenticate call because subsequent calls should not "by accident" use it but use 'from' instead.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14310/#review26379
-----------------------------------------------------------
On Sept. 24, 2013, 8:26 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14310/
> -----------------------------------------------------------
>
> (Updated Sept. 24, 2013, 8:26 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Initial SASL based authentication support. (I'll likely also be cleaning some stuff up as I deal with compilation issues with different libsasl2* versions.)
>
>
> Diffs
> -----
>
> configure.ac 96ac7a81311e9f3765bd54e078368566a3cc33ca
> include/mesos/mesos.proto 8f845cc9b7cd491622cb29a69f6909170510a664
> src/Makefile.am 3eae9642b639e78f101e7fe580aa803beadbebda
> src/messages/messages.proto 4d400c2d8f6ecb9f7cf8635b2a98341206bbb6c5
> src/sasl/authenticatee.hpp PRE-CREATION
> src/sasl/authenticator.hpp PRE-CREATION
> src/sasl/auxprop.hpp PRE-CREATION
> src/sasl/auxprop.cpp PRE-CREATION
> src/tests/sasl_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/14310/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 14310: Added SASL authentication support.
Posted by Benjamin Mahler <be...@gmail.com>.
Good catch: https://issues.apache.org/jira/browse/INFRA-6796
On Wed, Sep 25, 2013 at 5:31 PM, Vinod Kone <vi...@gmail.com> wrote:
> gotcha!
>
> on a side note, looks like our RB emails are still going to
> mesos-dev@incubator :/
>
>
> On Wed, Sep 25, 2013 at 5:27 PM, Benjamin Hindman <be...@berkeley.edu>wrote:
>
>> If a function was declared virtual I try and define "overrides" of that
>> function as virtual as well. Since the parent of AuthenticateeProcess
>> declared it's destructor virtual I declared the destructor of
>> AuthenticateeProcess as virtual as well. Make sense?
>>
>>
>> On Wed, Sep 25, 2013 at 4:13 PM, Vinod Kone <vi...@gmail.com> wrote:
>>
>>> This is an automatically generated e-mail. To reply, visit:
>>> https://reviews.apache.org/r/14310/
>>>
>>> On September 25th, 2013, 5:32 p.m. UTC, *Vinod Kone* wrote:
>>>
>>> src/sasl/authenticatee.hpp<https://reviews.apache.org/r/14310/diff/3/?file=356658#file356658line81> (Diff
>>> revision 3)
>>>
>>> 81
>>>
>>> virtual ~AuthenticateeProcess()
>>>
>>> Why virtual?
>>>
>>> On September 25th, 2013, 11 p.m. UTC, *Benjamin Hindman* wrote:
>>>
>>> http://stackoverflow.com/questions/461203/when-to-use-virtual-destructors
>>>
>>> i meant "are we expecting this class to be subclassed?". Do we defensively declare all our destructors virtual in our code base?
>>>
>>>
>>> - Vinod
>>>
>>> On September 24th, 2013, 8:26 p.m. UTC, Benjamin Hindman wrote:
>>> Review request for mesos, Ben Mahler and Vinod Kone.
>>> By Benjamin Hindman.
>>>
>>> *Updated Sept. 24, 2013, 8:26 p.m.*
>>> *Repository: * mesos-git
>>> Description
>>>
>>> Initial SASL based authentication support. (I'll likely also be cleaning some stuff up as I deal with compilation issues with different libsasl2* versions.)
>>>
>>> Testing
>>>
>>> make check
>>>
>>> Diffs
>>>
>>> - configure.ac (96ac7a81311e9f3765bd54e078368566a3cc33ca)
>>> - include/mesos/mesos.proto
>>> (8f845cc9b7cd491622cb29a69f6909170510a664)
>>> - src/Makefile.am (3eae9642b639e78f101e7fe580aa803beadbebda)
>>> - src/messages/messages.proto
>>> (4d400c2d8f6ecb9f7cf8635b2a98341206bbb6c5)
>>> - src/sasl/authenticatee.hpp (PRE-CREATION)
>>> - src/sasl/authenticator.hpp (PRE-CREATION)
>>> - src/sasl/auxprop.hpp (PRE-CREATION)
>>> - src/sasl/auxprop.cpp (PRE-CREATION)
>>> - src/tests/sasl_tests.cpp (PRE-CREATION)
>>>
>>> View Diff <https://reviews.apache.org/r/14310/diff/>
>>>
>>
>>
>
Re: Review Request 14310: Added SASL authentication support.
Posted by Vinod Kone <vi...@gmail.com>.
gotcha!
on a side note, looks like our RB emails are still going to
mesos-dev@incubator :/
On Wed, Sep 25, 2013 at 5:27 PM, Benjamin Hindman <be...@berkeley.edu> wrote:
> If a function was declared virtual I try and define "overrides" of that
> function as virtual as well. Since the parent of AuthenticateeProcess
> declared it's destructor virtual I declared the destructor of
> AuthenticateeProcess as virtual as well. Make sense?
>
>
> On Wed, Sep 25, 2013 at 4:13 PM, Vinod Kone <vi...@gmail.com> wrote:
>
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/14310/
>>
>> On September 25th, 2013, 5:32 p.m. UTC, *Vinod Kone* wrote:
>>
>> src/sasl/authenticatee.hpp<https://reviews.apache.org/r/14310/diff/3/?file=356658#file356658line81> (Diff
>> revision 3)
>>
>> 81
>>
>> virtual ~AuthenticateeProcess()
>>
>> Why virtual?
>>
>> On September 25th, 2013, 11 p.m. UTC, *Benjamin Hindman* wrote:
>>
>> http://stackoverflow.com/questions/461203/when-to-use-virtual-destructors
>>
>> i meant "are we expecting this class to be subclassed?". Do we defensively declare all our destructors virtual in our code base?
>>
>>
>> - Vinod
>>
>> On September 24th, 2013, 8:26 p.m. UTC, Benjamin Hindman wrote:
>> Review request for mesos, Ben Mahler and Vinod Kone.
>> By Benjamin Hindman.
>>
>> *Updated Sept. 24, 2013, 8:26 p.m.*
>> *Repository: * mesos-git
>> Description
>>
>> Initial SASL based authentication support. (I'll likely also be cleaning some stuff up as I deal with compilation issues with different libsasl2* versions.)
>>
>> Testing
>>
>> make check
>>
>> Diffs
>>
>> - configure.ac (96ac7a81311e9f3765bd54e078368566a3cc33ca)
>> - include/mesos/mesos.proto (8f845cc9b7cd491622cb29a69f6909170510a664)
>> - src/Makefile.am (3eae9642b639e78f101e7fe580aa803beadbebda)
>> - src/messages/messages.proto
>> (4d400c2d8f6ecb9f7cf8635b2a98341206bbb6c5)
>> - src/sasl/authenticatee.hpp (PRE-CREATION)
>> - src/sasl/authenticator.hpp (PRE-CREATION)
>> - src/sasl/auxprop.hpp (PRE-CREATION)
>> - src/sasl/auxprop.cpp (PRE-CREATION)
>> - src/tests/sasl_tests.cpp (PRE-CREATION)
>>
>> View Diff <https://reviews.apache.org/r/14310/diff/>
>>
>
>
Re: Review Request 14310: Added SASL authentication support.
Posted by Benjamin Hindman <be...@berkeley.edu>.
If a function was declared virtual I try and define "overrides" of that
function as virtual as well. Since the parent of AuthenticateeProcess
declared it's destructor virtual I declared the destructor of
AuthenticateeProcess as virtual as well. Make sense?
On Wed, Sep 25, 2013 at 4:13 PM, Vinod Kone <vi...@gmail.com> wrote:
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14310/
>
> On September 25th, 2013, 5:32 p.m. UTC, *Vinod Kone* wrote:
>
> src/sasl/authenticatee.hpp<https://reviews.apache.org/r/14310/diff/3/?file=356658#file356658line81> (Diff
> revision 3)
>
> 81
>
> virtual ~AuthenticateeProcess()
>
> Why virtual?
>
> On September 25th, 2013, 11 p.m. UTC, *Benjamin Hindman* wrote:
>
> http://stackoverflow.com/questions/461203/when-to-use-virtual-destructors
>
> i meant "are we expecting this class to be subclassed?". Do we defensively declare all our destructors virtual in our code base?
>
>
> - Vinod
>
> On September 24th, 2013, 8:26 p.m. UTC, Benjamin Hindman wrote:
> Review request for mesos, Ben Mahler and Vinod Kone.
> By Benjamin Hindman.
>
> *Updated Sept. 24, 2013, 8:26 p.m.*
> *Repository: * mesos-git
> Description
>
> Initial SASL based authentication support. (I'll likely also be cleaning some stuff up as I deal with compilation issues with different libsasl2* versions.)
>
> Testing
>
> make check
>
> Diffs
>
> - configure.ac (96ac7a81311e9f3765bd54e078368566a3cc33ca)
> - include/mesos/mesos.proto (8f845cc9b7cd491622cb29a69f6909170510a664)
> - src/Makefile.am (3eae9642b639e78f101e7fe580aa803beadbebda)
> - src/messages/messages.proto
> (4d400c2d8f6ecb9f7cf8635b2a98341206bbb6c5)
> - src/sasl/authenticatee.hpp (PRE-CREATION)
> - src/sasl/authenticator.hpp (PRE-CREATION)
> - src/sasl/auxprop.hpp (PRE-CREATION)
> - src/sasl/auxprop.cpp (PRE-CREATION)
> - src/tests/sasl_tests.cpp (PRE-CREATION)
>
> View Diff <https://reviews.apache.org/r/14310/diff/>
>
Re: Review Request 14310: Added SASL authentication support.
Posted by Vinod Kone <vi...@gmail.com>.
> On Sept. 25, 2013, 5:32 p.m., Vinod Kone wrote:
> > src/sasl/authenticatee.hpp, line 81
> > <https://reviews.apache.org/r/14310/diff/3/?file=356658#file356658line81>
> >
> > Why virtual?
>
> Benjamin Hindman wrote:
> http://stackoverflow.com/questions/461203/when-to-use-virtual-destructors
i meant "are we expecting this class to be subclassed?". Do we defensively declare all our destructors virtual in our code base?
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14310/#review26379
-----------------------------------------------------------
On Sept. 24, 2013, 8:26 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14310/
> -----------------------------------------------------------
>
> (Updated Sept. 24, 2013, 8:26 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Initial SASL based authentication support. (I'll likely also be cleaning some stuff up as I deal with compilation issues with different libsasl2* versions.)
>
>
> Diffs
> -----
>
> configure.ac 96ac7a81311e9f3765bd54e078368566a3cc33ca
> include/mesos/mesos.proto 8f845cc9b7cd491622cb29a69f6909170510a664
> src/Makefile.am 3eae9642b639e78f101e7fe580aa803beadbebda
> src/messages/messages.proto 4d400c2d8f6ecb9f7cf8635b2a98341206bbb6c5
> src/sasl/authenticatee.hpp PRE-CREATION
> src/sasl/authenticator.hpp PRE-CREATION
> src/sasl/auxprop.hpp PRE-CREATION
> src/sasl/auxprop.cpp PRE-CREATION
> src/tests/sasl_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/14310/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 14310: Added SASL authentication support.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14310/#review26379
-----------------------------------------------------------
Ship it!
configure.ac
<https://reviews.apache.org/r/14310/#comment51523>
I thought we needed couple other packages too. Can we check for them here?
include/mesos/mesos.proto
<https://reviews.apache.org/r/14310/#comment51524>
s/AuthenticationInfo/Credential/
Also add a required principal field.
Also, if we are anticipating the framework to authenticate more than one principal, then we could perhaps do?
message Credentials {
repeated Credential;
}
src/messages/messages.proto
<https://reviews.apache.org/r/14310/#comment51525>
Should the mechanisms be enums?
src/sasl/authenticatee.hpp
<https://reviews.apache.org/r/14310/#comment51526>
Since want users to not re-use Authenticatee for multiple attempts, how about taking the 'pid' as part of constructor argument? That probably drives the point home better?
src/sasl/authenticatee.hpp
<https://reviews.apache.org/r/14310/#comment51527>
Why virtual?
src/sasl/authenticatee.hpp
<https://reviews.apache.org/r/14310/#comment51528>
s/client/server/
src/sasl/authenticatee.hpp
<https://reviews.apache.org/r/14310/#comment51529>
Can you add a comment here on why you set it to false instead of an error?
src/sasl/authenticator.hpp
<https://reviews.apache.org/r/14310/#comment51530>
Same blocking issue applies here as it did in Authenticatee?
src/sasl/authenticator.hpp
<https://reviews.apache.org/r/14310/#comment51531>
Is there a benefit for 'link'ing the Authenticator? I would propose we either do it for both Authenticatee and Authenticator or none. I think the semantics would be easier to reason about then.
- Vinod Kone
On Sept. 24, 2013, 8:26 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14310/
> -----------------------------------------------------------
>
> (Updated Sept. 24, 2013, 8:26 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Initial SASL based authentication support. (I'll likely also be cleaning some stuff up as I deal with compilation issues with different libsasl2* versions.)
>
>
> Diffs
> -----
>
> configure.ac 96ac7a81311e9f3765bd54e078368566a3cc33ca
> include/mesos/mesos.proto 8f845cc9b7cd491622cb29a69f6909170510a664
> src/Makefile.am 3eae9642b639e78f101e7fe580aa803beadbebda
> src/messages/messages.proto 4d400c2d8f6ecb9f7cf8635b2a98341206bbb6c5
> src/sasl/authenticatee.hpp PRE-CREATION
> src/sasl/authenticator.hpp PRE-CREATION
> src/sasl/auxprop.hpp PRE-CREATION
> src/sasl/auxprop.cpp PRE-CREATION
> src/tests/sasl_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/14310/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 14310: Added SASL authentication support.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14310/
-----------------------------------------------------------
(Updated Sept. 24, 2013, 8:26 p.m.)
Review request for mesos, Ben Mahler and Vinod Kone.
Changes
-------
Bug fix.
Repository: mesos-git
Description
-------
Initial SASL based authentication support. (I'll likely also be cleaning some stuff up as I deal with compilation issues with different libsasl2* versions.)
Diffs (updated)
-----
configure.ac 96ac7a81311e9f3765bd54e078368566a3cc33ca
include/mesos/mesos.proto 8f845cc9b7cd491622cb29a69f6909170510a664
src/Makefile.am 3eae9642b639e78f101e7fe580aa803beadbebda
src/messages/messages.proto 4d400c2d8f6ecb9f7cf8635b2a98341206bbb6c5
src/sasl/authenticatee.hpp PRE-CREATION
src/sasl/authenticator.hpp PRE-CREATION
src/sasl/auxprop.hpp PRE-CREATION
src/sasl/auxprop.cpp PRE-CREATION
src/tests/sasl_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/14310/diff/
Testing
-------
make check
Thanks,
Benjamin Hindman
Re: Review Request 14310: Added SASL authentication support.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14310/
-----------------------------------------------------------
(Updated Sept. 24, 2013, 6:24 p.m.)
Review request for mesos, Ben Mahler and Vinod Kone.
Changes
-------
Updated description after pushing the new diff.
Repository: mesos-git
Description (updated)
-------
Initial SASL based authentication support. (I'll likely also be cleaning some stuff up as I deal with compilation issues with different libsasl2* versions.)
Diffs
-----
configure.ac 96ac7a81311e9f3765bd54e078368566a3cc33ca
include/mesos/mesos.proto 8f845cc9b7cd491622cb29a69f6909170510a664
src/Makefile.am 3eae9642b639e78f101e7fe580aa803beadbebda
src/messages/messages.proto 4d400c2d8f6ecb9f7cf8635b2a98341206bbb6c5
src/sasl/authenticatee.hpp PRE-CREATION
src/sasl/authenticator.hpp PRE-CREATION
src/sasl/auxprop.hpp PRE-CREATION
src/sasl/auxprop.cpp PRE-CREATION
src/tests/sasl_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/14310/diff/
Testing
-------
make check
Thanks,
Benjamin Hindman
Re: Review Request 14310: Added SASL authentication support.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14310/
-----------------------------------------------------------
(Updated Sept. 24, 2013, 6:24 p.m.)
Review request for mesos, Ben Mahler and Vinod Kone.
Changes
-------
Differentiated between authentication error and authentication failure.
Repository: mesos-git
Description
-------
Initial SASL based authentication support. I'll be adding more tests and cleaning up authentication error versus authentication failures. (I'll likely also be cleaning some stuff up as I deal with compilation issues with different libsasl2* versions.)
Diffs (updated)
-----
configure.ac 96ac7a81311e9f3765bd54e078368566a3cc33ca
include/mesos/mesos.proto 8f845cc9b7cd491622cb29a69f6909170510a664
src/Makefile.am 3eae9642b639e78f101e7fe580aa803beadbebda
src/messages/messages.proto 4d400c2d8f6ecb9f7cf8635b2a98341206bbb6c5
src/sasl/authenticatee.hpp PRE-CREATION
src/sasl/authenticator.hpp PRE-CREATION
src/sasl/auxprop.hpp PRE-CREATION
src/sasl/auxprop.cpp PRE-CREATION
src/tests/sasl_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/14310/diff/
Testing
-------
make check
Thanks,
Benjamin Hindman