gaurav singh has posted comments on this change. ( http://gerrit.cloudera.org:8080/21382 )
Change subject: IMPALA-12559: Support x5c Parameter in JSON Web Keys ...................................................................... Patch Set 7: (8 comments) http://gerrit.cloudera.org:8080/#/c/21382/6/be/src/util/jwt-util.cc File be/src/util/jwt-util.cc: http://gerrit.cloudera.org:8080/#/c/21382/6/be/src/util/jwt-util.cc@51 PS6, Line 51: x5c certificate > Is there hard limit for number of x5s certificate in RFC? As per RFC, there can be a chain of certificates, however at this point, we do not know how to process mulitiple certificates and/or have the examples of jwks with multiple x5c certificates. We plan to add that in the next patch. http://gerrit.cloudera.org:8080/#/c/21382/6/be/src/util/jwt-util.cc@150 PS6, Line 150: r()]; > nit: define a constant Done http://gerrit.cloudera.org:8080/#/c/21382/6/be/src/util/jwt-util.cc@200 PS6, Line 200: ing& nam > this variable is unused Removed. Thanks. http://gerrit.cloudera.org:8080/#/c/21382/6/be/src/util/jwt-util.cc@209 PS6, Line 209: re > nit: extra indent spaces Fixed. Thanks. http://gerrit.cloudera.org:8080/#/c/21382/6/be/src/util/jwt-util.cc@245 PS6, Line 245: error message upon > function return Status, not boolean value Updated the comment. http://gerrit.cloudera.org:8080/#/c/21382/6/be/src/util/jwt-util.cc@256 PS6, Line 256: value[i] = json_value[i].GetString(); \ : } \ : > nit: indent Done http://gerrit.cloudera.org:8080/#/c/21382/6/be/src/util/jwt-util.cc@323 PS6, Line 323: if (it_x5c != kv_map.end()) > since 'x5c' has priority over 'k', could we check 'x5c' before checking 'k' We check each key regardless to validate them. If "k" is invalid, then the whole jwks is incorrect and we will error out. Only at the time of consuming the values, we make a decision as to which one to prioritize. http://gerrit.cloudera.org:8080/#/c/21382/6/be/src/util/jwt-util.cc@385 PS6, Line 385: if (it_x5c != kv_map.end()) : pub_key_str = jwt::helper::convert_base64_der_to_pem(it_x5c->second); : else : pub_key_str = pub_key; : > since 'x5c' has priority over 'n' and 'e', could we check 'x5c' first? We check each key regardless to validate them. If "n" and "e" is invalid, then the whole jwks key is incorrect and we will error out. Only at the time of consuming the values, we make a decision as to which one to prioritize. -- To view, visit http://gerrit.cloudera.org:8080/21382 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I70be6f9f54190544aa005b2644e2ed8db6f6bb74 Gerrit-Change-Number: 21382 Gerrit-PatchSet: 7 Gerrit-Owner: gaurav singh <[email protected]> Gerrit-Reviewer: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Wenzhe Zhou <[email protected]> Gerrit-Reviewer: gaurav singh <[email protected]> Gerrit-Comment-Date: Thu, 02 May 2024 15:12:00 +0000 Gerrit-HasComments: Yes
