Define X509V*_VERSION constants.
The X509 version APIs confuse everyone, including our own tests
(v3name_test.cc was using the wrong value). Introduce constants. Also
document which version to use for each type. It is extra confusing that,
although certificates and CRLs use the same Version enum, RFC5280
defines X.509 v3 certificates with X.509 v2 CRLs. (There are no v3
CRLs.)
I'll send a similar patch to OpenSSL to keep upstream aligned.
Change-Id: If096138eb004156d43df87a6d74f695b04f67480
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45944
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/crypto/x509/x509_cmp.c b/crypto/x509/x509_cmp.c
index c9060dd..a4f0728 100644
--- a/crypto/x509/x509_cmp.c
+++ b/crypto/x509/x509_cmp.c
@@ -383,7 +383,7 @@
} else
i = 0;
- if (X509_get_version(x) != 2) {
+ if (X509_get_version(x) != X509V3_VERSION) {
rv = X509_V_ERR_SUITE_B_INVALID_VERSION;
/* Correct error depth */
i = 0;
@@ -401,7 +401,7 @@
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) != X509V3_VERSION) {
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 9193b20..4a57fb6 100644
--- a/crypto/x509/x509_set.c
+++ b/crypto/x509/x509_set.c
@@ -64,7 +64,7 @@
{
// The default version is v1(0).
if (x509->cert_info->version == NULL) {
- return 0;
+ return X509V1_VERSION;
}
return ASN1_INTEGER_get(x509->cert_info->version);
}
@@ -76,6 +76,7 @@
int X509_set_version(X509 *x, long version)
{
+ // TODO(davidben): Reject invalid version numbers.
if (x == NULL)
return (0);
if (version == 0) {
@@ -90,7 +91,7 @@
return (ASN1_INTEGER_set(x->cert_info->version, version));
}
-int X509_set_serialNumber(X509 *x, ASN1_INTEGER *serial)
+int X509_set_serialNumber(X509 *x, const ASN1_INTEGER *serial)
{
ASN1_INTEGER *in;
diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc
index 3a6740f..918f615 100644
--- a/crypto/x509/x509_test.cc
+++ b/crypto/x509/x509_test.cc
@@ -1599,7 +1599,7 @@
if (new_cert) {
cert.reset(X509_new());
// Fill in some fields for the certificate arbitrarily.
- EXPECT_TRUE(X509_set_version(cert.get(), 2 /* X.509v3 */));
+ EXPECT_TRUE(X509_set_version(cert.get(), X509V3_VERSION));
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 a997202..1b533bf 100644
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -2052,7 +2052,7 @@
}
/* Create new CRL */
crl = X509_CRL_new();
- if (!crl || !X509_CRL_set_version(crl, 1))
+ if (!crl || !X509_CRL_set_version(crl, X509V2_VERSION))
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 acb7602..66bf8da 100644
--- a/crypto/x509v3/v3_purp.c
+++ b/crypto/x509v3/v3_purp.c
@@ -440,7 +440,7 @@
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))
+ if (X509_get_version(x) == X509V1_VERSION)
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 2dcdd87..ca918a9 100644
--- a/crypto/x509v3/v3name_test.cc
+++ b/crypto/x509v3/v3name_test.cc
@@ -305,7 +305,7 @@
crt = X509_new();
if (crt == NULL)
goto out;
- if (!X509_set_version(crt, 3))
+ if (!X509_set_version(crt, X509V3_VERSION))
goto out;
ret = crt;
crt = NULL;
diff --git a/include/openssl/x509.h b/include/openssl/x509.h
index 49de433..01842e1 100644
--- a/include/openssl/x509.h
+++ b/include/openssl/x509.h
@@ -475,16 +475,34 @@
// it is safe to call mutating functions is a little tricky due to various
// internal caches.
-// X509_get_version returns the numerical value of |x509|'s version. That is,
-// it returns zero for X.509v1, one for X.509v2, and two for X.509v3. 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.
+// 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_get_version returns the numerical value of |x509|'s version. Callers may
+// compare the result to the |X509V*_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.
+//
+// If unsure, use |X509V3_VERSION|.
+OPENSSL_EXPORT int X509_set_version(X509 *x509, long version);
+
// X509_get0_serialNumber returns |x509|'s serial number.
OPENSSL_EXPORT const ASN1_INTEGER *X509_get0_serialNumber(const X509 *x509);
+// X509_set_serialNumber sets |x509|'s serial number to |serial|. It returns one
+// on success and zero on error.
+OPENSSL_EXPORT int X509_set_serialNumber(X509 *x509,
+ const ASN1_INTEGER *serial);
+
// X509_get0_notBefore returns |x509|'s notBefore time.
OPENSSL_EXPORT const ASN1_TIME *X509_get0_notBefore(const X509 *x509);
@@ -550,9 +568,9 @@
// |EXFLAG_INVALID| bit.
OPENSSL_EXPORT long X509_get_pathlen(X509 *x509);
-// X509_REQ_get_version returns the numerical value of |req|'s version. That is,
-// it returns zero for a v1 request. If |req| is invalid, it may return another
-// value, or -1 on overflow.
+// 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.
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
@@ -565,9 +583,9 @@
// X509_name_cmp is a legacy alias for |X509_NAME_cmp|.
#define X509_name_cmp(a, b) X509_NAME_cmp((a), (b))
-// X509_REQ_get_version returns the numerical value of |crl|'s version. That is,
-// it returns zero for a v1 CRL and one for a v2 CRL. If |crl| is invalid, it
-// may return another value, or -1 on overflow.
+// 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.
OPENSSL_EXPORT long X509_CRL_get_version(const X509_CRL *crl);
// X509_CRL_get0_lastUpdate returns |crl|'s lastUpdate time.
@@ -1071,8 +1089,6 @@
ASN1_BIT_STRING *signature, void *asn,
EVP_MD_CTX *ctx);
-OPENSSL_EXPORT int X509_set_version(X509 *x, long version);
-OPENSSL_EXPORT int X509_set_serialNumber(X509 *x, ASN1_INTEGER *serial);
OPENSSL_EXPORT ASN1_INTEGER *X509_get_serialNumber(X509 *x);
OPENSSL_EXPORT int X509_set_issuer_name(X509 *x, X509_NAME *name);
OPENSSL_EXPORT X509_NAME *X509_get_issuer_name(const X509 *a);
@@ -1085,7 +1101,11 @@
const X509 *x);
OPENSSL_EXPORT const X509_ALGOR *X509_get0_tbs_sigalg(const X509 *x);
-OPENSSL_EXPORT int X509_REQ_set_version(X509_REQ *x, long version);
+// X509_REQ_set_version sets |req|'s version to |version|, which should be
+// |X509V1_VERSION|. It returns one on success and zero on error.
+//
+// Note no versions other than |X509V1_VERSION| are defined for CSRs.
+OPENSSL_EXPORT int X509_REQ_set_version(X509_REQ *req, long version);
OPENSSL_EXPORT int X509_REQ_set_subject_name(X509_REQ *req, X509_NAME *name);
OPENSSL_EXPORT void X509_REQ_get0_signature(const X509_REQ *req,
const ASN1_BIT_STRING **psig,
@@ -1123,7 +1143,13 @@
const unsigned char *bytes,
int len);
-OPENSSL_EXPORT int X509_CRL_set_version(X509_CRL *x, long version);
+// 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
+// error.
+//
+// If unsure, use |X509V2_VERSION|. Note |X509V3_VERSION| is not defined for
+// CRLs.
+OPENSSL_EXPORT int X509_CRL_set_version(X509_CRL *crl, long version);
OPENSSL_EXPORT int X509_CRL_set_issuer_name(X509_CRL *x, X509_NAME *name);
OPENSSL_EXPORT int X509_CRL_sort(X509_CRL *crl);
OPENSSL_EXPORT int X509_CRL_up_ref(X509_CRL *crl);
