otherPrimeInfos is not optional in version 1 RSAPrivateKeys.
Currently, we correctly refuse to parse version 0 multi-prime keys, but we
still parse version 1 two-prime keys. Both should be rejected.
I missed an additional clause in the spec originally. It seems otherPrimeInfos
is marked OPTIONAL not because it is actually optional, but because they wanted
the two RSAPrivateKey forms to share one definition. The prose rules following
the definition imply that otherPrimeInfos' presence is entirely determined by
the version:
* version is the version number, for compatibility with future
revisions of this document. It shall be 0 for this version of the
document, unless multi-prime is used, in which case it shall be 1.
Version ::= INTEGER { two-prime(0), multi(1) }
(CONSTRAINED BY
{-- version must be multi if otherPrimeInfos present --})
and:
* otherPrimeInfos contains the information for the additional primes
r_3, ..., r_u, in order. It shall be omitted if version is 0 and
shall contain at least one instance of OtherPrimeInfo if version
is 1.
Change-Id: I458232a2e20ed68fddcc39c4c45333f33441f70b
Reviewed-on: https://boringssl-review.googlesource.com/7143
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/rsa/rsa_asn1.c b/crypto/rsa/rsa_asn1.c
index 83bae4d..36f6ee0 100644
--- a/crypto/rsa/rsa_asn1.c
+++ b/crypto/rsa/rsa_asn1.c
@@ -233,9 +233,11 @@
goto err;
}
- /* Multi-prime RSA requires a newer version. */
- if (version == kVersionMulti &&
- CBS_peek_asn1_tag(&child, CBS_ASN1_SEQUENCE)) {
+ if (version == kVersionMulti) {
+ /* Although otherPrimeInfos is written as OPTIONAL in RFC 3447, it later
+ * says "[otherPrimeInfos] shall be omitted if version is 0 and shall
+ * contain at least one instance of OtherPrimeInfo if version is 1. The
+ * OPTIONAL is just so both versions share a single definition. */
CBS other_prime_infos;
if (!CBS_get_asn1(&child, &other_prime_infos, CBS_ASN1_SEQUENCE) ||
CBS_len(&other_prime_infos) == 0) {
