From c90c469376e28e87caf02e96bf6568131f1c5d1b Mon Sep 17 00:00:00 2001 From: "Dr. David von Oheimb" Date: Fri, 26 Jun 2020 16:16:00 +0200 Subject: Correct confusing X509V3 conf error output by removing needless 'section:' etc. Reviewed-by: Paul Dale (Merged from https://github.com/openssl/openssl/pull/12296) --- crypto/x509/v3_addr.c | 19 ++++++++++--------- crypto/x509/v3_asid.c | 9 +++++---- crypto/x509/v3_bcons.c | 4 +++- crypto/x509/v3_bitst.c | 2 +- crypto/x509/v3_cpols.c | 20 +++++++++++--------- crypto/x509/v3_crld.c | 5 +++-- crypto/x509/v3_extku.c | 2 +- crypto/x509/v3_pci.c | 1 + crypto/x509/v3_pcons.c | 2 +- crypto/x509/v3_pmaps.c | 4 ++-- crypto/x509/v3_tlsf.c | 3 ++- crypto/x509/v3_utl.c | 5 +++-- crypto/x509/x509_local.h | 3 +++ 13 files changed, 46 insertions(+), 33 deletions(-) diff --git a/crypto/x509/v3_addr.c b/crypto/x509/v3_addr.c index 9e2b9d48a9..d965d74553 100644 --- a/crypto/x509/v3_addr.c +++ b/crypto/x509/v3_addr.c @@ -22,6 +22,7 @@ #include #include "crypto/x509.h" #include "ext_dat.h" +#include "x509_local.h" #ifndef OPENSSL_NO_RFC3779 @@ -925,7 +926,7 @@ static void *v2i_IPAddrBlocks(const struct v3_ext_method *method, } else { X509V3err(X509V3_F_V2I_IPADDRBLOCKS, X509V3_R_EXTENSION_NAME_ERROR); - X509V3_conf_err(val); + ERR_add_error_data(1, val->name); goto err; } @@ -949,7 +950,7 @@ static void *v2i_IPAddrBlocks(const struct v3_ext_method *method, t += strspn(t, " \t"); if (*safi > 0xFF || *t++ != ':') { X509V3err(X509V3_F_V2I_IPADDRBLOCKS, X509V3_R_INVALID_SAFI); - X509V3_conf_err(val); + X509V3_conf_add_error_name_value(val); goto err; } t += strspn(t, " \t"); @@ -970,7 +971,7 @@ static void *v2i_IPAddrBlocks(const struct v3_ext_method *method, if (!X509v3_addr_add_inherit(addr, afi, safi)) { X509V3err(X509V3_F_V2I_IPADDRBLOCKS, X509V3_R_INVALID_INHERITANCE); - X509V3_conf_err(val); + X509V3_conf_add_error_name_value(val); goto err; } OPENSSL_free(s); @@ -985,7 +986,7 @@ static void *v2i_IPAddrBlocks(const struct v3_ext_method *method, if (a2i_ipadd(min, s) != length) { X509V3err(X509V3_F_V2I_IPADDRBLOCKS, X509V3_R_INVALID_IPADDRESS); - X509V3_conf_err(val); + X509V3_conf_add_error_name_value(val); goto err; } @@ -995,7 +996,7 @@ static void *v2i_IPAddrBlocks(const struct v3_ext_method *method, if (t == s + i2 || *t != '\0') { X509V3err(X509V3_F_V2I_IPADDRBLOCKS, X509V3_R_EXTENSION_VALUE_ERROR); - X509V3_conf_err(val); + X509V3_conf_add_error_name_value(val); goto err; } if (!X509v3_addr_add_prefix(addr, afi, safi, min, prefixlen)) { @@ -1009,19 +1010,19 @@ static void *v2i_IPAddrBlocks(const struct v3_ext_method *method, if (i1 == i2 || s[i2] != '\0') { X509V3err(X509V3_F_V2I_IPADDRBLOCKS, X509V3_R_EXTENSION_VALUE_ERROR); - X509V3_conf_err(val); + X509V3_conf_add_error_name_value(val); goto err; } if (a2i_ipadd(max, s + i1) != length) { X509V3err(X509V3_F_V2I_IPADDRBLOCKS, X509V3_R_INVALID_IPADDRESS); - X509V3_conf_err(val); + X509V3_conf_add_error_name_value(val); goto err; } if (memcmp(min, max, length_from_afi(afi)) > 0) { X509V3err(X509V3_F_V2I_IPADDRBLOCKS, X509V3_R_EXTENSION_VALUE_ERROR); - X509V3_conf_err(val); + X509V3_conf_add_error_name_value(val); goto err; } if (!X509v3_addr_add_range(addr, afi, safi, min, max)) { @@ -1038,7 +1039,7 @@ static void *v2i_IPAddrBlocks(const struct v3_ext_method *method, default: X509V3err(X509V3_F_V2I_IPADDRBLOCKS, X509V3_R_EXTENSION_VALUE_ERROR); - X509V3_conf_err(val); + X509V3_conf_add_error_name_value(val); goto err; } diff --git a/crypto/x509/v3_asid.c b/crypto/x509/v3_asid.c index 0ff37073cf..0fc7641386 100644 --- a/crypto/x509/v3_asid.c +++ b/crypto/x509/v3_asid.c @@ -23,6 +23,7 @@ #include "crypto/x509.h" #include #include "ext_dat.h" +#include "x509_local.h" #ifndef OPENSSL_NO_RFC3779 @@ -545,7 +546,7 @@ static void *v2i_ASIdentifiers(const struct v3_ext_method *method, } else { X509V3err(X509V3_F_V2I_ASIDENTIFIERS, X509V3_R_EXTENSION_NAME_ERROR); - X509V3_conf_err(val); + X509V3_conf_add_error_name_value(val); goto err; } @@ -557,7 +558,7 @@ static void *v2i_ASIdentifiers(const struct v3_ext_method *method, continue; X509V3err(X509V3_F_V2I_ASIDENTIFIERS, X509V3_R_INVALID_INHERITANCE); - X509V3_conf_err(val); + X509V3_conf_add_error_name_value(val); goto err; } @@ -573,7 +574,7 @@ static void *v2i_ASIdentifiers(const struct v3_ext_method *method, if (val->value[i2] != '-') { X509V3err(X509V3_F_V2I_ASIDENTIFIERS, X509V3_R_INVALID_ASNUMBER); - X509V3_conf_err(val); + X509V3_conf_add_error_name_value(val); goto err; } i2++; @@ -582,7 +583,7 @@ static void *v2i_ASIdentifiers(const struct v3_ext_method *method, if (val->value[i3] != '\0') { X509V3err(X509V3_F_V2I_ASIDENTIFIERS, X509V3_R_INVALID_ASRANGE); - X509V3_conf_err(val); + X509V3_conf_add_error_name_value(val); goto err; } } diff --git a/crypto/x509/v3_bcons.c b/crypto/x509/v3_bcons.c index 6ab4aaf687..01d38473a3 100644 --- a/crypto/x509/v3_bcons.c +++ b/crypto/x509/v3_bcons.c @@ -14,6 +14,7 @@ #include #include #include "ext_dat.h" +#include "x509_local.h" DEFINE_STACK_OF(CONF_VALUE) @@ -73,9 +74,10 @@ static BASIC_CONSTRAINTS *v2i_BASIC_CONSTRAINTS(X509V3_EXT_METHOD *method, } else if (strcmp(val->name, "pathlen") == 0) { if (!X509V3_get_value_int(val, &bcons->pathlen)) goto err; + /* TODO add sanity check on int value - at least, must be >= 0 */ } else { X509V3err(X509V3_F_V2I_BASIC_CONSTRAINTS, X509V3_R_INVALID_NAME); - X509V3_conf_err(val); + X509V3_conf_add_error_name_value(val); goto err; } } diff --git a/crypto/x509/v3_bitst.c b/crypto/x509/v3_bitst.c index ec8fdc55a1..02d40863a6 100644 --- a/crypto/x509/v3_bitst.c +++ b/crypto/x509/v3_bitst.c @@ -86,7 +86,7 @@ ASN1_BIT_STRING *v2i_ASN1_BIT_STRING(X509V3_EXT_METHOD *method, if (!bnam->lname) { X509V3err(X509V3_F_V2I_ASN1_BIT_STRING, X509V3_R_UNKNOWN_BIT_STRING_ARGUMENT); - X509V3_conf_err(val); + ERR_add_error_data(1, val->name); ASN1_BIT_STRING_free(bs); return NULL; } diff --git a/crypto/x509/v3_cpols.c b/crypto/x509/v3_cpols.c index abbf5fbe60..6b507f40d7 100644 --- a/crypto/x509/v3_cpols.c +++ b/crypto/x509/v3_cpols.c @@ -14,6 +14,7 @@ #include #include +#include "x509_local.h" #include "pcy_local.h" #include "ext_dat.h" @@ -116,11 +117,10 @@ static STACK_OF(POLICYINFO) *r2i_certpol(X509V3_EXT_METHOD *method, ia5org = 0; for (i = 0; i < num; i++) { cnf = sk_CONF_VALUE_value(vals, i); - - if (cnf->value || !cnf->name) { + if (cnf->value != NULL || cnf->name == NULL) { X509V3err(X509V3_F_R2I_CERTPOL, X509V3_R_INVALID_POLICY_IDENTIFIER); - X509V3_conf_err(cnf); + X509V3_conf_add_error_name_value(cnf); goto err; } pstr = cnf->name; @@ -133,8 +133,7 @@ static STACK_OF(POLICYINFO) *r2i_certpol(X509V3_EXT_METHOD *method, polsect = X509V3_get_section(ctx, pstr + 1); if (polsect == NULL) { X509V3err(X509V3_F_R2I_CERTPOL, X509V3_R_INVALID_SECTION); - - X509V3_conf_err(cnf); + ERR_add_error_data(1, cnf->name); goto err; } pol = policy_section(ctx, polsect, ia5org); @@ -145,7 +144,7 @@ static STACK_OF(POLICYINFO) *r2i_certpol(X509V3_EXT_METHOD *method, if ((pobj = OBJ_txt2obj(cnf->name, 0)) == NULL) { X509V3err(X509V3_F_R2I_CERTPOL, X509V3_R_INVALID_OBJECT_IDENTIFIER); - X509V3_conf_err(cnf); + ERR_add_error_data(1, cnf->name); goto err; } pol = POLICYINFO_new(); @@ -184,6 +183,7 @@ static POLICYINFO *policy_section(X509V3_CTX *ctx, cnf = sk_CONF_VALUE_value(polstrs, i); if (strcmp(cnf->name, "policyIdentifier") == 0) { ASN1_OBJECT *pobj; + if ((pobj = OBJ_txt2obj(cnf->value, 0)) == NULL) { X509V3err(X509V3_F_POLICY_SECTION, X509V3_R_INVALID_OBJECT_IDENTIFIER); @@ -233,7 +233,6 @@ static POLICYINFO *policy_section(X509V3_CTX *ctx, goto merr; } else { X509V3err(X509V3_F_POLICY_SECTION, X509V3_R_INVALID_OPTION); - X509V3_conf_err(cnf); goto err; } @@ -307,6 +306,7 @@ static POLICYQUALINFO *notice_section(X509V3_CTX *ctx, qual->d.usernotice = not; for (i = 0; i < sk_CONF_VALUE_num(unot); i++) { cnf = sk_CONF_VALUE_value(unot, i); + value = cnf->value; if (strcmp(cnf->name, "explicitText") == 0) { tag = displaytext_str2tag(value, &tag_len); @@ -319,6 +319,7 @@ static POLICYQUALINFO *notice_section(X509V3_CTX *ctx, goto merr; } else if (strcmp(cnf->name, "organization") == 0) { NOTICEREF *nref; + if (!not->noticeref) { if ((nref = NOTICEREF_new()) == NULL) goto merr; @@ -334,6 +335,7 @@ static POLICYQUALINFO *notice_section(X509V3_CTX *ctx, goto merr; } else if (strcmp(cnf->name, "noticeNumbers") == 0) { NOTICEREF *nref; + STACK_OF(CONF_VALUE) *nos; if (!not->noticeref) { if ((nref = NOTICEREF_new()) == NULL) @@ -344,7 +346,7 @@ static POLICYQUALINFO *notice_section(X509V3_CTX *ctx, nos = X509V3_parse_list(cnf->value); if (!nos || !sk_CONF_VALUE_num(nos)) { X509V3err(X509V3_F_NOTICE_SECTION, X509V3_R_INVALID_NUMBERS); - X509V3_conf_err(cnf); + X509V3_conf_add_error_name_value(cnf); sk_CONF_VALUE_pop_free(nos, X509V3_conf_free); goto err; } @@ -354,7 +356,7 @@ static POLICYQUALINFO *notice_section(X509V3_CTX *ctx, goto err; } else { X509V3err(X509V3_F_NOTICE_SECTION, X509V3_R_INVALID_OPTION); - X509V3_conf_err(cnf); + X509V3_conf_add_error_name_value(cnf); goto err; } } diff --git a/crypto/x509/v3_crld.c b/crypto/x509/v3_crld.c index 21a1bfcd7d..b54346d036 100644 --- a/crypto/x509/v3_crld.c +++ b/crypto/x509/v3_crld.c @@ -16,6 +16,7 @@ #include "crypto/x509.h" #include "ext_dat.h" +#include "x509_local.h" DEFINE_STACK_OF(CONF_VALUE) DEFINE_STACK_OF(GENERAL_NAME) @@ -256,7 +257,7 @@ static void *v2i_crld(const X509V3_EXT_METHOD *method, DIST_POINT *point; cnf = sk_CONF_VALUE_value(nval, i); - if (!cnf->value) { + if (cnf->value == NULL) { STACK_OF(CONF_VALUE) *dpsect; dpsect = X509V3_get_section(ctx, cnf->name); if (!dpsect) @@ -398,7 +399,7 @@ static void *v2i_idp(const X509V3_EXT_METHOD *method, X509V3_CTX *ctx, goto err; } else { X509V3err(X509V3_F_V2I_IDP, X509V3_R_INVALID_NAME); - X509V3_conf_err(cnf); + X509V3_conf_add_error_name_value(cnf); goto err; } } diff --git a/crypto/x509/v3_extku.c b/crypto/x509/v3_extku.c index ed51b60f0c..7769bc9931 100644 --- a/crypto/x509/v3_extku.c +++ b/crypto/x509/v3_extku.c @@ -97,7 +97,7 @@ static void *v2i_EXTENDED_KEY_USAGE(const X509V3_EXT_METHOD *method, sk_ASN1_OBJECT_pop_free(extku, ASN1_OBJECT_free); X509V3err(X509V3_F_V2I_EXTENDED_KEY_USAGE, X509V3_R_INVALID_OBJECT_IDENTIFIER); - X509V3_conf_err(val); + ERR_add_error_data(1, extval); return NULL; } sk_ASN1_OBJECT_push(extku, objtmp); /* no failure as it was reserved */ diff --git a/crypto/x509/v3_pci.c b/crypto/x509/v3_pci.c index 30711149ce..714733684b 100644 --- a/crypto/x509/v3_pci.c +++ b/crypto/x509/v3_pci.c @@ -255,6 +255,7 @@ static PROXY_CERT_INFO_EXTENSION *r2i_pci(X509V3_EXT_METHOD *method, vals = X509V3_parse_list(value); for (i = 0; i < sk_CONF_VALUE_num(vals); i++) { CONF_VALUE *cnf = sk_CONF_VALUE_value(vals, i); + if (!cnf->name || (*cnf->name != '@' && !cnf->value)) { X509V3err(X509V3_F_R2I_PCI, X509V3_R_INVALID_PROXY_POLICY_SETTING); diff --git a/crypto/x509/v3_pcons.c b/crypto/x509/v3_pcons.c index e7bb7e9546..88a9497504 100644 --- a/crypto/x509/v3_pcons.c +++ b/crypto/x509/v3_pcons.c @@ -76,7 +76,7 @@ static void *v2i_POLICY_CONSTRAINTS(const X509V3_EXT_METHOD *method, goto err; } else { X509V3err(X509V3_F_V2I_POLICY_CONSTRAINTS, X509V3_R_INVALID_NAME); - X509V3_conf_err(val); + ERR_add_error_data(1, val->name); goto err; } } diff --git a/crypto/x509/v3_pmaps.c b/crypto/x509/v3_pmaps.c index d54384dac2..23aefb196c 100644 --- a/crypto/x509/v3_pmaps.c +++ b/crypto/x509/v3_pmaps.c @@ -85,7 +85,7 @@ static void *v2i_POLICY_MAPPINGS(const X509V3_EXT_METHOD *method, if (!val->value || !val->name) { X509V3err(X509V3_F_V2I_POLICY_MAPPINGS, X509V3_R_INVALID_OBJECT_IDENTIFIER); - X509V3_conf_err(val); + ERR_add_error_data(1, val->name); goto err; } obj1 = OBJ_txt2obj(val->name, 0); @@ -93,7 +93,7 @@ static void *v2i_POLICY_MAPPINGS(const X509V3_EXT_METHOD *method, if (!obj1 || !obj2) { X509V3err(X509V3_F_V2I_POLICY_MAPPINGS, X509V3_R_INVALID_OBJECT_IDENTIFIER); - X509V3_conf_err(val); + ERR_add_error_data(1, val->name); goto err; } pmap = POLICY_MAPPING_new(); diff --git a/crypto/x509/v3_tlsf.c b/crypto/x509/v3_tlsf.c index e494e4e8d1..81ce333a34 100644 --- a/crypto/x509/v3_tlsf.c +++ b/crypto/x509/v3_tlsf.c @@ -14,6 +14,7 @@ #include #include #include "ext_dat.h" +#include "x509_local.h" DEFINE_STACK_OF(ASN1_INTEGER) DEFINE_STACK_OF(CONF_VALUE) @@ -119,7 +120,7 @@ static TLS_FEATURE *v2i_TLS_FEATURE(const X509V3_EXT_METHOD *method, if (((*endptr) != '\0') || (extval == endptr) || (tlsextid < 0) || (tlsextid > 65535)) { X509V3err(X509V3_F_V2I_TLS_FEATURE, X509V3_R_INVALID_SYNTAX); - X509V3_conf_err(val); + X509V3_conf_add_error_name_value(val); goto err; } } diff --git a/crypto/x509/v3_utl.c b/crypto/x509/v3_utl.c index aefb589743..5f641b9d43 100644 --- a/crypto/x509/v3_utl.c +++ b/crypto/x509/v3_utl.c @@ -19,6 +19,7 @@ #include "crypto/x509.h" #include #include "ext_dat.h" +#include "x509_local.h" DEFINE_STACK_OF(CONF_VALUE) DEFINE_STACK_OF(GENERAL_NAME) @@ -271,7 +272,7 @@ int X509V3_get_value_bool(const CONF_VALUE *value, int *asn1_bool) err: X509V3err(X509V3_F_X509V3_GET_VALUE_BOOL, X509V3_R_INVALID_BOOLEAN_STRING); - X509V3_conf_err(value); + X509V3_conf_add_error_name_value(value); return 0; } @@ -280,7 +281,7 @@ int X509V3_get_value_int(const CONF_VALUE *value, ASN1_INTEGER **aint) ASN1_INTEGER *itmp; if ((itmp = s2i_ASN1_INTEGER(NULL, value->value)) == NULL) { - X509V3_conf_err(value); + X509V3_conf_add_error_name_value(value); return 0; } *aint = itmp; diff --git a/crypto/x509/x509_local.h b/crypto/x509/x509_local.h index e944d16afe..6a2137129c 100644 --- a/crypto/x509/x509_local.h +++ b/crypto/x509/x509_local.h @@ -9,6 +9,9 @@ #include "internal/refcount.h" +#define X509V3_conf_add_error_name_value(val) \ + ERR_add_error_data(4, "name=", (val)->name, ", value=", (val)->value) + /* * This structure holds all parameters associated with a verify operation by * including an X509_VERIFY_PARAM structure in related structures the -- cgit v1.2.3