diff options
author | tb <> | 2020-12-08 15:10:03 +0000 |
---|---|---|
committer | tb <> | 2020-12-08 15:10:03 +0000 |
commit | 25128aa86b3c1fab0a730b15592a21b839ae5a03 (patch) | |
tree | ab578539bc4c69bc884b6e42f1cb7e99e6eb0216 | |
parent | 3b56f0265346ac27187ab1c0aa41bc27260bea5b (diff) | |
download | openbsd-OPENBSD_6_7.tar.gz openbsd-OPENBSD_6_7.tar.bz2 openbsd-OPENBSD_6_7.zip |
Fix a NULL dereference in GENERAL_NAME_cmp()libressl-v3.1.5OPENBSD_6_7
Comparing two GENERAL_NAME structures containing an EDIPARTYNAME can lead
to a crash. This enables a denial of service attack for an attacker who can
control both sides of the comparison.
Issue reported to OpenSSL on Nov 9 by David Benjamin.
OpenSSL shared the information with us on Dec 1st.
Fix from Matt Caswell (OpenSSL) with a few small tweaks.
ok jsing
this is errata/6.7/031_asn1.patch.sig
-rw-r--r-- | src/lib/libcrypto/asn1/asn1.h | 3 | ||||
-rw-r--r-- | src/lib/libcrypto/asn1/asn1_err.c | 3 | ||||
-rw-r--r-- | src/lib/libcrypto/asn1/asn1_lib.c | 4 | ||||
-rw-r--r-- | src/lib/libcrypto/asn1/tasn_dec.c | 22 | ||||
-rw-r--r-- | src/lib/libcrypto/asn1/tasn_enc.c | 21 | ||||
-rw-r--r-- | src/lib/libcrypto/x509v3/v3_genn.c | 52 |
6 files changed, 94 insertions, 11 deletions
diff --git a/src/lib/libcrypto/asn1/asn1.h b/src/lib/libcrypto/asn1/asn1.h index 0a8da415fb..9cbc21238b 100644 --- a/src/lib/libcrypto/asn1/asn1.h +++ b/src/lib/libcrypto/asn1/asn1.h | |||
@@ -1,4 +1,4 @@ | |||
1 | /* $OpenBSD: asn1.h,v 1.53 2018/11/30 04:51:19 jeremy Exp $ */ | 1 | /* $OpenBSD: asn1.h,v 1.53.6.1 2020/12/08 15:10:03 tb Exp $ */ |
2 | /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) | 2 | /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) |
3 | * All rights reserved. | 3 | * All rights reserved. |
4 | * | 4 | * |
@@ -1137,6 +1137,7 @@ void ERR_load_ASN1_strings(void); | |||
1137 | #define ASN1_R_BAD_OBJECT_HEADER 102 | 1137 | #define ASN1_R_BAD_OBJECT_HEADER 102 |
1138 | #define ASN1_R_BAD_PASSWORD_READ 103 | 1138 | #define ASN1_R_BAD_PASSWORD_READ 103 |
1139 | #define ASN1_R_BAD_TAG 104 | 1139 | #define ASN1_R_BAD_TAG 104 |
1140 | #define ASN1_R_BAD_TEMPLATE 230 | ||
1140 | #define ASN1_R_BMPSTRING_IS_WRONG_LENGTH 214 | 1141 | #define ASN1_R_BMPSTRING_IS_WRONG_LENGTH 214 |
1141 | #define ASN1_R_BN_LIB 105 | 1142 | #define ASN1_R_BN_LIB 105 |
1142 | #define ASN1_R_BOOLEAN_IS_WRONG_LENGTH 106 | 1143 | #define ASN1_R_BOOLEAN_IS_WRONG_LENGTH 106 |
diff --git a/src/lib/libcrypto/asn1/asn1_err.c b/src/lib/libcrypto/asn1/asn1_err.c index 5cc355084f..bfa9fbf0b5 100644 --- a/src/lib/libcrypto/asn1/asn1_err.c +++ b/src/lib/libcrypto/asn1/asn1_err.c | |||
@@ -1,4 +1,4 @@ | |||
1 | /* $OpenBSD: asn1_err.c,v 1.21 2018/03/29 02:29:24 inoguchi Exp $ */ | 1 | /* $OpenBSD: asn1_err.c,v 1.21.8.1 2020/12/08 15:10:03 tb Exp $ */ |
2 | /* ==================================================================== | 2 | /* ==================================================================== |
3 | * Copyright (c) 1999-2011 The OpenSSL Project. All rights reserved. | 3 | * Copyright (c) 1999-2011 The OpenSSL Project. All rights reserved. |
4 | * | 4 | * |
@@ -85,6 +85,7 @@ static ERR_STRING_DATA ASN1_str_reasons[] = { | |||
85 | {ERR_REASON(ASN1_R_BAD_OBJECT_HEADER) , "bad object header"}, | 85 | {ERR_REASON(ASN1_R_BAD_OBJECT_HEADER) , "bad object header"}, |
86 | {ERR_REASON(ASN1_R_BAD_PASSWORD_READ) , "bad password read"}, | 86 | {ERR_REASON(ASN1_R_BAD_PASSWORD_READ) , "bad password read"}, |
87 | {ERR_REASON(ASN1_R_BAD_TAG) , "bad tag"}, | 87 | {ERR_REASON(ASN1_R_BAD_TAG) , "bad tag"}, |
88 | {ERR_REASON(ASN1_R_BAD_TEMPLATE) , "bad template"}, | ||
88 | {ERR_REASON(ASN1_R_BMPSTRING_IS_WRONG_LENGTH), "bmpstring is wrong length"}, | 89 | {ERR_REASON(ASN1_R_BMPSTRING_IS_WRONG_LENGTH), "bmpstring is wrong length"}, |
89 | {ERR_REASON(ASN1_R_BN_LIB) , "bn lib"}, | 90 | {ERR_REASON(ASN1_R_BN_LIB) , "bn lib"}, |
90 | {ERR_REASON(ASN1_R_BOOLEAN_IS_WRONG_LENGTH), "boolean is wrong length"}, | 91 | {ERR_REASON(ASN1_R_BOOLEAN_IS_WRONG_LENGTH), "boolean is wrong length"}, |
diff --git a/src/lib/libcrypto/asn1/asn1_lib.c b/src/lib/libcrypto/asn1/asn1_lib.c index 5dc520c428..5d79e6833f 100644 --- a/src/lib/libcrypto/asn1/asn1_lib.c +++ b/src/lib/libcrypto/asn1/asn1_lib.c | |||
@@ -1,4 +1,4 @@ | |||
1 | /* $OpenBSD: asn1_lib.c,v 1.44 2018/11/17 09:34:11 tb Exp $ */ | 1 | /* $OpenBSD: asn1_lib.c,v 1.44.6.1 2020/12/08 15:10:03 tb Exp $ */ |
2 | /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) | 2 | /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) |
3 | * All rights reserved. | 3 | * All rights reserved. |
4 | * | 4 | * |
@@ -388,6 +388,8 @@ ASN1_STRING_cmp(const ASN1_STRING *a, const ASN1_STRING *b) | |||
388 | { | 388 | { |
389 | int i; | 389 | int i; |
390 | 390 | ||
391 | if (a == NULL || b == NULL) | ||
392 | return -1; | ||
391 | i = (a->length - b->length); | 393 | i = (a->length - b->length); |
392 | if (i == 0) { | 394 | if (i == 0) { |
393 | i = memcmp(a->data, b->data, a->length); | 395 | i = memcmp(a->data, b->data, a->length); |
diff --git a/src/lib/libcrypto/asn1/tasn_dec.c b/src/lib/libcrypto/asn1/tasn_dec.c index 70dc355ca1..9e6ceacd4a 100644 --- a/src/lib/libcrypto/asn1/tasn_dec.c +++ b/src/lib/libcrypto/asn1/tasn_dec.c | |||
@@ -1,4 +1,4 @@ | |||
1 | /* $OpenBSD: tasn_dec.c,v 1.37 2019/04/01 15:48:04 jsing Exp $ */ | 1 | /* $OpenBSD: tasn_dec.c,v 1.37.6.1 2020/12/08 15:10:03 tb Exp $ */ |
2 | /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL | 2 | /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL |
3 | * project 2000. | 3 | * project 2000. |
4 | */ | 4 | */ |
@@ -210,6 +210,16 @@ asn1_item_ex_d2i(ASN1_VALUE **pval, const unsigned char **in, long len, | |||
210 | break; | 210 | break; |
211 | 211 | ||
212 | case ASN1_ITYPE_MSTRING: | 212 | case ASN1_ITYPE_MSTRING: |
213 | /* | ||
214 | * It never makes sense for multi-strings to have implicit | ||
215 | * tagging, so if tag != -1, then this looks like an error in | ||
216 | * the template. | ||
217 | */ | ||
218 | if (tag != -1) { | ||
219 | ASN1error(ASN1_R_BAD_TEMPLATE); | ||
220 | goto err; | ||
221 | } | ||
222 | |||
213 | p = *in; | 223 | p = *in; |
214 | /* Just read in tag and class */ | 224 | /* Just read in tag and class */ |
215 | ret = asn1_check_tlen(NULL, &otag, &oclass, NULL, NULL, | 225 | ret = asn1_check_tlen(NULL, &otag, &oclass, NULL, NULL, |
@@ -245,6 +255,16 @@ asn1_item_ex_d2i(ASN1_VALUE **pval, const unsigned char **in, long len, | |||
245 | it, tag, aclass, opt, ctx); | 255 | it, tag, aclass, opt, ctx); |
246 | 256 | ||
247 | case ASN1_ITYPE_CHOICE: | 257 | case ASN1_ITYPE_CHOICE: |
258 | /* | ||
259 | * It never makes sense for CHOICE types to have implicit | ||
260 | * tagging, so if tag != -1, then this looks like an error in | ||
261 | * the template. | ||
262 | */ | ||
263 | if (tag != -1) { | ||
264 | ASN1error(ASN1_R_BAD_TEMPLATE); | ||
265 | goto err; | ||
266 | } | ||
267 | |||
248 | if (asn1_cb && !asn1_cb(ASN1_OP_D2I_PRE, pval, it, NULL)) | 268 | if (asn1_cb && !asn1_cb(ASN1_OP_D2I_PRE, pval, it, NULL)) |
249 | goto auxerr; | 269 | goto auxerr; |
250 | 270 | ||
diff --git a/src/lib/libcrypto/asn1/tasn_enc.c b/src/lib/libcrypto/asn1/tasn_enc.c index d103c4d096..d0a569fd8a 100644 --- a/src/lib/libcrypto/asn1/tasn_enc.c +++ b/src/lib/libcrypto/asn1/tasn_enc.c | |||
@@ -1,4 +1,4 @@ | |||
1 | /* $OpenBSD: tasn_enc.c,v 1.22 2019/04/01 15:48:04 jsing Exp $ */ | 1 | /* $OpenBSD: tasn_enc.c,v 1.22.6.1 2020/12/08 15:10:03 tb Exp $ */ |
2 | /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL | 2 | /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL |
3 | * project 2000. | 3 | * project 2000. |
4 | */ | 4 | */ |
@@ -61,6 +61,7 @@ | |||
61 | 61 | ||
62 | #include <openssl/asn1.h> | 62 | #include <openssl/asn1.h> |
63 | #include <openssl/asn1t.h> | 63 | #include <openssl/asn1t.h> |
64 | #include <openssl/err.h> | ||
64 | #include <openssl/objects.h> | 65 | #include <openssl/objects.h> |
65 | 66 | ||
66 | static int asn1_i2d_ex_primitive(ASN1_VALUE **pval, unsigned char **out, | 67 | static int asn1_i2d_ex_primitive(ASN1_VALUE **pval, unsigned char **out, |
@@ -152,9 +153,27 @@ ASN1_item_ex_i2d(ASN1_VALUE **pval, unsigned char **out, const ASN1_ITEM *it, | |||
152 | break; | 153 | break; |
153 | 154 | ||
154 | case ASN1_ITYPE_MSTRING: | 155 | case ASN1_ITYPE_MSTRING: |
156 | /* | ||
157 | * It never makes sense for multi-strings to have implicit | ||
158 | * tagging, so if tag != -1, then this looks like an error in | ||
159 | * the template. | ||
160 | */ | ||
161 | if (tag != -1) { | ||
162 | ASN1error(ASN1_R_BAD_TEMPLATE); | ||
163 | return 0; | ||
164 | } | ||
155 | return asn1_i2d_ex_primitive(pval, out, it, -1, aclass); | 165 | return asn1_i2d_ex_primitive(pval, out, it, -1, aclass); |
156 | 166 | ||
157 | case ASN1_ITYPE_CHOICE: | 167 | case ASN1_ITYPE_CHOICE: |
168 | /* | ||
169 | * It never makes sense for CHOICE types to have implicit | ||
170 | * tagging, so if tag != -1, then this looks like an error in | ||
171 | * the template. | ||
172 | */ | ||
173 | if (tag != -1) { | ||
174 | ASN1error(ASN1_R_BAD_TEMPLATE); | ||
175 | return 0; | ||
176 | } | ||
158 | if (asn1_cb && !asn1_cb(ASN1_OP_I2D_PRE, pval, it, NULL)) | 177 | if (asn1_cb && !asn1_cb(ASN1_OP_I2D_PRE, pval, it, NULL)) |
159 | return 0; | 178 | return 0; |
160 | i = asn1_get_choice_selector(pval, it); | 179 | i = asn1_get_choice_selector(pval, it); |
diff --git a/src/lib/libcrypto/x509v3/v3_genn.c b/src/lib/libcrypto/x509v3/v3_genn.c index a6b7a18b17..b0269da5b0 100644 --- a/src/lib/libcrypto/x509v3/v3_genn.c +++ b/src/lib/libcrypto/x509v3/v3_genn.c | |||
@@ -1,4 +1,4 @@ | |||
1 | /* $OpenBSD: v3_genn.c,v 1.12 2015/09/26 17:38:41 jsing Exp $ */ | 1 | /* $OpenBSD: v3_genn.c,v 1.12.14.1 2020/12/08 15:10:03 tb Exp $ */ |
2 | /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL | 2 | /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL |
3 | * project 1999. | 3 | * project 1999. |
4 | */ | 4 | */ |
@@ -117,16 +117,17 @@ OTHERNAME_free(OTHERNAME *a) | |||
117 | ASN1_item_free((ASN1_VALUE *)a, &OTHERNAME_it); | 117 | ASN1_item_free((ASN1_VALUE *)a, &OTHERNAME_it); |
118 | } | 118 | } |
119 | 119 | ||
120 | /* Uses explicit tagging since DIRECTORYSTRING is a CHOICE type */ | ||
120 | static const ASN1_TEMPLATE EDIPARTYNAME_seq_tt[] = { | 121 | static const ASN1_TEMPLATE EDIPARTYNAME_seq_tt[] = { |
121 | { | 122 | { |
122 | .flags = ASN1_TFLG_IMPLICIT | ASN1_TFLG_OPTIONAL, | 123 | .flags = ASN1_TFLG_EXPLICIT | ASN1_TFLG_OPTIONAL, |
123 | .tag = 0, | 124 | .tag = 0, |
124 | .offset = offsetof(EDIPARTYNAME, nameAssigner), | 125 | .offset = offsetof(EDIPARTYNAME, nameAssigner), |
125 | .field_name = "nameAssigner", | 126 | .field_name = "nameAssigner", |
126 | .item = &DIRECTORYSTRING_it, | 127 | .item = &DIRECTORYSTRING_it, |
127 | }, | 128 | }, |
128 | { | 129 | { |
129 | .flags = ASN1_TFLG_IMPLICIT | ASN1_TFLG_OPTIONAL, | 130 | .flags = ASN1_TFLG_EXPLICIT, |
130 | .tag = 1, | 131 | .tag = 1, |
131 | .offset = offsetof(EDIPARTYNAME, partyName), | 132 | .offset = offsetof(EDIPARTYNAME, partyName), |
132 | .field_name = "partyName", | 133 | .field_name = "partyName", |
@@ -324,6 +325,37 @@ GENERAL_NAME_dup(GENERAL_NAME *a) | |||
324 | return ASN1_item_dup(&GENERAL_NAME_it, a); | 325 | return ASN1_item_dup(&GENERAL_NAME_it, a); |
325 | } | 326 | } |
326 | 327 | ||
328 | static int | ||
329 | EDIPARTYNAME_cmp(const EDIPARTYNAME *a, const EDIPARTYNAME *b) | ||
330 | { | ||
331 | int res; | ||
332 | |||
333 | /* | ||
334 | * Shouldn't be possible in a valid GENERAL_NAME, but we handle it | ||
335 | * anyway. OTHERNAME_cmp treats NULL != NULL, so we do the same here. | ||
336 | */ | ||
337 | if (a == NULL || b == NULL) | ||
338 | return -1; | ||
339 | if (a->nameAssigner == NULL && b->nameAssigner != NULL) | ||
340 | return -1; | ||
341 | if (a->nameAssigner != NULL && b->nameAssigner == NULL) | ||
342 | return 1; | ||
343 | /* If we get here, both have nameAssigner set or both unset. */ | ||
344 | if (a->nameAssigner != NULL) { | ||
345 | res = ASN1_STRING_cmp(a->nameAssigner, b->nameAssigner); | ||
346 | if (res != 0) | ||
347 | return res; | ||
348 | } | ||
349 | /* | ||
350 | * partyName is required, so these should never be NULL. We treat it in | ||
351 | * the same way as the a == NULL || b == NULL case above. | ||
352 | */ | ||
353 | if (a->partyName == NULL || b->partyName == NULL) | ||
354 | return -1; | ||
355 | |||
356 | return ASN1_STRING_cmp(a->partyName, b->partyName); | ||
357 | } | ||
358 | |||
327 | /* Returns 0 if they are equal, != 0 otherwise. */ | 359 | /* Returns 0 if they are equal, != 0 otherwise. */ |
328 | int | 360 | int |
329 | GENERAL_NAME_cmp(GENERAL_NAME *a, GENERAL_NAME *b) | 361 | GENERAL_NAME_cmp(GENERAL_NAME *a, GENERAL_NAME *b) |
@@ -334,8 +366,11 @@ GENERAL_NAME_cmp(GENERAL_NAME *a, GENERAL_NAME *b) | |||
334 | return -1; | 366 | return -1; |
335 | switch (a->type) { | 367 | switch (a->type) { |
336 | case GEN_X400: | 368 | case GEN_X400: |
369 | result = ASN1_TYPE_cmp(a->d.x400Address, b->d.x400Address); | ||
370 | break; | ||
371 | |||
337 | case GEN_EDIPARTY: | 372 | case GEN_EDIPARTY: |
338 | result = ASN1_TYPE_cmp(a->d.other, b->d.other); | 373 | result = EDIPARTYNAME_cmp(a->d.ediPartyName, b->d.ediPartyName); |
339 | break; | 374 | break; |
340 | 375 | ||
341 | case GEN_OTHERNAME: | 376 | case GEN_OTHERNAME: |
@@ -384,8 +419,11 @@ GENERAL_NAME_set0_value(GENERAL_NAME *a, int type, void *value) | |||
384 | { | 419 | { |
385 | switch (type) { | 420 | switch (type) { |
386 | case GEN_X400: | 421 | case GEN_X400: |
422 | a->d.x400Address = value; | ||
423 | break; | ||
424 | |||
387 | case GEN_EDIPARTY: | 425 | case GEN_EDIPARTY: |
388 | a->d.other = value; | 426 | a->d.ediPartyName = value; |
389 | break; | 427 | break; |
390 | 428 | ||
391 | case GEN_OTHERNAME: | 429 | case GEN_OTHERNAME: |
@@ -420,8 +458,10 @@ GENERAL_NAME_get0_value(GENERAL_NAME *a, int *ptype) | |||
420 | *ptype = a->type; | 458 | *ptype = a->type; |
421 | switch (a->type) { | 459 | switch (a->type) { |
422 | case GEN_X400: | 460 | case GEN_X400: |
461 | return a->d.x400Address; | ||
462 | |||
423 | case GEN_EDIPARTY: | 463 | case GEN_EDIPARTY: |
424 | return a->d.other; | 464 | return a->d.ediPartyName; |
425 | 465 | ||
426 | case GEN_OTHERNAME: | 466 | case GEN_OTHERNAME: |
427 | return a->d.otherName; | 467 | return a->d.otherName; |