Clarify CBS/CBB with respect to high tag number form.

We may need to implement high tag number form someday. CBS_get_asn1 has an unsigned output to allow for this, but CBB_add_asn1 takes a uint8_t (I think this might be my fault). Fix that which also fixes a -Wconversion warning. Simply leaving room in tag representation will still cause troubles because the class and constructed bits overlap with bits for tag numbers above 31. Probably the cleanest option would be to shift them to the top 3 bits of a u32 and thus not quite match the DER representation. Then CBS_get_asn1 and CBB_add_asn1 will internally munge that into the DER representation and consumers may continue to write things like: tag_number | CBS_ASN1_CONTEXT_SPECIFIC I haven't done that here, but in preparation for that, document that consumers need to use the values and should refrain from assuming the correspond to DER. Change-Id: Ibc76e51f0bc3b843e48e89adddfe2eaba4843d12 Reviewed-on: https://boringssl-review.googlesource.com/10502 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: Adam Langley <[email protected]> CQ-Verified: CQ bot account: [email protected] <[email protected]>
aws · Aug 26, 2016 · 1db42fb · 1db42fb
1 parent a6cd185
commit 1db42fb
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 6 deletions.
diff --git a/STYLE.md b/STYLE.md
@@ -159,7 +159,7 @@ For example,
     /* CBB_add_asn sets |*out_contents| to a |CBB| into which the contents of an
      * ASN.1 object can be written. The |tag| argument will be used as the tag for
      * the object. It returns one on success or zero on error. */
-    OPENSSL_EXPORT int CBB_add_asn1(CBB *cbb, CBB *out_contents, uint8_t tag);
+    OPENSSL_EXPORT int CBB_add_asn1(CBB *cbb, CBB *out_contents, unsigned tag);
 
 
 ## Documentation

diff --git a/crypto/bytestring/cbb.c b/crypto/bytestring/cbb.c
@@ -327,15 +327,18 @@ int CBB_add_u24_length_prefixed(CBB *cbb, CBB *out_contents) {
   return cbb_add_length_prefixed(cbb, out_contents, 3);
 }
 
-int CBB_add_asn1(CBB *cbb, CBB *out_contents, uint8_t tag) {
-  if ((tag & 0x1f) == 0x1f) {
-    /* Long form identifier octets are not supported. */
+int CBB_add_asn1(CBB *cbb, CBB *out_contents, unsigned tag) {
+  if (tag > 0xff ||
+      (tag & 0x1f) == 0x1f) {
+    /* Long form identifier octets are not supported. Further, all current valid
+     * tag serializations are 8 bits. */
     cbb->base->error = 1;
     return 0;
   }
 
   if (!CBB_flush(cbb) ||
-      !CBB_add_u8(cbb, tag)) {
+      /* |tag|'s representation matches the DER encoding. */
+      !CBB_add_u8(cbb, (uint8_t)tag)) {
     return 0;
   }
 

diff --git a/include/openssl/bytestring.h b/include/openssl/bytestring.h
@@ -125,6 +125,7 @@ OPENSSL_EXPORT int CBS_get_u24_length_prefixed(CBS *cbs, CBS *out);
 
 /* Parsing ASN.1 */
 
+/* The following values are tag numbers for UNIVERSAL elements. */
 #define CBS_ASN1_BOOLEAN 0x1
 #define CBS_ASN1_INTEGER 0x2
 #define CBS_ASN1_BITSTRING 0x3
@@ -148,8 +149,27 @@ OPENSSL_EXPORT int CBS_get_u24_length_prefixed(CBS *cbs, CBS *out);
 #define CBS_ASN1_UNIVERSALSTRING 0x1c
 #define CBS_ASN1_BMPSTRING 0x1e
 
+/* CBS_ASN1_CONSTRUCTED may be ORed into a tag to toggle the constructed
+ * bit. |CBS| and |CBB| APIs consider the constructed bit to be part of the
+ * tag. */
 #define CBS_ASN1_CONSTRUCTED 0x20
+
+/* The following values specify the constructed bit or tag class and may be ORed
+ * into a tag number to produce the final tag. If none is used, the tag will be
+ * UNIVERSAL.
+ *
+ * Note that although they currently match the DER serialization, consumers must
+ * use these bits rather than make assumptions about the representation. This is
+ * to allow for tag numbers beyond 31 in the future. */
+#define CBS_ASN1_APPLICATION 0x40
 #define CBS_ASN1_CONTEXT_SPECIFIC 0x80
+#define CBS_ASN1_PRIVATE 0xc0
+
+/* CBS_ASN1_CLASS_MASK may be ANDed with a tag to query its class. */
+#define CBS_ASN1_CLASS_MASK 0xc0
+
+/* CBS_ASN1_TAG_NUMBER_MASK may be ANDed with a tag to query its number. */
+#define CBS_ASN1_TAG_NUMBER_MASK 0x1f
 
 /* CBS_get_asn1 sets |*out| to the contents of DER-encoded, ASN.1 element (not
  * including tag and length bytes) and advances |cbs| over it. The ASN.1
@@ -345,7 +365,7 @@ OPENSSL_EXPORT int CBB_add_u24_length_prefixed(CBB *cbb, CBB *out_contents);
  * the object. Passing in |tag| number 31 will return in an error since only
  * single octet identifiers are supported. It returns one on success or zero
  * on error. */
-OPENSSL_EXPORT int CBB_add_asn1(CBB *cbb, CBB *out_contents, uint8_t tag);
+OPENSSL_EXPORT int CBB_add_asn1(CBB *cbb, CBB *out_contents, unsigned tag);
 
 /* CBB_add_bytes appends |len| bytes from |data| to |cbb|. It returns one on
  * success and zero otherwise. */