Kyuubi Hive JDBC: Replace UGI-based Kerberos authentication w/ JAAS by pan3793 · Pull Request #3023 · apache/kyuubi

pan3793 · 2022-07-06T08:47:14Z

Why are the changes needed?

The current JDBC Kerberos authentication and Hadoop UGI classes are strongly coupled, it's not friendly for downstream projects which do not have Hadoop dependencies, and users who are not familiar w/ Hadoop security mechanism.

This PR proposes to replace the UGI-based Kerberos authentication with JAAS-based.

The main logic is

if (param.auth == 'noSasl') {
    // no-sasl
    raw socket/http requrest
} else if (!param.principal) {
    // sasl/plain
    auth by param.username, param.password
} else if (param.principal && param.kerberosAuthType == 'fromSubject') {
    [0]
} else if (param.principal && param.kyuubiClientPrincipal && param.kyuubiClientKeytab) {
    [1]
} else if (param.principal && !param.kyuubiClientPrincipal && !param.kyuubiClientKeytab) {
    [3]
}

- [0][JDK] subject from context
- [1][JDK] principal + keytab
- [2][UGI] principal + keytab (no required, use [1] instead)
- [3][JDK] principal + tgt cache
- [4][UGI] principal + tgt cache (no required, use [3] instead)

To achieve this, we need to introduce new JDBC parameters kyuubiClientPrincipal and kyuubiClientKeytab.

The above description is pure JDK-based and totally removed Hadoop dependencies from Kyuubi Hive JDBC driver.

To minimize the scope of this PR, I separate the support of delegation token to a follow-up PR #3096, and it still needs UGI.

How was this patch tested?

Add some test cases that check the changes thoroughly including negative and positive cases if possible
Add screenshots for manual tests if appropriate
Run test locally before make a pull request

codecov-commenter · 2022-07-06T09:40:05Z

Codecov Report

Merging #3023 (caf6bef) into master (d5dae09) will decrease coverage by 0.14%.
The diff coverage is 1.69%.

❗ Current head caf6bef differs from pull request most recent head 7b45f47. Consider uploading reports for the commit 7b45f47 to get more accurate results

@@             Coverage Diff              @@
##             master    #3023      +/-   ##
============================================
- Coverage     51.26%   51.11%   -0.15%     
  Complexity        6        6              
============================================
  Files           458      458              
  Lines         25416    25495      +79     
  Branches       3535     3544       +9     
============================================
+ Hits          13030    13033       +3     
- Misses        11144    11219      +75     
- Partials       1242     1243       +1

Impacted Files	Coverage Δ
.../org/apache/kyuubi/jdbc/hive/KyuubiConnection.java	`4.11% <0.00%> (-0.08%)`	⬇️
.../jdbc/hive/auth/CachingKerberosAuthentication.java	`0.00% <0.00%> (ø)`
...a/org/apache/kyuubi/jdbc/hive/auth/HadoopShim.java	`0.00% <0.00%> (ø)`
...rg/apache/kyuubi/jdbc/hive/auth/HttpAuthUtils.java	`0.00% <0.00%> (ø)`
...yuubi/jdbc/hive/auth/HttpBasicAuthInterceptor.java	`0.00% <ø> (ø)`
...jdbc/hive/auth/HttpKerberosRequestInterceptor.java	`0.00% <0.00%> (ø)`
...ubi/jdbc/hive/auth/HttpRequestInterceptorBase.java	`0.00% <0.00%> (ø)`
...yuubi/jdbc/hive/auth/HttpTokenAuthInterceptor.java	`0.00% <ø> (ø)`
...ubi/jdbc/hive/auth/HttpXsrfRequestInterceptor.java	`0.00% <0.00%> (ø)`
.../kyuubi/jdbc/hive/auth/KerberosAuthentication.java	`0.00% <0.00%> (ø)`
... and 52 more

Help us with your feedback. Take ten seconds to tell us how you rate us.

...hive-jdbc/src/main/java/org/apache/kyuubi/jdbc/hive/auth/HttpKerberosRequestInterceptor.java

pan3793 · 2022-07-15T01:57:19Z

cc @yaooqinn @zhouyifan279 @cxzl25 @turboFei

### _Why are the changes needed?_ This PR is separated from #3023, to make #3023 change more clear. It is a pure code refactor, and should not break any functionality and public API. ### _How was this patch tested?_ - [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible - [ ] Add screenshots for manual tests if appropriate - [x] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request Closes #3094 from pan3793/noisy. Closes #3094 0cf2ac2 [Cheng Pan] Minor refactor Authored-by: Cheng Pan <chengpan@apache.org> Signed-off-by: Cheng Pan <chengpan@apache.org>

pan3793 · 2022-07-18T12:25:36Z

@yaooqinn for review convince, this PR has been split into multiple parts, I updated the PR description to reflect the changes, please take a look when you have time.

cxzl25 · 2022-07-21T03:34:41Z

kyuubi-hive-jdbc/src/main/java/org/apache/kyuubi/jdbc/hive/KyuubiConnection.java

  public static final Logger LOG = LoggerFactory.getLogger(KyuubiConnection.class.getName());
  public static final String BEELINE_MODE_PROPERTY = "BEELINE_MODE";
  public static final String HS2_PROXY_USER = "hive.server2.proxy.user";
-  public static final String HS2_CLIENT_TOKEN = "hiveserver2ClientToken";


n this PR we do not support the delegationToken authentication method, we will add it back in the subsequent PR #3096, right?

…instead

pan3793 · 2022-07-26T18:32:35Z

Since #3096 got approved, I'm going to merge this PR, thanks all for helping reviews.

… authentication w/ JAAS ### _Why are the changes needed?_ 1. `principal` supports `X/_HOSTEXAMPLE.COM` 2. `kyuubiClientPrincipal` supports headless keytab, `XEXAMPLE.COM` #3023 ### _How was this patch tested?_ - [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible - [ ] Add screenshots for manual tests if appropriate - [x] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request Closes #3346 from cxzl25/3023_followup. Closes #3023 1530929 [sychen] support principal _HOST and kyuubiClientPrincipal headless keytab Authored-by: sychen <sychen@ctrip.com> Signed-off-by: Cheng Pan <chengpan@apache.org>

… authentication w/ JAAS ### _Why are the changes needed?_ 1. `principal` supports `X/_HOSTEXAMPLE.COM` 2. `kyuubiClientPrincipal` supports headless keytab, `XEXAMPLE.COM` #3023 ### _How was this patch tested?_ - [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible - [ ] Add screenshots for manual tests if appropriate - [x] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request Closes #3346 from cxzl25/3023_followup. Closes #3023 1530929 [sychen] support principal _HOST and kyuubiClientPrincipal headless keytab Authored-by: sychen <sychen@ctrip.com> Signed-off-by: Cheng Pan <chengpan@apache.org> (cherry picked from commit 2b122ac) Signed-off-by: Cheng Pan <chengpan@apache.org>

turboFei · 2023-03-08T03:58:13Z

hi @pan3793

see that ugi.doAs does not work after this change if KRB5CCNAME env is set and does not set kerberosAuthType=fromSubject explicitly.

turboFei · 2023-03-08T03:58:29Z

cc @gabrywu

pan3793 · 2023-03-08T05:23:39Z

@turboFei @gabrywu thanks for reporting this issue, raised #4479 to fix it, please take a look

…doAs ### _Why are the changes needed?_ A typical use case of Hadoop UGI w/ Kyuubi Hive JDBC is ``` UserGroupInformation ugi = UserGroupInformation.getCurrentUser(); ugi.doAs(() -> { Connection conn = DriverManager.getConnection( "jdbc:kyuubi://host:10009/default;principal=kyuubi_HOST/ABC.ORG"); ... }); ``` After #3023, Kyuubi Hive JDBC implements the Kerberos authentication by using JDK directly instead of Hadoop `UserGroupInformation`, but it also introduce a breaking change for Hadoop users, including the above case. As workaround, user should add `kerberosAuthType=fromSubject` alongside w/ `principal=kyuubi_HOST/ABC.ORG` to make it work. This PR propose to restore the behavior before #3023 by handling UGI.doAs explicitly. And this PR makes the `clientPrincipal` `clientKeytab` as the highest priority, so in below cases, `clientPrincipal` `clientKeytab` take effects instead of UGI. ``` UserGroupInformation ugi = UserGroupInformation.getCurrentUser(); ugi.doAs(() -> { Connection conn = DriverManager.getConnection( "jdbc:kyuubi://host:10009/default;principal=kyuubi_HOST/ABC.ORG;" + "clientPrincipal=tom_HOST/ABC.ORG;clientKeytab=/path/xxx.keytab"); ... }); ``` ### _How was this patch tested?_ - [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible - [ ] Add screenshots for manual tests if appropriate - [ ] [Run test](https://kyuubi.readthedocs.io/en/master/develop_tools/testing.html#running-tests) locally before make a pull request Closes #4479 from pan3793/detect-ugi. Closes #4479 0e169ab [Cheng Pan] nit 19036e3 [Cheng Pan] reorder e8faf9c [Cheng Pan] Restore JDBC kerberos authentication behavior for UGI.doAs Authored-by: Cheng Pan <chengpan@apache.org> Signed-off-by: fwang12 <fwang12@ebay.com>

…doAs ### _Why are the changes needed?_ A typical use case of Hadoop UGI w/ Kyuubi Hive JDBC is ``` UserGroupInformation ugi = UserGroupInformation.getCurrentUser(); ugi.doAs(() -> { Connection conn = DriverManager.getConnection( "jdbc:kyuubi://host:10009/default;principal=kyuubi_HOST/ABC.ORG"); ... }); ``` After #3023, Kyuubi Hive JDBC implements the Kerberos authentication by using JDK directly instead of Hadoop `UserGroupInformation`, but it also introduce a breaking change for Hadoop users, including the above case. As workaround, user should add `kerberosAuthType=fromSubject` alongside w/ `principal=kyuubi_HOST/ABC.ORG` to make it work. This PR propose to restore the behavior before #3023 by handling UGI.doAs explicitly. And this PR makes the `clientPrincipal` `clientKeytab` as the highest priority, so in below cases, `clientPrincipal` `clientKeytab` take effects instead of UGI. ``` UserGroupInformation ugi = UserGroupInformation.getCurrentUser(); ugi.doAs(() -> { Connection conn = DriverManager.getConnection( "jdbc:kyuubi://host:10009/default;principal=kyuubi_HOST/ABC.ORG;" + "clientPrincipal=tom_HOST/ABC.ORG;clientKeytab=/path/xxx.keytab"); ... }); ``` ### _How was this patch tested?_ - [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible - [ ] Add screenshots for manual tests if appropriate - [ ] [Run test](https://kyuubi.readthedocs.io/en/master/develop_tools/testing.html#running-tests) locally before make a pull request Closes #4479 from pan3793/detect-ugi. Closes #4479 0e169ab [Cheng Pan] nit 19036e3 [Cheng Pan] reorder e8faf9c [Cheng Pan] Restore JDBC kerberos authentication behavior for UGI.doAs Authored-by: Cheng Pan <chengpan@apache.org> Signed-off-by: fwang12 <fwang12@ebay.com> (cherry picked from commit 17466ea) Signed-off-by: fwang12 <fwang12@ebay.com>

github-actions bot added module:ctl module:hive module:server labels Jul 6, 2022

github-actions bot added the kind:build label Jul 7, 2022

pan3793 force-pushed the ugi branch 2 times, most recently from 6a4feea to 1256321 Compare July 14, 2022 06:54

github-actions bot added the module:common label Jul 14, 2022

pan3793 force-pushed the ugi branch 3 times, most recently from c81a4ea to f3f7233 Compare July 14, 2022 12:30

pan3793 commented Jul 14, 2022

View reviewed changes

...hive-jdbc/src/main/java/org/apache/kyuubi/jdbc/hive/auth/HttpKerberosRequestInterceptor.java Show resolved Hide resolved

pan3793 marked this pull request as ready for review July 15, 2022 01:56

pan3793 force-pushed the ugi branch 5 times, most recently from 5b48c62 to d44d0ee Compare July 18, 2022 07:38

pan3793 mentioned this pull request Jul 18, 2022

Code refactor on Kyuubi Hive JDBC driver #3094

Closed

3 tasks

pan3793 force-pushed the ugi branch from d44d0ee to caf6bef Compare July 18, 2022 07:50

pan3793 force-pushed the ugi branch from caf6bef to 57a676b Compare July 18, 2022 12:02

pan3793 changed the title ~~Decouple Kyuubi JDBC Kerberos from UGI~~ Kyuubi Hive JDBC: Replace UGI-based Kerberos authentication by JAAS Jul 18, 2022

pan3793 mentioned this pull request Jul 18, 2022

Kyuubi Hive JDBC: UGI-based delegation token authentication #3096

Closed

3 tasks

pan3793 changed the title ~~Kyuubi Hive JDBC: Replace UGI-based Kerberos authentication by JAAS~~ Kyuubi Hive JDBC: Replace UGI-based Kerberos authentication w/ JAAS Jul 18, 2022

pan3793 self-assigned this Jul 18, 2022

pan3793 added this to the v1.6.0 milestone Jul 18, 2022

cxzl25 reviewed Jul 21, 2022

View reviewed changes

pan3793 added 2 commits July 27, 2022 01:50

Add util method isCommandAvailable

3876c3d

Remove UGI in JDBC driver and use JAAS-based Kerberos authentication …

7b45f47

…instead

pan3793 force-pushed the ugi branch from 57a676b to 7b45f47 Compare July 26, 2022 18:18

pan3793 closed this in 8778209 Jul 26, 2022

pan3793 deleted the ugi branch July 26, 2022 20:03

cxzl25 mentioned this pull request Aug 25, 2022

[KYUUBI #3023][FOLLOWUP] Kyuubi Hive JDBC: Replace UGI-based Kerberos authentication w/ JAAS #3346

Closed

3 tasks

pan3793 mentioned this pull request Dec 2, 2022

[Improvement] Provide kyuubiServerPrincipal as alias for principal in JDBC parameter #3887

Closed

5 tasks

pan3793 mentioned this pull request Mar 8, 2023

Restore JDBC Kerberos authentication behavior for UGI.doAs #4479

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Kyuubi Hive JDBC: Replace UGI-based Kerberos authentication w/ JAAS#3023

Kyuubi Hive JDBC: Replace UGI-based Kerberos authentication w/ JAAS#3023
pan3793 wants to merge 2 commits intoapache:masterfrom
pan3793:ugi

pan3793 commented Jul 6, 2022 •

edited

Loading

Uh oh!

codecov-commenter commented Jul 6, 2022 •

edited

Loading

Uh oh!

Uh oh!

pan3793 commented Jul 15, 2022

Uh oh!

pan3793 commented Jul 18, 2022

Uh oh!

cxzl25 Jul 21, 2022

Uh oh!

pan3793 Jul 21, 2022

Uh oh!

pan3793 commented Jul 26, 2022

Uh oh!

turboFei commented Mar 8, 2023

Uh oh!

turboFei commented Mar 8, 2023

Uh oh!

pan3793 commented Mar 8, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

pan3793 commented Jul 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why are the changes needed?

How was this patch tested?

Uh oh!

codecov-commenter commented Jul 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

pan3793 commented Jul 15, 2022

Uh oh!

pan3793 commented Jul 18, 2022

Uh oh!

cxzl25 Jul 21, 2022

Choose a reason for hiding this comment

Uh oh!

pan3793 Jul 21, 2022

Choose a reason for hiding this comment

Uh oh!

pan3793 commented Jul 26, 2022

Uh oh!

turboFei commented Mar 8, 2023

Uh oh!

turboFei commented Mar 8, 2023

Uh oh!

pan3793 commented Mar 8, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

pan3793 commented Jul 6, 2022 •

edited

Loading

codecov-commenter commented Jul 6, 2022 •

edited

Loading