From a96f4dd3822efdfbd371abc4d8ad0e1b9b497866 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Wed, 28 Apr 2021 11:17:17 -0400 Subject: [PATCH] Rename X509V*_VERSION constants. Upstream ultimately preferred a different naming convention, and type-specific constants. Align with them. Update-Note: This renames some BoringSSL-specific constants that we recently added. It doesn't look like anyone's used them yet. Change-Id: I580e0872a5f09fb1c5bab9127c35f1ed852680c0 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47164 Reviewed-by: Adam Langley --- crypto/x509/x509_cmp.c | 4 +-- crypto/x509/x509_set.c | 2 +- crypto/x509/x509_test.cc | 2 +- crypto/x509/x509_vfy.c | 2 +- crypto/x509v3/v3_purp.c | 2 +- crypto/x509v3/v3name_test.cc | 2 +- include/openssl/x509.h | 48 ++++++++++++++++++++++-------------- 7 files changed, 36 insertions(+), 26 deletions(-) diff --git a/crypto/x509/x509_cmp.c b/crypto/x509/x509_cmp.c index cf20dcd5a..5811f4402 100644 --- a/crypto/x509/x509_cmp.c +++ b/crypto/x509/x509_cmp.c @@ -384,7 +384,7 @@ int X509_chain_check_suiteb(int *perror_depth, X509 *x, STACK_OF(X509) *chain, } else i = 0; - if (X509_get_version(x) != X509V3_VERSION) { + if (X509_get_version(x) != X509_VERSION_3) { rv = X509_V_ERR_SUITE_B_INVALID_VERSION; /* Correct error depth */ i = 0; @@ -402,7 +402,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) != X509V3_VERSION) { + if (X509_get_version(x) != X509_VERSION_3) { rv = X509_V_ERR_SUITE_B_INVALID_VERSION; goto end; } diff --git a/crypto/x509/x509_set.c b/crypto/x509/x509_set.c index 77438683f..93bb6a396 100644 --- a/crypto/x509/x509_set.c +++ b/crypto/x509/x509_set.c @@ -67,7 +67,7 @@ long X509_get_version(const X509 *x509) { // The default version is v1(0). if (x509->cert_info->version == NULL) { - return X509V1_VERSION; + return X509_VERSION_1; } return ASN1_INTEGER_get(x509->cert_info->version); } diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc index e71cdeb18..89441a85c 100644 --- a/crypto/x509/x509_test.cc +++ b/crypto/x509/x509_test.cc @@ -1648,7 +1648,7 @@ TEST(X509Test, RSASignManual) { if (new_cert) { cert.reset(X509_new()); // Fill in some fields for the certificate arbitrarily. - EXPECT_TRUE(X509_set_version(cert.get(), X509V3_VERSION)); + EXPECT_TRUE(X509_set_version(cert.get(), X509_VERSION_3)); EXPECT_TRUE(ASN1_INTEGER_set(X509_get_serialNumber(cert.get()), 1)); EXPECT_TRUE(X509_gmtime_adj(X509_getm_notBefore(cert.get()), 0)); EXPECT_TRUE( diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c index 1b533bfcd..7bea47cff 100644 --- a/crypto/x509/x509_vfy.c +++ b/crypto/x509/x509_vfy.c @@ -2052,7 +2052,7 @@ X509_CRL *X509_CRL_diff(X509_CRL *base, X509_CRL *newer, } /* Create new CRL */ crl = X509_CRL_new(); - if (!crl || !X509_CRL_set_version(crl, X509V2_VERSION)) + if (!crl || !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/crypto/x509v3/v3_purp.c b/crypto/x509v3/v3_purp.c index 66bf8daab..2d7a4db87 100644 --- a/crypto/x509v3/v3_purp.c +++ b/crypto/x509v3/v3_purp.c @@ -440,7 +440,7 @@ int x509v3_cache_extensions(X509 *x) if (!X509_digest(x, EVP_sha1(), x->sha1_hash, NULL)) x->ex_flags |= EXFLAG_INVALID; /* V1 should mean no extensions ... */ - if (X509_get_version(x) == X509V1_VERSION) + if (X509_get_version(x) == X509_VERSION_1) x->ex_flags |= EXFLAG_V1; /* Handle basic constraints */ if ((bs = X509_get_ext_d2i(x, NID_basic_constraints, &j, NULL))) { diff --git a/crypto/x509v3/v3name_test.cc b/crypto/x509v3/v3name_test.cc index ca918a9c6..a501115cd 100644 --- a/crypto/x509v3/v3name_test.cc +++ b/crypto/x509v3/v3name_test.cc @@ -305,7 +305,7 @@ static X509 *make_cert(void) crt = X509_new(); if (crt == NULL) goto out; - if (!X509_set_version(crt, X509V3_VERSION)) + if (!X509_set_version(crt, X509_VERSION_3)) goto out; ret = crt; crt = NULL; diff --git a/include/openssl/x509.h b/include/openssl/x509.h index 949f12002..67153c424 100644 --- a/include/openssl/x509.h +++ b/include/openssl/x509.h @@ -453,24 +453,23 @@ extern "C" { // it is safe to call mutating functions is a little tricky due to various // internal caches. -// The following constants are version numbers of X.509-related structures. Note -// APIs typically return the numerical value of X.509 versions, which are one -// less than the named version. -#define X509V1_VERSION 0 -#define X509V2_VERSION 1 -#define X509V3_VERSION 2 +// X509_VERSION_* are X.509 version numbers. Note the numerical values of all +// defined X.509 versions are one less than the named version. +#define X509_VERSION_1 0 +#define X509_VERSION_2 1 +#define X509_VERSION_3 2 // X509_get_version returns the numerical value of |x509|'s version. Callers may -// compare the result to the |X509V*_VERSION| constants. Unknown versions are +// compare the result to the |X509_VERSION_*| constants. Unknown versions are // rejected by the parser, but a manually-created |X509| object may encode // invalid versions. In that case, the function will return the invalid version, // or -1 on overflow. OPENSSL_EXPORT long X509_get_version(const X509 *x509); // X509_set_version sets |x509|'s version to |version|, which should be one of -// the |X509V*_VERSION| constants. It returns one on success and zero on error. +// the |X509V_VERSION_*| constants. It returns one on success and zero on error. // -// If unsure, use |X509V3_VERSION|. +// If unsure, use |X509_VERSION_3|. OPENSSL_EXPORT int X509_set_version(X509 *x509, long version); // X509_get0_serialNumber returns |x509|'s serial number. @@ -542,9 +541,15 @@ OPENSSL_EXPORT void X509_get0_uids(const X509 *x509, // |EXFLAG_INVALID| bit. OPENSSL_EXPORT long X509_get_pathlen(X509 *x509); -// X509_REQ_get_version returns the numerical value of |req|'s version. Callers -// may compare the result to |X509V*_VERSION| constants. If |req| is invalid, it -// may return another value, or -1 on overflow. +// X509_REQ_VERSION_1 is the version constant for |X509_REQ| objects. Note no +// other versions are defined. +#define X509_REQ_VERSION_1 0 + +// X509_REQ_get_version returns the numerical value of |req|'s version. This +// will be |X509_REQ_VERSION_1| for valid certificate requests. If |req| is +// invalid, it may return another value, or -1 on overflow. +// +// TODO(davidben): Enforce the version number in the parser. OPENSSL_EXPORT long X509_REQ_get_version(const X509_REQ *req); // X509_REQ_get_subject_name returns |req|'s subject name. Note this function is @@ -557,9 +562,14 @@ OPENSSL_EXPORT X509_NAME *X509_REQ_get_subject_name(const X509_REQ *req); // X509_name_cmp is a legacy alias for |X509_NAME_cmp|. #define X509_name_cmp(a, b) X509_NAME_cmp((a), (b)) +#define X509_CRL_VERSION_1 0 +#define X509_CRL_VERSION_2 1 + // X509_CRL_get_version returns the numerical value of |crl|'s version. Callers -// may compare the result to |X509V*_VERSION| constants. If |crl| is invalid, -// it may return another value, or -1 on overflow. +// may compare the result to |X509_CRL_VERSION_*| constants. If |crl| is +// invalid, it may return another value, or -1 on overflow. +// +// TODO(davidben): Enforce the version number in the parser. OPENSSL_EXPORT long X509_CRL_get_version(const X509_CRL *crl); // X509_CRL_get0_lastUpdate returns |crl|'s lastUpdate time. @@ -1158,9 +1168,9 @@ OPENSSL_EXPORT const STACK_OF(X509_EXTENSION) *X509_get0_extensions( OPENSSL_EXPORT const X509_ALGOR *X509_get0_tbs_sigalg(const X509 *x509); // X509_REQ_set_version sets |req|'s version to |version|, which should be -// |X509V1_VERSION|. It returns one on success and zero on error. +// |X509_REQ_VERSION_1|. It returns one on success and zero on error. // -// Note no versions other than |X509V1_VERSION| are defined for CSRs. +// Note no versions other than |X509_REQ_VERSION_1| are defined for CSRs. OPENSSL_EXPORT int X509_REQ_set_version(X509_REQ *req, long version); // X509_REQ_set_subject_name sets |req|'s subject to a copy of |name|. It @@ -1292,11 +1302,11 @@ OPENSSL_EXPORT int X509_REQ_add1_attr_by_txt(X509_REQ *req, int len); // X509_CRL_set_version sets |crl|'s version to |version|, which should be one -// of the |X509V*_VERSION| constants. It returns one on success and zero on +// of the |X509_CRL_VERSION_*| constants. It returns one on success and zero on // error. // -// If unsure, use |X509V2_VERSION|. Note |X509V3_VERSION| is not defined for -// CRLs. +// If unsure, use |X509_CRL_VERSION_2|. Note that, unlike certificates, CRL +// versions are only defined up to v2. Callers should not use |X509_VERSION_3|. OPENSSL_EXPORT int X509_CRL_set_version(X509_CRL *crl, long version); // X509_CRL_set_issuer_name sets |crl|'s issuer to a copy of |name|. It returns