[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/8439 )

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..

IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

KRPC has some flags that turn on TLS. This patch sets those to enable
TLS communication.

Tests are added to rpc-mgr-test.

TODO: Kudu kerberos testing is disabled. Will re-enable as part of IMPALA-6448.

Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
Reviewed-on: http://gerrit.cloudera.org:8080/8439
Reviewed-by: Sailesh Mukil 
Tested-by: Impala Public Jenkins
---
M be/src/catalog/catalogd-main.cc
M be/src/rpc/authentication-test.cc
M be/src/rpc/rpc-mgr-test.cc
M be/src/rpc/rpc-mgr.cc
M be/src/rpc/rpc-mgr.h
M be/src/rpc/thrift-server.cc
M be/src/rpc/thrift-server.h
M be/src/runtime/exec-env.cc
M be/src/service/impala-server.cc
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestored-main.cc
M be/src/testutil/in-process-servers.cc
M be/src/util/openssl-util.cc
M be/src/util/openssl-util.h
15 files changed, 334 insertions(+), 73 deletions(-)

Approvals:
  Sailesh Mukil: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
Gerrit-Change-Number: 8439
Gerrit-PatchSet: 10
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8439 )

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..


Patch Set 9: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
Gerrit-Change-Number: 8439
Gerrit-PatchSet: 9
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 30 Jan 2018 23:23:50 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8439 )

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..


Patch Set 9:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1830/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
Gerrit-Change-Number: 8439
Gerrit-PatchSet: 9
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 30 Jan 2018 19:43:36 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Sailesh Mukil (Code Review)]

Sailesh Mukil has removed a vote on this change.

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..


Removed Verified-1 by Impala Public Jenkins (255)
--
To view, visit http://gerrit.cloudera.org:8080/8439
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
Gerrit-Change-Number: 8439
Gerrit-PatchSet: 9
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Sailesh Mukil (Code Review)]

Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8439 )

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..


Patch Set 9: Code-Review+2

Rebase, carry +2.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
Gerrit-Change-Number: 8439
Gerrit-PatchSet: 9
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 30 Jan 2018 05:58:31 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8439 )

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..


Patch Set 9:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1823/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
Gerrit-Change-Number: 8439
Gerrit-PatchSet: 9
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 30 Jan 2018 05:58:46 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Sailesh Mukil (Code Review)]

Hello Michael Ho, Dan Hecht, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..

IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

KRPC has some flags that turn on TLS. This patch sets those to enable
TLS communication.

Tests are added to rpc-mgr-test.

TODO: Kudu kerberos testing is disabled. Will re-enable as part of IMPALA-6448.

Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
---
M be/src/catalog/catalogd-main.cc
M be/src/rpc/authentication-test.cc
M be/src/rpc/rpc-mgr-test.cc
M be/src/rpc/rpc-mgr.cc
M be/src/rpc/rpc-mgr.h
M be/src/rpc/thrift-server.cc
M be/src/rpc/thrift-server.h
M be/src/runtime/exec-env.cc
M be/src/service/impala-server.cc
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestored-main.cc
M be/src/testutil/in-process-servers.cc
M be/src/util/openssl-util.cc
M be/src/util/openssl-util.h
15 files changed, 334 insertions(+), 73 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/39/8439/9
--
To view, visit http://gerrit.cloudera.org:8080/8439
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
Gerrit-Change-Number: 8439
Gerrit-PatchSet: 9
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8439 )

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..


Patch Set 8: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/1817/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
Gerrit-Change-Number: 8439
Gerrit-PatchSet: 8
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 30 Jan 2018 03:32:19 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8439 )

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..


Patch Set 8:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1817/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
Gerrit-Change-Number: 8439
Gerrit-PatchSet: 8
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 29 Jan 2018 23:54:40 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Sailesh Mukil (Code Review)]

Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8439 )

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..


Patch Set 8: Code-Review+2

Rebase, carry +2.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
Gerrit-Change-Number: 8439
Gerrit-PatchSet: 8
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 29 Jan 2018 23:54:24 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Sailesh Mukil (Code Review)]

Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8439 )

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..


Patch Set 7:

Disabled kerberized testing due to IMPALA-6448. Will re-enable after it's 
fixed. Will merge after IMPALA-6447 is fixed (it's currently breaking GVOs)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
Gerrit-Change-Number: 8439
Gerrit-PatchSet: 7
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 29 Jan 2018 19:46:04 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Sailesh Mukil (Code Review)]

Hello Michael Ho, Dan Hecht, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..

IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

KRPC has some flags that turn on TLS. This patch sets those to enable
TLS communication.

Tests are added to rpc-mgr-test.

TODO: Kudu kerberos testing is disabled. Will re-enable as part of IMPALA-6448.

Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
---
M be/src/catalog/catalogd-main.cc
M be/src/rpc/authentication-test.cc
M be/src/rpc/rpc-mgr-test.cc
M be/src/rpc/rpc-mgr.cc
M be/src/rpc/rpc-mgr.h
M be/src/rpc/thrift-server.cc
M be/src/rpc/thrift-server.h
M be/src/runtime/exec-env.cc
M be/src/service/impala-server.cc
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestored-main.cc
M be/src/testutil/in-process-servers.cc
M be/src/util/openssl-util.cc
M be/src/util/openssl-util.h
15 files changed, 332 insertions(+), 73 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/39/8439/7
--
To view, visit http://gerrit.cloudera.org:8080/8439
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
Gerrit-Change-Number: 8439
Gerrit-PatchSet: 7
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8439 )

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..


Patch Set 6: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/1809/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
Gerrit-Change-Number: 8439
Gerrit-PatchSet: 6
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Fri, 26 Jan 2018 23:20:52 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8439 )

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..


Patch Set 6:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1809/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
Gerrit-Change-Number: 8439
Gerrit-PatchSet: 6
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Fri, 26 Jan 2018 19:36:35 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Sailesh Mukil (Code Review)]

Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8439 )

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..


Patch Set 6: Code-Review+2

(2 comments)

Rebase. Carry +2.

Ran secure stress tests to confirm that there are no regressions. Also fixed a 
minor incorrect API usage bug.

http://gerrit.cloudera.org:8080/#/c/8439/5/be/src/rpc/rpc-mgr-test.cc
File be/src/rpc/rpc-mgr-test.cc:

http://gerrit.cloudera.org:8080/#/c/8439/5/be/src/rpc/rpc-mgr-test.cc@279
PS5, Line 279:
 : TEST_F(RpcMgrTest, MultipleServices) {
> delete
Done


http://gerrit.cloudera.org:8080/#/c/8439/5/be/src/rpc/rpc-mgr.cc
File be/src/rpc/rpc-mgr.cc:

http://gerrit.cloudera.org:8080/#/c/8439/5/be/src/rpc/rpc-mgr.cc@100
PS5, Line 100: }
> Makes me wonder if we should have a test case in which remote nodes somehow
We don't support that configuration, so I think we can leave it out for now. 
Also, in the worst case, it will be caught during connection negotiation.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
Gerrit-Change-Number: 8439
Gerrit-PatchSet: 6
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Fri, 26 Jan 2018 19:36:18 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Sailesh Mukil (Code Review)]

Hello Michael Ho, Dan Hecht,

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

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

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

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..

IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

KRPC has some flags that turn on TLS. This patch sets those to enable
TLS communication.

Tests are added to rpc-mgr-test.

Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
---
M be/src/catalog/catalogd-main.cc
M be/src/rpc/authentication-test.cc
M be/src/rpc/rpc-mgr-test.cc
M be/src/rpc/rpc-mgr.cc
M be/src/rpc/rpc-mgr.h
M be/src/rpc/thrift-server.cc
M be/src/rpc/thrift-server.h
M be/src/runtime/exec-env.cc
M be/src/service/impala-server.cc
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestored-main.cc
M be/src/testutil/in-process-servers.cc
M be/src/util/openssl-util.cc
M be/src/util/openssl-util.h
15 files changed, 329 insertions(+), 68 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/39/8439/6
--
To view, visit http://gerrit.cloudera.org:8080/8439
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
Gerrit-Change-Number: 8439
Gerrit-PatchSet: 6
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Michael Ho (Code Review)]

Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8439 )

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..


Patch Set 5: Code-Review+2

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8439/5/be/src/rpc/rpc-mgr-test.cc
File be/src/rpc/rpc-mgr-test.cc:

http://gerrit.cloudera.org:8080/#/c/8439/5/be/src/rpc/rpc-mgr-test.cc@279
PS5, Line 279:   //RunMultipleServicesTestTemplate(
 :   //static_cast(this), 
_mgr_, krpc_address_);
delete


http://gerrit.cloudera.org:8080/#/c/8439/5/be/src/rpc/rpc-mgr.cc
File be/src/rpc/rpc-mgr.cc:

http://gerrit.cloudera.org:8080/#/c/8439/5/be/src/rpc/rpc-mgr.cc@100
PS5, Line 100: bld.set_rpc_encryption("required");
Makes me wonder if we should have a test case in which remote nodes somehow 
doesn't support encryption ? That said, it's not a supported configuration.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
Gerrit-Change-Number: 8439
Gerrit-PatchSet: 5
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Sat, 20 Jan 2018 03:26:52 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Sailesh Mukil (Code Review)]

Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8439 )

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..


Patch Set 4:

(15 comments)

http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc
File be/src/rpc/rpc-mgr-test.cc:

http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@78
PS4, Line 78: string IMPALA_HOME(getenv("IMPALA_HOME"));
> const static
Done


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@90
PS4, Line 90: TLSv1.0 compatible ciphers
> Are there ways to detect the TLS version supported on the platform and choo
There actually is a way now, but the patch that introduces some basic utils for 
it is still in review here:
https://gerrit.cloudera.org/#/c/9060/

I can add tests for TLSv1.2 as a follow on patch.


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@268
PS4, Line 268: RunTlsTestTemplate
> What do you think about factoring line 331 to line 371 below into a single
Done. I removed RunTlsTestTemplate() and made every test use the new function 
RunMultipleServicesTestTemplate()


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@309
PS4, Line 309:
> nit: blank line
Done


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@331
PS4, Line 331:   // Test that a service can be started, and will respond to 
requests.
 :   unique_ptr ping_impl(
 :   new PingServiceImpl(tls_rpc_mgr.metric_entity(), 
tls_rpc_mgr.result_tracker()));
 :   ASSERT_OK(tls_rpc_mgr.RegisterService(10, 10, 
move(ping_impl)));
 :
 :   // Test that a second service, that verifies the RPC payload 
is not corrupted,
 :   // can be started.
 :   unique_ptr scan_mem_impl(
 :   new ScanMemServiceImpl(tls_rpc_mgr.metric_entity(), 
tls_rpc_mgr.result_tracker()));
 :   ASSERT_OK(tls_rpc_mgr.RegisterService(10, 10, 
move(scan_mem_impl)));
 :
 :   FLAGS_num_acceptor_threads = 2;
 :   FLAGS_num_reactor_threads = 10;
 :   ASSERT_OK(tls_rpc_mgr.StartServices(tls_krpc_address));
 :
 :   unique_ptr ping_proxy;
 :   
ASSERT_OK(tls_rpc_mgr.GetProxy(tls_krpc_address, 
_proxy));
 :
 :   unique_ptr scan_mem_proxy;
 :   
ASSERT_OK(tls_rpc_mgr.GetProxy(tls_krpc_address, 
_mem_proxy));
 :
 :   RpcController controller;
 :   srand(0);
 :   // Randomly invoke either services to make sure a RpcMgr can 
host multiple
 :   // services at the same time.
 :   for (int i = 0; i < 100; ++i) {
 : controller.Reset();
 : if (random() % 2 == 0) {
 :   PingRequestPB request;
 :   PingResponsePB response;
 :   kudu::Status status = ping_proxy->Ping(request, , 
);
 :   ASSERT_TRUE(status.ok());
 :   ASSERT_EQ(response.int_response(), 42);
 : } else {
 :   ScanMemRequestPB request;
 :   ScanMemResponsePB response;
 :   SetupScanMemRequest(, );
 :   kudu::Status status = scan_mem_proxy->ScanMem(request, 
, );
 :   ASSERT_TRUE(status.ok());
 : }
 :   }
> Please see comments above on how this may be factored into a function to be
Done


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@378
PS4, Line 378:   // Misconfigure TLS with bad CA certificate.
> nit: may as well move it to before line 377 as block comment to describe wh
Done


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@392
PS4, Line 392:
 :   // Misconfigure TLS with a bad password command for the 
passowrd protected private key.
> nit: may as well move it to before line 391 as block comment for this funct
Done


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@412
PS4, Line 412:   // Configure TLS with a password protected private key and the 
correct password for it.
> nit: same here.
Done


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@430
PS4, Line 430:   // Misconfigure TLS with a bad cipher.
> nit: same here
Done


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@446
PS4, Line 446:   // Enable TLS with a TLSv1 compatible cipher list.
> nit: same here.
Done


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr.h
File be/src/rpc/rpc-mgr.h:

http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr.h@100
PS4, Line 100: {
 :   }
> nit: one line { }
Done


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr.cc
File be/src/rpc/rpc-mgr.cc:

[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Sailesh Mukil (Code Review)]

Hello Michael Ho, Dan Hecht,

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

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

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

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..

IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

KRPC has some flags that turn on TLS. This patch sets those to enable
TLS communication.

Tests are added to rpc-mgr-test.

Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
---
M be/src/catalog/catalogd-main.cc
M be/src/rpc/authentication-test.cc
M be/src/rpc/rpc-mgr-test.cc
M be/src/rpc/rpc-mgr.cc
M be/src/rpc/rpc-mgr.h
M be/src/rpc/thrift-server.cc
M be/src/rpc/thrift-server.h
M be/src/runtime/exec-env.cc
M be/src/service/impala-server.cc
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestored-main.cc
M be/src/testutil/in-process-servers.cc
M be/src/util/openssl-util.cc
M be/src/util/openssl-util.h
15 files changed, 329 insertions(+), 68 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/39/8439/5
--
To view, visit http://gerrit.cloudera.org:8080/8439
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
Gerrit-Change-Number: 8439
Gerrit-PatchSet: 5
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Michael Ho (Code Review)]

Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8439 )

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..


Patch Set 4:

(15 comments)

http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc
File be/src/rpc/rpc-mgr-test.cc:

http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@78
PS4, Line 78: string IMPALA_HOME(getenv("IMPALA_HOME"));
const static


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@90
PS4, Line 90: TLSv1.0 compatible ciphers
Are there ways to detect the TLS version supported on the platform and choose 
some TLSv1.2 ciphers too if the platform supports it ?


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@268
PS4, Line 268: RunTlsTestTemplate
What do you think about factoring line 331 to line 371 below into a single 
function and use it in the tests below and  TEST_F(RpcMgrTest, 
MultipleServices)  ?

In this way, we can exercise more than a single ping RPC in the tests below 
when TLS is enabled.


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@309
PS4, Line 309:
nit: blank line


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@331
PS4, Line 331:   // Test that a service can be started, and will respond to 
requests.
 :   unique_ptr ping_impl(
 :   new PingServiceImpl(tls_rpc_mgr.metric_entity(), 
tls_rpc_mgr.result_tracker()));
 :   ASSERT_OK(tls_rpc_mgr.RegisterService(10, 10, 
move(ping_impl)));
 :
 :   // Test that a second service, that verifies the RPC payload 
is not corrupted,
 :   // can be started.
 :   unique_ptr scan_mem_impl(
 :   new ScanMemServiceImpl(tls_rpc_mgr.metric_entity(), 
tls_rpc_mgr.result_tracker()));
 :   ASSERT_OK(tls_rpc_mgr.RegisterService(10, 10, 
move(scan_mem_impl)));
 :
 :   FLAGS_num_acceptor_threads = 2;
 :   FLAGS_num_reactor_threads = 10;
 :   ASSERT_OK(tls_rpc_mgr.StartServices(tls_krpc_address));
 :
 :   unique_ptr ping_proxy;
 :   
ASSERT_OK(tls_rpc_mgr.GetProxy(tls_krpc_address, 
_proxy));
 :
 :   unique_ptr scan_mem_proxy;
 :   
ASSERT_OK(tls_rpc_mgr.GetProxy(tls_krpc_address, 
_mem_proxy));
 :
 :   RpcController controller;
 :   srand(0);
 :   // Randomly invoke either services to make sure a RpcMgr can 
host multiple
 :   // services at the same time.
 :   for (int i = 0; i < 100; ++i) {
 : controller.Reset();
 : if (random() % 2 == 0) {
 :   PingRequestPB request;
 :   PingResponsePB response;
 :   kudu::Status status = ping_proxy->Ping(request, , 
);
 :   ASSERT_TRUE(status.ok());
 :   ASSERT_EQ(response.int_response(), 42);
 : } else {
 :   ScanMemRequestPB request;
 :   ScanMemResponsePB response;
 :   SetupScanMemRequest(, );
 :   kudu::Status status = scan_mem_proxy->ScanMem(request, 
, );
 :   ASSERT_TRUE(status.ok());
 : }
 :   }
Please see comments above on how this may be factored into a function to be 
shared by multiple tests.


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@378
PS4, Line 378:   // Misconfigure TLS with bad CA certificate.
nit: may as well move it to before line 377 as block comment to describe what 
this function tests for.


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@392
PS4, Line 392:
 :   // Misconfigure TLS with a bad password command for the 
passowrd protected private key.
nit: may as well move it to before line 391 as block comment for this function.

Same for other functions.


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@412
PS4, Line 412:   // Configure TLS with a password protected private key and the 
correct password for it.
nit: same here.


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@430
PS4, Line 430:   // Misconfigure TLS with a bad cipher.
nit: same here


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@446
PS4, Line 446:   // Enable TLS with a TLSv1 compatible cipher list.
nit: same here.


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr.h
File be/src/rpc/rpc-mgr.h:

http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr.h@100
PS4, Line 100: {
 :   }
nit: one line { }


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr.cc
File be/src/rpc/rpc-mgr.cc:

http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr.cc@93
PS4, Line 93:
nit: blank 

[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Sailesh Mukil (Code Review)]

Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8439 )

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..


Patch Set 4:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/8439/3/be/src/rpc/rpc-mgr.h
File be/src/rpc/rpc-mgr.h:

http://gerrit.cloudera.org:8080/#/c/8439/3/be/src/rpc/rpc-mgr.h@105
PS3, Line 105:
> Seems more appropriate to pass it to the constructor of RpcMgr() instead of
Done


http://gerrit.cloudera.org:8080/#/c/8439/3/be/src/rpc/rpc-mgr.h@182
PS3, Line 182:
> True if TLS is configured for communication between Impalad backend servers
Done


http://gerrit.cloudera.org:8080/#/c/8439/3/be/src/rpc/rpc-mgr.h@183
PS3, Line 183: /// True if TLS is con
> const bool use_tls_; and initialize it in the ctor.
Done


http://gerrit.cloudera.org:8080/#/c/8439/3/be/src/rpc/rpc-mgr.cc
File be/src/rpc/rpc-mgr.cc:

http://gerrit.cloudera.org:8080/#/c/8439/3/be/src/rpc/rpc-mgr.cc@88
PS3, Line 88: bld.set_rpc_authentication("required");
:   }
> This may be worth fixing too in this patch if it's not too complicated.
Done


http://gerrit.cloudera.org:8080/#/c/8439/3/be/src/rpc/rpc-mgr.cc@117
PS3, Line 117:   RETURN_IF_ERROR(service_pool->Init(num_service_threads));
 :   KUDU_RETURN_IF_ERROR(
> One line ?
Done


http://gerrit.cloudera.org:8080/#/c/8439/3/be/src/util/openssl-util.h
File be/src/util/openssl-util.h:

http://gerrit.cloudera.org:8080/#/c/8439/3/be/src/util/openssl-util.h@31
PS3, Line 31: //
> nit: ///
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
Gerrit-Change-Number: 8439
Gerrit-PatchSet: 4
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 16 Jan 2018 21:38:02 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Michael Ho (Code Review)]

Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8439 )

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..


Patch Set 3:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/8439/3/be/src/rpc/rpc-mgr.h
File be/src/rpc/rpc-mgr.h:

http://gerrit.cloudera.org:8080/#/c/8439/3/be/src/rpc/rpc-mgr.h@105
PS3, Line 105: void use_tls(bool value) { use_tls_ = value; }
Seems more appropriate to pass it to the constructor of RpcMgr() instead of 
having this function.


http://gerrit.cloudera.org:8080/#/c/8439/3/be/src/rpc/rpc-mgr.h@182
PS3, Line 182:   /// messenger_ will be configured to use TLS if this is set.
True if TLS is configured for communication between Impalad backend servers. 
messenger_ 


http://gerrit.cloudera.org:8080/#/c/8439/3/be/src/rpc/rpc-mgr.h@183
PS3, Line 183: bool use_tls_ = false;
const bool use_tls_; and initialize it in the ctor.


http://gerrit.cloudera.org:8080/#/c/8439/3/be/src/rpc/rpc-mgr.cc
File be/src/rpc/rpc-mgr.cc:

http://gerrit.cloudera.org:8080/#/c/8439/3/be/src/rpc/rpc-mgr.cc@88
PS3, Line 88: // TODO: Once the Messenger can take more options pertaining 
to 'rpc_authentication'
: // and more, we need to explicitly set those options here. 
(KUDU-2228)
This may be worth fixing too in this patch if it's not too complicated.


http://gerrit.cloudera.org:8080/#/c/8439/3/be/src/rpc/rpc-mgr.cc@117
PS3, Line 117:   RETURN_IF_ERROR(
 :   service_pool->Init(num_service_threads));
One line ?


http://gerrit.cloudera.org:8080/#/c/8439/3/be/src/util/openssl-util.h
File be/src/util/openssl-util.h:

http://gerrit.cloudera.org:8080/#/c/8439/3/be/src/util/openssl-util.h@31
PS3, Line 31: //
nit: ///



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
Gerrit-Change-Number: 8439
Gerrit-PatchSet: 3
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Fri, 12 Jan 2018 22:33:41 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Sailesh Mukil (Code Review)]

Hello Michael Ho, Dan Hecht,

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

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

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

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..

IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

KRPC has some flags that turn on TLS. This patch sets those to enable
TLS communication.

Tests are added to rpc-mgr-test.

Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
---
M be/src/catalog/catalogd-main.cc
M be/src/rpc/authentication-test.cc
M be/src/rpc/rpc-mgr-test.cc
M be/src/rpc/rpc-mgr.cc
M be/src/rpc/rpc-mgr.h
M be/src/rpc/thrift-server.cc
M be/src/rpc/thrift-server.h
M be/src/runtime/exec-env.cc
M be/src/service/impala-server.cc
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestored-main.cc
M be/src/testutil/in-process-servers.cc
M be/src/util/openssl-util.cc
M be/src/util/openssl-util.h
15 files changed, 326 insertions(+), 36 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/39/8439/3
--
To view, visit http://gerrit.cloudera.org:8080/8439
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
Gerrit-Change-Number: 8439
Gerrit-PatchSet: 3
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Sailesh Mukil (Code Review)]

Hello Michael Ho, Dan Hecht,

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

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

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

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..

IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

KRPC has some flags that turn on TLS. This patch sets those to enable
TLS communication.

Tests are added to rpc-mgr-test.

Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
---
M be/src/catalog/catalogd-main.cc
M be/src/rpc/authentication-test.cc
M be/src/rpc/rpc-mgr-test.cc
M be/src/rpc/rpc-mgr.cc
M be/src/rpc/rpc-mgr.h
M be/src/rpc/thrift-server.cc
M be/src/rpc/thrift-server.h
M be/src/runtime/exec-env.cc
M be/src/service/impala-server.cc
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestored-main.cc
M be/src/testutil/in-process-servers.cc
M be/src/util/openssl-util.cc
M be/src/util/openssl-util.h
15 files changed, 327 insertions(+), 36 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/39/8439/2
--
To view, visit http://gerrit.cloudera.org:8080/8439
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
Gerrit-Change-Number: 8439
Gerrit-PatchSet: 2
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Michael Ho (Code Review)]

Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8439 )

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..


Patch Set 1:

The backport for KUDU-2228 is merged now.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
Gerrit-Change-Number: 8439
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Wed, 03 Jan 2018 18:33:14 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Michael Ho (Code Review)]

Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8439 )

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..


Patch Set 1:

The backport for the fix of KUDU-2228 is being reviewed here: 
https://gerrit.cloudera.org/#/c/8878/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
Gerrit-Change-Number: 8439
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Wed, 20 Dec 2017 01:08:36 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Sailesh Mukil (Code Review)]

Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8439 )

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..


Patch Set 1:

Update: This is now blocked on:
https://issues.apache.org/jira/browse/KUDU-2228

The reason is that we need to modify some KRPC gflags at runtime to enable TLS 
w/ KRPC. We found that this may affect the KuduClient as well, which is not 
what we want. To avoid this, we will convert all the flags to messenger options 
and make a modification in this patch to use the Messenger options.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
Gerrit-Change-Number: 8439
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 04 Dec 2017 04:59:02 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Michael Ho (Code Review)]

Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8439 )

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/8439/1/be/src/rpc/rpc-mgr.h
File be/src/rpc/rpc-mgr.h:

http://gerrit.cloudera.org:8080/#/c/8439/1/be/src/rpc/rpc-mgr.h@105
PS1, Line 105:   void use_tls(bool value) { use_tls_ = value; }
Doesn't seem necessary to have this if we can pass this in the constructor. If 
you decide to keep it, can you please rename it SetUseTLS(bool use_tls) ?


http://gerrit.cloudera.org:8080/#/c/8439/1/be/src/rpc/rpc-mgr.h@181
PS1, Line 181: bool use_tls_ = false;
const bool use_tls_; and initialize it in the constructor.


http://gerrit.cloudera.org:8080/#/c/8439/1/be/src/runtime/exec-env.cc
File be/src/runtime/exec-env.cc:

http://gerrit.cloudera.org:8080/#/c/8439/1/be/src/runtime/exec-env.cc@175
PS1, Line 175: rpc_mgr_->use_tls(IsInternalTlsConfigured());
This flag looks like something one should pass to the constructor of RpcMgr ?


http://gerrit.cloudera.org:8080/#/c/8439/1/be/src/util/openssl-util.h
File be/src/util/openssl-util.h:

http://gerrit.cloudera.org:8080/#/c/8439/1/be/src/util/openssl-util.h@28
PS1, Line 28: //
nit: ///



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
Gerrit-Change-Number: 8439
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Thu, 30 Nov 2017 01:58:58 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8439 )

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8439/1/be/src/rpc/rpc-mgr.h
File be/src/rpc/rpc-mgr.h:

http://gerrit.cloudera.org:8080/#/c/8439/1/be/src/rpc/rpc-mgr.h@183
PS1, Line 183:  /// The following strings preserve the Kudu flags original 
values to restore in
 :   /// Shutdown() as they will be modified by us.
 :   string flag_save_ca_certificate_file;
 :   string flag_save_rpc_private_key_file;
 :   string flag_save_rpc_certificate_file;
 :   string flag_save_rpc_private_key_password_cmd;
 :   string flag_save_rpc_tls_ciphers;
 :   string flag_save_rpc_tls_min_protocol;
> Ah you're right. I assumed that would have been called through some code pa
Please add a test case too.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
Gerrit-Change-Number: 8439
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 28 Nov 2017 22:30:35 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Sailesh Mukil (Code Review)]

Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8439 )

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8439/1/be/src/rpc/rpc-mgr.h
File be/src/rpc/rpc-mgr.h:

http://gerrit.cloudera.org:8080/#/c/8439/1/be/src/rpc/rpc-mgr.h@183
PS1, Line 183:  /// The following strings preserve the Kudu flags original 
values to restore in
 :   /// Shutdown() as they will be modified by us.
 :   string flag_save_ca_certificate_file;
 :   string flag_save_rpc_private_key_file;
 :   string flag_save_rpc_certificate_file;
 :   string flag_save_rpc_private_key_password_cmd;
 :   string flag_save_rpc_tls_ciphers;
 :   string flag_save_rpc_tls_min_protocol;
> There is no magic here :) My point is you need to trigger the validation so
Ah you're right. I assumed that would have been called through some code path, 
but it looks like that doesn't happen.

I'll try running it as a part of our Init routines and post an updated patch.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
Gerrit-Change-Number: 8439
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 28 Nov 2017 22:17:21 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Sailesh Mukil (Code Review)]

Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8439 )

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8439/1/be/src/rpc/rpc-mgr.h
File be/src/rpc/rpc-mgr.h:

http://gerrit.cloudera.org:8080/#/c/8439/1/be/src/rpc/rpc-mgr.h@183
PS1, Line 183:  /// The following strings preserve the Kudu flags original 
values to restore in
 :   /// Shutdown() as they will be modified by us.
 :   string flag_save_ca_certificate_file;
 :   string flag_save_rpc_private_key_file;
 :   string flag_save_rpc_certificate_file;
 :   string flag_save_rpc_private_key_password_cmd;
 :   string flag_save_rpc_tls_ciphers;
 :   string flag_save_rpc_tls_min_protocol;
> But in the tests, wouldn't each test case that creates a messenger need to
We don't set all of them for all the tests. We could set the ones we don't need 
as defaults for the tests that don't need them, but the defaults of some of 
these flags are pretty large. Eg, see the FLAGS_rpc_tls_ciphers:
https://github.com/apache/incubator-impala/blob/32baa695f499a936b72c5a51ae3649c408aa5a85/be/src/kudu/security/tls_context.cc#L53-L70

So it would be odd setting it to this hardcoded value before every test.

Some flags are only used by the builder but others are used at runtime too.

The validators are automatically run while creating Messenger objects.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
Gerrit-Change-Number: 8439
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 28 Nov 2017 18:31:22 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8439 )

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8439/1/be/src/rpc/rpc-mgr.h
File be/src/rpc/rpc-mgr.h:

http://gerrit.cloudera.org:8080/#/c/8439/1/be/src/rpc/rpc-mgr.h@183
PS1, Line 183:  /// The following strings preserve the Kudu flags original 
values to restore in
 :   /// Shutdown() as they will be modified by us.
 :   string flag_save_ca_certificate_file;
 :   string flag_save_rpc_private_key_file;
 :   string flag_save_rpc_certificate_file;
 :   string flag_save_rpc_private_key_password_cmd;
 :   string flag_save_rpc_tls_ciphers;
 :   string flag_save_rpc_tls_min_protocol;
> In our Impala process we only always start one Messenger object ever. Howev
But in the tests, wouldn't each test case that creates a messenger need to set 
all of these flags anyway.

Are these flags only used by the builder, or are they also used at runtime?

The krpc code seems to have validators that check the consistency of the flags. 
Should we run those validators after setting these flags?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
Gerrit-Change-Number: 8439
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 28 Nov 2017 17:51:23 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8439 )

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8439/1/be/src/rpc/rpc-mgr.h
File be/src/rpc/rpc-mgr.h:

http://gerrit.cloudera.org:8080/#/c/8439/1/be/src/rpc/rpc-mgr.h@183
PS1, Line 183:  /// The following strings preserve the Kudu flags original 
values to restore in
 :   /// Shutdown() as they will be modified by us.
 :   string flag_save_ca_certificate_file;
 :   string flag_save_rpc_private_key_file;
 :   string flag_save_rpc_certificate_file;
 :   string flag_save_rpc_private_key_password_cmd;
 :   string flag_save_rpc_tls_ciphers;
 :   string flag_save_rpc_tls_min_protocol;
why bother saving and restoring these flags?  what's the case that would 
benefit from this?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
Gerrit-Change-Number: 8439
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Tue, 28 Nov 2017 00:12:28 +
Gerrit-HasComments: Yes