Add X509 version constants.

The X509 version APIs return the numerical values of the version
numbers, which are one off from the names. This is a bit confusing.
Where they don't get it wrong (accidentally making an "X509v4"
certificate), callers tend to try commenting every call site to explain
the mismatch, including in OpenSSL itself.

Define constants for these values, so code can be self-documenting and
callers are nudged towards the right values.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14549)

diff --git a/apps/ca.c b/apps/ca.c
index 6c1df8d2e3e4c86f719820869ba43877f3a30f4a..2476343fddd33de648f839ebe364c226809579fd 100755 (executable)
--- a/apps/ca.c
+++ b/apps/ca.c
@@ -1255,8 +1255,8 @@ end_of_options:
             }
         }
         if (crl_ext != NULL || crl_v2) {
-            if (!X509_CRL_set_version(crl, 1))
-                goto end;       /* version 2 CRL */
+            if (!X509_CRL_set_version(crl, X509_CRL_VERSION_2))
+                goto end;
         }
 
         /* we have a CRL number that need updating */

diff --git a/apps/lib/apps.c b/apps/lib/apps.c
index e39e7cd061832b4520817466528e17714290ad7c..4b7b38cf5cf242a10072574ddce091da64947ed9 100644 (file)
--- a/apps/lib/apps.c
+++ b/apps/lib/apps.c
@@ -2209,7 +2209,7 @@ int do_X509_sign(X509 *cert, EVP_PKEY *pkey, const EVP_MD *md,
 
     if (sk_X509_EXTENSION_num(exts /* may be NULL */) > 0) {
         /* Prevent X509_V_ERR_EXTENSIONS_REQUIRE_VERSION_3 */
-        if (!X509_set_version(cert, 2)) /* Make sure cert is X509 v3 */
+        if (!X509_set_version(cert, X509_VERSION_3))
             goto end;
 
         /*

diff --git a/apps/req.c b/apps/req.c
index 89bde55b93d19eca45f859b3cfbc956eefd4390f..0a524118f09f6f7cc5bb7861b286664c4855ef5d 100644 (file)
--- a/apps/req.c
+++ b/apps/req.c
@@ -1117,7 +1117,8 @@ static int make_REQ(X509_REQ *req, EVP_PKEY *pkey, X509_NAME *fsubj,
         }
     }
 
-    if (!X509_REQ_set_version(req, 0L)) /* so far there is only version 1 */
+    /* so far there is only version 1 */
+    if (!X509_REQ_set_version(req, X509_REQ_VERSION_1))
         goto err;
 
     if (fsubj != NULL)

diff --git a/crypto/x509/t_crl.c b/crypto/x509/t_crl.c
index b9bffbb0c6d63c6ddcdb188d7208339b74c4fffb..48bcf5bb44e11ffbec6fe5b1e5152a6453ab9f33 100644 (file)
--- a/crypto/x509/t_crl.c
+++ b/crypto/x509/t_crl.c
@@ -48,7 +48,7 @@ int X509_CRL_print_ex(BIO *out, X509_CRL *x, unsigned long nmflag)
 
     BIO_printf(out, "Certificate Revocation List (CRL):\n");
     l = X509_CRL_get_version(x);
-    if (l >= 0 && l <= 1)
+    if (l >= X509_CRL_VERSION_1 && l <= X509_CRL_VERSION_2)
         BIO_printf(out, "%8sVersion %ld (0x%lx)\n", "", l + 1, (unsigned long)l);
     else
         BIO_printf(out, "%8sVersion unknown (%ld)\n", "", l);

diff --git a/crypto/x509/t_req.c b/crypto/x509/t_req.c
index 29479b08860c540ff565e01d0df98e0a2e27544c..095c16510099922c2ed94bf2344ca965d9367d2c 100644 (file)
--- a/crypto/x509/t_req.c
+++ b/crypto/x509/t_req.c
@@ -60,7 +60,7 @@ int X509_REQ_print_ex(BIO *bp, X509_REQ *x, unsigned long nmflags,
     }
     if (!(cflag & X509_FLAG_NO_VERSION)) {
         l = X509_REQ_get_version(x);
-        if (l >= 0 && l <= 2) {
+        if (l == X509_REQ_VERSION_1) {
             if (BIO_printf(bp, "%8sVersion: %ld (0x%lx)\n", "", l + 1, (unsigned long)l) <= 0)
                 goto err;
         } else {

diff --git a/crypto/x509/t_x509.c b/crypto/x509/t_x509.c
index 78d4452156122e506e4ece1d9cfd9aa95c8422ad..bdfb4cb08b9fa5055e65e07b1797910961aa44f1 100644 (file)
--- a/crypto/x509/t_x509.c
+++ b/crypto/x509/t_x509.c
@@ -71,7 +71,7 @@ int X509_print_ex(BIO *bp, X509 *x, unsigned long nmflags,
     }
     if (!(cflag & X509_FLAG_NO_VERSION)) {
         l = X509_get_version(x);
-        if (l >= 0 && l <= 2) {
+        if (l >= X509_VERSION_1 && l <= X509_VERSION_3) {
             if (BIO_printf(bp, "%8sVersion: %ld (0x%lx)\n", "", l + 1, (unsigned long)l) <= 0)
                 goto err;
         } else {

diff --git a/crypto/x509/v3_purp.c b/crypto/x509/v3_purp.c
index 5b13fd7445d05e22fb4cda944d150bdaa0d5453d..ede556d8ef0b3413a02a7f22160dc10f6cc3d5c6 100644 (file)
--- a/crypto/x509/v3_purp.c
+++ b/crypto/x509/v3_purp.c
@@ -425,7 +425,7 @@ int ossl_x509v3_cache_extensions(X509 *x)
     ERR_set_mark();
 
     /* V1 should mean no extensions ... */
-    if (X509_get_version(x) == 0)
+    if (X509_get_version(x) == X509_VERSION_1)
         x->ex_flags |= EXFLAG_V1;
 
     /* Handle basic constraints */

diff --git a/crypto/x509/x509_cmp.c b/crypto/x509/x509_cmp.c
index 0cc5ed7f5f7f87a1f30cf48bb3e3a6aec1b197ab..1c1a5e6a27fef8cb9fd220f41a4acd43fb0eb19a 100644 (file)
--- a/crypto/x509/x509_cmp.c
+++ b/crypto/x509/x509_cmp.c
@@ -486,7 +486,7 @@ int X509_chain_check_suiteb(int *perror_depth, X509 *x, STACK_OF(X509) *chain,
     if (chain == NULL)
         return check_suite_b(pk, -1, &tflags);
 
-    if (X509_get_version(x) != 2) {
+    if (X509_get_version(x) != X509_VERSION_3) {
         rv = X509_V_ERR_SUITE_B_INVALID_VERSION;
         /* Correct error depth */
         i = 0;
@@ -503,7 +503,7 @@ int X509_chain_check_suiteb(int *perror_depth, X509 *x, STACK_OF(X509) *chain,
     for (; i < sk_X509_num(chain); i++) {
         sign_nid = X509_get_signature_nid(x);
         x = sk_X509_value(chain, i);
-        if (X509_get_version(x) != 2) {
+        if (X509_get_version(x) != X509_VERSION_3) {
             rv = X509_V_ERR_SUITE_B_INVALID_VERSION;
             goto end;
         }

diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c
index cb541084dfa952482ca63b6553318328072d1e5e..4e6ce11f4efddd5146b9411846ce7bc610861187 100644 (file)
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -562,7 +562,7 @@ static int check_extensions(X509_STORE_CTX *ctx)
             CB_FAIL_IF(x->skid != NULL
                            && (x->ex_flags & EXFLAG_SKID_CRITICAL) != 0,
                        ctx, x, i, X509_V_ERR_SUBJECT_KEY_IDENTIFIER_CRITICAL);
-            if (X509_get_version(x) >= 2) { /* at least X.509v3 */
+            if (X509_get_version(x) >= X509_VERSION_3) {
                 /* Check AKID presence acc. to RFC 5280 section 4.2.1.1 */
                 CB_FAIL_IF(i + 1 < num /*
                                         * this means not last cert in chain,
@@ -2053,7 +2053,7 @@ X509_CRL *X509_CRL_diff(X509_CRL *base, X509_CRL *newer,
     }
     /* Create new CRL */
     crl = X509_CRL_new_ex(base->libctx, base->propq);
-    if (crl == NULL || !X509_CRL_set_version(crl, 1))
+    if (crl == NULL || !X509_CRL_set_version(crl, X509_CRL_VERSION_2))
         goto memerr;
     /* Set issuer name */
     if (!X509_CRL_set_issuer_name(crl, X509_CRL_get_issuer(newer)))

diff --git a/doc/man3/X509_get_version.pod b/doc/man3/X509_get_version.pod
index 2137ad174fe4c80836574893c3a2a18371537bc5..9aadcb7f949e7935853ac711aa208a9e91d9e60b 100644 (file)
--- a/doc/man3/X509_get_version.pod
+++ b/doc/man3/X509_get_version.pod
@@ -22,16 +22,18 @@ certificate request or CRL version
 =head1 DESCRIPTION
 
 X509_get_version() returns the numerical value of the version field of
-certificate B<x>. Note: this is defined by standards (X.509 et al) to be one
-less than the certificate version. So a version 3 certificate will return 2 and
-a version 1 certificate will return 0.
+certificate B<x>. These correspond to the constants B<X509_VERSION_1>,
+B<X509_VERSION_2>, and B<X509_VERSION_3>. Note: the values of these constants
+are defined by standards (X.509 et al) to be one less than the certificate
+version. So B<X509_VERSION_3> has value 2 and B<X509_VERSION_1> has value 0.
 
 X509_set_version() sets the numerical value of the version field of certificate
 B<x> to B<version>.
 
 Similarly X509_REQ_get_version(), X509_REQ_set_version(),
 X509_CRL_get_version() and X509_CRL_set_version() get and set the version
-number of certificate requests and CRLs.
+number of certificate requests and CRLs. They use constants
+B<X509_REQ_VERSION_1>, B<X509_CRL_VERSION_1>, and B<X509_CRL_VERSION_2>.
 
 =head1 NOTES
 

diff --git a/include/openssl/x509.h.in b/include/openssl/x509.h.in
index 0205781e0c18257a979d6020c6e8230c3c1ba224..cd28bd1d7031b26496b21842461784238c6e7e28 100644 (file)
--- a/include/openssl/x509.h.in
+++ b/include/openssl/x509.h.in
@@ -688,6 +688,10 @@ int ASN1_item_sign_ctx(const ASN1_ITEM *it, X509_ALGOR *algor1,
                        X509_ALGOR *algor2, ASN1_BIT_STRING *signature,
                        const void *data, EVP_MD_CTX *ctx);
 
+#define X509_VERSION_1 0
+#define X509_VERSION_2 1
+#define X509_VERSION_3 2
+
 long X509_get_version(const X509 *x);
 int X509_set_version(X509 *x, long version);
 int X509_set_serialNumber(X509 *x, ASN1_INTEGER *serial);
@@ -729,6 +733,8 @@ EVP_PKEY *X509_get0_pubkey(const X509 *x);
 EVP_PKEY *X509_get_pubkey(X509 *x);
 ASN1_BIT_STRING *X509_get0_pubkey_bitstr(const X509 *x);
 
+#define X509_REQ_VERSION_1 0
+
 long X509_REQ_get_version(const X509_REQ *req);
 int X509_REQ_set_version(X509_REQ *x, long version);
 X509_NAME *X509_REQ_get_subject_name(const X509_REQ *req); /* TODO change to get0_ */
@@ -767,6 +773,9 @@ int X509_REQ_add1_attr_by_txt(X509_REQ *req,
                               const char *attrname, int type,
                               const unsigned char *bytes, int len);
 
+#define X509_CRL_VERSION_1 0
+#define X509_CRL_VERSION_2 1
+
 int X509_CRL_set_version(X509_CRL *x, long version);
 int X509_CRL_set_issuer_name(X509_CRL *x, const X509_NAME *name);
 int X509_CRL_set1_lastUpdate(X509_CRL *x, const ASN1_TIME *tm);

diff --git a/test/v3nametest.c b/test/v3nametest.c
index e1eeb75f2f6ae3a504b5c1d37dfde09472ec5568..d11077fb3d6cdf4fd1035f3d8bd8578876efcfe8 100644 (file)
--- a/test/v3nametest.c
+++ b/test/v3nametest.c
@@ -257,7 +257,7 @@ static X509 *make_cert(void)
 
     if (!TEST_ptr(crt = X509_new()))
         return NULL;
-    if (!TEST_true(X509_set_version(crt, 2))) {
+    if (!TEST_true(X509_set_version(crt, X509_VERSION_3))) {
         X509_free(crt);
         return NULL;
     }