Todd Lipcon has posted comments on this change. Change subject: webserver: improve SSL certificate handling ......................................................................
Patch Set 2: (5 comments) http://gerrit.cloudera.org:8080/#/c/5015/2/src/kudu/security/test/test_certs.h File src/kudu/security/test/test_certs.h: Line 28: Status CreateTestSSLCerts(const std::string& dir, > Could you use instead or replace usage of CreateSSLServerCert/CreateSSLPriv This patch adds a TODO there -- the issue is that these certs use a password, but the RPC TLS implementation doesn't yet support using a password. I'll work on that next. http://gerrit.cloudera.org:8080/#/c/5015/2/src/kudu/server/webserver.cc File src/kudu/server/webserver.cc: Line 158: Status s = Subprocess::Call(argv, "" /* stdin */, &key_password, &stderr); > Adar and I traced through the other day and found that we never shelled out Yea, given it's close to startup I'd say it's safe. It's also the accepted way for people to tie in key/password management stuff in all of the other Hadoop ecosystem components, so we don't have much of a choice. (given this is close to startup I don't think it's worth preforking a separate "forker" process and communicating by a pipe or anything) Line 166: } else { > Is this else attached to the correct if? It would make more sense to me on Done PS2, Line 197: SimpleItoa > consider using std::to_string Done http://gerrit.cloudera.org:8080/#/c/5015/2/src/kudu/util/curl_util.cc File src/kudu/util/curl_util.cc: Line 72: RETURN_NOT_OK(TranslateError(curl_easy_setopt(curl_, CURLOPT_SSL_VERIFYPEER, 0))); > This might be simpler without the if block, and setting the value to veify_ Done -- To view, visit http://gerrit.cloudera.org:8080/5015 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4b508cebbe6f31556e6d5a5fba5e5e9fb44cf1b9 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: Dan Burkert <danburk...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes