Work around even more Estonian ID card misissuances.
Not content with signing negative RSA moduli, still other Estonian IDs have too
many leading zeros. Work around those too.
This workaround will be removed in six months.
BUG=534766
Change-Id: Ica23b1b1499f9dbe39e94cf7b540900860e8e135
Reviewed-on: https://boringssl-review.googlesource.com/5980
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/bn/bn_asn1.c b/crypto/bn/bn_asn1.c
index c84884c..9d70ba8 100644
--- a/crypto/bn/bn_asn1.c
+++ b/crypto/bn/bn_asn1.c
@@ -50,18 +50,9 @@
return 0;
}
- /* INTEGERs must be minimal. */
- if (CBS_data(&child)[0] == 0x00 &&
- CBS_len(&child) > 1 &&
- !(CBS_data(&child)[1] & 0x80)) {
- OPENSSL_PUT_ERROR(BN, BN_R_BAD_ENCODING);
- return 0;
- }
-
- /* This function intentionally does not reject negative numbers. Estonian IDs
- * issued between September 2014 to September 2015 are broken and use negative
- * moduli. They last five years and are common enough that we need to work
- * around this bug. See https://crbug.com/532048.
+ /* This function intentionally does not reject negative numbers or non-minimal
+ * encodings. Estonian IDs issued between September 2014 to September 2015 are
+ * broken. See https://crbug.com/532048 and https://crbug.com/534766.
*
* TODO(davidben): Remove this code and callers in March 2016. */
return BN_bin2bn(CBS_data(&child), CBS_len(&child), ret) != NULL;
diff --git a/crypto/bn/bn_test.cc b/crypto/bn/bn_test.cc
index 7bcdb39..47093a7 100644
--- a/crypto/bn/bn_test.cc
+++ b/crypto/bn/bn_test.cc
@@ -1774,15 +1774,16 @@
{"\x03\x01\x00", 3},
// Empty contents.
{"\x02\x00", 2},
- // Unnecessary leading zeros.
- {"\x02\x02\x00\x01", 4},
};
-// kASN1NegativeTests are encodings of negative numbers and how
-// |BN_cbs2unsigned_buggy| should interpret them.
-static const ASN1Test kASN1NegativeTests[] = {
+// kASN1BuggyTests are incorrect encodings and how |BN_cbs2unsigned_buggy|
+// should interpret them.
+static const ASN1Test kASN1BuggyTests[] = {
+ // Negative numbers.
{"128", "\x02\x01\x80", 3},
{"255", "\x02\x01\xff", 3},
+ // Unnecessary leading zeros.
+ {"1", "\x02\x02\x00\x01", 4},
};
static bool test_asn1() {
@@ -1861,8 +1862,8 @@
ERR_clear_error();
}
- for (const ASN1Test &test : kASN1NegativeTests) {
- // Negative numbers are rejected by |BN_cbs2unsigned|.
+ for (const ASN1Test &test : kASN1BuggyTests) {
+ // These broken encodings are rejected by |BN_cbs2unsigned|.
ScopedBIGNUM bn(BN_new());
if (!bn) {
return false;
diff --git a/crypto/evp/p_rsa_asn1.c b/crypto/evp/p_rsa_asn1.c
index 891e256..f60625b 100644
--- a/crypto/evp/p_rsa_asn1.c
+++ b/crypto/evp/p_rsa_asn1.c
@@ -92,9 +92,8 @@
return 0;
}
- /* Estonian IDs issued between September 2014 to September 2015 are broken and
- * use negative moduli. They last five years and are common enough that we
- * need to work around this bug. See https://crbug.com/532048.
+ /* Estonian IDs issued between September 2014 to September 2015 are
+ * broken. See https://crbug.com/532048 and https://crbug.com/534766.
*
* TODO(davidben): Switch this to the strict version in March 2016 or when
* Chromium can force client certificates down a different codepath, whichever
diff --git a/crypto/rsa/rsa_asn1.c b/crypto/rsa/rsa_asn1.c
index 100b2b4..e3756ba 100644
--- a/crypto/rsa/rsa_asn1.c
+++ b/crypto/rsa/rsa_asn1.c
@@ -116,9 +116,8 @@
}
RSA *RSA_parse_public_key_buggy(CBS *cbs) {
- /* Estonian IDs issued between September 2014 to September 2015 are broken and
- * use negative moduli. They last five years and are common enough that we
- * need to work around this bug. See https://crbug.com/532048.
+ /* Estonian IDs issued between September 2014 to September 2015 are
+ * broken. See https://crbug.com/532048 and https://crbug.com/534766.
*
* TODO(davidben): Remove this code and callers in March 2016. */
return parse_public_key(cbs, 1 /* buggy */);
diff --git a/include/openssl/bn.h b/include/openssl/bn.h
index d85ec86..8de8cc4 100644
--- a/include/openssl/bn.h
+++ b/include/openssl/bn.h
@@ -304,9 +304,8 @@
* result to |ret|. It returns one on success and zero on failure. */
OPENSSL_EXPORT int BN_cbs2unsigned(CBS *cbs, BIGNUM *ret);
-/* BN_cbs2unsigned_buggy acts like |BN_cbs2unsigned| but tolerates negative
- * INTEGERs. This is to work around buggy encoders which drop the 00 padding
- * byte. Do not use this function. */
+/* BN_cbs2unsigned_buggy acts like |BN_cbs2unsigned| but tolerates some invalid
+ * encodings. Do not use this function. */
OPENSSL_EXPORT int BN_cbs2unsigned_buggy(CBS *cbs, BIGNUM *ret);
/* BN_bn2cbb marshals |bn| as a non-negative DER INTEGER and appends the result
diff --git a/include/openssl/rsa.h b/include/openssl/rsa.h
index 8e361da..2be50dc 100644
--- a/include/openssl/rsa.h
+++ b/include/openssl/rsa.h
@@ -339,7 +339,7 @@
OPENSSL_EXPORT RSA *RSA_parse_public_key(CBS *cbs);
/* RSA_parse_public_key_buggy behaves like |RSA_parse_public_key|, but it
- * tolerates negative moduli. Do not use this function. */
+ * tolerates some invalid encodings. Do not use this function. */
OPENSSL_EXPORT RSA *RSA_parse_public_key_buggy(CBS *cbs);
/* RSA_public_key_from_bytes parses |in| as a DER-encoded RSAPublicKey structure
