You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Dan Burkert (Code Review)" <ge...@cloudera.org> on 2017/03/10 00:59:26 UTC

[kudu-CR] KUDU-1923: rpc encryption flag is not enforced 

Hello Todd Lipcon, Alexey Serbin,

I'd like you to do a code review.  Please visit

    http://gerrit.cloudera.org:8080/6340

to review the following change.

Change subject: KUDU-1923: rpc_encryption flag is not enforced
......................................................................

KUDU-1923: rpc_encryption flag is not enforced

The rpc_encryption flag wasn't being taken into account during
negotiation, so TLS encryption was in effect optional regardless of
configuration.

Change-Id: Ifdf17c6bb59ca4af1215376c5221a90bd1d93b64
---
M src/kudu/rpc/client_negotiation.cc
M src/kudu/rpc/client_negotiation.h
M src/kudu/rpc/constants.cc
M src/kudu/rpc/messenger.h
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/negotiation.h
M src/kudu/rpc/reactor.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
10 files changed, 159 insertions(+), 31 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/6340/1
-- 
To view, visit http://gerrit.cloudera.org:8080/6340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifdf17c6bb59ca4af1215376c5221a90bd1d93b64
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] KUDU-1923: rpc encryption flag is not enforced

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.

Change subject: KUDU-1923: rpc_encryption flag is not enforced
......................................................................


Patch Set 6: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/6340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifdf17c6bb59ca4af1215376c5221a90bd1d93b64
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@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

[kudu-CR] KUDU-1923: rpc encryption flag is not enforced

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.

Change subject: KUDU-1923: rpc_encryption flag is not enforced
......................................................................


Patch Set 4:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/6340/4/src/kudu/rpc/client_negotiation.h
File src/kudu/rpc/client_negotiation.h:

PS4, Line 61: explicit
nit: consider dropping 'explicit' -- it seems we don't bother about initializer-list assignments.


Line 217:   RpcEncryption encryption_;
Consider adding 'const' specifier.


http://gerrit.cloudera.org:8080/#/c/6340/4/src/kudu/rpc/server_negotiation.cc
File src/kudu/rpc/server_negotiation.cc:

PS4, Line 388: encryption_ != RpcEncryption::DISABLED &&
nit for here and elsewhere: I'm not sure whether compiler is capable of optimizing this on its own, but I would put the 'static' condition first, like

if (encryption_ != RpcEncryption::DISABLED && ...) {
   ...
}

because other criteria like ContainsKey() look more CPU consuming.


http://gerrit.cloudera.org:8080/#/c/6340/4/src/kudu/rpc/server_negotiation.h
File src/kudu/rpc/server_negotiation.h:

PS4, Line 58: explicit 
nit: consider dropping explicit since we don't bother with list-lie initializers, e.g. ServerNegotiation sn = { s, tls, verfier, enc };


Line 219:   RpcEncryption encryption_;
Consider adding const modifier -- it seems this member is not modified once the object is constructed.


-- 
To view, visit http://gerrit.cloudera.org:8080/6340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifdf17c6bb59ca4af1215376c5221a90bd1d93b64
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@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] KUDU-1923: rpc encryption flag is not enforced

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.

Change subject: KUDU-1923: rpc_encryption flag is not enforced
......................................................................


Patch Set 5:

> Not sure if Dan's working tonight so I'll address those nits on his
 > behalf (hoping to cut the rc tonight)

Thank you for taking care of this!

-- 
To view, visit http://gerrit.cloudera.org:8080/6340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifdf17c6bb59ca4af1215376c5221a90bd1d93b64
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@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

[kudu-CR] KUDU-1923: rpc encryption flag is not enforced

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

Change subject: KUDU-1923: rpc_encryption flag is not enforced
......................................................................


Patch Set 4:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/6340/4/src/kudu/rpc/client_negotiation.h
File src/kudu/rpc/client_negotiation.h:

PS4, Line 61: explicit
> nit: consider dropping 'explicit' -- it seems we don't bother about initial
Done


Line 217:   RpcEncryption encryption_;
> Consider adding 'const' specifier.
Done


http://gerrit.cloudera.org:8080/#/c/6340/4/src/kudu/rpc/server_negotiation.cc
File src/kudu/rpc/server_negotiation.cc:

PS4, Line 388: encryption_ != RpcEncryption::DISABLED &&
> nit for here and elsewhere: I'm not sure whether compiler is capable of opt
Done


http://gerrit.cloudera.org:8080/#/c/6340/4/src/kudu/rpc/server_negotiation.h
File src/kudu/rpc/server_negotiation.h:

PS4, Line 58: explicit 
> nit: consider dropping explicit since we don't bother with list-lie initial
Done


Line 219:   RpcEncryption encryption_;
> Consider adding const modifier -- it seems this member is not modified once
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/6340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifdf17c6bb59ca4af1215376c5221a90bd1d93b64
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@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] KUDU-1923: rpc encryption flag is not enforced

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

Change subject: KUDU-1923: rpc_encryption flag is not enforced
......................................................................


Patch Set 6: Code-Review+2

Carrying my own and Alexey's +2

-- 
To view, visit http://gerrit.cloudera.org:8080/6340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifdf17c6bb59ca4af1215376c5221a90bd1d93b64
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@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

[kudu-CR] KUDU-1923: rpc encryption flag is not enforced

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

Change subject: KUDU-1923: rpc_encryption flag is not enforced
......................................................................


Patch Set 4:

Not sure if Dan's working tonight so I'll address those nits on his behalf (hoping to cut the rc tonight)

-- 
To view, visit http://gerrit.cloudera.org:8080/6340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifdf17c6bb59ca4af1215376c5221a90bd1d93b64
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@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

[kudu-CR] KUDU-1923: rpc encryption flag is not enforced

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

Change subject: KUDU-1923: rpc_encryption flag is not enforced
......................................................................


Patch Set 2:

(2 comments)

I think it's worth a spot check integration test in security-itest that we can run a cluster with encryption disabled on the server side, just to make sure the whole thing works end to end. In particular I'm wondering what's gonna happen when the tservers generate certs and then have certs available but encryption disabled, and similar questions with tokens.

http://gerrit.cloudera.org:8080/#/c/6340/2/src/kudu/rpc/server_negotiation.cc
File src/kudu/rpc/server_negotiation.cc:

PS2, Line 144:     CHECK(tls_context_->has_cert());
is this a sure thing? during startup isn't it possible that we accept some RPC connections before adding a cert to tls context, or did I remember wrong? mayben given how late this is coming into the release we should just include this in the if() statement as before


Line 398:     CHECK(encryption_ != RpcEncryption::DISABLED);
seems redundant with the above


-- 
To view, visit http://gerrit.cloudera.org:8080/6340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifdf17c6bb59ca4af1215376c5221a90bd1d93b64
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1923: rpc encryption flag is not enforced

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/6340

to look at the new patch set (#6).

Change subject: KUDU-1923: rpc_encryption flag is not enforced
......................................................................

KUDU-1923: rpc_encryption flag is not enforced

The rpc_encryption flag wasn't being taken into account during
negotiation, so TLS encryption was in effect optional regardless of
configuration.

Change-Id: Ifdf17c6bb59ca4af1215376c5221a90bd1d93b64
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/rpc/client_negotiation.cc
M src/kudu/rpc/client_negotiation.h
M src/kudu/rpc/constants.cc
M src/kudu/rpc/messenger.h
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/negotiation.h
M src/kudu/rpc/reactor.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
11 files changed, 177 insertions(+), 37 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/6340/6
-- 
To view, visit http://gerrit.cloudera.org:8080/6340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifdf17c6bb59ca4af1215376c5221a90bd1d93b64
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@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] KUDU-1923: rpc encryption flag is not enforced

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/6340

to look at the new patch set (#2).

Change subject: KUDU-1923: rpc_encryption flag is not enforced
......................................................................

KUDU-1923: rpc_encryption flag is not enforced

The rpc_encryption flag wasn't being taken into account during
negotiation, so TLS encryption was in effect optional regardless of
configuration.

Change-Id: Ifdf17c6bb59ca4af1215376c5221a90bd1d93b64
---
M src/kudu/rpc/client_negotiation.cc
M src/kudu/rpc/client_negotiation.h
M src/kudu/rpc/constants.cc
M src/kudu/rpc/messenger.h
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/negotiation.h
M src/kudu/rpc/reactor.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
10 files changed, 162 insertions(+), 32 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/6340/2
-- 
To view, visit http://gerrit.cloudera.org:8080/6340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifdf17c6bb59ca4af1215376c5221a90bd1d93b64
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] KUDU-1923: rpc encryption flag is not enforced

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.

Change subject: KUDU-1923: rpc_encryption flag is not enforced
......................................................................


Patch Set 3:

(2 comments)

added security-itest which disables encryption and authentication.  Disabling encryption requires disabling authentication, which is why it's in the same test.

http://gerrit.cloudera.org:8080/#/c/6340/2/src/kudu/rpc/server_negotiation.cc
File src/kudu/rpc/server_negotiation.cc:

PS2, Line 144:       tls_context_->has_cert() &&
> is this a sure thing? during startup isn't it possible that we accept some 
Done


Line 398:              tls_context_->has_signed_cert()) {
> seems redundant with the above
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/6340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifdf17c6bb59ca4af1215376c5221a90bd1d93b64
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@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] KUDU-1923: rpc encryption flag is not enforced

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

Change subject: KUDU-1923: rpc_encryption flag is not enforced
......................................................................


Patch Set 5:

Needed a rebase

-- 
To view, visit http://gerrit.cloudera.org:8080/6340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifdf17c6bb59ca4af1215376c5221a90bd1d93b64
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@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

[kudu-CR] KUDU-1923: rpc encryption flag is not enforced

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.

Change subject: KUDU-1923: rpc_encryption flag is not enforced
......................................................................


Patch Set 5: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/6340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifdf17c6bb59ca4af1215376c5221a90bd1d93b64
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@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

[kudu-CR] KUDU-1923: rpc encryption flag is not enforced

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged.

Change subject: KUDU-1923: rpc_encryption flag is not enforced
......................................................................


KUDU-1923: rpc_encryption flag is not enforced

The rpc_encryption flag wasn't being taken into account during
negotiation, so TLS encryption was in effect optional regardless of
configuration.

Change-Id: Ifdf17c6bb59ca4af1215376c5221a90bd1d93b64
Reviewed-on: http://gerrit.cloudera.org:8080/6340
Reviewed-by: Todd Lipcon <to...@apache.org>
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Tested-by: Kudu Jenkins
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/rpc/client_negotiation.cc
M src/kudu/rpc/client_negotiation.h
M src/kudu/rpc/constants.cc
M src/kudu/rpc/messenger.h
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/negotiation.h
M src/kudu/rpc/reactor.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
11 files changed, 177 insertions(+), 37 deletions(-)

Approvals:
  Todd Lipcon: Looks good to me, approved
  Alexey Serbin: Looks good to me, approved
  Kudu Jenkins: Verified



-- 
To view, visit http://gerrit.cloudera.org:8080/6340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ifdf17c6bb59ca4af1215376c5221a90bd1d93b64
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@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] KUDU-1923: rpc encryption flag is not enforced

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.

Change subject: KUDU-1923: rpc_encryption flag is not enforced
......................................................................


Patch Set 4:

I think so.  I'm having trouble testing locally because of OS X issues.

-- 
To view, visit http://gerrit.cloudera.org:8080/6340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifdf17c6bb59ca4af1215376c5221a90bd1d93b64
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@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

[kudu-CR] KUDU-1923: rpc encryption flag is not enforced

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

Change subject: KUDU-1923: rpc_encryption flag is not enforced
......................................................................


Patch Set 3:

(1 comment)

seems like a bunch of real test failures

http://gerrit.cloudera.org:8080/#/c/6340/3/src/kudu/rpc/server_negotiation.cc
File src/kudu/rpc/server_negotiation.cc:

Line 416:   if (encryption_ != RpcEncryption::DISABLED) {
should be tls_context_->has_cert() && ...?


-- 
To view, visit http://gerrit.cloudera.org:8080/6340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifdf17c6bb59ca4af1215376c5221a90bd1d93b64
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@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] KUDU-1923: rpc encryption flag is not enforced

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/6340

to look at the new patch set (#4).

Change subject: KUDU-1923: rpc_encryption flag is not enforced
......................................................................

KUDU-1923: rpc_encryption flag is not enforced

The rpc_encryption flag wasn't being taken into account during
negotiation, so TLS encryption was in effect optional regardless of
configuration.

Change-Id: Ifdf17c6bb59ca4af1215376c5221a90bd1d93b64
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/rpc/client_negotiation.cc
M src/kudu/rpc/client_negotiation.h
M src/kudu/rpc/constants.cc
M src/kudu/rpc/messenger.h
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/negotiation.h
M src/kudu/rpc/reactor.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
11 files changed, 172 insertions(+), 32 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/6340/4
-- 
To view, visit http://gerrit.cloudera.org:8080/6340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifdf17c6bb59ca4af1215376c5221a90bd1d93b64
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@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] KUDU-1923: rpc encryption flag is not enforced

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/6340

to look at the new patch set (#3).

Change subject: KUDU-1923: rpc_encryption flag is not enforced
......................................................................

KUDU-1923: rpc_encryption flag is not enforced

The rpc_encryption flag wasn't being taken into account during
negotiation, so TLS encryption was in effect optional regardless of
configuration.

Change-Id: Ifdf17c6bb59ca4af1215376c5221a90bd1d93b64
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/rpc/client_negotiation.cc
M src/kudu/rpc/client_negotiation.h
M src/kudu/rpc/constants.cc
M src/kudu/rpc/messenger.h
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/negotiation.h
M src/kudu/rpc/reactor.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
11 files changed, 172 insertions(+), 32 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/6340/3
-- 
To view, visit http://gerrit.cloudera.org:8080/6340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifdf17c6bb59ca4af1215376c5221a90bd1d93b64
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] KUDU-1923: rpc encryption flag is not enforced

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/6340

to look at the new patch set (#5).

Change subject: KUDU-1923: rpc_encryption flag is not enforced
......................................................................

KUDU-1923: rpc_encryption flag is not enforced

The rpc_encryption flag wasn't being taken into account during
negotiation, so TLS encryption was in effect optional regardless of
configuration.

Change-Id: Ifdf17c6bb59ca4af1215376c5221a90bd1d93b64
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/rpc/client_negotiation.cc
M src/kudu/rpc/client_negotiation.h
M src/kudu/rpc/constants.cc
M src/kudu/rpc/messenger.h
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/negotiation.h
M src/kudu/rpc/reactor.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
11 files changed, 177 insertions(+), 37 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/6340/5
-- 
To view, visit http://gerrit.cloudera.org:8080/6340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifdf17c6bb59ca4af1215376c5221a90bd1d93b64
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@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] KUDU-1923: rpc encryption flag is not enforced

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

Change subject: KUDU-1923: rpc_encryption flag is not enforced
......................................................................


Patch Set 4: Code-Review+2

lgtm, was that the issue that caused the test failures?

-- 
To view, visit http://gerrit.cloudera.org:8080/6340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifdf17c6bb59ca4af1215376c5221a90bd1d93b64
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@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