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>

crypto/bn/bn_asn1.c

@@ -50,18 +50,9 @@ int BN_cbs2unsigned_buggy(CBS *cbs, BIGNUM *ret) {
     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;

crypto/bn/bn_test.cc

@@ -1774,15 +1774,16 @@ static const ASN1InvalidTest kASN1InvalidTests[] = {
     {"\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 @@ static bool test_asn1() {
     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;

crypto/evp/p_rsa_asn1.c

@@ -92,9 +92,8 @@ static int rsa_pub_decode(EVP_PKEY *pkey, X509_PUBKEY *pubkey) {
     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

crypto/rsa/rsa_asn1.c

@@ -116,9 +116,8 @@ RSA *RSA_parse_public_key(CBS *cbs) {
 }
 
 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 */);

include/openssl/bn.h

@@ -304,9 +304,8 @@ OPENSSL_EXPORT BN_ULONG BN_get_word(const BIGNUM *bn);
  * 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

include/openssl/rsa.h

@@ -339,7 +339,7 @@ OPENSSL_EXPORT int RSA_add_pkcs1_prefix(uint8_t **out_msg, size_t *out_msg_len,
 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