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)

apps/ca.c

@@ -1255,8 +1255,8 @@ end_of_options:
             }
         }
         if (crl_ext != NULL || crl_v2) {
-            if (!X509_CRL_set_version(crl, 1))
+            if (!X509_CRL_set_version(crl, X509_CRL_VERSION_2))
-                goto end;       /* version 2 CRL */
+                goto end;
         }
 
         /* we have a CRL number that need updating */

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;
 
         /*

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)

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);

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 {

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 {

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 */

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;
         }

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)))

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
+certificate B<x>. These correspond to the constants B<X509_VERSION_1>,
-less than the certificate version. So a version 3 certificate will return 2 and
+B<X509_VERSION_2>, and B<X509_VERSION_3>. Note: the values of these constants
-a version 1 certificate will return 0.
+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
 

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);

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;
     }