[Impala-ASF-CR] IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5 parse name() instead
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14433 ) Change subject: IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5_parse_name() instead .. IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5_parse_name() instead We want to use krb5_parse_name() to parse the principal instead of using custom code. When kerberos is initialized in Impala's copy of Kudu code, it stores a global context which is used when accessing the Krb5 library. To use this global context the code that parses the principal name is moved into the Impala Kudu code. This new code is then called from the existing ParseKerberosPrincipal method. Test done: Add two tests to authentication-test, one to verify parsing a principal containing a special character. The other to verify exception when parsing bad format principal, new error code is 2 instead of original 112 which is BAD_PRINCIPAL_FORMAT error code. Run impala-private-parameterized tests. Change-Id: Ifddafa7aae25d66ed7d9fa0306f17501a191cdac Reviewed-on: http://gerrit.cloudera.org:8080/14520 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin Change-Id: I0e64ebdc10f102dbdc5b87f6fe3f2a0310b1be24 Reviewed-on: http://gerrit.cloudera.org:8080/14433 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/kudu/security/init.cc M be/src/kudu/security/init.h M be/src/kudu/security/test/mini_kdc-test.cc M be/src/rpc/authentication-test.cc M be/src/util/auth-util.cc 5 files changed, 61 insertions(+), 15 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/14433 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I0e64ebdc10f102dbdc5b87f6fe3f2a0310b1be24 Gerrit-Change-Number: 14433 Gerrit-PatchSet: 9 Gerrit-Owner: Xiaomeng Zhang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Xiaomeng Zhang
[Impala-ASF-CR] IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5 parse name() instead
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14433 ) Change subject: IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5_parse_name() instead .. Patch Set 8: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/14433 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e64ebdc10f102dbdc5b87f6fe3f2a0310b1be24 Gerrit-Change-Number: 14433 Gerrit-PatchSet: 8 Gerrit-Owner: Xiaomeng Zhang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Xiaomeng Zhang Gerrit-Comment-Date: Wed, 30 Oct 2019 01:24:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5 parse name() instead
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14433 ) Change subject: IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5_parse_name() instead .. Patch Set 8: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5148/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/14433 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e64ebdc10f102dbdc5b87f6fe3f2a0310b1be24 Gerrit-Change-Number: 14433 Gerrit-PatchSet: 8 Gerrit-Owner: Xiaomeng Zhang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Xiaomeng Zhang Gerrit-Comment-Date: Tue, 29 Oct 2019 21:00:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5 parse name() instead
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14433 ) Change subject: IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5_parse_name() instead .. Patch Set 8: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14433 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e64ebdc10f102dbdc5b87f6fe3f2a0310b1be24 Gerrit-Change-Number: 14433 Gerrit-PatchSet: 8 Gerrit-Owner: Xiaomeng Zhang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Xiaomeng Zhang Gerrit-Comment-Date: Tue, 29 Oct 2019 21:00:44 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5 parse name() instead
Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/14433 ) Change subject: IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5_parse_name() instead .. Patch Set 7: Code-Review+2 LGTM, thank you -- To view, visit http://gerrit.cloudera.org:8080/14433 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e64ebdc10f102dbdc5b87f6fe3f2a0310b1be24 Gerrit-Change-Number: 14433 Gerrit-PatchSet: 7 Gerrit-Owner: Xiaomeng Zhang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Xiaomeng Zhang Gerrit-Comment-Date: Tue, 29 Oct 2019 21:00:15 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5 parse name() instead
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14433 ) Change subject: IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5_parse_name() instead .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4896/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/14433 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e64ebdc10f102dbdc5b87f6fe3f2a0310b1be24 Gerrit-Change-Number: 14433 Gerrit-PatchSet: 7 Gerrit-Owner: Xiaomeng Zhang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Xiaomeng Zhang Gerrit-Comment-Date: Mon, 28 Oct 2019 22:49:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5 parse name() instead
Xiaomeng Zhang has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/14433 ) Change subject: IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5_parse_name() instead .. IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5_parse_name() instead We want to use krb5_parse_name() to parse the principal instead of using custom code. When kerberos is initialized in Impala's copy of Kudu code, it stores a global context which is used when accessing the Krb5 library. To use this global context the code that parses the principal name is moved into the Impala Kudu code. This new code is then called from the existing ParseKerberosPrincipal method. Test done: Add two tests to authentication-test, one to verify parsing a principal containing a special character. The other to verify exception when parsing bad format principal, new error code is 2 instead of original 112 which is BAD_PRINCIPAL_FORMAT error code. Run impala-private-parameterized tests. Change-Id: Ifddafa7aae25d66ed7d9fa0306f17501a191cdac Reviewed-on: http://gerrit.cloudera.org:8080/14520 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin Change-Id: I0e64ebdc10f102dbdc5b87f6fe3f2a0310b1be24 --- M be/src/kudu/security/init.cc M be/src/kudu/security/init.h M be/src/kudu/security/test/mini_kdc-test.cc M be/src/rpc/authentication-test.cc M be/src/util/auth-util.cc 5 files changed, 61 insertions(+), 15 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/33/14433/7 -- To view, visit http://gerrit.cloudera.org:8080/14433 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0e64ebdc10f102dbdc5b87f6fe3f2a0310b1be24 Gerrit-Change-Number: 14433 Gerrit-PatchSet: 7 Gerrit-Owner: Xiaomeng Zhang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Xiaomeng Zhang
[Impala-ASF-CR] IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5 parse name() instead
Xiaomeng Zhang has posted comments on this change. (
http://gerrit.cloudera.org:8080/14433 )
Change subject: IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use
krb5_parse_name() instead
..
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14433/6/be/src/rpc/authentication-test.cc
File be/src/rpc/authentication-test.cc:
http://gerrit.cloudera.org:8080/#/c/14433/6/be/src/rpc/authentication-test.cc@200
PS6, Line 200: EXPECT_ERROR(sa.InitKerberos(" ", "/etc/hosts"), 2);
> This says we will get an error, but do we know it is the right error?
Expected error code is 2, this line is doing the comparison.
--
To view, visit http://gerrit.cloudera.org:8080/14433
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e64ebdc10f102dbdc5b87f6fe3f2a0310b1be24
Gerrit-Change-Number: 14433
Gerrit-PatchSet: 6
Gerrit-Owner: Xiaomeng Zhang
Gerrit-Reviewer: Andrew Sherman
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Xiaomeng Zhang
Gerrit-Comment-Date: Mon, 28 Oct 2019 22:05:45 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5 parse name() instead
Andrew Sherman has posted comments on this change. (
http://gerrit.cloudera.org:8080/14433 )
Change subject: IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use
krb5_parse_name() instead
..
Patch Set 6:
(3 comments)
Thanks for this update
http://gerrit.cloudera.org:8080/#/c/14433/5//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14433/5//COMMIT_MSG@21
PS5, Line 21: format principal, new error code is 2 instead of original 112
> What kind of end-to-end test? The one included in impala-private-parameteri
yes, maybe there's a better name but I think this name is generally understood
http://gerrit.cloudera.org:8080/#/c/14433/6//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14433/6//COMMIT_MSG@19
PS6, Line 19: Add two authentication-test
Add two tests to authentication-test
http://gerrit.cloudera.org:8080/#/c/14433/6/be/src/rpc/authentication-test.cc
File be/src/rpc/authentication-test.cc:
http://gerrit.cloudera.org:8080/#/c/14433/6/be/src/rpc/authentication-test.cc@200
PS6, Line 200: EXPECT_ERROR(sa.InitKerberos(" ", "/etc/hosts"), 2);
This says we will get an error, but do we know it is the right error?
We should check the return code or message or something more specific
--
To view, visit http://gerrit.cloudera.org:8080/14433
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e64ebdc10f102dbdc5b87f6fe3f2a0310b1be24
Gerrit-Change-Number: 14433
Gerrit-PatchSet: 6
Gerrit-Owner: Xiaomeng Zhang
Gerrit-Reviewer: Andrew Sherman
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Xiaomeng Zhang
Gerrit-Comment-Date: Wed, 23 Oct 2019 17:03:19 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5 parse name() instead
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14433 ) Change subject: IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5_parse_name() instead .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4856/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/14433 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e64ebdc10f102dbdc5b87f6fe3f2a0310b1be24 Gerrit-Change-Number: 14433 Gerrit-PatchSet: 6 Gerrit-Owner: Xiaomeng Zhang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Xiaomeng Zhang Gerrit-Comment-Date: Wed, 23 Oct 2019 01:55:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5 parse name() instead
Xiaomeng Zhang has posted comments on this change. (
http://gerrit.cloudera.org:8080/14433 )
Change subject: IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use
krb5_parse_name() instead
..
Patch Set 6:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/14433/5//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14433/5//COMMIT_MSG@10
PS5, Line 10: using custom code.
> instead of using custom code.
Done
http://gerrit.cloudera.org:8080/#/c/14433/5//COMMIT_MSG@12
PS5, Line 12: When kerberos is initialized in Impala's copy of Kudu code, it
stores a
> The reader of commit messages is developers like you and me.
Done
http://gerrit.cloudera.org:8080/#/c/14433/5//COMMIT_MSG@18
PS5, Line 18: Test done:
> to verify parsing a principal nae containing a special character.
Done
http://gerrit.cloudera.org:8080/#/c/14433/5//COMMIT_MSG@19
PS5, Line 19: Add two authentication-test, one to verify parsing a principal
containing
> Is it possible to have an automated test for this?
Yes, added a new test BadPrincipalFormat
http://gerrit.cloudera.org:8080/#/c/14433/5//COMMIT_MSG@21
PS5, Line 21: format principal, new error code is 2 instead of original 112
> Do you want to mention running end-to-end tests?
What kind of end-to-end test? The one included in impala-private-parameterized?
http://gerrit.cloudera.org:8080/#/c/14433/5/be/src/kudu/security/init.h
File be/src/kudu/security/init.h:
http://gerrit.cloudera.org:8080/#/c/14433/5/be/src/kudu/security/init.h@39
PS5, Line 39: // Parses the given Kerberos principal into service name,
hostname, and realm.
> Maybe: "Parse a kerberos principal name and extract the ervice_name, hostna
Done
http://gerrit.cloudera.org:8080/#/c/14433/5/be/src/util/auth-util.cc
File be/src/util/auth-util.cc:
http://gerrit.cloudera.org:8080/#/c/14433/5/be/src/util/auth-util.cc@92
PS5, Line 92: hostname, realm), strings::Substitute("bad principal format
$0", principal));
> It is more idiomatic and marginally more efficient to use:
Done
--
To view, visit http://gerrit.cloudera.org:8080/14433
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e64ebdc10f102dbdc5b87f6fe3f2a0310b1be24
Gerrit-Change-Number: 14433
Gerrit-PatchSet: 6
Gerrit-Owner: Xiaomeng Zhang
Gerrit-Reviewer: Andrew Sherman
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Xiaomeng Zhang
Gerrit-Comment-Date: Wed, 23 Oct 2019 01:13:17 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5 parse name() instead
Xiaomeng Zhang has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/14433 ) Change subject: IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5_parse_name() instead .. IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5_parse_name() instead We want to use krb5_parse_name() to parse the principal instead of using custom code. When kerberos is initialized in Impala's copy of Kudu code, it stores a global context which is used when accessing the Krb5 library. To use this global context the code that parses the principal name is moved into the Impala Kudu code. This new code is then called from the existing ParseKerberosPrincipal method. Test done: Add two authentication-test, one to verify parsing a principal containing a special character. The other to verify exception when parsing bad format principal, new error code is 2 instead of original 112 which is BAD_PRINCIPAL_FORMAT error code. Change-Id: I0e64ebdc10f102dbdc5b87f6fe3f2a0310b1be24 --- M be/src/kudu/security/init.cc M be/src/kudu/security/init.h M be/src/rpc/authentication-test.cc M be/src/util/auth-util.cc 4 files changed, 41 insertions(+), 15 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/33/14433/6 -- To view, visit http://gerrit.cloudera.org:8080/14433 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0e64ebdc10f102dbdc5b87f6fe3f2a0310b1be24 Gerrit-Change-Number: 14433 Gerrit-PatchSet: 6 Gerrit-Owner: Xiaomeng Zhang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Xiaomeng Zhang
[Impala-ASF-CR] IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5 parse name() instead
Andrew Sherman has posted comments on this change. (
http://gerrit.cloudera.org:8080/14433 )
Change subject: IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use
krb5_parse_name() instead
..
Patch Set 5:
(7 comments)
This is all looking close. I have some picky comments
http://gerrit.cloudera.org:8080/#/c/14433/5//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14433/5//COMMIT_MSG@10
PS5, Line 10: creating our own.
instead of using custom code.
http://gerrit.cloudera.org:8080/#/c/14433/5//COMMIT_MSG@12
PS5, Line 12: As src/kudu/security/init.cc already have g_krb5_ctx initialized,
The reader of commit messages is developers like you and me.
Will the reader know what 'g_krb5_ctx' is?
I think you want to say something like:
"When kerberos is initialized in Impala's copy of Kudu code, it stores a global
context which is used when accessing the Krb5 library. To use this global
context the code that parses the principal name is moved into the Impala Kudu
code. This new code is then called from the existing ParseKerberosPrincipal
method."
http://gerrit.cloudera.org:8080/#/c/14433/5//COMMIT_MSG@18
PS5, Line 18: Add an authentication-test to verify principal with special
character.
to verify parsing a principal nae containing a special character.
http://gerrit.cloudera.org:8080/#/c/14433/5//COMMIT_MSG@19
PS5, Line 19: Manually tested with bad format principal, throw out error code 2
Is it possible to have an automated test for this?
http://gerrit.cloudera.org:8080/#/c/14433/5//COMMIT_MSG@21
PS5, Line 21:
Do you want to mention running end-to-end tests?
http://gerrit.cloudera.org:8080/#/c/14433/5/be/src/kudu/security/init.h
File be/src/kudu/security/init.h:
http://gerrit.cloudera.org:8080/#/c/14433/5/be/src/kudu/security/init.h@39
PS5, Line 39: // Convert a string representation of a principal name to a
krb5_principal structure.
Maybe: "Parse a kerberos principal name and extract the ervice_name, hostname,
and realm from it."
http://gerrit.cloudera.org:8080/#/c/14433/5/be/src/util/auth-util.cc
File be/src/util/auth-util.cc:
http://gerrit.cloudera.org:8080/#/c/14433/5/be/src/util/auth-util.cc@92
PS5, Line 92: hostname, realm),"bad principal format " + principal);
It is more idiomatic and marginally more efficient to use:
Substitute("bad principal name $0", principal)
--
To view, visit http://gerrit.cloudera.org:8080/14433
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e64ebdc10f102dbdc5b87f6fe3f2a0310b1be24
Gerrit-Change-Number: 14433
Gerrit-PatchSet: 5
Gerrit-Owner: Xiaomeng Zhang
Gerrit-Reviewer: Andrew Sherman
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Xiaomeng Zhang
Gerrit-Comment-Date: Tue, 22 Oct 2019 00:08:11 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5 parse name() instead
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14433 ) Change subject: IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5_parse_name() instead .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4849/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/14433 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e64ebdc10f102dbdc5b87f6fe3f2a0310b1be24 Gerrit-Change-Number: 14433 Gerrit-PatchSet: 5 Gerrit-Owner: Xiaomeng Zhang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Xiaomeng Zhang Gerrit-Comment-Date: Mon, 21 Oct 2019 23:26:55 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5 parse name() instead
Xiaomeng Zhang has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/14433 ) Change subject: IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5_parse_name() instead .. IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5_parse_name() instead We want to use krb5_parse_name() to parse the principal instead of creating our own. As src/kudu/security/init.cc already have g_krb5_ctx initialized, we want to leverage the code in KUDU, and create a wrap up function which can be called from IMPALA Krb5parseName(const string& principal, string* service_name, string* hostname, string* realm) Test done: Add an authentication-test to verify principal with special character. Manually tested with bad format principal, throw out error code 2 instead of original 112 which is BAD_PRINCIPAL_FORMAT error code. Change-Id: I0e64ebdc10f102dbdc5b87f6fe3f2a0310b1be24 --- M be/src/kudu/security/init.cc M be/src/kudu/security/init.h M be/src/rpc/authentication-test.cc M be/src/util/auth-util.cc 4 files changed, 36 insertions(+), 15 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/33/14433/5 -- To view, visit http://gerrit.cloudera.org:8080/14433 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0e64ebdc10f102dbdc5b87f6fe3f2a0310b1be24 Gerrit-Change-Number: 14433 Gerrit-PatchSet: 5 Gerrit-Owner: Xiaomeng Zhang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Xiaomeng Zhang
[Impala-ASF-CR] IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5 parse name() instead
Xiaomeng Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14433 ) Change subject: IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5_parse_name() instead .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/14433/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14433/4//COMMIT_MSG@7 PS4, Line 7: IMPALA-7504 ParseKerberosPrincipal() should use krb5_parse_name() instead > See comments on commit msg in patch set 2. Sorry, missed this comments. Fixed in new patch. -- To view, visit http://gerrit.cloudera.org:8080/14433 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e64ebdc10f102dbdc5b87f6fe3f2a0310b1be24 Gerrit-Change-Number: 14433 Gerrit-PatchSet: 4 Gerrit-Owner: Xiaomeng Zhang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Xiaomeng Zhang Gerrit-Comment-Date: Mon, 21 Oct 2019 22:46:51 + Gerrit-HasComments: Yes
