Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/8247 )
Change subject: Allow configuration of values passed into kerberos env vars ...................................................................... Patch Set 2: (6 comments) http://gerrit.cloudera.org:8080/#/c/8247/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8247/1//COMMIT_MSG@7 PS1, Line 7: Allow configuration of values passed into kerberos env vars > typo Done http://gerrit.cloudera.org:8080/#/c/8247/1//COMMIT_MSG@15 PS1, Line 15: arguments to InitKerberosForServer(). > mind adding a sentence here saying _why_ you want to make them configurable Yup, done. http://gerrit.cloudera.org:8080/#/c/8247/1/src/kudu/security/init.h File src/kudu/security/init.h: http://gerrit.cloudera.org:8080/#/c/8247/1/src/kudu/security/init.h@41 PS1, Line 41: he kerberos replay cache by setting > I think using boost::optional<> is nicer here I reverted to just using the flag here now, so this is not necessary anymore. http://gerrit.cloudera.org:8080/#/c/8247/1/src/kudu/security/init.cc File src/kudu/security/init.cc: http://gerrit.cloudera.org:8080/#/c/8247/1/src/kudu/security/init.cc@470 PS1, Line 470: // per-connection server-generated nonce to protect against replay attacks > hrm, given that this is already configurable via a flag, why do we need it That's true, I just thought it would be a little weird if some are configurable via parameters and others and configured via flags. But I guess this is something we should fix in the "make the Messenger configurable" patch later. I've removed the 'krb5_ktname' parameter and just used the flag now. http://gerrit.cloudera.org:8080/#/c/8247/1/src/kudu/security/init.cc@467 PS1, Line 467: : if (disable_krb5_replay_cache) { : // KUDU-1897: disable the Kerberos replay cache. The KRPC protocol includes a : // per-connection server-generated nonce to protect against replay attacks : > maybe just scratch the defaults and instead change the call site in server_ I removed the krb5_ktname param, so we don't have this problem anymore. I left the other 2 as defaults. http://gerrit.cloudera.org:8080/#/c/8247/1/src/kudu/security/init.cc@473 PS1, Line 473: setenv("KRB5RCACHETYPE", "none", 1); > why do you need to configure this? We found that the replay cache was a hug Yes, it's because the Thrift part would lack replay protection. I looked around a bit and couldn't find a way to selectively disable it, since we're only able to set it via an environment variable. -- To view, visit http://gerrit.cloudera.org:8080/8247 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iab4ce72c04ec4056dc89fb4c1c540a6fdaca4404 Gerrit-Change-Number: 8247 Gerrit-PatchSet: 2 Gerrit-Owner: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Fri, 13 Oct 2017 06:41:56 +0000 Gerrit-HasComments: Yes
