google-oss-prow[bot]
commented
1 year ago [APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: davidben, nareddyt
The full list of commands accepted by this bot can be found here.
The pull request process is described here
Needs approval from an approver in each of these files:
- ~~[OWNERS](https://github.com/google/jwt_verify_lib/blob/master/OWNERS)~~ [nareddyt]
Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
(This imports the Google-internal change cl/532158532.)
This change is necessary to avoid build errors with future versions of BoringSSL. Reaching into private structs isn't supported and we'll be, matching OpenSSL, hiding them in the future.
Also add a call to RSA_check_key, to reject invalid keys earlier. If BoringSSL considers the keys invalid, no signatures will be accepted. This should have been part of RSA object initializtion, but due to some poor OpenSSL API decisions, we're stuck with this sort of multi-step initialization.
(RSA_new and RSA_set0_key are effectively infallible, so this is kinda silly. RSA_new can only fail on malloc failure. RSA_set0_key can only fail on programmer error. But I didn't see any uses of CHECK in this library, so I just added the error checks. If there's a simpler pattern to handle programmer errors, happy to switch to that instead.)
BoringSSL has a much better API available now, RSA_new_public_key, which does the whole thing in a single function, and avoids the awkward ownership transfer when RSA_set0_key fails. I didn't use it for two reasons. First, this project's BoringSSL version hasn't been updated for three years (https://github.com/google/jwt_verify_lib/issues/97). Second, this project seems to be used in Envoy and AIUI Envoy needs support some very old BoringSSL revisions. So, this PR uses the less convenient APIs for now and leaves the TODO that, perhaps in a year or two, can be resolved.