From 2c9ac5855b2f45cf94ac1004b945c1d4aa77cc21 Mon Sep 17 00:00:00 2001 From: Ed Maste Date: Tue, 8 Dec 2020 16:43:35 +0000 Subject: [PATCH] OpenSSL: address CVE-2020-1971 OpenSSL commit 3db2c9f3: Complain if we are attempting to encode with an invalid ASN.1 template OpenSSL commit 43a7033: Check that multi-strings/CHOICE types don't use implicit tagging OpenSSL commit f960d812: Correctly compare EdiPartyName in GENERAL_NAME_cmp() Obtained from: OpenSSL 3db2c9f3, 43a7033, f960d812 Security: CVE-2020-1971 --- crypto/openssl/crypto/asn1/asn1_err.c | 4 +- crypto/openssl/crypto/asn1/tasn_dec.c | 19 +++++++++ crypto/openssl/crypto/asn1/tasn_enc.c | 16 ++++++++ crypto/openssl/crypto/err/openssl.txt | 2 + crypto/openssl/crypto/x509v3/v3_genn.c | 50 +++++++++++++++++++++--- crypto/openssl/include/openssl/asn1err.h | 8 ++-- 6 files changed, 89 insertions(+), 10 deletions(-) diff --git a/crypto/openssl/crypto/asn1/asn1_err.c b/crypto/openssl/crypto/asn1/asn1_err.c index 613f9ae71333..cc0a59ca4c8b 100644 --- a/crypto/openssl/crypto/asn1/asn1_err.c +++ b/crypto/openssl/crypto/asn1/asn1_err.c @@ -1,6 +1,6 @@ /* * Generated by util/mkerr.pl DO NOT EDIT - * Copyright 1995-2018 The OpenSSL Project Authors. All Rights Reserved. + * Copyright 1995-2020 The OpenSSL Project Authors. All Rights Reserved. * * Licensed under the OpenSSL license (the "License"). You may not use * this file except in compliance with the License. You can obtain a copy @@ -49,6 +49,7 @@ static const ERR_STRING_DATA ASN1_str_functs[] = { "asn1_item_embed_d2i"}, {ERR_PACK(ERR_LIB_ASN1, ASN1_F_ASN1_ITEM_EMBED_NEW, 0), "asn1_item_embed_new"}, + {ERR_PACK(ERR_LIB_ASN1, ASN1_F_ASN1_ITEM_EX_I2D, 0), "ASN1_item_ex_i2d"}, {ERR_PACK(ERR_LIB_ASN1, ASN1_F_ASN1_ITEM_FLAGS_I2D, 0), "asn1_item_flags_i2d"}, {ERR_PACK(ERR_LIB_ASN1, ASN1_F_ASN1_ITEM_I2D_BIO, 0), "ASN1_item_i2d_bio"}, @@ -160,6 +161,7 @@ static const ERR_STRING_DATA ASN1_str_reasons[] = { "asn1 sig parse error"}, {ERR_PACK(ERR_LIB_ASN1, 0, ASN1_R_AUX_ERROR), "aux error"}, {ERR_PACK(ERR_LIB_ASN1, 0, ASN1_R_BAD_OBJECT_HEADER), "bad object header"}, + {ERR_PACK(ERR_LIB_ASN1, 0, ASN1_R_BAD_TEMPLATE), "bad template"}, {ERR_PACK(ERR_LIB_ASN1, 0, ASN1_R_BMPSTRING_IS_WRONG_LENGTH), "bmpstring is wrong length"}, {ERR_PACK(ERR_LIB_ASN1, 0, ASN1_R_BN_LIB), "bn lib"}, diff --git a/crypto/openssl/crypto/asn1/tasn_dec.c b/crypto/openssl/crypto/asn1/tasn_dec.c index 2332b204edc1..1021705f4346 100644 --- a/crypto/openssl/crypto/asn1/tasn_dec.c +++ b/crypto/openssl/crypto/asn1/tasn_dec.c @@ -182,6 +182,15 @@ static int asn1_item_embed_d2i(ASN1_VALUE **pval, const unsigned char **in, tag, aclass, opt, ctx); case ASN1_ITYPE_MSTRING: + /* + * It never makes sense for multi-strings to have implicit tagging, so + * if tag != -1, then this looks like an error in the template. + */ + if (tag != -1) { + ASN1err(ASN1_F_ASN1_ITEM_EMBED_D2I, ASN1_R_BAD_TEMPLATE); + goto err; + } + p = *in; /* Just read in tag and class */ ret = asn1_check_tlen(NULL, &otag, &oclass, NULL, NULL, @@ -199,6 +208,7 @@ static int asn1_item_embed_d2i(ASN1_VALUE **pval, const unsigned char **in, ASN1err(ASN1_F_ASN1_ITEM_EMBED_D2I, ASN1_R_MSTRING_NOT_UNIVERSAL); goto err; } + /* Check tag matches bit map */ if (!(ASN1_tag2bit(otag) & it->utype)) { /* If OPTIONAL, assume this is OK */ @@ -215,6 +225,15 @@ static int asn1_item_embed_d2i(ASN1_VALUE **pval, const unsigned char **in, return ef->asn1_ex_d2i(pval, in, len, it, tag, aclass, opt, ctx); case ASN1_ITYPE_CHOICE: + /* + * It never makes sense for CHOICE types to have implicit tagging, so + * if tag != -1, then this looks like an error in the template. + */ + if (tag != -1) { + ASN1err(ASN1_F_ASN1_ITEM_EMBED_D2I, ASN1_R_BAD_TEMPLATE); + goto err; + } + if (asn1_cb && !asn1_cb(ASN1_OP_D2I_PRE, pval, it, NULL)) goto auxerr; if (*pval) { diff --git a/crypto/openssl/crypto/asn1/tasn_enc.c b/crypto/openssl/crypto/asn1/tasn_enc.c index d600c7a53804..52a051d5b1c1 100644 --- a/crypto/openssl/crypto/asn1/tasn_enc.c +++ b/crypto/openssl/crypto/asn1/tasn_enc.c @@ -103,9 +103,25 @@ int ASN1_item_ex_i2d(ASN1_VALUE **pval, unsigned char **out, return asn1_i2d_ex_primitive(pval, out, it, tag, aclass); case ASN1_ITYPE_MSTRING: + /* + * It never makes sense for multi-strings to have implicit tagging, so + * if tag != -1, then this looks like an error in the template. + */ + if (tag != -1) { + ASN1err(ASN1_F_ASN1_ITEM_EX_I2D, ASN1_R_BAD_TEMPLATE); + return -1; + } return asn1_i2d_ex_primitive(pval, out, it, -1, aclass); case ASN1_ITYPE_CHOICE: + /* + * It never makes sense for CHOICE types to have implicit tagging, so + * if tag != -1, then this looks like an error in the template. + */ + if (tag != -1) { + ASN1err(ASN1_F_ASN1_ITEM_EX_I2D, ASN1_R_BAD_TEMPLATE); + return -1; + } if (asn1_cb && !asn1_cb(ASN1_OP_I2D_PRE, pval, it, NULL)) return 0; i = asn1_get_choice_selector(pval, it); diff --git a/crypto/openssl/crypto/err/openssl.txt b/crypto/openssl/crypto/err/openssl.txt index 0b5873ebbcb7..815460b24f67 100644 --- a/crypto/openssl/crypto/err/openssl.txt +++ b/crypto/openssl/crypto/err/openssl.txt @@ -36,6 +36,7 @@ ASN1_F_ASN1_ITEM_D2I_FP:206:ASN1_item_d2i_fp ASN1_F_ASN1_ITEM_DUP:191:ASN1_item_dup ASN1_F_ASN1_ITEM_EMBED_D2I:120:asn1_item_embed_d2i ASN1_F_ASN1_ITEM_EMBED_NEW:121:asn1_item_embed_new +ASN1_F_ASN1_ITEM_EX_I2D:144:ASN1_item_ex_i2d ASN1_F_ASN1_ITEM_FLAGS_I2D:118:asn1_item_flags_i2d ASN1_F_ASN1_ITEM_I2D_BIO:192:ASN1_item_i2d_bio ASN1_F_ASN1_ITEM_I2D_FP:193:ASN1_item_i2d_fp @@ -1771,6 +1772,7 @@ ASN1_R_ASN1_PARSE_ERROR:203:asn1 parse error ASN1_R_ASN1_SIG_PARSE_ERROR:204:asn1 sig parse error ASN1_R_AUX_ERROR:100:aux error ASN1_R_BAD_OBJECT_HEADER:102:bad object header +ASN1_R_BAD_TEMPLATE:230:bad template ASN1_R_BMPSTRING_IS_WRONG_LENGTH:214:bmpstring is wrong length ASN1_R_BN_LIB:105:bn lib ASN1_R_BOOLEAN_IS_WRONG_LENGTH:106:boolean is wrong length diff --git a/crypto/openssl/crypto/x509v3/v3_genn.c b/crypto/openssl/crypto/x509v3/v3_genn.c index 23e3bc45653c..6f0a347cced7 100644 --- a/crypto/openssl/crypto/x509v3/v3_genn.c +++ b/crypto/openssl/crypto/x509v3/v3_genn.c @@ -22,8 +22,9 @@ ASN1_SEQUENCE(OTHERNAME) = { IMPLEMENT_ASN1_FUNCTIONS(OTHERNAME) ASN1_SEQUENCE(EDIPARTYNAME) = { - ASN1_IMP_OPT(EDIPARTYNAME, nameAssigner, DIRECTORYSTRING, 0), - ASN1_IMP_OPT(EDIPARTYNAME, partyName, DIRECTORYSTRING, 1) + /* DirectoryString is a CHOICE type so use explicit tagging */ + ASN1_EXP_OPT(EDIPARTYNAME, nameAssigner, DIRECTORYSTRING, 0), + ASN1_EXP(EDIPARTYNAME, partyName, DIRECTORYSTRING, 1) } ASN1_SEQUENCE_END(EDIPARTYNAME) IMPLEMENT_ASN1_FUNCTIONS(EDIPARTYNAME) @@ -57,6 +58,37 @@ GENERAL_NAME *GENERAL_NAME_dup(GENERAL_NAME *a) (char *)a); } +static int edipartyname_cmp(const EDIPARTYNAME *a, const EDIPARTYNAME *b) +{ + int res; + + if (a == NULL || b == NULL) { + /* + * Shouldn't be possible in a valid GENERAL_NAME, but we handle it + * anyway. OTHERNAME_cmp treats NULL != NULL so we do the same here + */ + return -1; + } + if (a->nameAssigner == NULL && b->nameAssigner != NULL) + return -1; + if (a->nameAssigner != NULL && b->nameAssigner == NULL) + return 1; + /* If we get here then both have nameAssigner set, or both unset */ + if (a->nameAssigner != NULL) { + res = ASN1_STRING_cmp(a->nameAssigner, b->nameAssigner); + if (res != 0) + return res; + } + /* + * partyName is required, so these should never be NULL. We treat it in + * the same way as the a == NULL || b == NULL case above + */ + if (a->partyName == NULL || b->partyName == NULL) + return -1; + + return ASN1_STRING_cmp(a->partyName, b->partyName); +} + /* Returns 0 if they are equal, != 0 otherwise. */ int GENERAL_NAME_cmp(GENERAL_NAME *a, GENERAL_NAME *b) { @@ -66,8 +98,11 @@ int GENERAL_NAME_cmp(GENERAL_NAME *a, GENERAL_NAME *b) return -1; switch (a->type) { case GEN_X400: + result = ASN1_TYPE_cmp(a->d.x400Address, b->d.x400Address); + break; + case GEN_EDIPARTY: - result = ASN1_TYPE_cmp(a->d.other, b->d.other); + result = edipartyname_cmp(a->d.ediPartyName, b->d.ediPartyName); break; case GEN_OTHERNAME: @@ -114,8 +149,11 @@ void GENERAL_NAME_set0_value(GENERAL_NAME *a, int type, void *value) { switch (type) { case GEN_X400: + a->d.x400Address = value; + break; + case GEN_EDIPARTY: - a->d.other = value; + a->d.ediPartyName = value; break; case GEN_OTHERNAME: @@ -149,8 +187,10 @@ void *GENERAL_NAME_get0_value(const GENERAL_NAME *a, int *ptype) *ptype = a->type; switch (a->type) { case GEN_X400: + return a->d.x400Address; + case GEN_EDIPARTY: - return a->d.other; + return a->d.ediPartyName; case GEN_OTHERNAME: return a->d.otherName; diff --git a/crypto/openssl/include/openssl/asn1err.h b/crypto/openssl/include/openssl/asn1err.h index faed5a55180d..e1ad1fefeca7 100644 --- a/crypto/openssl/include/openssl/asn1err.h +++ b/crypto/openssl/include/openssl/asn1err.h @@ -1,6 +1,6 @@ /* * Generated by util/mkerr.pl DO NOT EDIT - * Copyright 1995-2019 The OpenSSL Project Authors. All Rights Reserved. + * Copyright 1995-2020 The OpenSSL Project Authors. All Rights Reserved. * * Licensed under the OpenSSL license (the "License"). You may not use * this file except in compliance with the License. You can obtain a copy @@ -11,9 +11,7 @@ #ifndef HEADER_ASN1ERR_H # define HEADER_ASN1ERR_H -# ifndef HEADER_SYMHACKS_H -# include -# endif +# include # ifdef __cplusplus extern "C" @@ -53,6 +51,7 @@ int ERR_load_ASN1_strings(void); # define ASN1_F_ASN1_ITEM_DUP 191 # define ASN1_F_ASN1_ITEM_EMBED_D2I 120 # define ASN1_F_ASN1_ITEM_EMBED_NEW 121 +# define ASN1_F_ASN1_ITEM_EX_I2D 144 # define ASN1_F_ASN1_ITEM_FLAGS_I2D 118 # define ASN1_F_ASN1_ITEM_I2D_BIO 192 # define ASN1_F_ASN1_ITEM_I2D_FP 193 @@ -145,6 +144,7 @@ int ERR_load_ASN1_strings(void); # define ASN1_R_ASN1_SIG_PARSE_ERROR 204 # define ASN1_R_AUX_ERROR 100 # define ASN1_R_BAD_OBJECT_HEADER 102 +# define ASN1_R_BAD_TEMPLATE 230 # define ASN1_R_BMPSTRING_IS_WRONG_LENGTH 214 # define ASN1_R_BN_LIB 105 # define ASN1_R_BOOLEAN_IS_WRONG_LENGTH 106