[kudu-CR] webserver: improve SSL certificate handling

["Todd Lipcon (Code Review)" <ge...@cloudera.org>]

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 <to...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes