Reimplement OBJ_obj2txt.
The old implementation had a lot of size_t/int confusion. It also
accepted non-minimally-encoded OIDs. Unlike the old implementation, the
new one does not fall back to BIGNUMs and does not attempt to
pretty-print OIDs with components which do not fit in a uint64_t. Add
tests for these cases.
With this new implementation, hopefully we'll have a much easier time
enabling MSVC's size_t truncation warning later.
Change-Id: I602102b97cf9b02d874644f8ef67fe9bac70e45e
Reviewed-on: https://boringssl-review.googlesource.com/9131
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/obj/obj.c b/crypto/obj/obj.c
index 65366eb..1e4959d 100644
--- a/crypto/obj/obj.c
+++ b/crypto/obj/obj.c
@@ -54,8 +54,13 @@
* copied and put under another distribution licence
* [including the GNU Public Licence.] */
+#if !defined(__STDC_FORMAT_MACROS)
+#define __STDC_FORMAT_MACROS
+#endif
+
#include <openssl/obj.h>
+#include <inttypes.h>
#include <limits.h>
#include <string.h>
@@ -409,148 +414,109 @@
return op;
}
-int OBJ_obj2txt(char *out, int out_len, const ASN1_OBJECT *obj, int dont_return_name) {
- int i, n = 0, len, nid, first, use_bn;
- BIGNUM *bl;
- unsigned long l;
- const unsigned char *p;
- char tbuf[DECIMAL_SIZE(i) + DECIMAL_SIZE(l) + 2];
-
- if (out && out_len > 0) {
- out[0] = 0;
+static int strlcpy_int(char *dst, const char *src, int dst_size) {
+ size_t ret = BUF_strlcpy(dst, src, dst_size < 0 ? 0 : (size_t)dst_size);
+ if (ret > INT_MAX) {
+ OPENSSL_PUT_ERROR(OBJ, ERR_R_OVERFLOW);
+ return -1;
}
+ return (int)ret;
+}
- if (obj == NULL || obj->data == NULL) {
- return 0;
- }
-
- if (!dont_return_name && (nid = OBJ_obj2nid(obj)) != NID_undef) {
- const char *s;
- s = OBJ_nid2ln(nid);
- if (s == NULL) {
- s = OBJ_nid2sn(nid);
+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 (s) {
- if (out) {
- BUF_strlcpy(out, s, out_len);
+ 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 dont_return_name) {
+ if (!dont_return_name) {
+ int nid = OBJ_obj2nid(obj);
+ if (nid != NID_undef) {
+ const char *name = OBJ_nid2ln(nid);
+ if (name == NULL) {
+ name = OBJ_nid2sn(nid);
}
- return strlen(s);
+ if (name != NULL) {
+ return strlcpy_int(out, name, out_len);
+ }
}
}
- len = obj->length;
- p = obj->data;
+ CBB cbb;
+ if (!CBB_init(&cbb, 32)) {
+ goto err;
+ }
- first = 1;
- bl = NULL;
+ CBS cbs;
+ CBS_init(&cbs, obj->data, obj->length);
- while (len > 0) {
- l = 0;
- use_bn = 0;
- for (;;) {
- unsigned char c = *p++;
- len--;
- if (len == 0 && (c & 0x80)) {
- goto err;
- }
- if (use_bn) {
- if (!BN_add_word(bl, c & 0x7f)) {
- goto err;
- }
- } else {
- l |= c & 0x7f;
- }
- if (!(c & 0x80)) {
- break;
- }
- if (!use_bn && (l > (ULONG_MAX >> 7L))) {
- if (!bl && !(bl = BN_new())) {
- goto err;
- }
- if (!BN_set_word(bl, l)) {
- goto err;
- }
- use_bn = 1;
- }
- if (use_bn) {
- if (!BN_lshift(bl, bl, 7)) {
- goto err;
- }
- } else {
- l <<= 7L;
- }
+ /* 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;
}
+ } else if (!add_decimal(&cbb, v / 40) ||
+ !CBB_add_u8(&cbb, '.') ||
+ !add_decimal(&cbb, v % 40)) {
+ goto err;
+ }
- if (first) {
- first = 0;
- if (l >= 80) {
- i = 2;
- if (use_bn) {
- if (!BN_sub_word(bl, 80)) {
- goto err;
- }
- } else {
- l -= 80;
- }
- } else {
- i = (int)(l / 40);
- l -= (long)(i * 40);
- }
- if (out && out_len > 1) {
- *out++ = i + '0';
- *out = '0';
- out_len--;
- }
- n++;
- }
-
- if (use_bn) {
- char *bndec;
- bndec = BN_bn2dec(bl);
- if (!bndec) {
- goto err;
- }
- i = strlen(bndec);
- if (out) {
- if (out_len > 1) {
- *out++ = '.';
- *out = 0;
- out_len--;
- }
- BUF_strlcpy(out, bndec, out_len);
- if (i > out_len) {
- out += out_len;
- out_len = 0;
- } else {
- out += i;
- out_len -= i;
- }
- }
- n++;
- n += i;
- OPENSSL_free(bndec);
- } else {
- BIO_snprintf(tbuf, sizeof(tbuf), ".%lu", l);
- i = strlen(tbuf);
- if (out && out_len > 0) {
- BUF_strlcpy(out, tbuf, out_len);
- if (i > out_len) {
- out += out_len;
- out_len = 0;
- } else {
- out += i;
- out_len -= i;
- }
- }
- n += i;
+ while (CBS_len(&cbs) != 0) {
+ if (!parse_oid_component(&cbs, &v) ||
+ !CBB_add_u8(&cbb, '.') ||
+ !add_decimal(&cbb, v)) {
+ goto err;
}
}
- BN_free(bl);
- return n;
+ 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);
+ OPENSSL_free(txt);
+ return ret;
err:
- BN_free(bl);
+ CBB_cleanup(&cbb);
+ if (out_len > 0) {
+ out[0] = '\0';
+ }
return -1;
}
diff --git a/crypto/obj/obj_test.cc b/crypto/obj/obj_test.cc
index 9f610bd..8bec2b4 100644
--- a/crypto/obj/obj_test.cc
+++ b/crypto/obj/obj_test.cc
@@ -182,6 +182,51 @@
return false;
}
+ ASN1_OBJECT obj;
+ memset(&obj, 0, sizeof(obj));
+
+ if (OBJ_obj2txt(NULL, 0, &obj, 0) != -1) {
+ fprintf(stderr, "OBJ_obj2txt accepted the empty OID.\n");
+ return false;
+ }
+
+ // kNonMinimalOID is kBasicConstraints with the final component non-minimally
+ // encoded.
+ static const uint8_t kNonMinimalOID[] = {
+ 0x55, 0x1d, 0x80, 0x13,
+ };
+ obj.data = kNonMinimalOID;
+ obj.length = sizeof(kNonMinimalOID);
+ if (OBJ_obj2txt(NULL, 0, &obj, 0) != -1) {
+ fprintf(stderr, "OBJ_obj2txt accepted non-minimal OIDs.\n");
+ return false;
+ }
+
+ // kOverflowOID is the DER representation of
+ // 1.2.840.113554.4.1.72585.18446744073709551616. (The final value is 2^64.)
+ static const uint8_t kOverflowOID[] = {
+ 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x12, 0x04, 0x01, 0x84, 0xb7, 0x09,
+ 0x82, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x00,
+ };
+ obj.data = kOverflowOID;
+ obj.length = sizeof(kOverflowOID);
+ if (OBJ_obj2txt(NULL, 0, &obj, 0) != -1) {
+ fprintf(stderr, "OBJ_obj2txt accepted an OID with a large component.\n");
+ return false;
+ }
+
+ // kInvalidOID is a mis-encoded version of kBasicConstraints with the final
+ // octet having the high bit set.
+ static const uint8_t kInvalidOID[] = {
+ 0x55, 0x1d, 0x93,
+ };
+ obj.data = kInvalidOID;
+ obj.length = sizeof(kInvalidOID);
+ if (OBJ_obj2txt(NULL, 0, &obj, 0) != -1) {
+ fprintf(stderr, "OBJ_obj2txt accepted a mis-encoded OID.\n");
+ return false;
+ }
+
return true;
}
