DER writer: Add the possibility to abandon empty SEQUENCEs
authorRichard Levitte <levitte@openssl.org>
Sat, 2 May 2020 11:33:24 +0000 (13:33 +0200)
committerRichard Levitte <levitte@openssl.org>
Thu, 14 May 2020 10:16:35 +0000 (12:16 +0200)
In some cases, a SEQUENCE that contains only optional fields may end
up empty.  In some cases, this may be represented by dropping the
SEQUENCE entirely from the encoded DER.

To do this, we detect the case where WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH
is used, and adapt accordingly.

Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/11710)

crypto/der_writer.c

index 26fd88592da612cedaeb8c42a43a47d613fdbea9..876278750470374c1c3577122374082729ff7aed 100644 (file)
@@ -24,12 +24,24 @@ static int int_start_context(WPACKET *pkt, int tag)
 
 static int int_end_context(WPACKET *pkt, int tag)
 {
 
 static int int_end_context(WPACKET *pkt, int tag)
 {
+    /*
+     * If someone set the flag WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH on this
+     * sub-packet and this sub-packet has nothing written to it, the DER length
+     * will not be written, and the total written size will be unchanged before
+     * and after WPACKET_close().  We use size1 and size2 to determine if
+     * anything was written, and only write our tag if it has.
+     *
+     */
+    size_t size1, size2;
+
     if (tag < 0)
         return 1;
     if (!ossl_assert(tag <= 30))
         return 0;
     if (tag < 0)
         return 1;
     if (!ossl_assert(tag <= 30))
         return 0;
-    return WPACKET_close(pkt)
-        && WPACKET_put_bytes_u8(pkt, DER_C_CONTEXT | tag);
+    return WPACKET_get_total_written(pkt, &size1)
+        && WPACKET_close(pkt)
+        && WPACKET_get_total_written(pkt, &size2)
+        && (size1 == size2 || WPACKET_put_bytes_u8(pkt, DER_C_CONTEXT | tag));
 }
 
 int DER_w_precompiled(WPACKET *pkt, int tag,
 }
 
 int DER_w_precompiled(WPACKET *pkt, int tag,
@@ -136,7 +148,24 @@ int DER_w_begin_sequence(WPACKET *pkt, int tag)
 
 int DER_w_end_sequence(WPACKET *pkt, int tag)
 {
 
 int DER_w_end_sequence(WPACKET *pkt, int tag)
 {
-    return WPACKET_close(pkt)
-        && WPACKET_put_bytes_u8(pkt, DER_F_CONSTRUCTED | DER_P_SEQUENCE)
+    /*
+     * If someone set the flag WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH on this
+     * sub-packet and this sub-packet has nothing written to it, the DER length
+     * will not be written, and the total written size will be unchanged before
+     * and after WPACKET_close().  We use size1 and size2 to determine if
+     * anything was written, and only write our tag if it has.
+     *
+     * Because we know that int_end_context() needs to do the same check,
+     * we reproduce this flag if the written length was unchanged, or we will
+     * have an erroneous context tag.
+     */
+    size_t size1, size2;
+
+    return WPACKET_get_total_written(pkt, &size1)
+        && WPACKET_close(pkt)
+        && WPACKET_get_total_written(pkt, &size2)
+        && (size1 == size2
+            ? WPACKET_set_flags(pkt, WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH)
+            : WPACKET_put_bytes_u8(pkt, DER_F_CONSTRUCTED | DER_P_SEQUENCE))
         && int_end_context(pkt, tag);
 }
         && int_end_context(pkt, tag);
 }