Dan Burkert has posted comments on this change. Change subject: [security] security-flags ......................................................................
Patch Set 5: (5 comments) http://gerrit.cloudera.org:8080/#/c/6052/4/src/kudu/rpc/messenger.cc File src/kudu/rpc/messenger.cc: Line 62: > The new tri-state flags should be tagged as stable. Done PS4, Line 84: tions with > --rpc_certificate_file ? Done Line 164: auto cleanup = MakeScopedCleanup([&] () { > Would be nice to do this validation in a per-gflag validator. Better experi It does fail 'faster', but this still gets run synchronously as part of the server startup, I think. I originally implemented it as a gflags validator, but there is no way (that I can find) to supply an error message with the validator API, so the error message shown to the user is very generic and unhelpful. PS4, Line 198: uto* tls_context = new_msgr->mutable_tls_context(); : : if (!FLAGS_rpc_certificate_file.empty() && : !FLAGS_rpc_private_key_file.empty() && : !FLAGS_rpc_ca_certificate_file.empty()) { : > Likewise with this. same reasoning. http://gerrit.cloudera.org:8080/#/c/6052/4/src/kudu/rpc/negotiation.cc File src/kudu/rpc/negotiation.cc: Line 206: // TODO(PKI): this should be enabling PLAIN if authn < required, and GSSAPI if > shouldn't this be checking whether the authentication = 'required'? for 'en Yes I think you are correct. That being said, there are a bunch of places where this needs to be fixed up, so I'm going to add a TODO and punt for now. I'll come back soon and make more comprehensive fixes for this and related issues. -- To view, visit http://gerrit.cloudera.org:8080/6052 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iaa53348b8969e83d9f794e1e0553bdec12252d9a Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
