Add CBB_add_asn1_[u]int64_with_tag. CBB_add_asn1_uint64 doesn't work if you're encoding an implicitly-tagged INTEGER. Take a leaf from Go cryptobyte and add a "with tag" variant, rather than a "contents" variant, which is a little more convenient to use. It also avoids us having to decide how to name the contents field. Change-Id: I6072e55017230c513577c44c5a7ed86e778255b3 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54685 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com> Auto-Submit: David Benjamin <davidben@google.com>
diff --git a/crypto/bytestring/bytestring_test.cc b/crypto/bytestring/bytestring_test.cc index b6b716e..27aae79 100644 --- a/crypto/bytestring/bytestring_test.cc +++ b/crypto/bytestring/bytestring_test.cc
@@ -862,12 +862,28 @@ EXPECT_EQ(0, is_negative); EXPECT_TRUE(CBS_is_unsigned_asn1_integer(&child)); - bssl::ScopedCBB cbb; - ASSERT_TRUE(CBB_init(cbb.get(), 0)); - ASSERT_TRUE(CBB_add_asn1_uint64(cbb.get(), test.value)); - ASSERT_TRUE(CBB_finish(cbb.get(), &out, &len)); - bssl::UniquePtr<uint8_t> scoper(out); - EXPECT_EQ(Bytes(test.encoding, test.encoding_len), Bytes(out, len)); + { + bssl::ScopedCBB cbb; + ASSERT_TRUE(CBB_init(cbb.get(), 0)); + ASSERT_TRUE(CBB_add_asn1_uint64(cbb.get(), test.value)); + ASSERT_TRUE(CBB_finish(cbb.get(), &out, &len)); + bssl::UniquePtr<uint8_t> scoper(out); + EXPECT_EQ(Bytes(test.encoding, test.encoding_len), Bytes(out, len)); + } + + { + // Overwrite the tag. + bssl::ScopedCBB cbb; + ASSERT_TRUE(CBB_init(cbb.get(), 0)); + ASSERT_TRUE(CBB_add_asn1_uint64_with_tag(cbb.get(), test.value, + CBS_ASN1_CONTEXT_SPECIFIC | 1)); + ASSERT_TRUE(CBB_finish(cbb.get(), &out, &len)); + bssl::UniquePtr<uint8_t> scoper(out); + std::vector<uint8_t> expected(test.encoding, + test.encoding + test.encoding_len); + expected[0] = 0x81; + EXPECT_EQ(Bytes(expected), Bytes(out, len)); + } } for (const ASN1InvalidUint64Test &test : kASN1InvalidUint64Tests) { @@ -952,12 +968,28 @@ EXPECT_EQ(test.value < 0, !!is_negative); EXPECT_EQ(test.value >= 0, !!CBS_is_unsigned_asn1_integer(&child)); - bssl::ScopedCBB cbb; - ASSERT_TRUE(CBB_init(cbb.get(), 0)); - ASSERT_TRUE(CBB_add_asn1_int64(cbb.get(), test.value)); - ASSERT_TRUE(CBB_finish(cbb.get(), &out, &len)); - bssl::UniquePtr<uint8_t> scoper(out); - EXPECT_EQ(Bytes(test.encoding, test.encoding_len), Bytes(out, len)); + { + bssl::ScopedCBB cbb; + ASSERT_TRUE(CBB_init(cbb.get(), 0)); + ASSERT_TRUE(CBB_add_asn1_int64(cbb.get(), test.value)); + ASSERT_TRUE(CBB_finish(cbb.get(), &out, &len)); + bssl::UniquePtr<uint8_t> scoper(out); + EXPECT_EQ(Bytes(test.encoding, test.encoding_len), Bytes(out, len)); + } + + { + // Overwrite the tag. + bssl::ScopedCBB cbb; + ASSERT_TRUE(CBB_init(cbb.get(), 0)); + ASSERT_TRUE(CBB_add_asn1_int64_with_tag(cbb.get(), test.value, + CBS_ASN1_CONTEXT_SPECIFIC | 1)); + ASSERT_TRUE(CBB_finish(cbb.get(), &out, &len)); + bssl::UniquePtr<uint8_t> scoper(out); + std::vector<uint8_t> expected(test.encoding, + test.encoding + test.encoding_len); + expected[0] = 0x81; + EXPECT_EQ(Bytes(expected), Bytes(out, len)); + } } for (const ASN1InvalidInt64Test &test : kASN1InvalidInt64Tests) {
diff --git a/crypto/bytestring/cbb.c b/crypto/bytestring/cbb.c index 6ce20ad..100c713 100644 --- a/crypto/bytestring/cbb.c +++ b/crypto/bytestring/cbb.c
@@ -503,13 +503,16 @@ } int CBB_add_asn1_uint64(CBB *cbb, uint64_t value) { - CBB child; - int started = 0; + return CBB_add_asn1_uint64_with_tag(cbb, value, CBS_ASN1_INTEGER); +} - if (!CBB_add_asn1(cbb, &child, CBS_ASN1_INTEGER)) { +int CBB_add_asn1_uint64_with_tag(CBB *cbb, uint64_t value, unsigned tag) { + CBB child; + if (!CBB_add_asn1(cbb, &child, tag)) { return 0; } + int started = 0; for (size_t i = 0; i < 8; i++) { uint8_t byte = (value >> 8*(7-i)) & 0xff; if (!started) { @@ -538,8 +541,12 @@ } int CBB_add_asn1_int64(CBB *cbb, int64_t value) { + return CBB_add_asn1_int64_with_tag(cbb, value, CBS_ASN1_INTEGER); +} + +int CBB_add_asn1_int64_with_tag(CBB *cbb, int64_t value, unsigned tag) { if (value >= 0) { - return CBB_add_asn1_uint64(cbb, value); + return CBB_add_asn1_uint64_with_tag(cbb, (uint64_t)value, tag); } uint8_t bytes[sizeof(int64_t)]; @@ -551,7 +558,7 @@ } CBB child; - if (!CBB_add_asn1(cbb, &child, CBS_ASN1_INTEGER)) { + if (!CBB_add_asn1(cbb, &child, tag)) { return 0; } for (int i = start; i >= 0; i--) {
diff --git a/include/openssl/bytestring.h b/include/openssl/bytestring.h index 1d47f60..33a417c 100644 --- a/include/openssl/bytestring.h +++ b/include/openssl/bytestring.h
@@ -557,11 +557,23 @@ // error. OPENSSL_EXPORT int CBB_add_asn1_uint64(CBB *cbb, uint64_t value); +// CBB_add_asn1_uint64_with_tag behaves like |CBB_add_asn1_uint64| but uses +// |tag| as the tag instead of INTEGER. This is useful if the INTEGER type uses +// implicit tagging. +OPENSSL_EXPORT int CBB_add_asn1_uint64_with_tag(CBB *cbb, uint64_t value, + unsigned tag); + // CBB_add_asn1_int64 writes an ASN.1 INTEGER into |cbb| using |CBB_add_asn1| // and writes |value| in its contents. It returns one on success and zero on // error. OPENSSL_EXPORT int CBB_add_asn1_int64(CBB *cbb, int64_t value); +// CBB_add_asn1_int64_with_tag behaves like |CBB_add_asn1_int64| but uses |tag| +// as the tag instead of INTEGER. This is useful if the INTEGER type uses +// implicit tagging. +OPENSSL_EXPORT int CBB_add_asn1_int64_with_tag(CBB *cbb, int64_t value, + unsigned tag); + // CBB_add_asn1_octet_string writes an ASN.1 OCTET STRING into |cbb| with the // given contents. It returns one on success and zero on error. OPENSSL_EXPORT int CBB_add_asn1_octet_string(CBB *cbb, const uint8_t *data,