fix X509_PURPOSE_add() to take |sname| as primary key and handle |id|…

… in a backwd compat way for new purpose Fixes openssl#25873
siemens · Dec 20, 2024 · 98091bf · 98091bf
1 parent 6c2d635
commit 98091bf
Show file tree

Hide file tree

Showing 10 changed files with 258 additions and 72 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -70,6 +70,14 @@ OpenSSL 3.5
 
    *David von Oheimb*
 
+ * `X509_PURPOSE_add()` has been fixed to take as the primary purpose identifier
+   not the `id` but the `sname` parameter.
+   For its convenient use, `X509_PURPOSE_get_fresh_id()` has been added.
+
+   This work was sponsored by Siemens AG.
+
+   *David von Oheimb*
+
  * Optionally allow the FIPS provider to use the `JITTER` entropy source.
    Note that using this option will require the resulting FIPS provider
    to undergo entropy source validation [ESV] by the [CMVP], without this

diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt
@@ -1811,6 +1811,7 @@ X509V3_R_POLICY_PATH_LENGTH_ALREADY_DEFINED:157:\
 	policy path length already defined
 X509V3_R_POLICY_WHEN_PROXY_LANGUAGE_REQUIRES_NO_POLICY:159:\
 	policy when proxy language requires no policy
+X509V3_R_PURPOSE_NOT_UNIQUE:173:purpose not unique
 X509V3_R_SECTION_NOT_FOUND:150:section not found
 X509V3_R_UNABLE_TO_GET_ISSUER_DETAILS:122:unable to get issuer details
 X509V3_R_UNABLE_TO_GET_ISSUER_KEYID:123:unable to get issuer keyid

diff --git a/crypto/x509/v3_purp.c b/crypto/x509/v3_purp.c
@@ -42,6 +42,7 @@ static int check_purpose_ocsp_helper(const X509_PURPOSE *xp, const X509 *x,
 static int xp_cmp(const X509_PURPOSE *const *a, const X509_PURPOSE *const *b);
 static void xptable_free(X509_PURPOSE *p);
 
+/* note that the id must be unique and for the standard entries == idx + 1 */
 static X509_PURPOSE xstandard[] = {
     {X509_PURPOSE_SSL_CLIENT, X509_TRUST_SSL_CLIENT, 0,
      check_purpose_ssl_client, "SSL client", "sslclient", NULL},
@@ -70,6 +71,7 @@ static X509_PURPOSE xstandard[] = {
 
 #define X509_PURPOSE_COUNT OSSL_NELEM(xstandard)
 
+/* the id must be unique, but there may be gaps and maybe table is not sorted */
 static STACK_OF(X509_PURPOSE) *xptable = NULL;
 
 static int xp_cmp(const X509_PURPOSE *const *a, const X509_PURPOSE *const *b)
@@ -118,6 +120,16 @@ int X509_PURPOSE_get_count(void)
     return sk_X509_PURPOSE_num(xptable) + X509_PURPOSE_COUNT;
 }
 
+/* find smallest identifier not yet taken - note there might be gaps */
+int X509_PURPOSE_get_fresh_id(void)
+{
+    int id = X509_PURPOSE_MAX + 1;
+
+    while (X509_PURPOSE_get_by_id(id) != -1)
+        id++;
+    return id; /* is guaranteed to be unique and > X509_PURPOSE_MAX and != 0 */
+}
+
 X509_PURPOSE *X509_PURPOSE_get0(int idx)
 {
     if (idx < 0)
@@ -157,26 +169,49 @@ int X509_PURPOSE_get_by_id(int purpose)
     return idx + X509_PURPOSE_COUNT;
 }
 
+/*
+ * Add purpose entry identified by |sname|. |id| must be >= X509_PURPOSE_MIN.
+ * May also be used to modify existing entry, including changing its id.
+ */
 int X509_PURPOSE_add(int id, int trust, int flags,
                      int (*ck) (const X509_PURPOSE *, const X509 *, int),
                      const char *name, const char *sname, void *arg)
 {
+    int old_id = 0;
     int idx;
     X509_PURPOSE *ptmp;
 
+    if (id < X509_PURPOSE_MIN) {
+        ERR_raise(ERR_LIB_X509V3, X509V3_R_INVALID_PURPOSE);
+        return 0;
+    }
+    if (trust < X509_TRUST_DEFAULT || name == NULL || sname == NULL || ck == NULL) {
+        ERR_raise(ERR_LIB_X509, ERR_R_PASSED_INVALID_ARGUMENT);
+        return 0;
+    }
+
     /* This is set according to what we change: application can't set it */
     flags &= ~X509_PURPOSE_DYNAMIC;
     /* This will always be set for application modified trust entries */
     flags |= X509_PURPOSE_DYNAMIC_NAME;
+
     /* Get existing entry if any */
-    idx = X509_PURPOSE_get_by_id(id);
-    /* Need a new entry */
-    if (idx == -1) {
+    idx = X509_PURPOSE_get_by_sname(sname);
+    if (idx == -1) { /* Need a new entry */
+        if (X509_PURPOSE_get_by_id(id) != -1) {
+            ERR_raise(ERR_LIB_X509V3, X509V3_R_PURPOSE_NOT_UNIQUE);
+            return 0;
+        }
         if ((ptmp = OPENSSL_malloc(sizeof(*ptmp))) == NULL)
             return 0;
         ptmp->flags = X509_PURPOSE_DYNAMIC;
     } else {
         ptmp = X509_PURPOSE_get0(idx);
+        old_id = ptmp->purpose;
+        if (id != old_id && X509_PURPOSE_get_by_id(id) != -1) {
+            ERR_raise(ERR_LIB_X509V3, X509V3_R_PURPOSE_NOT_UNIQUE);
+            return 0;
+        }
     }
 
     /* OPENSSL_free existing name if dynamic */
@@ -210,6 +245,9 @@ int X509_PURPOSE_add(int id, int trust, int flags,
             ERR_raise(ERR_LIB_X509V3, ERR_R_CRYPTO_LIB);
             goto err;
         }
+    } else if (id != old_id) {
+        /* on changing existing entry id, make sure to reset 'sorted' */
+        (void)sk_X509_PURPOSE_set(xptable, idx, ptmp);
     }
     return 1;
  err:

diff --git a/crypto/x509/v3err.c b/crypto/x509/v3err.c
@@ -1,6 +1,6 @@
 /*
  * Generated by util/mkerr.pl DO NOT EDIT
- * Copyright 1995-2022 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 1995-2024 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
@@ -21,119 +21,121 @@ static const ERR_STRING_DATA X509V3_str_reasons[] = {
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_BAD_VALUE), "bad value"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_BN_DEC2BN_ERROR), "bn dec2bn error"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_BN_TO_ASN1_INTEGER_ERROR),
-    "bn to asn1 integer error"},
+     "bn to asn1 integer error"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_DIRNAME_ERROR), "dirname error"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_DISTPOINT_ALREADY_SET),
-    "distpoint already set"},
+     "distpoint already set"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_DUPLICATE_ZONE_ID),
-    "duplicate zone id"},
+     "duplicate zone id"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_EMPTY_KEY_USAGE), "empty key usage"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_ERROR_CONVERTING_ZONE),
-    "error converting zone"},
+     "error converting zone"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_ERROR_CREATING_EXTENSION),
-    "error creating extension"},
+     "error creating extension"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_ERROR_IN_EXTENSION),
-    "error in extension"},
+     "error in extension"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_EXPECTED_A_SECTION_NAME),
-    "expected a section name"},
+     "expected a section name"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_EXTENSION_EXISTS),
-    "extension exists"},
+     "extension exists"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_EXTENSION_NAME_ERROR),
-    "extension name error"},
+     "extension name error"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_EXTENSION_NOT_FOUND),
-    "extension not found"},
+     "extension not found"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_EXTENSION_SETTING_NOT_SUPPORTED),
-    "extension setting not supported"},
+     "extension setting not supported"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_EXTENSION_VALUE_ERROR),
-    "extension value error"},
+     "extension value error"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_ILLEGAL_EMPTY_EXTENSION),
-    "illegal empty extension"},
+     "illegal empty extension"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_INCORRECT_POLICY_SYNTAX_TAG),
-    "incorrect policy syntax tag"},
+     "incorrect policy syntax tag"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_INVALID_ASNUMBER),
-    "invalid asnumber"},
+     "invalid asnumber"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_INVALID_ASRANGE), "invalid asrange"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_INVALID_BOOLEAN_STRING),
-    "invalid boolean string"},
+     "invalid boolean string"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_INVALID_CERTIFICATE),
-    "invalid certificate"},
+     "invalid certificate"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_INVALID_EMPTY_NAME),
-    "invalid empty name"},
+     "invalid empty name"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_INVALID_EXTENSION_STRING),
-    "invalid extension string"},
+     "invalid extension string"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_INVALID_INHERITANCE),
-    "invalid inheritance"},
+     "invalid inheritance"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_INVALID_IPADDRESS),
-    "invalid ipaddress"},
+     "invalid ipaddress"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_INVALID_MULTIPLE_RDNS),
-    "invalid multiple rdns"},
+     "invalid multiple rdns"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_INVALID_NAME), "invalid name"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_INVALID_NULL_ARGUMENT),
-    "invalid null argument"},
+     "invalid null argument"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_INVALID_NULL_VALUE),
-    "invalid null value"},
+     "invalid null value"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_INVALID_NUMBER), "invalid number"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_INVALID_NUMBERS), "invalid numbers"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_INVALID_OBJECT_IDENTIFIER),
-    "invalid object identifier"},
+     "invalid object identifier"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_INVALID_OPTION), "invalid option"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_INVALID_POLICY_IDENTIFIER),
-    "invalid policy identifier"},
+     "invalid policy identifier"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_INVALID_PROXY_POLICY_SETTING),
-    "invalid proxy policy setting"},
+     "invalid proxy policy setting"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_INVALID_PURPOSE), "invalid purpose"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_INVALID_SAFI), "invalid safi"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_INVALID_SECTION), "invalid section"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_INVALID_SYNTAX), "invalid syntax"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_ISSUER_DECODE_ERROR),
-    "issuer decode error"},
+     "issuer decode error"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_MISSING_VALUE), "missing value"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_NEED_ORGANIZATION_AND_NUMBERS),
-    "need organization and numbers"},
+     "need organization and numbers"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_NEGATIVE_PATHLEN),
-    "negative pathlen"},
+     "negative pathlen"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_NO_CONFIG_DATABASE),
-    "no config database"},
+     "no config database"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_NO_ISSUER_CERTIFICATE),
-    "no issuer certificate"},
+     "no issuer certificate"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_NO_ISSUER_DETAILS),
-    "no issuer details"},
+     "no issuer details"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_NO_POLICY_IDENTIFIER),
-    "no policy identifier"},
+     "no policy identifier"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_NO_PROXY_CERT_POLICY_LANGUAGE_DEFINED),
-    "no proxy cert policy language defined"},
+     "no proxy cert policy language defined"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_NO_PUBLIC_KEY), "no public key"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_NO_SUBJECT_DETAILS),
-    "no subject details"},
+     "no subject details"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_OPERATION_NOT_DEFINED),
-    "operation not defined"},
+     "operation not defined"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_OTHERNAME_ERROR), "othername error"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_POLICY_LANGUAGE_ALREADY_DEFINED),
-    "policy language already defined"},
+     "policy language already defined"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_POLICY_PATH_LENGTH),
-    "policy path length"},
+     "policy path length"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_POLICY_PATH_LENGTH_ALREADY_DEFINED),
-    "policy path length already defined"},
+     "policy path length already defined"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_POLICY_WHEN_PROXY_LANGUAGE_REQUIRES_NO_POLICY),
-    "policy when proxy language requires no policy"},
+     "policy when proxy language requires no policy"},
+    {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_PURPOSE_NOT_UNIQUE),
+     "purpose not unique"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_SECTION_NOT_FOUND),
-    "section not found"},
+     "section not found"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_UNABLE_TO_GET_ISSUER_DETAILS),
-    "unable to get issuer details"},
+     "unable to get issuer details"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_UNABLE_TO_GET_ISSUER_KEYID),
-    "unable to get issuer keyid"},
+     "unable to get issuer keyid"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_UNKNOWN_BIT_STRING_ARGUMENT),
-    "unknown bit string argument"},
+     "unknown bit string argument"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_UNKNOWN_EXTENSION),
-    "unknown extension"},
+     "unknown extension"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_UNKNOWN_EXTENSION_NAME),
-    "unknown extension name"},
+     "unknown extension name"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_UNKNOWN_OPTION), "unknown option"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_UNKNOWN_VALUE), "unknown value"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_UNSUPPORTED_OPTION),
-    "unsupported option"},
+     "unsupported option"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_UNSUPPORTED_TYPE),
-    "unsupported type"},
+     "unsupported type"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_USER_TOO_LONG), "user too long"},
     {0, NULL}
 };