Fix error-handling for i2a_ASN1_OBJECT.
Some BIO_write failures weren't handled. Otherwise would successfully
write truncated results. The other i2a functions all report -1 on
truncation, so match. While I'm here, write a test to make sure I didn't
break this.
Change-Id: If17d0209e75c15b3f37bceb1cdfb480fd2c62c4d
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49931
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/asn1/a_object.c b/crypto/asn1/a_object.c
index b88f06b..0de3141 100644
--- a/crypto/asn1/a_object.c
+++ b/crypto/asn1/a_object.c
@@ -110,26 +110,37 @@
return OBJ_obj2txt(buf, buf_len, a, 0);
}
+static int write_str(BIO *bp, const char *str)
+{
+ int len = strlen(str);
+ return BIO_write(bp, str, len) == len ? len : -1;
+}
+
int i2a_ASN1_OBJECT(BIO *bp, const ASN1_OBJECT *a)
{
- char buf[80], *p = buf;
- int i;
-
- if ((a == NULL) || (a->data == NULL))
- return (BIO_write(bp, "NULL", 4));
- i = i2t_ASN1_OBJECT(buf, sizeof buf, a);
- if (i > (int)(sizeof(buf) - 1)) {
- p = OPENSSL_malloc(i + 1);
- if (!p)
- return -1;
- i2t_ASN1_OBJECT(p, i + 1, a);
+ if (a == NULL || a->data == NULL) {
+ return write_str(bp, "NULL");
}
- if (i <= 0)
- return BIO_write(bp, "<INVALID>", 9);
- BIO_write(bp, p, i);
- if (p != buf)
- OPENSSL_free(p);
- return (i);
+
+ char buf[80], *allocated = NULL;
+ const char *str = buf;
+ int len = i2t_ASN1_OBJECT(buf, sizeof(buf), a);
+ if (len > (int)sizeof(buf) - 1) {
+ /* The input was truncated. Allocate a buffer that fits. */
+ allocated = OPENSSL_malloc(len + 1);
+ if (allocated == NULL) {
+ return -1;
+ }
+ len = i2t_ASN1_OBJECT(allocated, len + 1, a);
+ str = allocated;
+ }
+ if (len <= 0) {
+ str = "<INVALID>";
+ }
+
+ int ret = write_str(bp, str);
+ OPENSSL_free(allocated);
+ return ret;
}
ASN1_OBJECT *d2i_ASN1_OBJECT(ASN1_OBJECT **a, const unsigned char **pp,
diff --git a/crypto/asn1/asn1_test.cc b/crypto/asn1/asn1_test.cc
index bb2b0a8..1381169 100644
--- a/crypto/asn1/asn1_test.cc
+++ b/crypto/asn1/asn1_test.cc
@@ -1587,6 +1587,88 @@
}
}
+TEST(ASN1Test, PrintASN1Object) {
+ const struct {
+ std::vector<uint8_t> in;
+ const char *expected;
+ } kDataTests[] = {
+ // Known OIDs print as the name.
+ {{0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x01}, "rsaEncryption"},
+
+ // Unknown OIDs print in decimal.
+ {{0x2a, 0x86, 0x48, 0x86, 0xf7, 0x12, 0x04, 0x01, 0x84, 0xb7, 0x09, 0x00},
+ "1.2.840.113554.4.1.72585.0"},
+
+ // Inputs which cannot be parsed as OIDs print as "<INVALID>".
+ {{0xff}, "<INVALID>"},
+
+ // The function has an internal 80-byte buffer. Test inputs at that
+ // boundary. First, 78 characters.
+ {{0x2a, 0x86, 0x48, 0x86, 0xf7, 0x12, 0x04, 0x01, 0x84, 0xb7,
+ 0x09, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01},
+ "1.2.840.113554.4.1.72585.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0."
+ "0.0.0.1"},
+ // 79 characters.
+ {{0x2a, 0x86, 0x48, 0x86, 0xf7, 0x12, 0x04, 0x01, 0x84, 0xb7,
+ 0x09, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0a},
+ "1.2.840.113554.4.1.72585.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0."
+ "0.0.0.10"},
+ // 80 characters.
+ {{0x2a, 0x86, 0x48, 0x86, 0xf7, 0x12, 0x04, 0x01, 0x84, 0xb7,
+ 0x09, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x64},
+ "1.2.840.113554.4.1.72585.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0."
+ "0.0.0.100"},
+ // 81 characters.
+ {{0x2a, 0x86, 0x48, 0x86, 0xf7, 0x12, 0x04, 0x01, 0x84, 0xb7,
+ 0x09, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x87, 0x68},
+ "1.2.840.113554.4.1.72585.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0."
+ "0.0.0.1000"},
+ // 82 characters.
+ {{0x2a, 0x86, 0x48, 0x86, 0xf7, 0x12, 0x04, 0x01, 0x84, 0xb7,
+ 0x09, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xce, 0x10},
+ "1.2.840.113554.4.1.72585.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0."
+ "0.0.0.10000"},
+ };
+ for (const auto &t : kDataTests) {
+ SCOPED_TRACE(Bytes(t.in));
+ bssl::UniquePtr<ASN1_OBJECT> obj(ASN1_OBJECT_create(
+ NID_undef, t.in.data(), t.in.size(), /*sn=*/nullptr, /*ln=*/nullptr));
+ ASSERT_TRUE(obj);
+ bssl::UniquePtr<BIO> bio(BIO_new(BIO_s_mem()));
+ ASSERT_TRUE(bio);
+
+ int len = i2a_ASN1_OBJECT(bio.get(), obj.get());
+ EXPECT_EQ(len, static_cast<int>(strlen(t.expected)));
+
+ const uint8_t *bio_data;
+ size_t bio_len;
+ BIO_mem_contents(bio.get(), &bio_data, &bio_len);
+ EXPECT_EQ(t.expected,
+ std::string(reinterpret_cast<const char *>(bio_data), bio_len));
+ }
+
+ // Test writing NULL.
+ bssl::UniquePtr<BIO> bio(BIO_new(BIO_s_mem()));
+ ASSERT_TRUE(bio);
+ int len = i2a_ASN1_OBJECT(bio.get(), nullptr);
+ EXPECT_EQ(len, 4);
+ const uint8_t *bio_data;
+ size_t bio_len;
+ BIO_mem_contents(bio.get(), &bio_data, &bio_len);
+ EXPECT_EQ("NULL",
+ std::string(reinterpret_cast<const char *>(bio_data), bio_len));
+}
+
// The ASN.1 macros do not work on Windows shared library builds, where usage of
// |OPENSSL_EXPORT| is a bit stricter.
#if !defined(OPENSSL_WINDOWS) || !defined(BORINGSSL_SHARED_LIBRARY)
