From ab72b8b8c221a7e276a078466106e0297ab6d900 Mon Sep 17 00:00:00 2001 From: jsing Date: Mon, 9 May 2022 19:19:33 +0000 Subject: [PATCH] Simplify X509_ATTRIBUTE ASN.1 encoding. For some unknown historical reason, X509_ATTRIBUTE allows for a single ASN.1 value or an ASN.1 SET OF, rather than requiring an ASN.1 SET OF. Simplify encoding and remove support for single values - this is similar to OpenSSL e20b57270dec. This removes the last use of COMBINE in the ASN.1 decoder. ok tb@ --- lib/libcrypto/asn1/t_req.c | 21 ++++-------- lib/libcrypto/asn1/x_attrib.c | 60 ++++++--------------------------- lib/libcrypto/pkcs12/p12_attr.c | 10 ++---- lib/libcrypto/pkcs7/pk7_doit.c | 10 ++---- lib/libcrypto/x509/x509_att.c | 27 ++++++--------- lib/libcrypto/x509/x509_lcl.h | 7 +--- 6 files changed, 34 insertions(+), 101 deletions(-) diff --git a/lib/libcrypto/asn1/t_req.c b/lib/libcrypto/asn1/t_req.c index cc9da46439a..4b27a4ddbe1 100644 --- a/lib/libcrypto/asn1/t_req.c +++ b/lib/libcrypto/asn1/t_req.c @@ -1,4 +1,4 @@ -/* $OpenBSD: t_req.c,v 1.21 2021/12/25 13:17:48 jsing Exp $ */ +/* $OpenBSD: t_req.c,v 1.22 2022/05/09 19:19:33 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -176,7 +176,6 @@ X509_REQ_print_ex(BIO *bp, X509_REQ *x, unsigned long nmflags, ASN1_TYPE *at; X509_ATTRIBUTE *a; ASN1_BIT_STRING *bs = NULL; - ASN1_TYPE *t; int j, type = 0, count = 1, ii = 0; a = sk_X509_ATTRIBUTE_value(sk, i); @@ -186,20 +185,12 @@ X509_REQ_print_ex(BIO *bp, X509_REQ *x, unsigned long nmflags, if (BIO_printf(bp, "%12s", "") <= 0) goto err; if ((j = i2a_ASN1_OBJECT(bp, a->object)) > 0) { - if (a->single) { - t = a->value.single; - type = t->type; - bs = t->value.bit_string; - } else { - ii = 0; - count = sk_ASN1_TYPE_num( - a->value.set); + ii = 0; + count = sk_ASN1_TYPE_num(a->set); get_next: - at = sk_ASN1_TYPE_value( - a->value.set, ii); - type = at->type; - bs = at->value.asn1_string; - } + at = sk_ASN1_TYPE_value(a->set, ii); + type = at->type; + bs = at->value.asn1_string; } for (j = 25 - j; j > 0; j--) if (BIO_write(bp, " ", 1) != 1) diff --git a/lib/libcrypto/asn1/x_attrib.c b/lib/libcrypto/asn1/x_attrib.c index 47b5afd95d2..e8822a33a5a 100644 --- a/lib/libcrypto/asn1/x_attrib.c +++ b/lib/libcrypto/asn1/x_attrib.c @@ -1,4 +1,4 @@ -/* $OpenBSD: x_attrib.c,v 1.16 2021/12/25 13:17:48 jsing Exp $ */ +/* $OpenBSD: x_attrib.c,v 1.17 2022/05/09 19:19:33 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -64,48 +64,14 @@ #include "x509_lcl.h" -/* X509_ATTRIBUTE: this has the following form: - * - * typedef struct x509_attributes_st - * { - * ASN1_OBJECT *object; - * int single; - * union { - * char *ptr; - * STACK_OF(ASN1_TYPE) *set; - * ASN1_TYPE *single; - * } value; - * } X509_ATTRIBUTE; - * - * this needs some extra thought because the CHOICE type is - * merged with the main structure and because the value can - * be anything at all we *must* try the SET OF first because - * the ASN1_ANY type will swallow anything including the whole - * SET OF structure. +/* + * XXX - remove X509_ATTRIBUTE_SET_it with next major bump. */ - -static const ASN1_TEMPLATE X509_ATTRIBUTE_SET_ch_tt[] = { - { - .flags = ASN1_TFLG_SET_OF, - .tag = 0, - .offset = offsetof(X509_ATTRIBUTE, value.set), - .field_name = "value.set", - .item = &ASN1_ANY_it, - }, - { - .flags = 0, - .tag = 0, - .offset = offsetof(X509_ATTRIBUTE, value.single), - .field_name = "value.single", - .item = &ASN1_ANY_it, - }, -}; - const ASN1_ITEM X509_ATTRIBUTE_SET_it = { .itype = ASN1_ITYPE_CHOICE, - .utype = offsetof(X509_ATTRIBUTE, single), - .templates = X509_ATTRIBUTE_SET_ch_tt, - .tcount = sizeof(X509_ATTRIBUTE_SET_ch_tt) / sizeof(ASN1_TEMPLATE), + .utype = 0, + .templates = NULL, + .tcount = 0, .funcs = NULL, .size = sizeof(X509_ATTRIBUTE), .sname = "X509_ATTRIBUTE", @@ -119,13 +85,12 @@ static const ASN1_TEMPLATE X509_ATTRIBUTE_seq_tt[] = { .field_name = "object", .item = &ASN1_OBJECT_it, }, - /* CHOICE type merged with parent */ { - .flags = 0 | ASN1_TFLG_COMBINE, + .flags = ASN1_TFLG_SET_OF, .tag = 0, - .offset = 0, - .field_name = NULL, - .item = &X509_ATTRIBUTE_SET_it, + .offset = offsetof(X509_ATTRIBUTE, set), + .field_name = "set", + .item = &ASN1_ANY_it, }, }; @@ -183,12 +148,9 @@ X509_ATTRIBUTE_create(int nid, int atrtype, void *value) if ((ret = X509_ATTRIBUTE_new()) == NULL) return (NULL); ret->object = oid; - ret->single = 0; - if ((ret->value.set = sk_ASN1_TYPE_new_null()) == NULL) - goto err; if ((val = ASN1_TYPE_new()) == NULL) goto err; - if (!sk_ASN1_TYPE_push(ret->value.set, val)) + if (!sk_ASN1_TYPE_push(ret->set, val)) goto err; ASN1_TYPE_set(val, atrtype, value); diff --git a/lib/libcrypto/pkcs12/p12_attr.c b/lib/libcrypto/pkcs12/p12_attr.c index dc38b7c897c..a35a148b11e 100644 --- a/lib/libcrypto/pkcs12/p12_attr.c +++ b/lib/libcrypto/pkcs12/p12_attr.c @@ -1,4 +1,4 @@ -/* $OpenBSD: p12_attr.c,v 1.14 2021/11/01 20:53:08 tb Exp $ */ +/* $OpenBSD: p12_attr.c,v 1.15 2022/05/09 19:19:33 jsing Exp $ */ /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL * project 1999. */ @@ -129,12 +129,8 @@ PKCS12_get_attr_gen(const STACK_OF(X509_ATTRIBUTE) *attrs, int attr_nid) return NULL; for (i = 0; i < sk_X509_ATTRIBUTE_num(attrs); i++) { attrib = sk_X509_ATTRIBUTE_value(attrs, i); - if (OBJ_obj2nid(attrib->object) == attr_nid) { - if (sk_ASN1_TYPE_num(attrib->value.set)) - return sk_ASN1_TYPE_value(attrib->value.set, 0); - else - return NULL; - } + if (OBJ_obj2nid(attrib->object) == attr_nid) + return sk_ASN1_TYPE_value(attrib->set, 0); } return NULL; } diff --git a/lib/libcrypto/pkcs7/pk7_doit.c b/lib/libcrypto/pkcs7/pk7_doit.c index c9d64bcf342..b3140696104 100644 --- a/lib/libcrypto/pkcs7/pk7_doit.c +++ b/lib/libcrypto/pkcs7/pk7_doit.c @@ -1,4 +1,4 @@ -/* $OpenBSD: pk7_doit.c,v 1.46 2021/12/12 21:30:14 tb Exp $ */ +/* $OpenBSD: pk7_doit.c,v 1.47 2022/05/09 19:19:33 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -1129,12 +1129,8 @@ get_attribute(STACK_OF(X509_ATTRIBUTE) *sk, int nid) return (NULL); for (i = 0; i < sk_X509_ATTRIBUTE_num(sk); i++) { xa = sk_X509_ATTRIBUTE_value(sk, i); - if (OBJ_cmp(xa->object, o) == 0) { - if (!xa->single && sk_ASN1_TYPE_num(xa->value.set)) - return (sk_ASN1_TYPE_value(xa->value.set, 0)); - else - return (NULL); - } + if (OBJ_cmp(xa->object, o) == 0) + return (sk_ASN1_TYPE_value(xa->set, 0)); } return (NULL); } diff --git a/lib/libcrypto/x509/x509_att.c b/lib/libcrypto/x509/x509_att.c index 38aa0631432..8d369df9006 100644 --- a/lib/libcrypto/x509/x509_att.c +++ b/lib/libcrypto/x509/x509_att.c @@ -1,4 +1,4 @@ -/* $OpenBSD: x509_att.c,v 1.18 2021/11/01 20:53:08 tb Exp $ */ +/* $OpenBSD: x509_att.c,v 1.19 2022/05/09 19:19:33 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -324,10 +324,8 @@ X509_ATTRIBUTE_set1_data(X509_ATTRIBUTE *attr, int attrtype, const void *data, goto err; atype = attrtype; } - if (!(attr->value.set = sk_ASN1_TYPE_new_null())) - goto err; - attr->single = 0; - /* This is a bit naughty because the attribute should really have + /* + * This is a bit naughty because the attribute should really have * at least one value but some types use and zero length SET and * require this. */ @@ -343,7 +341,7 @@ X509_ATTRIBUTE_set1_data(X509_ATTRIBUTE *attr, int attrtype, const void *data, goto err; } else ASN1_TYPE_set(ttmp, atype, stmp); - if (!sk_ASN1_TYPE_push(attr->value.set, ttmp)) + if (!sk_ASN1_TYPE_push(attr->set, ttmp)) goto err; return 1; @@ -357,11 +355,10 @@ err: int X509_ATTRIBUTE_count(const X509_ATTRIBUTE *attr) { - if (!attr->single) - return sk_ASN1_TYPE_num(attr->value.set); - if (attr->value.single) - return 1; - return 0; + if (attr == NULL) + return 0; + + return sk_ASN1_TYPE_num(attr->set); } ASN1_OBJECT * @@ -392,10 +389,6 @@ X509_ATTRIBUTE_get0_type(X509_ATTRIBUTE *attr, int idx) { if (attr == NULL) return (NULL); - if (idx >= X509_ATTRIBUTE_count(attr)) - return NULL; - if (!attr->single) - return sk_ASN1_TYPE_value(attr->value.set, idx); - else - return attr->value.single; + + return sk_ASN1_TYPE_value(attr->set, idx); } diff --git a/lib/libcrypto/x509/x509_lcl.h b/lib/libcrypto/x509/x509_lcl.h index e7eb733f7d9..5beef8a94dc 100644 --- a/lib/libcrypto/x509/x509_lcl.h +++ b/lib/libcrypto/x509/x509_lcl.h @@ -109,12 +109,7 @@ struct X509_extension_st { struct x509_attributes_st { ASN1_OBJECT *object; - int single; /* 0 for a set, 1 for a single item (which is wrong) */ - union { - char *ptr; -/* 0 */ STACK_OF(ASN1_TYPE) *set; -/* 1 */ ASN1_TYPE *single; - } value; + STACK_OF(ASN1_TYPE) *set; } /* X509_ATTRIBUTE */; struct X509_req_info_st { -- 2.20.1