Also add a decoupled OBJ_obj2txt.
We need it in both directions. Also I missed that in OBJ_obj2txt we
allowed uint64_t components, but in my new OBJ_txt2obj we only allowed
uint32_t. For consistency, upgrade that to uint64_t.
Bug: chromium:706445
Change-Id: I38cfeea8ff64b9acf7998e552727c6c3b2cc600f
Reviewed-on: https://boringssl-review.googlesource.com/23544
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/crypto/bytestring/bytestring_test.cc b/crypto/bytestring/bytestring_test.cc
index 6ce7035..1969e73 100644
--- a/crypto/bytestring/bytestring_test.cc
+++ b/crypto/bytestring/bytestring_test.cc
@@ -889,54 +889,95 @@
TEST(CBBTest, AddOIDFromText) {
const struct {
- const char *in;
- bool ok;
- std::vector<uint8_t> out;
- } kTests[] = {
+ const char *text;
+ std::vector<uint8_t> der;
+ } kValidOIDs[] = {
// Some valid values.
- {"1.2.3.4", true, {0x2a, 0x3, 0x4}},
+ {"0.0", {0x00}},
+ {"0.2.3.4", {0x2, 0x3, 0x4}},
+ {"1.2.3.4", {0x2a, 0x3, 0x4}},
+ {"2.2.3.4", {0x52, 0x3, 0x4}},
{"1.2.840.113554.4.1.72585",
- true,
{0x2a, 0x86, 0x48, 0x86, 0xf7, 0x12, 0x04, 0x01, 0x84, 0xb7, 0x09}},
// Test edge cases around the first component.
- {"0.39", true, {0x27}},
- {"0.40", false, {}},
- {"1.0", true, {0x28}},
- {"1.39", true, {0x4f}},
- {"1.40", false, {}},
- {"2.0", true, {0x50}},
- {"2.1", true, {0x51}},
- {"2.40", true, {0x78}},
- // The empty string is not an OID.
- {"", false, {}},
- // No empty components.
- {".1.2.3.4.5", false, {}},
- {"1..2.3.4.5", false, {}},
- {"1.2.3.4.5.", false, {}},
- // There must be at least two components.
- {"1", false, {}},
- // No extra leading zeros.
- {"00.1.2.3.4", false, {}},
- {"01.1.2.3.4", false, {}},
- // Check for overflow.
- {"1.2.4294967295", true, {0x2a, 0x8f, 0xff, 0xff, 0xff, 0x7f}},
- {"1.2.4294967296", false, {}},
- // 40*A + B overflows.
- {"2.4294967215", true, {0x8f, 0xff, 0xff, 0xff, 0x7f}},
- {"2.4294967216", false, {}},
+ {"0.39", {0x27}},
+ {"1.0", {0x28}},
+ {"1.39", {0x4f}},
+ {"2.0", {0x50}},
+ {"2.1", {0x51}},
+ {"2.40", {0x78}},
+ // Edge cases near an overflow.
+ {"1.2.18446744073709551615",
+ {0x2a, 0x81, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x7f}},
+ {"2.18446744073709551535",
+ {0x81, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x7f}},
};
- for (const auto &t : kTests) {
- SCOPED_TRACE(t.in);
+
+ const char *kInvalidTexts[] = {
+ // Invalid second component.
+ "0.40",
+ "1.40",
+ // Invalid first component.
+ "3.1",
+ // The empty string is not an OID.
+ "",
+ // No empty components.
+ ".1.2.3.4.5",
+ "1..2.3.4.5",
+ "1.2.3.4.5.",
+ // There must be at least two components.
+ "1",
+ // No extra leading zeros.
+ "00.1.2.3.4",
+ "01.1.2.3.4",
+ // Overflow for both components or 40*A + B.
+ "1.2.18446744073709551616",
+ "2.18446744073709551536",
+ };
+
+ const std::vector<uint8_t> kInvalidDER[] = {
+ // The empty string is not an OID.
+ {},
+ // Non-minimal representation.
+ {0x80, 0x01},
+ // Overflow. This is the DER representation of
+ // 1.2.840.113554.4.1.72585.18446744073709551616. (The final value is
+ // 2^64.)
+ {0x2a, 0x86, 0x48, 0x86, 0xf7, 0x12, 0x04, 0x01, 0x84, 0xb7, 0x09,
+ 0x82, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x00},
+ };
+
+ for (const auto &t : kValidOIDs) {
+ SCOPED_TRACE(t.text);
+
bssl::ScopedCBB cbb;
ASSERT_TRUE(CBB_init(cbb.get(), 0));
- int ok = CBB_add_asn1_oid_from_text(cbb.get(), t.in, strlen(t.in));
- EXPECT_EQ(t.ok, static_cast<bool>(ok));
- if (ok) {
- uint8_t *out;
- size_t len;
- ASSERT_TRUE(CBB_finish(cbb.get(), &out, &len));
- bssl::UniquePtr<uint8_t> free_out(out);
- EXPECT_EQ(Bytes(t.out), Bytes(out, len));
- }
+ ASSERT_TRUE(CBB_add_asn1_oid_from_text(cbb.get(), t.text, strlen(t.text)));
+ uint8_t *out;
+ size_t len;
+ ASSERT_TRUE(CBB_finish(cbb.get(), &out, &len));
+ bssl::UniquePtr<uint8_t> free_out(out);
+ EXPECT_EQ(Bytes(t.der), Bytes(out, len));
+
+ CBS cbs;
+ CBS_init(&cbs, t.der.data(), t.der.size());
+ bssl::UniquePtr<char> text(CBS_asn1_oid_to_text(&cbs));
+ ASSERT_TRUE(text.get());
+ EXPECT_STREQ(t.text, text.get());
+ }
+
+ for (const char *t : kInvalidTexts) {
+ SCOPED_TRACE(t);
+ bssl::ScopedCBB cbb;
+ ASSERT_TRUE(CBB_init(cbb.get(), 0));
+ EXPECT_FALSE(CBB_add_asn1_oid_from_text(cbb.get(), t, strlen(t)));
+ }
+
+ for (const auto &t : kInvalidDER) {
+ SCOPED_TRACE(Bytes(t));
+ CBS cbs;
+ CBS_init(&cbs, t.data(), t.size());
+ bssl::UniquePtr<char> text(CBS_asn1_oid_to_text(&cbs));
+ EXPECT_FALSE(text);
}
}
diff --git a/crypto/bytestring/cbb.c b/crypto/bytestring/cbb.c
index 5a1c45b..b12a66c 100644
--- a/crypto/bytestring/cbb.c
+++ b/crypto/bytestring/cbb.c
@@ -332,9 +332,9 @@
// add_base128_integer encodes |v| as a big-endian base-128 integer where the
// high bit of each byte indicates where there is more data. This is the
// encoding used in DER for both high tag number form and OID components.
-static int add_base128_integer(CBB *cbb, uint32_t v) {
+static int add_base128_integer(CBB *cbb, uint64_t v) {
unsigned len_len = 0;
- unsigned copy = v;
+ uint64_t copy = v;
while (copy > 0) {
len_len++;
copy >>= 7;
@@ -508,7 +508,7 @@
// an OID literal, e.g., "1.2.840.113554.4.1.72585". It consumes both the
// component and the dot, so |cbs| may be passed into the function again for the
// next value.
-static int parse_dotted_decimal(CBS *cbs, uint32_t *out) {
+static int parse_dotted_decimal(CBS *cbs, uint64_t *out) {
*out = 0;
int seen_digit = 0;
for (;;) {
@@ -524,8 +524,8 @@
// Forbid stray leading zeros.
(seen_digit && *out == 0) ||
// Check for overflow.
- *out > UINT32_MAX / 10 ||
- *out * 10 > UINT32_MAX - (u - '0')) {
+ *out > UINT64_MAX / 10 ||
+ *out * 10 > UINT64_MAX - (u - '0')) {
return 0;
}
*out = *out * 10 + (u - '0');
@@ -544,7 +544,7 @@
CBS_init(&cbs, (const uint8_t *)text, len);
// OIDs must have at least two components.
- uint32_t a, b;
+ uint64_t a, b;
if (!parse_dotted_decimal(&cbs, &a) ||
!parse_dotted_decimal(&cbs, &b)) {
return 0;
@@ -554,8 +554,8 @@
// 0, 1, or 2 and that, when it is 0 or 1, |b| is at most 39.
if (a > 2 ||
(a < 2 && b > 39) ||
- b > UINT32_MAX - 80 ||
- !add_base128_integer(cbb, 40 * a + b)) {
+ b > UINT64_MAX - 80 ||
+ !add_base128_integer(cbb, 40u * a + b)) {
return 0;
}
diff --git a/crypto/bytestring/cbs.c b/crypto/bytestring/cbs.c
index d96371c..e456330 100644
--- a/crypto/bytestring/cbs.c
+++ b/crypto/bytestring/cbs.c
@@ -12,11 +12,16 @@
* OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
* CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */
+#if !defined(__STDC_FORMAT_MACROS)
+#define __STDC_FORMAT_MACROS
+#endif
+
#include <openssl/buf.h>
#include <openssl/mem.h>
#include <openssl/bytestring.h>
#include <assert.h>
+#include <inttypes.h>
#include <string.h>
#include "internal.h"
@@ -175,6 +180,33 @@
return cbs_get_length_prefixed(cbs, out, 3);
}
+// parse_base128_integer reads a big-endian base-128 integer from |cbs| and sets
+// |*out| to the result. This is the encoding used in DER for both high tag
+// number form and OID components.
+static int parse_base128_integer(CBS *cbs, uint64_t *out) {
+ uint64_t v = 0;
+ uint8_t b;
+ do {
+ if (!CBS_get_u8(cbs, &b)) {
+ return 0;
+ }
+ if ((v >> (64 - 7)) != 0) {
+ // The value is too large.
+ return 0;
+ }
+ if (v == 0 && b == 0x80) {
+ // The value must be minimally encoded.
+ return 0;
+ }
+ v = (v << 7) | (b & 0x7f);
+
+ // Values end at an octet with the high bit cleared.
+ } while (b & 0x80);
+
+ *out = v;
+ return 1;
+}
+
static int parse_asn1_tag(CBS *cbs, unsigned *out) {
uint8_t tag_byte;
if (!CBS_get_u8(cbs, &tag_byte)) {
@@ -191,27 +223,15 @@
unsigned tag = ((unsigned)tag_byte & 0xe0) << CBS_ASN1_TAG_SHIFT;
unsigned tag_number = tag_byte & 0x1f;
if (tag_number == 0x1f) {
- tag_number = 0;
- for (;;) {
- if (!CBS_get_u8(cbs, &tag_byte) ||
- ((tag_number << 7) >> 7) != tag_number) {
- return 0;
- }
- tag_number = (tag_number << 7) | (tag_byte & 0x7f);
- // The tag must be represented in the minimal number of bytes.
- if (tag_number == 0) {
- return 0;
- }
- if ((tag_byte & 0x80) == 0) {
- break;
- }
- }
- if (// Check the tag number is within our supported bounds.
- tag_number > CBS_ASN1_TAG_NUMBER_MASK ||
+ uint64_t v;
+ if (!parse_base128_integer(cbs, &v) ||
+ // Check the tag number is within our supported bounds.
+ v > CBS_ASN1_TAG_NUMBER_MASK ||
// Small tag numbers should have used low tag number form.
- tag_number < 0x1f) {
+ v < 0x1f) {
return 0;
}
+ tag_number = (unsigned)v;
}
tag |= tag_number;
@@ -527,3 +547,55 @@
return byte_num < CBS_len(cbs) &&
(CBS_data(cbs)[byte_num] & (1 << bit_num)) != 0;
}
+
+static int add_decimal(CBB *out, uint64_t v) {
+ char buf[DECIMAL_SIZE(uint64_t) + 1];
+ BIO_snprintf(buf, sizeof(buf), "%" PRIu64, v);
+ return CBB_add_bytes(out, (const uint8_t *)buf, strlen(buf));
+}
+
+char *CBS_asn1_oid_to_text(const CBS *cbs) {
+ CBB cbb;
+ if (!CBB_init(&cbb, 32)) {
+ goto err;
+ }
+
+ CBS copy = *cbs;
+ // The first component is 40 * value1 + value2, where value1 is 0, 1, or 2.
+ uint64_t v;
+ if (!parse_base128_integer(©, &v)) {
+ goto err;
+ }
+
+ if (v >= 80) {
+ if (!CBB_add_bytes(&cbb, (const uint8_t *)"2.", 2) ||
+ !add_decimal(&cbb, v - 80)) {
+ goto err;
+ }
+ } else if (!add_decimal(&cbb, v / 40) ||
+ !CBB_add_u8(&cbb, '.') ||
+ !add_decimal(&cbb, v % 40)) {
+ goto err;
+ }
+
+ while (CBS_len(©) != 0) {
+ if (!parse_base128_integer(©, &v) ||
+ !CBB_add_u8(&cbb, '.') ||
+ !add_decimal(&cbb, v)) {
+ goto err;
+ }
+ }
+
+ uint8_t *txt;
+ size_t txt_len;
+ if (!CBB_add_u8(&cbb, '\0') ||
+ !CBB_finish(&cbb, &txt, &txt_len)) {
+ goto err;
+ }
+
+ return (char *)txt;
+
+err:
+ CBB_cleanup(&cbb);
+ return NULL;
+}
diff --git a/crypto/obj/obj.c b/crypto/obj/obj.c
index 8e1e6b1..a34d6dc 100644
--- a/crypto/obj/obj.c
+++ b/crypto/obj/obj.c
@@ -434,36 +434,6 @@
return (int)ret;
}
-static int parse_oid_component(CBS *cbs, uint64_t *out) {
- uint64_t v = 0;
- uint8_t b;
- do {
- if (!CBS_get_u8(cbs, &b)) {
- return 0;
- }
- if ((v >> (64 - 7)) != 0) {
- // The component is too large.
- return 0;
- }
- if (v == 0 && b == 0x80) {
- // The component must be minimally encoded.
- return 0;
- }
- v = (v << 7) | (b & 0x7f);
-
- // Components end at an octet with the high bit cleared.
- } while (b & 0x80);
-
- *out = v;
- return 1;
-}
-
-static int add_decimal(CBB *out, uint64_t v) {
- char buf[DECIMAL_SIZE(uint64_t) + 1];
- BIO_snprintf(buf, sizeof(buf), "%" PRIu64, v);
- return CBB_add_bytes(out, (const uint8_t *)buf, strlen(buf));
-}
-
int OBJ_obj2txt(char *out, int out_len, const ASN1_OBJECT *obj,
int always_return_oid) {
// Python depends on the empty OID successfully encoding as the empty
@@ -485,56 +455,19 @@
}
}
- CBB cbb;
- if (!CBB_init(&cbb, 32)) {
- goto err;
- }
-
CBS cbs;
CBS_init(&cbs, obj->data, obj->length);
-
- // The first component is 40 * value1 + value2, where value1 is 0, 1, or 2.
- uint64_t v;
- if (!parse_oid_component(&cbs, &v)) {
- goto err;
- }
-
- if (v >= 80) {
- if (!CBB_add_bytes(&cbb, (const uint8_t *)"2.", 2) ||
- !add_decimal(&cbb, v - 80)) {
- goto err;
+ char *txt = CBS_asn1_oid_to_text(&cbs);
+ if (txt == NULL) {
+ if (out_len > 0) {
+ out[0] = '\0';
}
- } else if (!add_decimal(&cbb, v / 40) ||
- !CBB_add_u8(&cbb, '.') ||
- !add_decimal(&cbb, v % 40)) {
- goto err;
+ return -1;
}
- while (CBS_len(&cbs) != 0) {
- if (!parse_oid_component(&cbs, &v) ||
- !CBB_add_u8(&cbb, '.') ||
- !add_decimal(&cbb, v)) {
- goto err;
- }
- }
-
- uint8_t *txt;
- size_t txt_len;
- if (!CBB_add_u8(&cbb, '\0') ||
- !CBB_finish(&cbb, &txt, &txt_len)) {
- goto err;
- }
-
- int ret = strlcpy_int(out, (const char *)txt, out_len);
+ int ret = strlcpy_int(out, txt, out_len);
OPENSSL_free(txt);
return ret;
-
-err:
- CBB_cleanup(&cbb);
- if (out_len > 0) {
- out[0] = '\0';
- }
- return -1;
}
static uint32_t hash_nid(const ASN1_OBJECT *obj) {
diff --git a/include/openssl/bytestring.h b/include/openssl/bytestring.h
index abf8885..43349e9 100644
--- a/include/openssl/bytestring.h
+++ b/include/openssl/bytestring.h
@@ -279,6 +279,13 @@
// is indexed starting from zero.
OPENSSL_EXPORT int CBS_asn1_bitstring_has_bit(const CBS *cbs, unsigned bit);
+// CBS_asn1_oid_to_text interprets |cbs| as DER-encoded ASN.1 OBJECT IDENTIFIER
+// contents (not including the element framing) and returns the ASCII
+// representation (e.g., "1.2.840.113554.4.1.72585") in a newly-allocated
+// string, or NULL on failure. The caller must release the result with
+// |OPENSSL_free|.
+OPENSSL_EXPORT char *CBS_asn1_oid_to_text(const CBS *cbs);
+
// CRYPTO ByteBuilder.
//
@@ -450,7 +457,7 @@
// the element's contents.
//
// This function considers OID strings with components which do not fit in a
-// |uint32_t| to be invalid.
+// |uint64_t| to be invalid.
OPENSSL_EXPORT int CBB_add_asn1_oid_from_text(CBB *cbb, const char *text,
size_t len);
