You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org> on 2021/07/09 02:46:54 UTC

[Impala-ASF-CR] IMPALA-10489 part2: Support RSASSA-PSS and EC Algorithms for JWT

Wenzhe Zhou has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17663


Change subject: IMPALA-10489 part2: Support RSASSA-PSS and EC Algorithms for JWT
......................................................................

IMPALA-10489 part2: Support RSASSA-PSS and EC Algorithms for JWT

This patch added additional JSON Web Algorithms (JWA) for JWT
authentication, including RSASSA-PSS algorithms - PS256/PS384/PS512,
and Elliptic Curve family of algorithms - ES256/ES384/ES512.
JWTs can be signed using a public/private key pair using RSA or ECDSA.

Added BE unit-tests for PS256, PS384, PS512, ES256, ES384, and ES512
algorithms.

Testing:
 - Passed core run.
 - Passed BE test jwt-util-test with ASAN build.

Change-Id: Ib4dc30c51f503b609dd311ce4387080abc5a0832
---
M be/src/util/jwt-util-internal.h
M be/src/util/jwt-util-test.cc
M be/src/util/jwt-util.cc
3 files changed, 550 insertions(+), 6 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/63/17663/1
-- 
To view, visit http://gerrit.cloudera.org:8080/17663
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib4dc30c51f503b609dd311ce4387080abc5a0832
Gerrit-Change-Number: 17663
Gerrit-PatchSet: 1
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>

[Impala-ASF-CR] IMPALA-10489 part2: Support RSASSA-PSS and EC Algorithms for JWT

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/17663 )

Change subject: IMPALA-10489 part2: Support RSASSA-PSS and EC Algorithms for JWT
......................................................................


Patch Set 2: Code-Review+2

(1 comment)

This looks good. Thanks!

http://gerrit.cloudera.org:8080/#/c/17663/1/be/src/util/jwt-util.cc
File be/src/util/jwt-util.cc:

http://gerrit.cloudera.org:8080/#/c/17663/1/be/src/util/jwt-util.cc@313
PS1, Line 313:     if (algorithm == "rs256") {
             :       jwt_pub_key = new RS256JWTPublicKey(algorithm, pub_key);
             :     } else if (algorithm == "rs384") {
             :       jwt_pub_key = new RS384JWTPublicKey(algorithm, pub_key);
             :     } else if (algorithm == "rs512") {
             :       jwt_pub_key = new RS512JWTPublicKey(algorithm, pub_key);
             :     } else if (algorithm == "ps256") {
             :       jwt_pub_key = new PS256JWTPublicKey(algorithm, pub_key);
             :     } else if (algorithm == "ps384") {
             :       jwt_pub_key = new PS384JWTPublicKey(algorithm, pub_key);
             :     } else if (algorithm == "ps512") {
             :       jwt_pub_key = new PS512JWTPublicKey(algorithm, pub_key);
> replaced compare() with "==". Tried "switch" for string, but got compiling 
Oh, I must have been thinking of a different language.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4dc30c51f503b609dd311ce4387080abc5a0832
Gerrit-Change-Number: 17663
Gerrit-PatchSet: 2
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 15 Jul 2021 03:48:38 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10489 part2: Support RSASSA-PSS and EC Algorithms for JWT

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17663 )

Change subject: IMPALA-10489 part2: Support RSASSA-PSS and EC Algorithms for JWT
......................................................................


Patch Set 2:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/7300/ DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4dc30c51f503b609dd311ce4387080abc5a0832
Gerrit-Change-Number: 17663
Gerrit-PatchSet: 2
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 15 Jul 2021 04:01:12 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10489 part2: Support RSASSA-PSS and EC Algorithms for JWT

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17663 )

Change subject: IMPALA-10489 part2: Support RSASSA-PSS and EC Algorithms for JWT
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9094/ : 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/17663
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4dc30c51f503b609dd311ce4387080abc5a0832
Gerrit-Change-Number: 17663
Gerrit-PatchSet: 2
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 14 Jul 2021 16:16:06 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10489 part2: Support RSASSA-PSS and EC Algorithms for JWT

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/17663 )

Change subject: IMPALA-10489 part2: Support RSASSA-PSS and EC Algorithms for JWT
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/17663/1/be/src/util/jwt-util-test.cc
File be/src/util/jwt-util-test.cc:

http://gerrit.cloudera.org:8080/#/c/17663/1/be/src/util/jwt-util-test.cc@634
PS1, Line 634:   TempTestDataFile jwks_file(Substitute(jwks_rsa_file_format, kid_1, "PS384",
             :       rsa_pub_key_jwk_n, rsa_pub_key_jwk_e, kid_2, "PS384", rsa_invalid_pub_key_jwk_n,
             :       rsa_pub_key_jwk_e));
> I'm seeing that PS256/PS384/PS512 use the same RSA public key to initialize
Added 3 different sizes of RSA keys, and use 1024 bits RSA key for PS256, 2048 bits RSA key for PS384, and 4096 bits RSA key for PS512.


http://gerrit.cloudera.org:8080/#/c/17663/1/be/src/util/jwt-util.cc
File be/src/util/jwt-util.cc:

http://gerrit.cloudera.org:8080/#/c/17663/1/be/src/util/jwt-util.cc@313
PS1, Line 313:     if (algorithm.compare("rs256") == 0) {
             :       jwt_pub_key = new RS256JWTPublicKey(algorithm, pub_key);
             :     } else if (algorithm.compare("rs384") == 0) {
             :       jwt_pub_key = new RS384JWTPublicKey(algorithm, pub_key);
             :     } else if (algorithm.compare("rs512") == 0) {
             :       jwt_pub_key = new RS512JWTPublicKey(algorithm, pub_key);
             :     } else if (algorithm.compare("ps256") == 0) {
             :       jwt_pub_key = new PS256JWTPublicKey(algorithm, pub_key);
             :     } else if (algorithm.compare("ps384") == 0) {
             :       jwt_pub_key = new PS384JWTPublicKey(algorithm, pub_key);
             :     } else if (algorithm.compare("ps512") == 0) {
             :       jwt_pub_key = new PS512JWTPublicKey(algorithm, pub_key);
> Nit: Very minor thing, but the C++ std library implements the "==" relation
replaced compare() with "==". Tried "switch" for string, but got compiling error. It seems we cannot simply use "switch" for string statement. One work around is to convert string to enum.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4dc30c51f503b609dd311ce4387080abc5a0832
Gerrit-Change-Number: 17663
Gerrit-PatchSet: 1
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 14 Jul 2021 15:50:44 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10489 part2: Support RSASSA-PSS and EC Algorithms for JWT

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17663 )

Change subject: IMPALA-10489 part2: Support RSASSA-PSS and EC Algorithms for JWT
......................................................................


Patch Set 3: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4dc30c51f503b609dd311ce4387080abc5a0832
Gerrit-Change-Number: 17663
Gerrit-PatchSet: 3
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 15 Jul 2021 22:37:51 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10489 part2: Support RSASSA-PSS and EC Algorithms for JWT

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17663 )

Change subject: IMPALA-10489 part2: Support RSASSA-PSS and EC Algorithms for JWT
......................................................................


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4dc30c51f503b609dd311ce4387080abc5a0832
Gerrit-Change-Number: 17663
Gerrit-PatchSet: 3
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 15 Jul 2021 16:16:10 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10489 part2: Support RSASSA-PSS and EC Algorithms for JWT

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/17663 )

Change subject: IMPALA-10489 part2: Support RSASSA-PSS and EC Algorithms for JWT
......................................................................


Patch Set 2:

Verification failed due to "java.io.IOException: No space left on device"


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4dc30c51f503b609dd311ce4387080abc5a0832
Gerrit-Change-Number: 17663
Gerrit-PatchSet: 2
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 15 Jul 2021 15:46:08 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10489 part2: Support RSASSA-PSS and EC Algorithms for JWT

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17663 )

Change subject: IMPALA-10489 part2: Support RSASSA-PSS and EC Algorithms for JWT
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9054/ : 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/17663
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4dc30c51f503b609dd311ce4387080abc5a0832
Gerrit-Change-Number: 17663
Gerrit-PatchSet: 1
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Fri, 09 Jul 2021 03:09:15 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10489 part2: Support RSASSA-PSS and EC Algorithms for JWT

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17663 )

Change subject: IMPALA-10489 part2: Support RSASSA-PSS and EC Algorithms for JWT
......................................................................


Patch Set 2: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4dc30c51f503b609dd311ce4387080abc5a0832
Gerrit-Change-Number: 17663
Gerrit-PatchSet: 2
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 15 Jul 2021 08:39:32 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10489 part2: Support RSASSA-PSS and EC Algorithms for JWT

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17663 )

Change subject: IMPALA-10489 part2: Support RSASSA-PSS and EC Algorithms for JWT
......................................................................


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/7304/ DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4dc30c51f503b609dd311ce4387080abc5a0832
Gerrit-Change-Number: 17663
Gerrit-PatchSet: 3
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 15 Jul 2021 16:16:11 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10489 part2: Support RSASSA-PSS and EC Algorithms for JWT

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17663 )

Change subject: IMPALA-10489 part2: Support RSASSA-PSS and EC Algorithms for JWT
......................................................................

IMPALA-10489 part2: Support RSASSA-PSS and EC Algorithms for JWT

This patch added additional JSON Web Algorithms (JWA) for JWT
authentication, including RSASSA-PSS algorithms - PS256/PS384/PS512,
and Elliptic Curve family of algorithms - ES256/ES384/ES512.
JWTs can be signed using a public/private key pair using RSA or ECDSA.

Added BE unit-tests for PS256, PS384, PS512, ES256, ES384, and ES512
algorithms.

Testing:
 - Passed core run.
 - Passed BE test jwt-util-test with ASAN build.

Change-Id: Ib4dc30c51f503b609dd311ce4387080abc5a0832
Reviewed-on: http://gerrit.cloudera.org:8080/17663
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/util/jwt-util-internal.h
M be/src/util/jwt-util-test.cc
M be/src/util/jwt-util.cc
3 files changed, 701 insertions(+), 16 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib4dc30c51f503b609dd311ce4387080abc5a0832
Gerrit-Change-Number: 17663
Gerrit-PatchSet: 4
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-10489 part2: Support RSASSA-PSS and EC Algorithms for JWT

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/17663 )

Change subject: IMPALA-10489 part2: Support RSASSA-PSS and EC Algorithms for JWT
......................................................................

IMPALA-10489 part2: Support RSASSA-PSS and EC Algorithms for JWT

This patch added additional JSON Web Algorithms (JWA) for JWT
authentication, including RSASSA-PSS algorithms - PS256/PS384/PS512,
and Elliptic Curve family of algorithms - ES256/ES384/ES512.
JWTs can be signed using a public/private key pair using RSA or ECDSA.

Added BE unit-tests for PS256, PS384, PS512, ES256, ES384, and ES512
algorithms.

Testing:
 - Passed core run.
 - Passed BE test jwt-util-test with ASAN build.

Change-Id: Ib4dc30c51f503b609dd311ce4387080abc5a0832
---
M be/src/util/jwt-util-internal.h
M be/src/util/jwt-util-test.cc
M be/src/util/jwt-util.cc
3 files changed, 701 insertions(+), 16 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib4dc30c51f503b609dd311ce4387080abc5a0832
Gerrit-Change-Number: 17663
Gerrit-PatchSet: 2
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-10489 part2: Support RSASSA-PSS and EC Algorithms for JWT

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/17663 )

Change subject: IMPALA-10489 part2: Support RSASSA-PSS and EC Algorithms for JWT
......................................................................


Patch Set 1:

(2 comments)

Thank you for this follow-on change. I only have minor nits.

http://gerrit.cloudera.org:8080/#/c/17663/1/be/src/util/jwt-util-test.cc
File be/src/util/jwt-util-test.cc:

http://gerrit.cloudera.org:8080/#/c/17663/1/be/src/util/jwt-util-test.cc@634
PS1, Line 634:   TempTestDataFile jwks_file(Substitute(jwks_rsa_file_format, kid_1, "PS384",
             :       rsa_pub_key_jwk_n, rsa_pub_key_jwk_e, kid_2, "PS384", rsa_invalid_pub_key_jwk_n,
             :       rsa_pub_key_jwk_e));
I'm seeing that PS256/PS384/PS512 use the same RSA public key to initialize the JWKS file. What are the rules around that? Would we get any testing value from having different keys for each size?


http://gerrit.cloudera.org:8080/#/c/17663/1/be/src/util/jwt-util.cc
File be/src/util/jwt-util.cc:

http://gerrit.cloudera.org:8080/#/c/17663/1/be/src/util/jwt-util.cc@313
PS1, Line 313:     if (algorithm.compare("rs256") == 0) {
             :       jwt_pub_key = new RS256JWTPublicKey(algorithm, pub_key);
             :     } else if (algorithm.compare("rs384") == 0) {
             :       jwt_pub_key = new RS384JWTPublicKey(algorithm, pub_key);
             :     } else if (algorithm.compare("rs512") == 0) {
             :       jwt_pub_key = new RS512JWTPublicKey(algorithm, pub_key);
             :     } else if (algorithm.compare("ps256") == 0) {
             :       jwt_pub_key = new PS256JWTPublicKey(algorithm, pub_key);
             :     } else if (algorithm.compare("ps384") == 0) {
             :       jwt_pub_key = new PS384JWTPublicKey(algorithm, pub_key);
             :     } else if (algorithm.compare("ps512") == 0) {
             :       jwt_pub_key = new PS512JWTPublicKey(algorithm, pub_key);
Nit: Very minor thing, but the C++ std library implements the "==" relational operator for strings. For exact matching, you can do "str1 == str2" directly. (compare() could still be useful for substring matching.)
https://www.cplusplus.com/reference/string/string/operators/

You also have the option of using a switch statement like:

switch (algorithm) {
case "rs256":
   etc etc
   break;
...
default:
   return Status(etc);
}

I'm ok with either, but I would prefer one of these as opposed to compare() across these files. (But this is minor and I'm open to disagreement.)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4dc30c51f503b609dd311ce4387080abc5a0832
Gerrit-Change-Number: 17663
Gerrit-PatchSet: 1
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Tue, 13 Jul 2021 23:27:42 +0000
Gerrit-HasComments: Yes