-
Notifications
You must be signed in to change notification settings - Fork 118
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Map certs with ITUT X509 to our RSA implementation #1754
Conversation
// RSA ITU-T X.509. These are rare and we map them to the more | ||
// common |NID_rsaEncryption| instead for simplicity. | ||
{NID_md5WithRSA, NID_md5, NID_rsaEncryption}, | ||
{NID_sha1WithRSA, NID_sha1, NID_rsaEncryption}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested this change and can confirm that this addition to the kTriples table resolves this s2n-tls issue: aws/s2n-tls#4691
dc48cd1
to
7efc93d
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1754 +/- ##
==========================================
- Coverage 78.56% 78.55% -0.01%
==========================================
Files 583 583
Lines 98807 98808 +1
Branches 14161 14160 -1
==========================================
- Hits 77623 77621 -2
- Misses 20558 20559 +1
- Partials 626 628 +2 ☔ View full report in Codecov by Sentry. |
73adc70
to
a483ea9
Compare
a483ea9
to
ecbfa2a
Compare
ecbfa2a
to
9a12688
Compare
There exists a rarer OID of RSA public keys which comes from the ITU-T version of X.509 (links below). Links: https://www.itu.int/ITU-T/formal-language/itu-t/x/x509/2008/AlgorithmObjectIdentifiers.html * https://www.ivi.co/jsse/2009/06/13/0220.html OpenSSL accepts the `sign_nid` and `pkey_nid` for this draft. * `NID_md5WithRSA` * `NID_sha1WithRSA` * `NID_rsa` We can parse both `sign_nid`s correctly. But we lack the logic to interpret `NID_rsa` in a certificate or map the parsed `sign_nid`s to any RSA implementation. OpenSSL maps the corresponding nids from the same draft, which means `NID_md5WithRSA` and `NID_sha1WithRSA` [are mapped](https://github.com/openssl/openssl/blob/8e0d479b98357bb20ab1bd073cf75f7d42531553/crypto/objects/obj_xref.h#L30-L32) to `NID_rsa` and an additional `EVP_PKEY_RSA2` is used to represent `NID_rsa`. The additional `EVP_PKEY` type introduces additional complexities however and the underlying functionality in `EVP_PKEY_RSA2` is [identical](https://github.com/openssl/openssl/blob/8e0d479b98357bb20ab1bd073cf75f7d42531553/crypto/rsa/rsa_ameth.c#L965-L1012) to it's more common `EVP_PKEY_RSA` counterpart. We've been burned by multiple OpenSSL cert parsing differences in the past, so we should have logic to understand certs that have `NID_rsa`. We don't want to duplicate all the function pointers for `EVP_PKEY_RSA2` however. Since the underlying RSA functions pointers are the same, I think it should be OK for us to 1. Map `NID_md5WithRSA` and `NID_sha1WithRSA` to our RSA implementation (`NID_rsa_encryption`). 2. Map `NID_rsa` to `EVP_PKEY_RSA` when parsing certificates. Unfortunately, there's some stricter parsing in `rsa_pub_decode` that I've had to remove since it doesn't apply to both RFCs for RSA public keys. This is aligning with OpenSSL, which happens to ignore both of the early parameters specified in either. * Documented in aws@f6094e0 * OpenSSL code (which ignores the parameter): https://github.com/openssl/openssl/blob/8e0d479b98357bb20ab1bd073cf75f7d42531553/crypto/rsa/rsa_ameth.c#L76-L90 Although we have `EVP_PKEY_RSA2` and we could map the logic for that to `EVP_PKEY_RSA` in `EVP_PKEY_type`, I've intentionally left that out for now. We still want to discourage people from using this form, we just need to understand how to parse certificates that have it. Relevant historic commits: * Removed support for `NID_rsa`: aws@f6094e0 * Sigalgs mapping removed: aws@720ff53 * RSA stricter parsing introduced: aws@68772b3 (cherry picked from commit 4916fe8)
… version to v2.0.16 (#1893) This is a cherry pick of 4916fe8 to the fips-2022-11-02 branch. The changes are outside the fipsmodule and were modified from the original commit to match the return values of `parse_key_type()` in `evp_asn1.c` without changing the fipsmodule files. Release version is updated to v2.0.16.
Description of changes:
There exists a rarer OID of RSA public keys which comes from the ITU-T version of X.509 (links below).
Links:
OpenSSL accepts the
sign_nid
andpkey_nid
for this draft.NID_md5WithRSA
NID_sha1WithRSA
NID_rsa
We can parse both
sign_nid
s correctly. But we lack the logic to interpretNID_rsa
in a certificate or map the parsedsign_nid
s to any RSA implementation.OpenSSL maps the corresponding nids from the same draft, which means
NID_md5WithRSA
andNID_sha1WithRSA
are mapped toNID_rsa
and an additionalEVP_PKEY_RSA2
is used to representNID_rsa
. The additionalEVP_PKEY
type introduces additional complexities however and the underlying functionality inEVP_PKEY_RSA2
is identical to it's more commonEVP_PKEY_RSA
counterpart.We've been burned by multiple OpenSSL cert parsing differences in the past, so we should have logic to understand certs that have
NID_rsa
. We don't want to duplicate all the function pointers forEVP_PKEY_RSA2
however. Since the underlying RSA functions pointers are the same, I think it should be OK for us toNID_md5WithRSA
andNID_sha1WithRSA
to our RSA implementation (NID_rsa_encryption
).NID_rsa
toEVP_PKEY_RSA
when parsing certificates.Unfortunately, there's some stricter parsing in
rsa_pub_decode
that I've had to remove since it doesn't apply to both RFCs for RSA public keys. This is aligning with OpenSSL, which happens to ignore both of the early parameters specified in either.Call-outs:
Although we have
EVP_PKEY_RSA2
and we could map the logic for that toEVP_PKEY_RSA
inEVP_PKEY_type
, I've intentionally left that out for now. We still want to discourage people from using this form, we just need to understand how to parse certificates that have it.Relevant historic commits:
NID_rsa
: f6094e0Testing:
WIP
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.