v2i_AUTHORITY_KEYID(): Improve error reporting on parsing config values/options

Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/16345)

diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt
index 6e75af9b8bba3407717e9615ee06a0fce6f85715..3c59fce96c6491ba1c0ad5fec7c49f12a55577be 100644 (file)
--- a/crypto/err/openssl.txt
+++ b/crypto/err/openssl.txt
@@ -1585,6 +1585,8 @@ UI_R_UNKNOWN_TTYGET_ERRNO_VALUE:108:unknown ttyget errno value
 UI_R_USER_DATA_DUPLICATION_UNSUPPORTED:112:user data duplication unsupported
 X509V3_R_BAD_IP_ADDRESS:118:bad ip address
 X509V3_R_BAD_OBJECT:119:bad object
+X509V3_R_BAD_OPTION:170:bad option
+X509V3_R_BAD_VALUE:171:bad value
 X509V3_R_BN_DEC2BN_ERROR:100:bn dec2bn error
 X509V3_R_BN_TO_ASN1_INTEGER_ERROR:101:bn to asn1 integer error
 X509V3_R_DIRNAME_ERROR:149:dirname error
@@ -1651,6 +1653,7 @@ X509V3_R_UNKNOWN_BIT_STRING_ARGUMENT:111:unknown bit string argument
 X509V3_R_UNKNOWN_EXTENSION:129:unknown extension
 X509V3_R_UNKNOWN_EXTENSION_NAME:130:unknown extension name
 X509V3_R_UNKNOWN_OPTION:120:unknown option
+X509V3_R_UNKNOWN_VALUE:172:unknown value
 X509V3_R_UNSUPPORTED_OPTION:117:unsupported option
 X509V3_R_UNSUPPORTED_TYPE:167:unsupported type
 X509V3_R_USER_TOO_LONG:132:user too long

diff --git a/crypto/x509/v3_akid.c b/crypto/x509/v3_akid.c
index 2a993dd5bcbe4f0c773272b391369168fefc8097..209f32cbf7658eaad8756c50c626b24eb6447da1 100644 (file)
--- a/crypto/x509/v3_akid.c
+++ b/crypto/x509/v3_akid.c
@@ -85,14 +85,14 @@ static STACK_OF(CONF_VALUE) *i2v_AUTHORITY_KEYID(X509V3_EXT_METHOD *method,
 }
 
 /*-
- * Currently two options:
- * keyid: use the issuers subject keyid, the value 'always' means its is
- * an error if the issuer certificate doesn't have a key id.
- * issuer: use the issuers cert issuer and serial number. The default is
- * to only use this if keyid is not present. With the option 'always'
+ * Three explicit tags may be given, where 'keyid' and 'issuer' may be combined:
+ * 'none': do not add any authority key identifier.
+ * 'keyid': use the issuer's subject keyid; the option 'always' means its is
+ * an error if the issuer certificate doesn't have a subject key id.
+ * 'issuer': use the issuer's cert issuer and serial number. The default is
+ * to only use this if 'keyid' is not present. With the option 'always'
  * this is always included.
  */
-
 static AUTHORITY_KEYID *v2i_AUTHORITY_KEYID(X509V3_EXT_METHOD *method,
                                             X509V3_CTX *ctx,
                                             STACK_OF(CONF_VALUE) *values)
@@ -119,16 +119,27 @@ static AUTHORITY_KEYID *v2i_AUTHORITY_KEYID(X509V3_EXT_METHOD *method,
 
     for (i = 0; i < n; i++) {
         cnf = sk_CONF_VALUE_value(values, i);
-        if (strcmp(cnf->name, "keyid") == 0) {
+        if (cnf->value != NULL && strcmp(cnf->value, "always") != 0) {
+            ERR_raise_data(ERR_LIB_X509V3, X509V3_R_UNKNOWN_OPTION,
+                           "name=%s option=%s", cnf->name, cnf->value);
+            goto err;
+        }
+        if (strcmp(cnf->name, "keyid") == 0 && keyid == 0) {
             keyid = 1;
-            if (cnf->value && strcmp(cnf->value, "always") == 0)
+            if (cnf->value != NULL)
                 keyid = 2;
-        } else if (strcmp(cnf->name, "issuer") == 0) {
+        } else if (strcmp(cnf->name, "issuer") == 0 && issuer == 0) {
             issuer = 1;
-            if (cnf->value && strcmp(cnf->value, "always") == 0)
+            if (cnf->value != NULL)
                 issuer = 2;
+        } else if (strcmp(cnf->name, "none") == 0
+                   || strcmp(cnf->name, "keyid") == 0
+                   || strcmp(cnf->name, "issuer") == 0) {
+            ERR_raise_data(ERR_LIB_X509V3, X509V3_R_BAD_VALUE,
+                           "name=%s", cnf->name);
+            goto err;
         } else {
-            ERR_raise_data(ERR_LIB_X509V3, X509V3_R_UNKNOWN_OPTION,
+            ERR_raise_data(ERR_LIB_X509V3, X509V3_R_UNKNOWN_VALUE,
                            "name=%s", cnf->name);
             goto err;
         }

diff --git a/crypto/x509/v3err.c b/crypto/x509/v3err.c
index 6f38034c1afe765d58a8a7f4cd4f8ff051e06474..b52f16f597fadcf9c9d8b92859a36cfabb701584 100644 (file)
--- a/crypto/x509/v3err.c
+++ b/crypto/x509/v3err.c
@@ -17,6 +17,8 @@
 static const ERR_STRING_DATA X509V3_str_reasons[] = {
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_BAD_IP_ADDRESS), "bad ip address"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_BAD_OBJECT), "bad object"},
+    {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_BAD_OPTION), "bad option"},
+    {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"},
@@ -127,6 +129,7 @@ static const ERR_STRING_DATA X509V3_str_reasons[] = {
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_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"},
     {ERR_PACK(ERR_LIB_X509V3, 0, X509V3_R_UNSUPPORTED_TYPE),

diff --git a/doc/man5/x509v3_config.pod b/doc/man5/x509v3_config.pod
index fb9e562d7fa762af08805169220beb82bcf71017..97251d5d09eb0fa844871b7ce285f7d4a60e23cd 100644 (file)
--- a/doc/man5/x509v3_config.pod
+++ b/doc/man5/x509v3_config.pod
@@ -195,8 +195,8 @@ or both of them, separated by C<,>.
 Either or both can have the option B<always>,
 indicated by putting a colon C<:> between the value and this option.
 For self-signed certificates the AKID is suppressed unless B<always> is present.
-By default the B<x509>, B<req>, and B<ca> apps behave as if
-"none" was given for self-signed certificates and "keyid, issuer" otherwise.
+By default the B<x509>, B<req>, and B<ca> apps behave as if B<none> was given
+for self-signed certificates and B<keyid>C<,> B<issuer> otherwise.
 
 If B<keyid> is present, an attempt is made to
 copy the subject key identifier (SKID) from the issuer certificate except if

diff --git a/include/openssl/x509v3err.h b/include/openssl/x509v3err.h
index 1ae3a56209ae87e97c2a723a01835e675d6aa366..fcad1bfca5b7726e6fbbb21d11da0a2703dfc522 100644 (file)
--- a/include/openssl/x509v3err.h
+++ b/include/openssl/x509v3err.h
@@ -23,6 +23,8 @@
  */
 # define X509V3_R_BAD_IP_ADDRESS                          118
 # define X509V3_R_BAD_OBJECT                              119
+# define X509V3_R_BAD_OPTION                              170
+# define X509V3_R_BAD_VALUE                               171
 # define X509V3_R_BN_DEC2BN_ERROR                         100
 # define X509V3_R_BN_TO_ASN1_INTEGER_ERROR                101
 # define X509V3_R_DIRNAME_ERROR                           149
@@ -86,6 +88,7 @@
 # define X509V3_R_UNKNOWN_EXTENSION                       129
 # define X509V3_R_UNKNOWN_EXTENSION_NAME                  130
 # define X509V3_R_UNKNOWN_OPTION                          120
+# define X509V3_R_UNKNOWN_VALUE                           172
 # define X509V3_R_UNSUPPORTED_OPTION                      117
 # define X509V3_R_UNSUPPORTED_TYPE                        167
 # define X509V3_R_USER_TOO_LONG                           132