Alexey Serbin has posted comments on this change. Change subject: security: generate certs on the tserver, sign them on the master ......................................................................
Patch Set 2: (5 comments) http://gerrit.cloudera.org:8080/#/c/5766/2/src/kudu/master/master.cc File src/kudu/master/master.cc: Line 98: cert_manager_.reset(new MasterCertManager(fs_manager_->uuid())); Probably, this should be optional. I.e., if running Kudu with no security option or using external PKI, then cert_manager_ is not needed. http://gerrit.cloudera.org:8080/#/c/5766/2/src/kudu/master/master.proto File src/kudu/master/master.proto: Line 257: optional bytes csr_der = 5; Why not to depend on security_ca.proto and use X509CsrPB here instead? Line 284: optional bytes signed_cert_der = 7; Ditto for X509CertPB from security_ca.proto http://gerrit.cloudera.org:8080/#/c/5766/2/src/kudu/tserver/tablet_server.h File src/kudu/tserver/tablet_server.h: Line 108: std::unique_ptr<TSCertManager> cert_manager_; > Looks like this doesn't necessarily need to be wrapped in unique_ptr, any r Probably, in case of non-TLS case, it's just a wrapper around nil? http://gerrit.cloudera.org:8080/#/c/5766/2/src/kudu/tserver/ts_cert_manager.cc File src/kudu/tserver/ts_cert_manager.cc: Line 61: // TODO(aserbin): do these fields actually have to be set? ok, I'll take a look. Basically, I just need to make sure whether we are loading openssl.conf or not. I think we should not (need just to double-check). And if so, then only common name (uuid) is needed. Comment might make sense if we want to add some Kudu-specific information as well. And either hostnames or ips are necessary. -- To view, visit http://gerrit.cloudera.org:8080/5766 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3eb8ab4edc17e2fa1a54e0123a06dabc59a0489b Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
