You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2017/02/01 01:07:07 UTC
[kudu-CR] Allow configuring TlsContext with key wrappers
Hello Dan Burkert, Alexey Serbin,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/5845
to review the following change.
Change subject: Allow configuring TlsContext with key wrappers
......................................................................
Allow configuring TlsContext with key wrappers
Change-Id: Idb82427aea5f1bd29bad7529f2d54638b90811e2
---
M src/kudu/rpc/messenger.cc
M src/kudu/security/cert.cc
M src/kudu/security/cert.h
M src/kudu/security/tls_context.cc
M src/kudu/security/tls_context.h
M src/kudu/security/tls_handshake-test.cc
6 files changed, 126 insertions(+), 38 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/45/5845/1
--
To view, visit http://gerrit.cloudera.org:8080/5845
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Idb82427aea5f1bd29bad7529f2d54638b90811e2
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
[kudu-CR] Allow configuring TlsContext with key wrappers
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: Allow configuring TlsContext with key wrappers
......................................................................
Patch Set 4:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/5845/4/src/kudu/security/tls_context.cc
File src/kudu/security/tls_context.cc:
PS4, Line 95: err == X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT
> In what case we would get a self-signed certificate here which is not in th
yea, if you add the self-signed cert before you mark yourself as a "trusted CA", then you hit this path.
In some other review, Dan convinced me that if you are just making a self-signed cert, you shouldn't really need to add yourself as a CA. For example, Test_ClientNone_ServerSelfSigned uses a self-signed cert without doing any "trusting", which I think is reasonable.
PS4, Line 101: cur_cert
> might it happen that it's nullptr?
good catch!
PS4, Line 112: ERR_clear_error();
> consider removing this here and elsewhere: if using OPENSSL_RET_NOT_OK() wr
I see your point, but also it's possible that some other library in our same thread might not be so well behaved (eg in the case of the client). So it's kind of nice to clear the errors before we do any SSL stuff.
Line 122:
> paranoic nit: consider adding has_cert_.Store(false) before calling pair of
I don't think that actually helps anything, because there is no locking here. What's the goal?
http://gerrit.cloudera.org:8080/#/c/5845/4/src/kudu/security/tls_context.h
File src/kudu/security/tls_context.h:
PS4, Line 67: oad the server certificate.
> Could you please document that the key and cert in those files should be PE
Done
http://gerrit.cloudera.org:8080/#/c/5845/4/src/kudu/security/tls_handshake-test.cc
File src/kudu/security/tls_handshake-test.cc:
PS4, Line 165: ASSERT_OK(client_tls_.LoadCertificateAndKey(cert_path_, key_path_));
: ASSERT_OK(client_tls_.LoadCertificateAuthority(cert_path_));
> Should this sequence be reversed? I.e., I would expect the cert authority
given that it's a self-signed cert, that's not necessary. The trusting is only necessary because this client is going to verify the _other_ cert, which happens to be using the same cert.
Perhaps these tests would be more realistic if we used two different certs for client/server as mentioned above - then I think the CA stuff would be clearer.
--
To view, visit http://gerrit.cloudera.org:8080/5845
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Idb82427aea5f1bd29bad7529f2d54638b90811e2
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] Allow configuring TlsContext with key wrappers
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/5845
to look at the new patch set (#4).
Change subject: Allow configuring TlsContext with key wrappers
......................................................................
Allow configuring TlsContext with key wrappers
Change-Id: Idb82427aea5f1bd29bad7529f2d54638b90811e2
---
M src/kudu/rpc/messenger.cc
M src/kudu/security/ca/cert_management-test.cc
M src/kudu/security/cert.cc
M src/kudu/security/cert.h
M src/kudu/security/tls_context.cc
M src/kudu/security/tls_context.h
M src/kudu/security/tls_handshake-test.cc
7 files changed, 137 insertions(+), 40 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/45/5845/4
--
To view, visit http://gerrit.cloudera.org:8080/5845
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idb82427aea5f1bd29bad7529f2d54638b90811e2
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] Allow configuring TlsContext with key wrappers
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: Allow configuring TlsContext with key wrappers
......................................................................
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/5845/3/src/kudu/security/cert.cc
File src/kudu/security/cert.cc:
PS3, Line 50: char* str = X509_NAME_oneline(name, nullptr, 0);
: string ret(str);
: free(str);
: return ret;
What if allocate some long buffer (4K) on the stack and then pass it into the X509_NAME_oneline() to avoid extra malloc/free pair?
http://gerrit.cloudera.org:8080/#/c/5845/3/src/kudu/security/tls_context.cc
File src/kudu/security/tls_context.cc:
PS3, Line 112: ERR_clear_error();
What is it for?
http://gerrit.cloudera.org:8080/#/c/5845/2/src/kudu/security/tls_context.h
File src/kudu/security/tls_context.h:
PS2, Line 46: the
nit: drop
--
To view, visit http://gerrit.cloudera.org:8080/5845
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Idb82427aea5f1bd29bad7529f2d54638b90811e2
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] Allow configuring TlsContext with key wrappers
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: Allow configuring TlsContext with key wrappers
......................................................................
Patch Set 6: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/5845
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Idb82427aea5f1bd29bad7529f2d54638b90811e2
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] Allow configuring TlsContext with key wrappers
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: Allow configuring TlsContext with key wrappers
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5845/1/src/kudu/security/tls_context.h
File src/kudu/security/tls_context.h:
Line 68: Status LoadCertificateAndKey(const std::string& certificate_path,
> Is the plan to eventually remove these? I can't think of a reason they wou
yea, I think it's worth considering removing them. I left them since messenger.cc is still calling them to handle the flag-based paths it can be provided, but we could just as well move the wrapper implementation there.
--
To view, visit http://gerrit.cloudera.org:8080/5845
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Idb82427aea5f1bd29bad7529f2d54638b90811e2
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] Allow configuring TlsContext with key wrappers
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: Allow configuring TlsContext with key wrappers
......................................................................
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/5845/3/src/kudu/security/cert.cc
File src/kudu/security/cert.cc:
PS3, Line 50: std::unique_ptr<char> str(X509_NAME_oneline(name, nullptr, 0));
: return str.get();
: }
:
> What if allocate some long buffer (4K) on the stack and then pass it into t
actually looking at the manpage I learned that this function is "strongly discouraged", so I"m going to switch to a new one altogether.
http://gerrit.cloudera.org:8080/#/c/5845/3/src/kudu/security/tls_context.cc
File src/kudu/security/tls_context.cc:
PS3, Line 112: ERR_clear_error();
> This clears the OpenSSL error queue. I'm not sure how necessary it is to c
yea, just left it there since it was there before.
http://gerrit.cloudera.org:8080/#/c/5845/2/src/kudu/security/tls_context.h
File src/kudu/security/tls_context.h:
PS2, Line 46: the
> nit: drop
Done
--
To view, visit http://gerrit.cloudera.org:8080/5845
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Idb82427aea5f1bd29bad7529f2d54638b90811e2
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] Allow configuring TlsContext with key wrappers
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: Allow configuring TlsContext with key wrappers
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5845/6/src/kudu/security/cert.h
File src/kudu/security/cert.h:
Line 36: std::string X509NameToString(X509_NAME* name);
I think this could be an internal (static or anon namespace), right?
--
To view, visit http://gerrit.cloudera.org:8080/5845
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Idb82427aea5f1bd29bad7529f2d54638b90811e2
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] Allow configuring TlsContext with key wrappers
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: Allow configuring TlsContext with key wrappers
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/5845
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Idb82427aea5f1bd29bad7529f2d54638b90811e2
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] Allow configuring TlsContext with key wrappers
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged.
Change subject: Allow configuring TlsContext with key wrappers
......................................................................
Allow configuring TlsContext with key wrappers
Change-Id: Idb82427aea5f1bd29bad7529f2d54638b90811e2
Reviewed-on: http://gerrit.cloudera.org:8080/5845
Tested-by: Todd Lipcon <to...@apache.org>
Reviewed-by: Dan Burkert <da...@apache.org>
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M src/kudu/rpc/messenger.cc
M src/kudu/rpc/negotiation-test.cc
M src/kudu/security/ca/cert_management-test.cc
M src/kudu/security/cert.cc
M src/kudu/security/cert.h
M src/kudu/security/tls_context.cc
M src/kudu/security/tls_context.h
M src/kudu/security/tls_handshake-test.cc
8 files changed, 144 insertions(+), 43 deletions(-)
Approvals:
Dan Burkert: Looks good to me, approved
Todd Lipcon: Verified
Alexey Serbin: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/5845
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Idb82427aea5f1bd29bad7529f2d54638b90811e2
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] Allow configuring TlsContext with key wrappers
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: Allow configuring TlsContext with key wrappers
......................................................................
Patch Set 3: Code-Review+1
(2 comments)
http://gerrit.cloudera.org:8080/#/c/5845/3/src/kudu/security/tls_context.cc
File src/kudu/security/tls_context.cc:
PS3, Line 112: ERR_clear_error();
> What is it for?
This clears the OpenSSL error queue. I'm not sure how necessary it is to call, since we should be doing a good job of always clearing the queue right after an error happens. I think it doesn't hurt to include it, though.
http://gerrit.cloudera.org:8080/#/c/5845/1/src/kudu/security/tls_context.h
File src/kudu/security/tls_context.h:
Line 68: Status LoadCertificateAndKey(const std::string& certificate_path,
Is the plan to eventually remove these? I can't think of a reason they would be used, except currently in tests.
--
To view, visit http://gerrit.cloudera.org:8080/5845
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Idb82427aea5f1bd29bad7529f2d54638b90811e2
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] Allow configuring TlsContext with key wrappers
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/5845
to look at the new patch set (#5).
Change subject: Allow configuring TlsContext with key wrappers
......................................................................
Allow configuring TlsContext with key wrappers
Change-Id: Idb82427aea5f1bd29bad7529f2d54638b90811e2
---
M src/kudu/rpc/messenger.cc
M src/kudu/security/ca/cert_management-test.cc
M src/kudu/security/cert.cc
M src/kudu/security/cert.h
M src/kudu/security/tls_context.cc
M src/kudu/security/tls_context.h
M src/kudu/security/tls_handshake-test.cc
7 files changed, 143 insertions(+), 41 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/45/5845/5
--
To view, visit http://gerrit.cloudera.org:8080/5845
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idb82427aea5f1bd29bad7529f2d54638b90811e2
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] Allow configuring TlsContext with key wrappers
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Alexey Serbin, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/5845
to look at the new patch set (#3).
Change subject: Allow configuring TlsContext with key wrappers
......................................................................
Allow configuring TlsContext with key wrappers
Change-Id: Idb82427aea5f1bd29bad7529f2d54638b90811e2
---
M src/kudu/rpc/messenger.cc
M src/kudu/security/cert.cc
M src/kudu/security/cert.h
M src/kudu/security/tls_context.cc
M src/kudu/security/tls_context.h
M src/kudu/security/tls_handshake-test.cc
6 files changed, 128 insertions(+), 38 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/45/5845/3
--
To view, visit http://gerrit.cloudera.org:8080/5845
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idb82427aea5f1bd29bad7529f2d54638b90811e2
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Allow configuring TlsContext with key wrappers
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: Allow configuring TlsContext with key wrappers
......................................................................
Patch Set 6: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5845/4/src/kudu/security/tls_context.cc
File src/kudu/security/tls_context.cc:
Line 122:
> I don't think that actually helps anything, because there is no locking her
I was thinking about calling this method on already established context with key and certificate set up. In that case, if an error happens after adding private key but before adding certificate, then has_cert_ is true, but the TlsContext instance would not be functional.
If that's not the use case at all, then of course it's not needed.
--
To view, visit http://gerrit.cloudera.org:8080/5845
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Idb82427aea5f1bd29bad7529f2d54638b90811e2
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] Allow configuring TlsContext with key wrappers
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: Allow configuring TlsContext with key wrappers
......................................................................
Patch Set 4:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/5845/4/src/kudu/security/tls_context.cc
File src/kudu/security/tls_context.cc:
PS4, Line 95: err == X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT
In what case we would get a self-signed certificate here which is not in the list of trusted certificates? I mean, does this error happen even if calling AddTrustedCertificate() for the self-signed cert?
PS4, Line 101: cur_cert
might it happen that it's nullptr?
PS4, Line 112: ERR_clear_error();
consider removing this here and elsewhere: if using OPENSSL_RET_NOT_OK() wrapper macro while calling OpenSSL functions, the stack of errors should not contain any left-over errors.
Line 122:
paranoic nit: consider adding has_cert_.Store(false) before calling pair of SSL_CTX_use_{PrivateKey, certificate}() methods
http://gerrit.cloudera.org:8080/#/c/5845/4/src/kudu/security/tls_context.h
File src/kudu/security/tls_context.h:
PS4, Line 67: oad the server certificate.
Could you please document that the key and cert in those files should be PEM-encoded?
http://gerrit.cloudera.org:8080/#/c/5845/4/src/kudu/security/tls_handshake-test.cc
File src/kudu/security/tls_handshake-test.cc:
PS4, Line 165: ASSERT_OK(client_tls_.LoadCertificateAndKey(cert_path_, key_path_));
: ASSERT_OK(client_tls_.LoadCertificateAuthority(cert_path_));
Should this sequence be reversed? I.e., I would expect the cert authority to loaded first, so the loaded certificate would be checked to be trusted by the CA.
--
To view, visit http://gerrit.cloudera.org:8080/5845
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Idb82427aea5f1bd29bad7529f2d54638b90811e2
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] Allow configuring TlsContext with key wrappers
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/5845
to look at the new patch set (#6).
Change subject: Allow configuring TlsContext with key wrappers
......................................................................
Allow configuring TlsContext with key wrappers
Change-Id: Idb82427aea5f1bd29bad7529f2d54638b90811e2
---
M src/kudu/rpc/messenger.cc
M src/kudu/rpc/negotiation-test.cc
M src/kudu/security/ca/cert_management-test.cc
M src/kudu/security/cert.cc
M src/kudu/security/cert.h
M src/kudu/security/tls_context.cc
M src/kudu/security/tls_context.h
M src/kudu/security/tls_handshake-test.cc
8 files changed, 144 insertions(+), 43 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/45/5845/6
--
To view, visit http://gerrit.cloudera.org:8080/5845
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idb82427aea5f1bd29bad7529f2d54638b90811e2
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] Allow configuring TlsContext with key wrappers
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: Allow configuring TlsContext with key wrappers
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5845/6/src/kudu/security/cert.h
File src/kudu/security/cert.h:
Line 36: std::string X509NameToString(X509_NAME* name);
> I think this could be an internal (static or anon namespace), right?
it's also used in tls_context.cc
--
To view, visit http://gerrit.cloudera.org:8080/5845
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Idb82427aea5f1bd29bad7529f2d54638b90811e2
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] Allow configuring TlsContext with key wrappers
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: Allow configuring TlsContext with key wrappers
......................................................................
Patch Set 6:
Failure was unrelated (some tsan hang - emailed the tsan mailing list about it)
--
To view, visit http://gerrit.cloudera.org:8080/5845
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Idb82427aea5f1bd29bad7529f2d54638b90811e2
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No