[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
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 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 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 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
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 Build failed: 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 09:41:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
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 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
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 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 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 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 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
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 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 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 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
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 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), &rpc_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 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, &ping_proxy)); : : unique_ptr scan_mem_proxy; : ASSERT_OK(tls_rpc_mgr.GetProxy(tls_krpc_address, &scan_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, &response, &controller); : ASSERT_TRUE(status.ok()); : ASSERT_EQ(response.int_response(), 42); : } else { : ScanMemRequestPB request; : ScanMemResponsePB response; : SetupScanMemRequest(&request, &controller); : kudu::Status status = scan_mem_proxy->ScanMem(request, &response, &controller); : 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
[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
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 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, &ping_proxy)); : : unique_ptr scan_mem_proxy; : ASSERT_OK(tls_rpc_mgr.GetProxy(tls_krpc_address, &scan_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, &response, &controller); : ASSERT_TRUE(status.ok()); : ASSERT_EQ(response.int_response(), 42); : } else { : ScanMemRequestPB request; : ScanMemResponsePB response; : SetupScanMemRequest(&request, &controller); : kudu::Status status = scan_mem_proxy->ScanMem(request, &response, &controller); : 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:8
[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
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 (#4). 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(+), 41 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/39/8439/4 -- 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: 4 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 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 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
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
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 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 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 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 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 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 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
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; > The flag validators created with DEFINE_validators() are run automatically There is no magic here :) My point is you need to trigger the validation somehow, and currently the only way that happens is via ParseCommandLineFlags() (see the call chain I posted above). The documentation you pointed at says that as well. -- 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:11:09 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
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; > I don't see where the validators are run when creating Messenger objects. T The flag validators created with DEFINE_validators() are run automatically when these flags are assigned values. https://github.com/apache/incubator-impala/blob/master/be/src/kudu/rpc/messenger.cc#L123-L201 https://gflags.github.io/gflags/#validate I should have been more clear, it's not when a Messenger object is created. -- 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:02:09 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
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; > We don't set all of them for all the tests. We could set the ones we don't I don't see where the validators are run when creating Messenger objects. The code path I see is: ParseCommandLineFlags -> HandleCommonFlags -> RunCustomValidators. -- 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:37:52 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
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 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
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; > why bother saving and restoring these flags? what's the case that would be In our Impala process we only always start one Messenger object ever. However, in our tests, we start multiple Messenger objects within the context of the same process. So if we don't save and restore the flags on exit, we leak the configuration of one Messenger object into the following ones. This is isn't great as we would ideally have all these as messenger options instead of process wide flags, but that's something not done yet on the Kudu side. I have a WIP patch for that but we decided against going forward with it now since that would change the APIs to use KRPC quite a bit. But it is something we'll need to pick up again in the future. https://gerrit.cloudera.org/#/c/6520/ -- 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:04:33 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
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