From 2275ff656c6d2043b40663686ec6627613d68318 Mon Sep 17 00:00:00 2001 From: Richard Levitte Date: Sat, 2 May 2020 13:33:24 +0200 Subject: [PATCH] DER writer: Add the possibility to abandon empty SEQUENCEs 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 (Merged from https://github.com/openssl/openssl/pull/11710) --- crypto/der_writer.c | 37 +++++++++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/crypto/der_writer.c b/crypto/der_writer.c index 26fd88592d..8762787504 100644 --- a/crypto/der_writer.c +++ b/crypto/der_writer.c @@ -24,12 +24,24 @@ static int int_start_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; - 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, @@ -136,7 +148,24 @@ int DER_w_begin_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); } -- 2.34.1