Alexey Serbin has posted comments on this change. Change subject: [security] groundwork for cert signing service ......................................................................
Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/5671/4/src/kudu/security/ca/cert_management.cc File src/kudu/security/ca/cert_management.cc: PS4, Line 632: : : > per comment on the earlier revision, can you make this look less like a bug Good idea. I didn't find the proper constant in OpenSSL header files and forgot about this. http://gerrit.cloudera.org:8080/#/c/5671/1/src/kudu/security/crypto/cert_management.cc File src/kudu/security/crypto/cert_management.cc: Line 113: "BIO_get_mem_ptr() failed"); > I generally feel like if the only way you'd hit this is a memory allocation ok, this sounds reasonable -- thank you for the clarification. Will update accordingly. Line 315: "Error setting parameters for RSA key generation"); > See above -- yea, I think crashing on malloc failure is appropriate. Done Line 349: CERT_RET_IF_NULL(name, "Null subject name sub-structure in X509 CSR"); > yep (see above) Done http://gerrit.cloudera.org:8080/#/c/5671/1/src/kudu/security/crypto/cert_management.h File src/kudu/security/crypto/cert_management.h: PS1, Line 174: // The configuration and hostnames/ips are made separate parameters to : // work around limitations of old gcc 4.4.x compiler. Ideally, both hostnames : // and ips should have been fields of the Config structure. > hrm, you removed the comment, but now it doesn't really make sense why it's Oh! Thanks for pointing at this. It seems I removed the comment but I missed the crucial part on clarifying on the issue. Yes, it's fine if compiling with the clang++ from the devtoolset. IIRC, there was an issue while I was trying to do that C++11-style structure initialization if compiling with the default system compiler. Should be OK now -- I put those fields into the configuration structure. Let's see how it behaves on Jenkins build machines. -- To view, visit http://gerrit.cloudera.org:8080/5671 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic2ce55d38f4d06172fadaaa702f4550997d9bc8f Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Dan Burkert <danburk...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
