diff options
author | Dr. David von Oheimb <David.von.Oheimb@siemens.com> | 2022-08-01 16:33:35 +0200 |
---|---|---|
committer | Dr. David von Oheimb <dev@ddvo.net> | 2022-08-24 11:25:04 +0200 |
commit | ba9e3721febb073397248154a846f2088efd6409 (patch) | |
tree | a4aecbae0c4dffb9dbf211aa0d120c71baeb0247 /crypto | |
parent | 47dc828c6b652feb9cef5b0e4186d010986f197c (diff) |
x509_att.c: improve error checking and reporting and coding style
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com>
(Merged from https://github.com/openssl/openssl/pull/18931)
Diffstat (limited to 'crypto')
-rw-r--r-- | crypto/err/openssl.txt | 1 | ||||
-rw-r--r-- | crypto/pkcs12/p12_attr.c | 10 | ||||
-rw-r--r-- | crypto/pkcs12/p12_crt.c | 10 | ||||
-rw-r--r-- | crypto/pkcs7/pk7_doit.c | 10 | ||||
-rw-r--r-- | crypto/x509/x509_att.c | 96 | ||||
-rw-r--r-- | crypto/x509/x509_err.c | 4 | ||||
-rw-r--r-- | crypto/x509/x509_req.c | 2 |
7 files changed, 76 insertions, 57 deletions
diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt index 3f1d844e04..e6db85e96c 100644 --- a/crypto/err/openssl.txt +++ b/crypto/err/openssl.txt @@ -1691,6 +1691,7 @@ X509_R_CERTIFICATE_VERIFICATION_FAILED:139:certificate verification failed X509_R_CERT_ALREADY_IN_HASH_TABLE:101:cert already in hash table X509_R_CRL_ALREADY_DELTA:127:crl already delta X509_R_CRL_VERIFY_FAILURE:131:crl verify failure +X509_R_DUPLICATE_ATTRIBUTE:140:duplicate attribute X509_R_ERROR_GETTING_MD_BY_NID:141:error getting md by nid X509_R_ERROR_USING_SIGINF_SET:142:error using siginf set X509_R_IDP_MISMATCH:128:idp mismatch diff --git a/crypto/pkcs12/p12_attr.c b/crypto/pkcs12/p12_attr.c index da228336eb..568a32a55e 100644 --- a/crypto/pkcs12/p12_attr.c +++ b/crypto/pkcs12/p12_attr.c @@ -95,11 +95,11 @@ int PKCS12_add1_attr_by_txt(PKCS12_SAFEBAG *bag, const char *attrname, int type, ASN1_TYPE *PKCS12_get_attr_gen(const STACK_OF(X509_ATTRIBUTE) *attrs, int attr_nid) { - X509_ATTRIBUTE *attrib; - int i; - i = X509at_get_attr_by_NID(attrs, attr_nid, -1); - attrib = X509at_get_attr(attrs, i); - return X509_ATTRIBUTE_get0_type(attrib, 0); + int i = X509at_get_attr_by_NID(attrs, attr_nid, -1); + + if (i < 0) + return NULL; + return X509_ATTRIBUTE_get0_type(X509at_get_attr(attrs, i), 0); } char *PKCS12_get_friendlyname(PKCS12_SAFEBAG *bag) diff --git a/crypto/pkcs12/p12_crt.c b/crypto/pkcs12/p12_crt.c index 00c7129746..7889842b6f 100644 --- a/crypto/pkcs12/p12_crt.c +++ b/crypto/pkcs12/p12_crt.c @@ -17,15 +17,11 @@ static int pkcs12_add_bag(STACK_OF(PKCS12_SAFEBAG) **pbags, static int copy_bag_attr(PKCS12_SAFEBAG *bag, EVP_PKEY *pkey, int nid) { - int idx; - X509_ATTRIBUTE *attr; - idx = EVP_PKEY_get_attr_by_NID(pkey, nid, -1); + int idx = EVP_PKEY_get_attr_by_NID(pkey, nid, -1); + if (idx < 0) return 1; - attr = EVP_PKEY_get_attr(pkey, idx); - if (!X509at_add1_attr(&bag->attrib, attr)) - return 0; - return 1; + return X509at_add1_attr(&bag->attrib, EVP_PKEY_get_attr(pkey, idx)) != NULL; } PKCS12 *PKCS12_create_ex(const char *pass, const char *name, EVP_PKEY *pkey, diff --git a/crypto/pkcs7/pk7_doit.c b/crypto/pkcs7/pk7_doit.c index 4a13070a0a..e68aaca466 100644 --- a/crypto/pkcs7/pk7_doit.c +++ b/crypto/pkcs7/pk7_doit.c @@ -1162,11 +1162,11 @@ ASN1_TYPE *PKCS7_get_attribute(const PKCS7_SIGNER_INFO *si, int nid) static ASN1_TYPE *get_attribute(const STACK_OF(X509_ATTRIBUTE) *sk, int nid) { - int idx; - X509_ATTRIBUTE *xa; - idx = X509at_get_attr_by_NID(sk, nid, -1); - xa = X509at_get_attr(sk, idx); - return X509_ATTRIBUTE_get0_type(xa, 0); + int idx = X509at_get_attr_by_NID(sk, nid, -1); + + if (idx < 0) + return NULL; + return X509_ATTRIBUTE_get0_type(X509at_get_attr(sk, idx), 0); } ASN1_OCTET_STRING *PKCS7_digest_from_attributes(STACK_OF(X509_ATTRIBUTE) *sk) diff --git a/crypto/x509/x509_att.c b/crypto/x509/x509_att.c index 73ac59454d..9e6434187c 100644 --- a/crypto/x509/x509_att.c +++ b/crypto/x509/x509_att.c @@ -55,20 +55,28 @@ int X509at_get_attr_by_OBJ(const STACK_OF(X509_ATTRIBUTE) *sk, X509_ATTRIBUTE *X509at_get_attr(const STACK_OF(X509_ATTRIBUTE) *x, int loc) { - if (x == NULL || sk_X509_ATTRIBUTE_num(x) <= loc || loc < 0) + if (x == NULL) { + ERR_raise(ERR_LIB_X509, ERR_R_PASSED_NULL_PARAMETER); return NULL; - + } + if (sk_X509_ATTRIBUTE_num(x) <= loc || loc < 0) { + ERR_raise(ERR_LIB_X509, ERR_R_PASSED_INVALID_ARGUMENT); + return NULL; + } return sk_X509_ATTRIBUTE_value(x, loc); } X509_ATTRIBUTE *X509at_delete_attr(STACK_OF(X509_ATTRIBUTE) *x, int loc) { - X509_ATTRIBUTE *ret; - - if (x == NULL || sk_X509_ATTRIBUTE_num(x) <= loc || loc < 0) + if (x == NULL) { + ERR_raise(ERR_LIB_X509, ERR_R_PASSED_NULL_PARAMETER); return NULL; - ret = sk_X509_ATTRIBUTE_delete(x, loc); - return ret; + } + if (sk_X509_ATTRIBUTE_num(x) <= loc || loc < 0) { + ERR_raise(ERR_LIB_X509, ERR_R_PASSED_INVALID_ARGUMENT); + return NULL; + } + return sk_X509_ATTRIBUTE_delete(x, loc); } STACK_OF(X509_ATTRIBUTE) *X509at_add1_attr(STACK_OF(X509_ATTRIBUTE) **x, @@ -77,10 +85,14 @@ STACK_OF(X509_ATTRIBUTE) *X509at_add1_attr(STACK_OF(X509_ATTRIBUTE) **x, X509_ATTRIBUTE *new_attr = NULL; STACK_OF(X509_ATTRIBUTE) *sk = NULL; - if (x == NULL) { + if (x == NULL || attr == NULL) { ERR_raise(ERR_LIB_X509, ERR_R_PASSED_NULL_PARAMETER); return NULL; } + if (X509at_get_attr_by_OBJ(sk, attr->object, -1) != -1) { + ERR_raise(ERR_LIB_X509, X509_R_DUPLICATE_ATTRIBUTE); + return NULL; + } if (*x == NULL) { if ((sk = sk_X509_ATTRIBUTE_new_null()) == NULL) @@ -113,8 +125,9 @@ STACK_OF(X509_ATTRIBUTE) *X509at_add1_attr_by_OBJ(STACK_OF(X509_ATTRIBUTE) { X509_ATTRIBUTE *attr; STACK_OF(X509_ATTRIBUTE) *ret; + attr = X509_ATTRIBUTE_create_by_OBJ(NULL, obj, type, bytes, len); - if (!attr) + if (attr == NULL) return 0; ret = X509at_add1_attr(x, attr); X509_ATTRIBUTE_free(attr); @@ -128,8 +141,9 @@ STACK_OF(X509_ATTRIBUTE) *X509at_add1_attr_by_NID(STACK_OF(X509_ATTRIBUTE) { X509_ATTRIBUTE *attr; STACK_OF(X509_ATTRIBUTE) *ret; + attr = X509_ATTRIBUTE_create_by_NID(NULL, nid, type, bytes, len); - if (!attr) + if (attr == NULL) return 0; ret = X509at_add1_attr(x, attr); X509_ATTRIBUTE_free(attr); @@ -144,8 +158,9 @@ STACK_OF(X509_ATTRIBUTE) *X509at_add1_attr_by_txt(STACK_OF(X509_ATTRIBUTE) { X509_ATTRIBUTE *attr; STACK_OF(X509_ATTRIBUTE) *ret; + attr = X509_ATTRIBUTE_create_by_txt(NULL, attrname, type, bytes, len); - if (!attr) + if (attr == NULL) return 0; ret = X509at_add1_attr(x, attr); X509_ATTRIBUTE_free(attr); @@ -155,29 +170,26 @@ STACK_OF(X509_ATTRIBUTE) *X509at_add1_attr_by_txt(STACK_OF(X509_ATTRIBUTE) void *X509at_get0_data_by_OBJ(const STACK_OF(X509_ATTRIBUTE) *x, const ASN1_OBJECT *obj, int lastpos, int type) { - int i; + int i = X509at_get_attr_by_OBJ(x, obj, lastpos); X509_ATTRIBUTE *at; - i = X509at_get_attr_by_OBJ(x, obj, lastpos); + if (i == -1) return NULL; - if ((lastpos <= -2) && (X509at_get_attr_by_OBJ(x, obj, i) != -1)) + if (lastpos <= -2 && X509at_get_attr_by_OBJ(x, obj, i) != -1) return NULL; at = X509at_get_attr(x, i); - if (lastpos <= -3 && (X509_ATTRIBUTE_count(at) != 1)) + if (lastpos <= -3 && X509_ATTRIBUTE_count(at) != 1) return NULL; return X509_ATTRIBUTE_get0_data(at, 0, type, NULL); } STACK_OF(X509_ATTRIBUTE) *ossl_x509at_dup(const STACK_OF(X509_ATTRIBUTE) *x) { - int i, n; + int i, n = sk_X509_ATTRIBUTE_num(x); STACK_OF(X509_ATTRIBUTE) *sk = NULL; - n = sk_X509_ATTRIBUTE_num(x); for (i = 0; i < n; ++i) { - X509_ATTRIBUTE *attr = sk_X509_ATTRIBUTE_value(x, i); - - if (X509at_add1_attr(&sk, attr) == NULL) { + if (X509at_add1_attr(&sk, sk_X509_ATTRIBUTE_value(x, i)) == NULL) { sk_X509_ATTRIBUTE_pop_free(sk, X509_ATTRIBUTE_free); return NULL; } @@ -189,10 +201,9 @@ X509_ATTRIBUTE *X509_ATTRIBUTE_create_by_NID(X509_ATTRIBUTE **attr, int nid, int atrtype, const void *data, int len) { - ASN1_OBJECT *obj; + ASN1_OBJECT *obj = OBJ_nid2obj(nid); X509_ATTRIBUTE *ret; - obj = OBJ_nid2obj(nid); if (obj == NULL) { ERR_raise(ERR_LIB_X509, X509_R_UNKNOWN_NID); return NULL; @@ -210,24 +221,25 @@ X509_ATTRIBUTE *X509_ATTRIBUTE_create_by_OBJ(X509_ATTRIBUTE **attr, { X509_ATTRIBUTE *ret; - if ((attr == NULL) || (*attr == NULL)) { + if (attr == NULL || *attr == NULL) { if ((ret = X509_ATTRIBUTE_new()) == NULL) { ERR_raise(ERR_LIB_X509, ERR_R_MALLOC_FAILURE); return NULL; } - } else + } else { ret = *attr; + } if (!X509_ATTRIBUTE_set1_object(ret, obj)) goto err; if (!X509_ATTRIBUTE_set1_data(ret, atrtype, data, len)) goto err; - if ((attr != NULL) && (*attr == NULL)) + if (attr != NULL && *attr == NULL) *attr = ret; return ret; err: - if ((attr == NULL) || (ret != *attr)) + if (attr == NULL || ret != *attr) X509_ATTRIBUTE_free(ret); return NULL; } @@ -237,10 +249,9 @@ X509_ATTRIBUTE *X509_ATTRIBUTE_create_by_txt(X509_ATTRIBUTE **attr, const unsigned char *bytes, int len) { - ASN1_OBJECT *obj; + ASN1_OBJECT *obj = OBJ_txt2obj(atrname, 0); X509_ATTRIBUTE *nattr; - obj = OBJ_txt2obj(atrname, 0); if (obj == NULL) { ERR_raise_data(ERR_LIB_X509, X509_R_INVALID_FIELD_NAME, "name=%s", atrname); @@ -253,8 +264,10 @@ X509_ATTRIBUTE *X509_ATTRIBUTE_create_by_txt(X509_ATTRIBUTE **attr, int X509_ATTRIBUTE_set1_object(X509_ATTRIBUTE *attr, const ASN1_OBJECT *obj) { - if ((attr == NULL) || (obj == NULL)) + if (attr == NULL || obj == NULL) { + ERR_raise(ERR_LIB_X509, ERR_R_PASSED_NULL_PARAMETER); return 0; + } ASN1_OBJECT_free(attr->object); attr->object = OBJ_dup(obj); return attr->object != NULL; @@ -266,12 +279,15 @@ int X509_ATTRIBUTE_set1_data(X509_ATTRIBUTE *attr, int attrtype, ASN1_TYPE *ttmp = NULL; ASN1_STRING *stmp = NULL; int atype = 0; - if (!attr) + + if (attr == NULL) { + ERR_raise(ERR_LIB_X509, ERR_R_PASSED_NULL_PARAMETER); return 0; - if (attrtype & MBSTRING_FLAG) { + } + if ((attrtype & MBSTRING_FLAG) != 0) { stmp = ASN1_STRING_set_by_NID(NULL, data, len, attrtype, OBJ_obj2nid(attr->object)); - if (!stmp) { + if (stmp == NULL) { ERR_raise(ERR_LIB_X509, ERR_R_ASN1_LIB); return 0; } @@ -294,7 +310,7 @@ int X509_ATTRIBUTE_set1_data(X509_ATTRIBUTE *attr, int attrtype, } if ((ttmp = ASN1_TYPE_new()) == NULL) goto err; - if ((len == -1) && !(attrtype & MBSTRING_FLAG)) { + if (len == -1 && (attrtype & MBSTRING_FLAG) == 0) { if (!ASN1_TYPE_set1(ttmp, attrtype, data)) goto err; } else { @@ -320,17 +336,19 @@ int X509_ATTRIBUTE_count(const X509_ATTRIBUTE *attr) ASN1_OBJECT *X509_ATTRIBUTE_get0_object(X509_ATTRIBUTE *attr) { - if (attr == NULL) + if (attr == NULL) { + ERR_raise(ERR_LIB_X509, ERR_R_PASSED_NULL_PARAMETER); return NULL; + } return attr->object; } void *X509_ATTRIBUTE_get0_data(X509_ATTRIBUTE *attr, int idx, int atrtype, void *data) { - ASN1_TYPE *ttmp; - ttmp = X509_ATTRIBUTE_get0_type(attr, idx); - if (!ttmp) + ASN1_TYPE *ttmp = X509_ATTRIBUTE_get0_type(attr, idx); + + if (ttmp == NULL) return NULL; if (atrtype == V_ASN1_BOOLEAN || atrtype == V_ASN1_NULL @@ -343,7 +361,9 @@ void *X509_ATTRIBUTE_get0_data(X509_ATTRIBUTE *attr, int idx, ASN1_TYPE *X509_ATTRIBUTE_get0_type(X509_ATTRIBUTE *attr, int idx) { - if (attr == NULL) + if (attr == NULL) { + ERR_raise(ERR_LIB_X509, ERR_R_PASSED_NULL_PARAMETER); return NULL; + } return sk_ASN1_TYPE_value(attr->set, idx); } diff --git a/crypto/x509/x509_err.c b/crypto/x509/x509_err.c index a933aeef35..61f1e3ecc1 100644 --- a/crypto/x509/x509_err.c +++ b/crypto/x509/x509_err.c @@ -1,6 +1,6 @@ /* * Generated by util/mkerr.pl DO NOT EDIT - * Copyright 1995-2021 The OpenSSL Project Authors. All Rights Reserved. + * Copyright 1995-2022 The OpenSSL Project Authors. All Rights Reserved. * * Licensed under the Apache License 2.0 (the "License"). You may not use * this file except in compliance with the License. You can obtain a copy @@ -28,6 +28,8 @@ static const ERR_STRING_DATA X509_str_reasons[] = { {ERR_PACK(ERR_LIB_X509, 0, X509_R_CRL_ALREADY_DELTA), "crl already delta"}, {ERR_PACK(ERR_LIB_X509, 0, X509_R_CRL_VERIFY_FAILURE), "crl verify failure"}, + {ERR_PACK(ERR_LIB_X509, 0, X509_R_DUPLICATE_ATTRIBUTE), + "duplicate attribute"}, {ERR_PACK(ERR_LIB_X509, 0, X509_R_ERROR_GETTING_MD_BY_NID), "error getting md by nid"}, {ERR_PACK(ERR_LIB_X509, 0, X509_R_ERROR_USING_SIGINF_SET), diff --git a/crypto/x509/x509_req.c b/crypto/x509/x509_req.c index 9e926fbe29..858c6c566c 100644 --- a/crypto/x509/x509_req.c +++ b/crypto/x509/x509_req.c @@ -146,7 +146,7 @@ STACK_OF(X509_EXTENSION) *X509_REQ_get_extensions(X509_REQ *req) return NULL; for (pnid = ext_nids; *pnid != NID_undef; pnid++) { idx = X509_REQ_get_attr_by_NID(req, *pnid, -1); - if (idx == -1) + if (idx < 0) continue; attr = X509_REQ_get_attr(req, idx); ext = X509_ATTRIBUTE_get0_type(attr, 0); |