Const-correct X509_NAME and test thread-safety
Finally!
Fixed: 42290269
Change-Id: Icb0e885b9f2afa02bbc2d1ef99358461d7990335
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81892
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/crypto/x509/by_dir.cc b/crypto/x509/by_dir.cc
index a6df712..c6ed9e7 100644
--- a/crypto/x509/by_dir.cc
+++ b/crypto/x509/by_dir.cc
@@ -47,7 +47,7 @@
static int new_dir(X509_LOOKUP *lu);
static void free_dir(X509_LOOKUP *lu);
static int add_cert_dir(BY_DIR *ctx, const char *dir, int type);
-static int get_cert_by_subject(X509_LOOKUP *xl, int type, X509_NAME *name,
+static int get_cert_by_subject(X509_LOOKUP *xl, int type, const X509_NAME *name,
X509_OBJECT *ret);
static const X509_LOOKUP_METHOD x509_dir_lookup = {
new_dir, // new
@@ -189,7 +189,7 @@
return 1;
}
-static int get_cert_by_subject(X509_LOOKUP *xl, int type, X509_NAME *name,
+static int get_cert_by_subject(X509_LOOKUP *xl, int type, const X509_NAME *name,
X509_OBJECT *ret) {
bssl::UniquePtr<X509> lookup_cert;
bssl::UniquePtr<X509_CRL> lookup_crl;
diff --git a/crypto/x509/internal.h b/crypto/x509/internal.h
index 3bee7cd..f5d2dc4 100644
--- a/crypto/x509/internal.h
+++ b/crypto/x509/internal.h
@@ -149,7 +149,7 @@
CRYPTO_MUTEX lock;
} /* X509 */;
-int x509_marshal_tbs_cert(CBB *cbb, X509 *x509);
+int x509_marshal_tbs_cert(CBB *cbb, const X509 *x509);
// X509 is an |ASN1_ITEM| whose ASN.1 type is X.509 Certificate (RFC 5280) and C
// type is |X509*|.
@@ -164,9 +164,7 @@
STACK_OF(X509_ATTRIBUTE) *attributes; // [ 0 ]
} X509_REQ_INFO;
-// TODO(https://crbug.com/boringssl/407): This is not const because it contains
-// an |X509_NAME|.
-DECLARE_ASN1_FUNCTIONS(X509_REQ_INFO)
+DECLARE_ASN1_FUNCTIONS_const(X509_REQ_INFO)
struct X509_req_st {
X509_REQ_INFO *req_info;
@@ -202,9 +200,7 @@
ASN1_ENCODING enc;
} X509_CRL_INFO;
-// TODO(https://crbug.com/boringssl/407): This is not const because it contains
-// an |X509_NAME|.
-DECLARE_ASN1_FUNCTIONS(X509_CRL_INFO)
+DECLARE_ASN1_FUNCTIONS_const(X509_CRL_INFO)
// Values in idp_flags field
// IDP present
@@ -291,7 +287,7 @@
void (*free)(X509_LOOKUP *ctx);
int (*ctrl)(X509_LOOKUP *ctx, int cmd, const char *argc, long argl,
char **ret);
- int (*get_by_subject)(X509_LOOKUP *ctx, int type, X509_NAME *name,
+ int (*get_by_subject)(X509_LOOKUP *ctx, int type, const X509_NAME *name,
X509_OBJECT *ret);
} /* X509_LOOKUP_METHOD */;
@@ -432,7 +428,8 @@
// TODO(davidben): Reduce the scope of the verify callback and remove this. The
// callback only runs with |X509_V_FLAG_CB_ISSUER_CHECK|, which is only used by
// one internal project and rust-openssl, who use it by mistake.
-int x509_check_issued_with_callback(X509_STORE_CTX *ctx, X509 *x, X509 *issuer);
+int x509_check_issued_with_callback(X509_STORE_CTX *ctx, const X509 *x,
+ const X509 *issuer);
// x509v3_bytes_to_hex encodes |len| bytes from |in| to hex and returns a
// newly-allocated NUL-terminated string containing the result, or NULL on
@@ -562,9 +559,7 @@
const X509V3_CTX *ctx,
const STACK_OF(CONF_VALUE) *nval);
-// TODO(https://crbug.com/boringssl/407): Make |issuer| const once the
-// |X509_NAME| issue is resolved.
-int X509_check_akid(X509 *issuer, const AUTHORITY_KEYID *akid);
+int X509_check_akid(const X509 *issuer, const AUTHORITY_KEYID *akid);
int X509_is_valid_trust_id(int trust);
diff --git a/crypto/x509/v3_akeya.cc b/crypto/x509/v3_akeya.cc
index d5a8460..a9ef7d4 100644
--- a/crypto/x509/v3_akeya.cc
+++ b/crypto/x509/v3_akeya.cc
@@ -28,4 +28,4 @@
ASN1_IMP_OPT(AUTHORITY_KEYID, serial, ASN1_INTEGER, 2),
} ASN1_SEQUENCE_END(AUTHORITY_KEYID)
-IMPLEMENT_ASN1_FUNCTIONS(AUTHORITY_KEYID)
+IMPLEMENT_ASN1_FUNCTIONS_const(AUTHORITY_KEYID)
diff --git a/crypto/x509/v3_crld.cc b/crypto/x509/v3_crld.cc
index 5ba89d1..b8fd313 100644
--- a/crypto/x509/v3_crld.cc
+++ b/crypto/x509/v3_crld.cc
@@ -365,7 +365,7 @@
ASN1_TFLG_SEQUENCE_OF, 0, CRLDistributionPoints, DIST_POINT)
ASN1_ITEM_TEMPLATE_END(CRL_DIST_POINTS)
-IMPLEMENT_ASN1_FUNCTIONS(CRL_DIST_POINTS)
+IMPLEMENT_ASN1_FUNCTIONS_const(CRL_DIST_POINTS)
ASN1_SEQUENCE(ISSUING_DIST_POINT) = {
ASN1_EXP_OPT(ISSUING_DIST_POINT, distpoint, DIST_POINT_NAME, 0),
@@ -376,7 +376,7 @@
ASN1_IMP_OPT(ISSUING_DIST_POINT, onlyattr, ASN1_FBOOLEAN, 5),
} ASN1_SEQUENCE_END(ISSUING_DIST_POINT)
-IMPLEMENT_ASN1_FUNCTIONS(ISSUING_DIST_POINT)
+IMPLEMENT_ASN1_FUNCTIONS_const(ISSUING_DIST_POINT)
static int i2r_idp(const X509V3_EXT_METHOD *method, void *pidp, BIO *out,
int indent);
diff --git a/crypto/x509/v3_genn.cc b/crypto/x509/v3_genn.cc
index 63d4cf2..628f1ba 100644
--- a/crypto/x509/v3_genn.cc
+++ b/crypto/x509/v3_genn.cc
@@ -53,16 +53,16 @@
ASN1_IMP(GENERAL_NAME, d.registeredID, ASN1_OBJECT, GEN_RID),
} ASN1_CHOICE_END(GENERAL_NAME)
-IMPLEMENT_ASN1_FUNCTIONS(GENERAL_NAME)
+IMPLEMENT_ASN1_FUNCTIONS_const(GENERAL_NAME)
ASN1_ITEM_TEMPLATE(GENERAL_NAMES) = ASN1_EX_TEMPLATE_TYPE(ASN1_TFLG_SEQUENCE_OF,
0, GeneralNames,
GENERAL_NAME)
ASN1_ITEM_TEMPLATE_END(GENERAL_NAMES)
-IMPLEMENT_ASN1_FUNCTIONS(GENERAL_NAMES)
+IMPLEMENT_ASN1_FUNCTIONS_const(GENERAL_NAMES)
-IMPLEMENT_ASN1_DUP_FUNCTION(GENERAL_NAME)
+IMPLEMENT_ASN1_DUP_FUNCTION_const(GENERAL_NAME)
static int edipartyname_cmp(const EDIPARTYNAME *a, const EDIPARTYNAME *b) {
// nameAssigner is optional and may be NULL.
diff --git a/crypto/x509/v3_info.cc b/crypto/x509/v3_info.cc
index 00b4744..573734f 100644
--- a/crypto/x509/v3_info.cc
+++ b/crypto/x509/v3_info.cc
@@ -77,7 +77,7 @@
ASN1_TFLG_SEQUENCE_OF, 0, GeneralNames, ACCESS_DESCRIPTION)
ASN1_ITEM_TEMPLATE_END(AUTHORITY_INFO_ACCESS)
-IMPLEMENT_ASN1_FUNCTIONS(AUTHORITY_INFO_ACCESS)
+IMPLEMENT_ASN1_FUNCTIONS_const(AUTHORITY_INFO_ACCESS)
static STACK_OF(CONF_VALUE) *i2v_AUTHORITY_INFO_ACCESS(
const X509V3_EXT_METHOD *method, void *ext, STACK_OF(CONF_VALUE) *ret) {
diff --git a/crypto/x509/v3_ncons.cc b/crypto/x509/v3_ncons.cc
index c94bb20..e130c9c 100644
--- a/crypto/x509/v3_ncons.cc
+++ b/crypto/x509/v3_ncons.cc
@@ -32,13 +32,13 @@
static int i2r_NAME_CONSTRAINTS(const X509V3_EXT_METHOD *method, void *a,
BIO *bp, int ind);
static int do_i2r_name_constraints(const X509V3_EXT_METHOD *method,
- STACK_OF(GENERAL_SUBTREE) *trees, BIO *bp,
- int ind, const char *name);
+ const STACK_OF(GENERAL_SUBTREE) *trees,
+ BIO *bp, int ind, const char *name);
static int print_nc_ipadd(BIO *bp, const ASN1_OCTET_STRING *ip);
-static int nc_match(GENERAL_NAME *gen, NAME_CONSTRAINTS *nc);
-static int nc_match_single(GENERAL_NAME *sub, GENERAL_NAME *gen);
-static int nc_dn(X509_NAME *sub, X509_NAME *nm);
+static int nc_match(const GENERAL_NAME *gen, const NAME_CONSTRAINTS *nc);
+static int nc_match_single(const GENERAL_NAME *sub, const GENERAL_NAME *gen);
+static int nc_dn(const X509_NAME *sub, const X509_NAME *nm);
static int nc_dns(const ASN1_IA5STRING *sub, const ASN1_IA5STRING *dns);
static int nc_email(const ASN1_IA5STRING *sub, const ASN1_IA5STRING *eml);
static int nc_uri(const ASN1_IA5STRING *uri, const ASN1_IA5STRING *base);
@@ -132,15 +132,12 @@
}
static int do_i2r_name_constraints(const X509V3_EXT_METHOD *method,
- STACK_OF(GENERAL_SUBTREE) *trees, BIO *bp,
- int ind, const char *name) {
- GENERAL_SUBTREE *tree;
- size_t i;
+ const STACK_OF(GENERAL_SUBTREE) *trees,
+ BIO *bp, int ind, const char *name) {
if (sk_GENERAL_SUBTREE_num(trees) > 0) {
BIO_printf(bp, "%*s%s:\n", ind, "", name);
}
- for (i = 0; i < sk_GENERAL_SUBTREE_num(trees); i++) {
- tree = sk_GENERAL_SUBTREE_value(trees, i);
+ for (const GENERAL_SUBTREE *tree : trees) {
BIO_printf(bp, "%*s", ind + 2, "");
if (tree->base->type == GEN_IPADD) {
print_nc_ipadd(bp, tree->base->d.ip);
@@ -191,12 +188,8 @@
// syntax.
// X509_V_ERR_UNSUPPORTED_NAME_SYNTAX: Bad or unsupported syntax of name.
-int NAME_CONSTRAINTS_check(X509 *x, NAME_CONSTRAINTS *nc) {
- int r, i;
- size_t j;
- X509_NAME *nm;
-
- nm = X509_get_subject_name(x);
+int NAME_CONSTRAINTS_check(const X509 *x, const NAME_CONSTRAINTS *nc) {
+ const X509_NAME *nm = X509_get_subject_name(x);
// Guard against certificates with an excessive number of names or
// constraints causing a computationally expensive name constraints
@@ -216,10 +209,9 @@
if (X509_NAME_entry_count(nm) > 0) {
GENERAL_NAME gntmp;
gntmp.type = GEN_DIRNAME;
- gntmp.d.directoryName = nm;
+ gntmp.d.directoryName = const_cast<X509_NAME*>(nm);
- r = nc_match(&gntmp, nc);
-
+ int r = nc_match(&gntmp, nc);
if (r != X509_V_OK) {
return r;
}
@@ -227,8 +219,7 @@
gntmp.type = GEN_EMAIL;
// Process any email address attributes in subject name
-
- for (i = -1;;) {
+ for (int i = -1;;) {
i = X509_NAME_get_index_by_NID(nm, NID_pkcs9_emailAddress, i);
if (i == -1) {
break;
@@ -240,16 +231,14 @@
}
r = nc_match(&gntmp, nc);
-
if (r != X509_V_OK) {
return r;
}
}
}
- for (j = 0; j < sk_GENERAL_NAME_num(x->altname); j++) {
- GENERAL_NAME *gen = sk_GENERAL_NAME_value(x->altname, j);
- r = nc_match(gen, nc);
+ for (const GENERAL_NAME *gen : x->altname) {
+ int r = nc_match(gen, nc);
if (r != X509_V_OK) {
return r;
}
@@ -258,16 +247,11 @@
return X509_V_OK;
}
-static int nc_match(GENERAL_NAME *gen, NAME_CONSTRAINTS *nc) {
- GENERAL_SUBTREE *sub;
- int r, match = 0;
- size_t i;
-
+static int nc_match(const GENERAL_NAME *gen, const NAME_CONSTRAINTS *nc) {
// Permitted subtrees: if any subtrees exist of matching the type at
// least one subtree must match.
-
- for (i = 0; i < sk_GENERAL_SUBTREE_num(nc->permittedSubtrees); i++) {
- sub = sk_GENERAL_SUBTREE_value(nc->permittedSubtrees, i);
+ int match = 0;
+ for (const GENERAL_SUBTREE *sub : nc->permittedSubtrees) {
if (gen->type != sub->base->type) {
continue;
}
@@ -281,7 +265,7 @@
if (match == 0) {
match = 1;
}
- r = nc_match_single(gen, sub->base);
+ int r = nc_match_single(gen, sub->base);
if (r == X509_V_OK) {
match = 2;
} else if (r != X509_V_ERR_PERMITTED_VIOLATION) {
@@ -294,9 +278,7 @@
}
// Excluded subtrees: must not match any of these
-
- for (i = 0; i < sk_GENERAL_SUBTREE_num(nc->excludedSubtrees); i++) {
- sub = sk_GENERAL_SUBTREE_value(nc->excludedSubtrees, i);
+ for (const GENERAL_SUBTREE *sub : nc->excludedSubtrees) {
if (gen->type != sub->base->type) {
continue;
}
@@ -304,7 +286,7 @@
return X509_V_ERR_SUBTREE_MINMAX;
}
- r = nc_match_single(gen, sub->base);
+ int r = nc_match_single(gen, sub->base);
if (r == X509_V_OK) {
return X509_V_ERR_EXCLUDED_VIOLATION;
} else if (r != X509_V_ERR_PERMITTED_VIOLATION) {
@@ -315,7 +297,7 @@
return X509_V_OK;
}
-static int nc_match_single(GENERAL_NAME *gen, GENERAL_NAME *base) {
+static int nc_match_single(const GENERAL_NAME *gen, const GENERAL_NAME *base) {
switch (base->type) {
case GEN_DIRNAME:
return nc_dn(gen->d.directoryName, base->d.directoryName);
@@ -339,7 +321,7 @@
// X509_NAME makes this comparison easy. It is matched if the subtree is a
// subset of the name.
-static int nc_dn(X509_NAME *nm, X509_NAME *base) {
+static int nc_dn(const X509_NAME *nm, const X509_NAME *base) {
const X509_NAME_CACHE *nm_cache = x509_name_get_cache(nm);
if (nm_cache == nullptr) {
return X509_V_ERR_OUT_OF_MEM;
diff --git a/crypto/x509/v3_purp.cc b/crypto/x509/v3_purp.cc
index 5e5b57c..04f67e8 100644
--- a/crypto/x509/v3_purp.cc
+++ b/crypto/x509/v3_purp.cc
@@ -356,8 +356,8 @@
return ((x->ex_flags & EXFLAG_BCONS) && (x->ex_flags & EXFLAG_CA));
}
-int X509_check_ca(X509 *x) {
- if (!x509v3_cache_extensions(x)) {
+int X509_check_ca(const X509 *x) {
+ if (!x509v3_cache_extensions(const_cast<X509*>(x))) {
return 0;
}
return check_ca(x);
@@ -460,12 +460,13 @@
static int no_check(const X509_PURPOSE *xp, const X509 *x, int ca) { return 1; }
-int X509_check_issued(X509 *issuer, X509 *subject) {
+int X509_check_issued(const X509 *issuer, const X509 *subject) {
if (X509_NAME_cmp(X509_get_subject_name(issuer),
X509_get_issuer_name(subject))) {
return X509_V_ERR_SUBJECT_ISSUER_MISMATCH;
}
- if (!x509v3_cache_extensions(issuer) || !x509v3_cache_extensions(subject)) {
+ if (!x509v3_cache_extensions(const_cast<X509 *>(issuer)) ||
+ !x509v3_cache_extensions(const_cast<X509 *>(subject))) {
return X509_V_ERR_UNSPECIFIED;
}
@@ -482,7 +483,7 @@
return X509_V_OK;
}
-int X509_check_akid(X509 *issuer, const AUTHORITY_KEYID *akid) {
+int X509_check_akid(const X509 *issuer, const AUTHORITY_KEYID *akid) {
if (!akid) {
return X509_V_OK;
}
@@ -494,7 +495,7 @@
}
// Check serial number
if (akid->serial &&
- ASN1_INTEGER_cmp(X509_get_serialNumber(issuer), akid->serial)) {
+ ASN1_INTEGER_cmp(X509_get0_serialNumber(issuer), akid->serial)) {
return X509_V_ERR_AKID_ISSUER_SERIAL_MISMATCH;
}
// Check issuer name
@@ -502,13 +503,8 @@
// Ugh, for some peculiar reason AKID includes SEQUENCE OF
// GeneralName. So look for a DirName. There may be more than one but
// we only take any notice of the first.
- GENERAL_NAMES *gens;
- GENERAL_NAME *gen;
- X509_NAME *nm = NULL;
- size_t i;
- gens = akid->issuer;
- for (i = 0; i < sk_GENERAL_NAME_num(gens); i++) {
- gen = sk_GENERAL_NAME_value(gens, i);
+ const X509_NAME *nm = nullptr;
+ for (const GENERAL_NAME *gen : akid->issuer) {
if (gen->type == GEN_DIRNAME) {
nm = gen->d.dirn;
break;
diff --git a/crypto/x509/x509_cmp.cc b/crypto/x509/x509_cmp.cc
index 3e5a784..e0a4b2a 100644
--- a/crypto/x509/x509_cmp.cc
+++ b/crypto/x509/x509_cmp.cc
@@ -49,9 +49,11 @@
return a->issuer;
}
-uint32_t X509_issuer_name_hash(X509 *x) { return X509_NAME_hash(x->issuer); }
+uint32_t X509_issuer_name_hash(const X509 *x) {
+ return X509_NAME_hash(x->issuer);
+}
-uint32_t X509_issuer_name_hash_old(X509 *x) {
+uint32_t X509_issuer_name_hash_old(const X509 *x) {
return X509_NAME_hash_old(x->issuer);
}
@@ -66,9 +68,11 @@
return &x509->serialNumber;
}
-uint32_t X509_subject_name_hash(X509 *x) { return X509_NAME_hash(x->subject); }
+uint32_t X509_subject_name_hash(const X509 *x) {
+ return X509_NAME_hash(x->subject);
+}
-uint32_t X509_subject_name_hash_old(X509 *x) {
+uint32_t X509_subject_name_hash_old(const X509 *x) {
return X509_NAME_hash_old(x->subject);
}
@@ -120,7 +124,7 @@
return 0;
}
-uint32_t X509_NAME_hash(X509_NAME *x) {
+uint32_t X509_NAME_hash(const X509_NAME *x) {
const X509_NAME_CACHE *cache = x509_name_get_cache(x);
if (cache == nullptr) {
return 0;
@@ -133,7 +137,7 @@
// I now DER encode the name and hash it. Since I cache the DER encoding,
// this is reasonably efficient.
-uint32_t X509_NAME_hash_old(X509_NAME *x) {
+uint32_t X509_NAME_hash_old(const X509_NAME *x) {
const X509_NAME_CACHE *cache = x509_name_get_cache(x);
if (cache == nullptr) {
return 0;
@@ -143,7 +147,8 @@
return CRYPTO_load_u32_le(md);
}
-X509 *X509_find_by_issuer_and_serial(const STACK_OF(X509) *sk, X509_NAME *name,
+X509 *X509_find_by_issuer_and_serial(const STACK_OF(X509) *sk,
+ const X509_NAME *name,
const ASN1_INTEGER *serial) {
if (serial->type != V_ASN1_INTEGER && serial->type != V_ASN1_NEG_INTEGER) {
return NULL;
@@ -159,7 +164,7 @@
return NULL;
}
-X509 *X509_find_by_subject(const STACK_OF(X509) *sk, X509_NAME *name) {
+X509 *X509_find_by_subject(const STACK_OF(X509) *sk, const X509_NAME *name) {
for (size_t i = 0; i < sk_X509_num(sk); i++) {
X509 *x509 = sk_X509_value(sk, i);
if (X509_NAME_cmp(X509_get_subject_name(x509), name) == 0) {
diff --git a/crypto/x509/x509_lu.cc b/crypto/x509/x509_lu.cc
index 6646f4d..d3c0630 100644
--- a/crypto/x509/x509_lu.cc
+++ b/crypto/x509/x509_lu.cc
@@ -23,17 +23,18 @@
static int X509_OBJECT_idx_by_subject(STACK_OF(X509_OBJECT) *h, int type,
- X509_NAME *name);
+ const X509_NAME *name);
static X509_OBJECT *X509_OBJECT_retrieve_by_subject(STACK_OF(X509_OBJECT) *h,
- int type, X509_NAME *name);
+ int type,
+ const X509_NAME *name);
static X509_OBJECT *X509_OBJECT_retrieve_match(STACK_OF(X509_OBJECT) *h,
X509_OBJECT *x);
static int X509_OBJECT_up_ref_count(X509_OBJECT *a);
static X509_LOOKUP *X509_LOOKUP_new(const X509_LOOKUP_METHOD *method,
X509_STORE *store);
-static int X509_LOOKUP_by_subject(X509_LOOKUP *ctx, int type, X509_NAME *name,
- X509_OBJECT *ret);
+static int X509_LOOKUP_by_subject(X509_LOOKUP *ctx, int type,
+ const X509_NAME *name, X509_OBJECT *ret);
static X509_LOOKUP *X509_LOOKUP_new(const X509_LOOKUP_METHOD *method,
X509_STORE *store) {
@@ -74,8 +75,8 @@
}
}
-static int X509_LOOKUP_by_subject(X509_LOOKUP *ctx, int type, X509_NAME *name,
- X509_OBJECT *ret) {
+static int X509_LOOKUP_by_subject(X509_LOOKUP *ctx, int type,
+ const X509_NAME *name, X509_OBJECT *ret) {
if (ctx->method == NULL || ctx->method->get_by_subject == NULL) {
return 0;
}
@@ -163,8 +164,8 @@
return lu;
}
-int X509_STORE_CTX_get_by_subject(X509_STORE_CTX *vs, int type, X509_NAME *name,
- X509_OBJECT *ret) {
+int X509_STORE_CTX_get_by_subject(X509_STORE_CTX *vs, int type,
+ const X509_NAME *name, X509_OBJECT *ret) {
X509_STORE *ctx = vs->ctx;
X509_OBJECT stmp;
CRYPTO_MUTEX_lock_write(&ctx->objs_lock);
@@ -284,7 +285,7 @@
}
static int x509_object_idx_cnt(STACK_OF(X509_OBJECT) *h, int type,
- X509_NAME *name, int *pnmatch) {
+ const X509_NAME *name, int *pnmatch) {
X509_OBJECT stmp;
X509 x509_s;
X509_CRL crl_s;
@@ -294,12 +295,12 @@
switch (type) {
case X509_LU_X509:
stmp.data.x509 = &x509_s;
- x509_s.subject = name;
+ x509_s.subject = const_cast<X509_NAME*>(name);
break;
case X509_LU_CRL:
stmp.data.crl = &crl_s;
crl_s.crl = &crl_info_s;
- crl_info_s.issuer = name;
+ crl_info_s.issuer = const_cast<X509_NAME*>(name);
break;
default:
// abort();
@@ -327,12 +328,13 @@
}
static int X509_OBJECT_idx_by_subject(STACK_OF(X509_OBJECT) *h, int type,
- X509_NAME *name) {
+ const X509_NAME *name) {
return x509_object_idx_cnt(h, type, name, NULL);
}
static X509_OBJECT *X509_OBJECT_retrieve_by_subject(STACK_OF(X509_OBJECT) *h,
- int type, X509_NAME *name) {
+ int type,
+ const X509_NAME *name) {
int idx;
idx = X509_OBJECT_idx_by_subject(h, type, name);
if (idx == -1) {
@@ -364,7 +366,8 @@
return store->objs;
}
-STACK_OF(X509) *X509_STORE_CTX_get1_certs(X509_STORE_CTX *ctx, X509_NAME *nm) {
+STACK_OF(X509) *X509_STORE_CTX_get1_certs(X509_STORE_CTX *ctx,
+ const X509_NAME *nm) {
int cnt;
STACK_OF(X509) *sk = sk_X509_new_null();
if (sk == NULL) {
@@ -405,7 +408,7 @@
}
STACK_OF(X509_CRL) *X509_STORE_CTX_get1_crls(X509_STORE_CTX *ctx,
- X509_NAME *nm) {
+ const X509_NAME *nm) {
int cnt;
X509_OBJECT xobj;
STACK_OF(X509_CRL) *sk = sk_X509_CRL_new_null();
@@ -473,7 +476,7 @@
}
int X509_STORE_CTX_get1_issuer(X509 **out_issuer, X509_STORE_CTX *ctx,
- X509 *x) {
+ const X509 *x) {
X509_NAME *xn;
X509_OBJECT obj, *pobj;
int idx, ret;
diff --git a/crypto/x509/x509_set.cc b/crypto/x509/x509_set.cc
index 98ab04f..0da8cc5 100644
--- a/crypto/x509/x509_set.cc
+++ b/crypto/x509/x509_set.cc
@@ -46,14 +46,14 @@
return ASN1_STRING_copy(&x->serialNumber, serial);
}
-int X509_set_issuer_name(X509 *x, X509_NAME *name) {
+int X509_set_issuer_name(X509 *x, const X509_NAME *name) {
if (x == NULL) {
return 0;
}
return (X509_NAME_set(&x->issuer, name));
}
-int X509_set_subject_name(X509 *x, X509_NAME *name) {
+int X509_set_subject_name(X509 *x, const X509_NAME *name) {
if (x == NULL) {
return 0;
}
diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc
index b9094d0..354955e 100644
--- a/crypto/x509/x509_test.cc
+++ b/crypto/x509/x509_test.cc
@@ -1396,6 +1396,74 @@
thread.join();
}
}
+
+// Test that serializing a modified |X509_NAME| object on multiple threads is
+// thread-safe. This historically wasn't because OpenSSL's |X509_NAME| object
+// maintains a number of caches.
+TEST(X509Test, SerializeModifiedNameThreads) {
+ bssl::UniquePtr<X509_NAME> name(X509_NAME_new());
+ ASSERT_TRUE(name);
+ ASSERT_TRUE(
+ X509_NAME_add_entry_by_txt(name.get(), "CN", MBSTRING_UTF8,
+ reinterpret_cast<const uint8_t *>("Test"),
+ /*len=*/-1, /*loc=*/-1, /*set=*/0));
+
+ const size_t kNumThreads = 10;
+ std::vector<std::thread> threads;
+ for (size_t i = 0; i < kNumThreads; i++) {
+ threads.emplace_back([&] {
+ uint8_t *der = nullptr;
+ int der_len = i2d_X509_NAME(name.get(), &der);
+ ASSERT_GT(der_len, 0);
+ OPENSSL_free(der);
+ });
+ }
+ for (auto &thread : threads) {
+ thread.join();
+ }
+}
+
+// Test that serializing a modified |X509| object on multiple threads is
+// thread-safe. This historically wasn't true because OpenSSL's |X509_NAME|
+// object maintains a number of caches, and because |X509_get_issuer_name| and
+// |X509_get_subject_name| aren't const-correct and allow direct, mutable access
+// to the |X509|'s subject and issuer.
+TEST(X509Test, SerializeModifiedCertThreads) {
+ bssl::UniquePtr<X509> cert = CertFromPEM(kLeafPEM);
+ ASSERT_TRUE(cert);
+
+ // Re-encode the TBSCertificate, dropping the cached encoding. As currently
+ // implemented, once the cached encoding is dropped, it is never recreated.
+ // Instead, it's assumed that the DER encoder will reproduce the expected
+ // encoding. https://crbug.com/443261873 discusses changing this model.
+ uint8_t *tbs = nullptr;
+ int tbs_len = i2d_re_X509_tbs(cert.get(), &tbs);
+ ASSERT_GT(tbs_len, 0);
+ OPENSSL_free(tbs);
+
+ // Modify the subject name directly, now that there is no cached encoding.
+ ASSERT_TRUE(X509_NAME_add_entry_by_txt(
+ X509_get_subject_name(cert.get()), "CN", MBSTRING_UTF8,
+ reinterpret_cast<const uint8_t *>("Test"),
+ /*len=*/-1, /*loc=*/-1, /*set=*/0));
+
+ // Now serialize the certificate in parallel. This should be safe to use
+ // across threads. Historically, this would expose the underlying |X509_NAME|
+ // encoder not being thread-safe.
+ const size_t kNumThreads = 10;
+ std::vector<std::thread> threads;
+ for (size_t i = 0; i < kNumThreads; i++) {
+ threads.emplace_back([&] {
+ uint8_t *der = nullptr;
+ int der_len = i2d_X509(cert.get(), &der);
+ ASSERT_GT(der_len, 0);
+ OPENSSL_free(der);
+ });
+ }
+ for (auto &thread : threads) {
+ thread.join();
+ }
+}
#endif // OPENSSL_THREADS
static const char kHostname[] = "example.com";
diff --git a/crypto/x509/x509_vfy.cc b/crypto/x509/x509_vfy.cc
index 87a714a..5b007e5 100644
--- a/crypto/x509/x509_vfy.cc
+++ b/crypto/x509/x509_vfy.cc
@@ -423,8 +423,8 @@
// Given a possible certificate and issuer check them
-int x509_check_issued_with_callback(X509_STORE_CTX *ctx, X509 *x,
- X509 *issuer) {
+int x509_check_issued_with_callback(X509_STORE_CTX *ctx, const X509 *x,
+ const X509 *issuer) {
int ret;
ret = X509_check_issued(issuer, x);
if (ret == X509_V_OK) {
@@ -436,7 +436,7 @@
}
ctx->error = ret;
- ctx->current_cert = x;
+ ctx->current_cert = const_cast<X509 *>(x);
return call_verify_cb(0, ctx);
}
diff --git a/crypto/x509/x509cset.cc b/crypto/x509/x509cset.cc
index 75e07e0..3ef23d3 100644
--- a/crypto/x509/x509cset.cc
+++ b/crypto/x509/x509cset.cc
@@ -47,7 +47,7 @@
return ASN1_INTEGER_set_int64(x->crl->version, version);
}
-int X509_CRL_set_issuer_name(X509_CRL *x, X509_NAME *name) {
+int X509_CRL_set_issuer_name(X509_CRL *x, const X509_NAME *name) {
if ((x == NULL) || (x->crl == NULL)) {
return 0;
}
diff --git a/crypto/x509/x_all.cc b/crypto/x509/x_all.cc
index e98d046..9167aa6 100644
--- a/crypto/x509/x_all.cc
+++ b/crypto/x509/x_all.cc
@@ -262,8 +262,7 @@
int X509_digest(const X509 *x509, const EVP_MD *md, uint8_t *out,
unsigned *out_len) {
uint8_t *der = NULL;
- // TODO(https://crbug.com/boringssl/407): This function is not const-correct.
- int der_len = i2d_X509((X509 *)x509, &der);
+ int der_len = i2d_X509(x509, &der);
if (der_len < 0) {
return 0;
}
diff --git a/crypto/x509/x_crl.cc b/crypto/x509/x_crl.cc
index 2d486ae..0c0874e 100644
--- a/crypto/x509/x_crl.cc
+++ b/crypto/x509/x_crl.cc
@@ -243,15 +243,12 @@
ASN1_SIMPLE(X509_CRL, signature, ASN1_BIT_STRING),
} ASN1_SEQUENCE_END_ref(X509_CRL, X509_CRL)
-// Although |X509_REVOKED| contains an |X509_NAME|, it can be const. It is not
-// affected by https://crbug.com/boringssl/407 because the |X509_NAME| does
-// not participate in serialization.
IMPLEMENT_ASN1_FUNCTIONS_const(X509_REVOKED)
IMPLEMENT_ASN1_DUP_FUNCTION_const(X509_REVOKED)
-IMPLEMENT_ASN1_FUNCTIONS(X509_CRL_INFO)
-IMPLEMENT_ASN1_FUNCTIONS(X509_CRL)
-IMPLEMENT_ASN1_DUP_FUNCTION(X509_CRL)
+IMPLEMENT_ASN1_FUNCTIONS_const(X509_CRL_INFO)
+IMPLEMENT_ASN1_FUNCTIONS_const(X509_CRL)
+IMPLEMENT_ASN1_DUP_FUNCTION_const(X509_CRL)
static int X509_REVOKED_cmp(const X509_REVOKED *const *a,
const X509_REVOKED *const *b) {
diff --git a/crypto/x509/x_name.cc b/crypto/x509/x_name.cc
index 0e80ab4..973741a 100644
--- a/crypto/x509/x_name.cc
+++ b/crypto/x509/x_name.cc
@@ -263,7 +263,7 @@
return CBB_add_bytes(out, cache->der, cache->der_len);
}
-X509_NAME *X509_NAME_dup(X509_NAME *name) {
+X509_NAME *X509_NAME_dup(const X509_NAME *name) {
const X509_NAME_CACHE *cache = x509_name_get_cache(name);
if (cache == nullptr) {
return nullptr;
@@ -289,7 +289,7 @@
});
}
-int i2d_X509_NAME(X509_NAME *in, uint8_t **outp) {
+int i2d_X509_NAME(const X509_NAME *in, uint8_t **outp) {
const X509_NAME_CACHE *cache = x509_name_get_cache(in);
if (cache == nullptr) {
return -1;
@@ -384,18 +384,19 @@
return CBB_flush(cbb);
}
-int X509_NAME_set(X509_NAME **xn, X509_NAME *name) {
- if ((name = X509_NAME_dup(name)) == NULL) {
+int X509_NAME_set(X509_NAME **xn, const X509_NAME *name) {
+ bssl::UniquePtr<X509_NAME> copy(X509_NAME_dup(name));
+ if (copy == nullptr) {
return 0;
}
X509_NAME_free(*xn);
- *xn = name;
+ *xn = copy.release();
return 1;
}
int X509_NAME_ENTRY_set(const X509_NAME_ENTRY *ne) { return ne->set; }
-int X509_NAME_get0_der(X509_NAME *nm, const unsigned char **out_der,
+int X509_NAME_get0_der(const X509_NAME *nm, const unsigned char **out_der,
size_t *out_der_len) {
const X509_NAME_CACHE *cache = x509_name_get_cache(nm);
if (cache == nullptr) {
diff --git a/crypto/x509/x_req.cc b/crypto/x509/x_req.cc
index ca74904..b8c0f84 100644
--- a/crypto/x509/x_req.cc
+++ b/crypto/x509/x_req.cc
@@ -60,7 +60,7 @@
ASN1_IMP_SET_OF_OPT(X509_REQ_INFO, attributes, X509_ATTRIBUTE, 0),
} ASN1_SEQUENCE_END_enc(X509_REQ_INFO, X509_REQ_INFO)
-IMPLEMENT_ASN1_FUNCTIONS(X509_REQ_INFO)
+IMPLEMENT_ASN1_FUNCTIONS_const(X509_REQ_INFO)
ASN1_SEQUENCE(X509_REQ) = {
ASN1_SIMPLE(X509_REQ, req_info, X509_REQ_INFO),
@@ -68,6 +68,6 @@
ASN1_SIMPLE(X509_REQ, signature, ASN1_BIT_STRING),
} ASN1_SEQUENCE_END(X509_REQ)
-IMPLEMENT_ASN1_FUNCTIONS(X509_REQ)
+IMPLEMENT_ASN1_FUNCTIONS_const(X509_REQ)
-IMPLEMENT_ASN1_DUP_FUNCTION(X509_REQ)
+IMPLEMENT_ASN1_DUP_FUNCTION_const(X509_REQ)
diff --git a/crypto/x509/x_x509.cc b/crypto/x509/x_x509.cc
index 442c827..c9cc309 100644
--- a/crypto/x509/x_x509.cc
+++ b/crypto/x509/x_x509.cc
@@ -256,7 +256,7 @@
return bssl::UniquePtr<X509>(X509_parse_from_buffer(buf.get()));
}
-int x509_marshal_tbs_cert(CBB *cbb, X509 *x509) {
+int x509_marshal_tbs_cert(CBB *cbb, const X509 *x509) {
if (x509->buf != nullptr) {
// Replay the saved TBSCertificate from the |CRYPTO_BUFFER|, to verify
// exactly what we parsed. The |CRYPTO_BUFFER| contains the full
@@ -310,7 +310,7 @@
return CBB_flush(cbb);
}
-static int x509_marshal(CBB *cbb, X509 *x509) {
+static int x509_marshal(CBB *cbb, const X509 *x509) {
CBB cert;
return CBB_add_asn1(cbb, &cert, CBS_ASN1_SEQUENCE) &&
x509_marshal_tbs_cert(&cert, x509) &&
@@ -323,7 +323,7 @@
return bssl::D2IFromCBS(out, inp, len, x509_parse);
}
-int i2d_X509(X509 *x509, uint8_t **outp) {
+int i2d_X509(const X509 *x509, uint8_t **outp) {
if (x509 == NULL) {
OPENSSL_PUT_ERROR(ASN1, ASN1_R_MISSING_VALUE);
return -1;
@@ -369,7 +369,7 @@
x509_parse_cb, x509_i2d_cb};
IMPLEMENT_EXTERN_ASN1(X509, x509_extern_funcs)
-X509 *X509_dup(X509 *x509) {
+X509 *X509_dup(const X509 *x509) {
uint8_t *der = NULL;
int len = i2d_X509(x509, &der);
if (len < 0) {
@@ -441,7 +441,7 @@
// length if pp == NULL. We ultimately want to avoid modifying *pp in the
// error path, but that depends on similar hygiene in lower-level functions.
// Here we avoid compounding the problem.
-static int i2d_x509_aux_internal(X509 *a, unsigned char **pp) {
+static int i2d_x509_aux_internal(const X509 *a, unsigned char **pp) {
int length, tmplen;
unsigned char *start = pp != NULL ? *pp : NULL;
@@ -476,7 +476,7 @@
// we're writing two ASN.1 objects back to back, we can't have i2d_X509() do
// the allocation, nor can we allow i2d_X509_CERT_AUX() to increment the
// allocated buffer.
-int i2d_X509_AUX(X509 *a, unsigned char **pp) {
+int i2d_X509_AUX(const X509 *a, unsigned char **pp) {
int length;
unsigned char *tmp;
@@ -505,13 +505,13 @@
return length;
}
-int i2d_re_X509_tbs(X509 *x509, unsigned char **outp) {
+int i2d_re_X509_tbs(X509 *x509, uint8_t **outp) {
CRYPTO_BUFFER_free(x509->buf);
x509->buf = nullptr;
return i2d_X509_tbs(x509, outp);
}
-int i2d_X509_tbs(X509 *x509, unsigned char **outp) {
+int i2d_X509_tbs(const X509 *x509, uint8_t **outp) {
return bssl::I2DFromCBB(/*initial_capacity=*/128, outp, [&](CBB *cbb) -> bool {
return x509_marshal_tbs_cert(cbb, x509);
});
diff --git a/include/openssl/x509.h b/include/openssl/x509.h
index e490931..fb61dee 100644
--- a/include/openssl/x509.h
+++ b/include/openssl/x509.h
@@ -87,10 +87,7 @@
// function works by serializing the structure, so auxiliary properties (see
// |i2d_X509_AUX|) are not preserved. Additionally, if |x509| is incomplete,
// this function may fail.
-//
-// TODO(crbug.com/42290269): This function should be const and thread-safe but
-// is currently neither in some cases, notably if |crl| was mutated.
-OPENSSL_EXPORT X509 *X509_dup(X509 *x509);
+OPENSSL_EXPORT X509 *X509_dup(const X509 *x509);
// X509_free decrements |x509|'s reference count and, if zero, releases memory
// associated with |x509|.
@@ -118,10 +115,7 @@
// i2d_X509 marshals |x509| as a DER-encoded X.509 Certificate (RFC 5280), as
// described in |i2d_SAMPLE|.
-//
-// TODO(crbug.com/42290269): This function should be const and thread-safe but
-// is currently neither in some cases, notably if |x509| was mutated.
-OPENSSL_EXPORT int i2d_X509(X509 *x509, uint8_t **outp);
+OPENSSL_EXPORT int i2d_X509(const X509 *x509, uint8_t **outp);
// X509_VERSION_* are X.509 version numbers. Note the numerical values of all
// defined X.509 versions are one less than the named version.
@@ -393,7 +387,7 @@
// not reflect modifications made to |x509|. It may be used to manually verify
// the signature of an existing certificate. To generate certificates, use
// |i2d_re_X509_tbs| instead.
-OPENSSL_EXPORT int i2d_X509_tbs(X509 *x509, unsigned char **outp);
+OPENSSL_EXPORT int i2d_X509_tbs(const X509 *x509, uint8_t **outp);
// X509_verify checks that |x509| has a valid signature by |pkey|. It returns
// one if the signature is valid and zero otherwise. Note this function only
@@ -436,9 +430,6 @@
// serialized, the current behavior is to compare all unencodable certificates
// as equal. This function should only be used with |X509| objects that were
// parsed from bytes and never mutated.
-//
-// TODO(crbug.com/42290269): This function is const, but it is not always
-// thread-safe, notably if |a| and |b| were mutated.
OPENSSL_EXPORT int X509_cmp(const X509 *a, const X509 *b);
@@ -480,11 +471,11 @@
// X509_set_issuer_name sets |x509|'s issuer to a copy of |name|. It returns one
// on success and zero on error.
-OPENSSL_EXPORT int X509_set_issuer_name(X509 *x509, X509_NAME *name);
+OPENSSL_EXPORT int X509_set_issuer_name(X509 *x509, const X509_NAME *name);
// X509_set_subject_name sets |x509|'s subject to a copy of |name|. It returns
// one on success and zero on error.
-OPENSSL_EXPORT int X509_set_subject_name(X509 *x509, X509_NAME *name);
+OPENSSL_EXPORT int X509_set_subject_name(X509 *x509, const X509_NAME *name);
// X509_set_pubkey sets |x509|'s public key to |pkey|. It returns one on success
// and zero on error. This function does not take ownership of |pkey| and
@@ -537,7 +528,13 @@
// This function re-encodes the TBSCertificate and may not reflect |x509|'s
// original encoding. It may be used to manually generate a signature for a new
// certificate. To verify certificates, use |i2d_X509_tbs| instead.
-OPENSSL_EXPORT int i2d_re_X509_tbs(X509 *x509, unsigned char **outp);
+//
+// Unlike |i2d_X509_tbs|, this function is not |const| and thus may not be to
+// use concurrently with other functions that access |x509|. It mutates |x509|
+// by dropping the cached encoding. This function is intended to be used during
+// certificate construction, where |x509| is still single-threaded and being
+// mutated.
+OPENSSL_EXPORT int i2d_re_X509_tbs(X509 *x509, uint8_t **outp);
// X509_set1_signature_algo sets |x509|'s signature algorithm to |algo| and
// returns one on success or zero on error. It updates both the signature field
@@ -571,9 +568,7 @@
// Unlike similarly-named functions, this function does not output a single
// ASN.1 element. Directly embedding the output in a larger ASN.1 structure will
// not behave correctly.
-//
-// TODO(crbug.com/42290269): |x509| should be const.
-OPENSSL_EXPORT int i2d_X509_AUX(X509 *x509, uint8_t **outp);
+OPENSSL_EXPORT int i2d_X509_AUX(const X509 *x509, uint8_t **outp);
// d2i_X509_AUX parses up to |length| bytes from |*inp| as a DER-encoded X.509
// Certificate (RFC 5280), followed optionally by a separate, OpenSSL-specific
@@ -682,10 +677,7 @@
// X509_CRL_dup returns a newly-allocated copy of |crl|, or NULL on error. This
// function works by serializing the structure, so if |crl| is incomplete, it
// may fail.
-//
-// TODO(crbug.com/42290269): This function should be const and thread-safe but
-// is currently neither in some cases, notably if |crl| was mutated.
-OPENSSL_EXPORT X509_CRL *X509_CRL_dup(X509_CRL *crl);
+OPENSSL_EXPORT X509_CRL *X509_CRL_dup(const X509_CRL *crl);
// X509_CRL_free decrements |crl|'s reference count and, if zero, releases
// memory associated with |crl|.
@@ -698,10 +690,7 @@
// i2d_X509_CRL marshals |crl| as a X.509 CertificateList (RFC 5280), as
// described in |i2d_SAMPLE|.
-//
-// TODO(crbug.com/42290269): This function should be const and thread-safe but
-// is currently neither in some cases, notably if |crl| was mutated.
-OPENSSL_EXPORT int i2d_X509_CRL(X509_CRL *crl, uint8_t **outp);
+OPENSSL_EXPORT int i2d_X509_CRL(const X509_CRL *crl, uint8_t **outp);
// X509_CRL_match compares |a| and |b| and returns zero if they are equal, a
// negative number if |b| sorts after |a| and a negative number if |a| sorts
@@ -848,7 +837,8 @@
// X509_CRL_set_issuer_name sets |crl|'s issuer to a copy of |name|. It returns
// one on success and zero on error.
-OPENSSL_EXPORT int X509_CRL_set_issuer_name(X509_CRL *crl, X509_NAME *name);
+OPENSSL_EXPORT int X509_CRL_set_issuer_name(X509_CRL *crl,
+ const X509_NAME *name);
// X509_CRL_set1_lastUpdate sets |crl|'s thisUpdate time to |tm|. It returns one
// on success and zero on error. The OpenSSL API refers to this field as
@@ -1073,10 +1063,7 @@
// X509_REQ_dup returns a newly-allocated copy of |req|, or NULL on error. This
// function works by serializing the structure, so if |req| is incomplete, it
// may fail.
-//
-// TODO(crbug.com/42290269): This function should be const and thread-safe but
-// is currently neither in some cases, notably if |req| was mutated.
-OPENSSL_EXPORT X509_REQ *X509_REQ_dup(X509_REQ *req);
+OPENSSL_EXPORT X509_REQ *X509_REQ_dup(const X509_REQ *req);
// X509_REQ_free releases memory associated with |req|.
OPENSSL_EXPORT void X509_REQ_free(X509_REQ *req);
@@ -1088,10 +1075,7 @@
// i2d_X509_REQ marshals |req| as a CertificateRequest (RFC 2986), as described
// in |i2d_SAMPLE|.
-//
-// TODO(crbug.com/42290269): This function should be const and thread-safe but
-// is currently neither in some cases, notably if |req| was mutated.
-OPENSSL_EXPORT int i2d_X509_REQ(X509_REQ *req, uint8_t **outp);
+OPENSSL_EXPORT int i2d_X509_REQ(const X509_REQ *req, uint8_t **outp);
// X509_REQ_VERSION_1 is the version constant for |X509_REQ| objects. No other
// versions are defined.
@@ -1355,16 +1339,10 @@
// i2d_X509_NAME marshals |in| as a DER-encoded X.509 Name (RFC 5280), as
// described in |i2d_SAMPLE|.
-//
-// TODO(crbug.com/42290269): This function should be const and thread-safe but
-// is currently neither in some cases, notably if |in| was mutated.
-OPENSSL_EXPORT int i2d_X509_NAME(X509_NAME *in, uint8_t **outp);
+OPENSSL_EXPORT int i2d_X509_NAME(const X509_NAME *in, uint8_t **outp);
// X509_NAME_dup returns a newly-allocated copy of |name|, or NULL on error.
-//
-// TODO(crbug.com/42290269): This function should be const and thread-safe but
-// is currently neither in some cases, notably if |name| was mutated.
-OPENSSL_EXPORT X509_NAME *X509_NAME_dup(X509_NAME *name);
+OPENSSL_EXPORT X509_NAME *X509_NAME_dup(const X509_NAME *name);
// X509_NAME_cmp compares |a| and |b|'s canonicalized forms. It returns zero if
// they are equal, one if |a| sorts after |b|, -1 if |b| sorts after |a|, and -2
@@ -1381,20 +1359,13 @@
// containing the result. Otherwise, it returns zero. |*out_der| is owned by
// |name| and must not be freed by the caller. It is invalidated after |name| is
// mutated or freed.
-//
-// Avoid this function and prefer |i2d_X509_NAME|. It is one of the reasons
-// |X509_NAME| functions, including this one, are not consistently thread-safe
-// or const-correct. Depending on the resolution of
-// crbug.com/42290269, this function may be removed or cause poor performance.
-OPENSSL_EXPORT int X509_NAME_get0_der(X509_NAME *name, const uint8_t **out_der,
+OPENSSL_EXPORT int X509_NAME_get0_der(const X509_NAME *name,
+ const uint8_t **out_der,
size_t *out_der_len);
// X509_NAME_set makes a copy of |name|. On success, it frees |*xn|, sets |*xn|
// to the copy, and returns one. Otherwise, it returns zero.
-//
-// TODO(crbug.com/42290269): This function should be const and thread-safe but
-// is currently neither in some cases, notably if |name| was mutated.
-OPENSSL_EXPORT int X509_NAME_set(X509_NAME **xn, X509_NAME *name);
+OPENSSL_EXPORT int X509_NAME_set(X509_NAME **xn, const X509_NAME *name);
// X509_NAME_entry_count returns the number of entries in |name|.
OPENSSL_EXPORT int X509_NAME_entry_count(const X509_NAME *name);
@@ -2089,20 +2060,12 @@
// i2d_GENERAL_NAME marshals |in| as a DER-encoded X.509 GeneralName (RFC 5280),
// as described in |i2d_SAMPLE|.
-//
-// TODO(crbug.com/42290269): This function should be const and thread-safe but
-// is currently neither in some cases, notably if |in| is an directoryName and
-// the |X509_NAME| has been modified.
-OPENSSL_EXPORT int i2d_GENERAL_NAME(GENERAL_NAME *in, uint8_t **outp);
+OPENSSL_EXPORT int i2d_GENERAL_NAME(const GENERAL_NAME *in, uint8_t **outp);
// GENERAL_NAME_dup returns a newly-allocated copy of |gen|, or NULL on error.
// This function works by serializing the structure, so it will fail if |gen| is
// empty.
-//
-// TODO(crbug.com/42290269): This function should be const and thread-safe but
-// is currently neither in some cases, notably if |gen| is an directoryName and
-// the |X509_NAME| has been modified.
-OPENSSL_EXPORT GENERAL_NAME *GENERAL_NAME_dup(GENERAL_NAME *gen);
+OPENSSL_EXPORT GENERAL_NAME *GENERAL_NAME_dup(const GENERAL_NAME *gen);
// GENERAL_NAMES_new returns a new, empty |GENERAL_NAMES|, or NULL on error.
OPENSSL_EXPORT GENERAL_NAMES *GENERAL_NAMES_new(void);
@@ -2117,11 +2080,7 @@
// i2d_GENERAL_NAMES marshals |in| as a DER-encoded SEQUENCE OF GeneralName, as
// described in |i2d_SAMPLE|.
-//
-// TODO(crbug.com/42290269): This function should be const and thread-safe but
-// is currently neither in some cases, notably if some element of |in| is an
-// directoryName and the |X509_NAME| has been modified.
-OPENSSL_EXPORT int i2d_GENERAL_NAMES(GENERAL_NAMES *in, uint8_t **outp);
+OPENSSL_EXPORT int i2d_GENERAL_NAMES(const GENERAL_NAMES *in, uint8_t **outp);
// OTHERNAME_new returns a new, empty |OTHERNAME|, or NULL on error.
OPENSSL_EXPORT OTHERNAME *OTHERNAME_new(void);
@@ -2225,10 +2184,8 @@
// i2d_AUTHORITY_KEYID marshals |akid| as a DER-encoded AuthorityKeyIdentifier
// (RFC 5280), as described in |i2d_SAMPLE|.
-//
-// TODO(crbug.com/42290269): |akid| is not const because it contains an
-// |X509_NAME|.
-OPENSSL_EXPORT int i2d_AUTHORITY_KEYID(AUTHORITY_KEYID *akid, uint8_t **outp);
+OPENSSL_EXPORT int i2d_AUTHORITY_KEYID(const AUTHORITY_KEYID *akid,
+ uint8_t **outp);
// Name constraints.
@@ -2318,10 +2275,7 @@
// i2d_AUTHORITY_INFO_ACCESS marshals |aia| as a DER-encoded
// AuthorityInfoAccessSyntax (RFC 5280), as described in |i2d_SAMPLE|.
-//
-// TODO(crbug.com/42290269): |aia| is not const because it contains an
-// |X509_NAME|.
-OPENSSL_EXPORT int i2d_AUTHORITY_INFO_ACCESS(AUTHORITY_INFO_ACCESS *aia,
+OPENSSL_EXPORT int i2d_AUTHORITY_INFO_ACCESS(const AUTHORITY_INFO_ACCESS *aia,
uint8_t **outp);
@@ -2393,10 +2347,8 @@
// i2d_CRL_DIST_POINTS marshals |crldp| as a DER-encoded CRLDistributionPoints
// (RFC 5280), as described in |i2d_SAMPLE|.
-//
-// TODO(crbug.com/42290269): |crldp| is not const because it contains an
-// |X509_NAME|.
-OPENSSL_EXPORT int i2d_CRL_DIST_POINTS(CRL_DIST_POINTS *crldp, uint8_t **outp);
+OPENSSL_EXPORT int i2d_CRL_DIST_POINTS(const CRL_DIST_POINTS *crldp,
+ uint8_t **outp);
// A ISSUING_DIST_POINT_st, aka |ISSUING_DIST_POINT|, represents a
// IssuingDistributionPoint structure (RFC 5280).
@@ -2427,10 +2379,7 @@
// i2d_ISSUING_DIST_POINT marshals |idp| as a DER-encoded
// IssuingDistributionPoint (RFC 5280), as described in |i2d_SAMPLE|.
-//
-// TODO(crbug.com/42290269): |idp| is not const because it contains an
-// |X509_NAME|.
-OPENSSL_EXPORT int i2d_ISSUING_DIST_POINT(ISSUING_DIST_POINT *idp,
+OPENSSL_EXPORT int i2d_ISSUING_DIST_POINT(const ISSUING_DIST_POINT *idp,
uint8_t **outp);
@@ -3705,10 +3654,7 @@
// not suitable for general-purpose X.509 name processing. It is very short, so
// there will be hash collisions. It also depends on an OpenSSL-specific
// canonicalization process.
-//
-// TODO(crbug.com/42290269): This should be const and thread-safe but currently
-// is neither, notably if |name| was modified from its parsed value.
-OPENSSL_EXPORT uint32_t X509_NAME_hash(X509_NAME *name);
+OPENSSL_EXPORT uint32_t X509_NAME_hash(const X509_NAME *name);
// X509_NAME_hash_old returns a hash of |name|, or zero on error. This is the
// legacy hash used by |X509_LOOKUP_add_dir|, which is still supported for
@@ -3717,10 +3663,7 @@
// This hash is specific to the |X509_LOOKUP_add_dir| filesystem format and is
// not suitable for general-purpose X.509 name processing. It is very short, so
// there will be hash collisions.
-//
-// TODO(crbug.com/42290269): This should be const and thread-safe but currently
-// is neither, notably if |name| was modified from its parsed value.
-OPENSSL_EXPORT uint32_t X509_NAME_hash_old(X509_NAME *name);
+OPENSSL_EXPORT uint32_t X509_NAME_hash_old(const X509_NAME *name);
// X509_STORE_set_default_paths configures |store| to read from some "default"
// filesystem paths. It returns one on success and zero on error. The filesystem
@@ -4375,13 +4318,13 @@
// and serial are |name| and |serial|, respectively. If no match is found, it
// returns NULL.
OPENSSL_EXPORT X509 *X509_find_by_issuer_and_serial(const STACK_OF(X509) *sk,
- X509_NAME *name,
+ const X509_NAME *name,
const ASN1_INTEGER *serial);
// X509_find_by_subject returns the first |X509| in |sk| whose subject is
// |name|. If no match is found, it returns NULL.
OPENSSL_EXPORT X509 *X509_find_by_subject(const STACK_OF(X509) *sk,
- X509_NAME *name);
+ const X509_NAME *name);
// X509_cmp_time compares |s| against |*t|. On success, it returns a negative
// number if |s| <= |*t| and a positive number if |s| > |*t|. On error, it
@@ -4438,10 +4381,7 @@
// not suitable for general-purpose X.509 name processing. It is very short, so
// there will be hash collisions. It also depends on an OpenSSL-specific
// canonicalization process.
-//
-// TODO(crbug.com/42290269): This should be const and thread-safe but currently
-// is neither, notably if |x509| was modified from its parsed value.
-OPENSSL_EXPORT uint32_t X509_issuer_name_hash(X509 *x509);
+OPENSSL_EXPORT uint32_t X509_issuer_name_hash(const X509 *x509);
// X509_subject_name_hash returns the hash of |x509|'s subject name with
// |X509_NAME_hash|.
@@ -4450,10 +4390,7 @@
// not suitable for general-purpose X.509 name processing. It is very short, so
// there will be hash collisions. It also depends on an OpenSSL-specific
// canonicalization process.
-//
-// TODO(crbug.com/42290269): This should be const and thread-safe but currently
-// is neither, notably if |x509| was modified from its parsed value.
-OPENSSL_EXPORT uint32_t X509_subject_name_hash(X509 *x509);
+OPENSSL_EXPORT uint32_t X509_subject_name_hash(const X509 *x509);
// X509_issuer_name_hash_old returns the hash of |x509|'s issuer name with
// |X509_NAME_hash_old|.
@@ -4461,10 +4398,7 @@
// This hash is specific to the |X509_LOOKUP_add_dir| filesystem format and is
// not suitable for general-purpose X.509 name processing. It is very short, so
// there will be hash collisions.
-//
-// TODO(crbug.com/42290269): This should be const and thread-safe but currently
-// is neither, notably if |x509| was modified from its parsed value.
-OPENSSL_EXPORT uint32_t X509_issuer_name_hash_old(X509 *x509);
+OPENSSL_EXPORT uint32_t X509_issuer_name_hash_old(const X509 *x509);
// X509_subject_name_hash_old returns the hash of |x509|'s usjbect name with
// |X509_NAME_hash_old|.
@@ -4472,10 +4406,7 @@
// This hash is specific to the |X509_LOOKUP_add_dir| filesystem format and is
// not suitable for general-purpose X.509 name processing. It is very short, so
// there will be hash collisions.
-//
-// TODO(crbug.com/42290269): This should be const and thread-safe but currently
-// is neither, notably if |x509| was modified from its parsed value.
-OPENSSL_EXPORT uint32_t X509_subject_name_hash_old(X509 *x509);
+OPENSSL_EXPORT uint32_t X509_subject_name_hash_old(const X509 *x509);
// ex_data functions.
@@ -4598,9 +4529,7 @@
//
// This function returning one does not indicate that |x509| is trusted, only
// that it is eligible to be a CA.
-//
-// TODO(crbug.com/42290269): |x509| should be const.
-OPENSSL_EXPORT int X509_check_ca(X509 *x509);
+OPENSSL_EXPORT int X509_check_ca(const X509 *x509);
// X509_check_issued checks if |issuer| and |subject|'s name, authority key
// identifier, and key usage fields allow |issuer| to have issued |subject|. It
@@ -4609,15 +4538,12 @@
// This function does not check the signature on |subject|. Rather, it is
// intended to prune the set of possible issuer certificates during
// path-building.
-//
-// TODO(crbug.com/42290269): Both parameters should be const.
-OPENSSL_EXPORT int X509_check_issued(X509 *issuer, X509 *subject);
+OPENSSL_EXPORT int X509_check_issued(const X509 *issuer, const X509 *subject);
// NAME_CONSTRAINTS_check checks if |x509| satisfies name constraints in |nc|.
// It returns |X509_V_OK| on success and some |X509_V_ERR_*| constant on error.
-//
-// TODO(crbug.com/42290269): Both parameters should be const.
-OPENSSL_EXPORT int NAME_CONSTRAINTS_check(X509 *x509, NAME_CONSTRAINTS *nc);
+OPENSSL_EXPORT int NAME_CONSTRAINTS_check(const X509 *x509,
+ const NAME_CONSTRAINTS *nc);
// X509_check_host checks if |x509| matches the DNS name |chk|. It returns one
// on match, zero on mismatch, or a negative number on error. |flags| should be
@@ -4695,10 +4621,9 @@
//
// This function only searches for trusted issuers. It does not consider
// untrusted intermediates passed in to |X509_STORE_CTX_init|.
-//
-// TODO(crbug.com/42290269): |x509| should be const.
OPENSSL_EXPORT int X509_STORE_CTX_get1_issuer(X509 **out_issuer,
- X509_STORE_CTX *ctx, X509 *x509);
+ X509_STORE_CTX *ctx,
+ const X509 *x509);
// X509_check_purpose performs checks if |x509|'s basic constraints, key usage,
// and extended key usage extensions for the specified purpose. |purpose| should
@@ -4729,19 +4654,15 @@
// trusted certificates in |ctx|'s |X509_STORE| whose subject matches |name|, or
// NULL on error. The caller must release the result with |sk_X509_pop_free| and
// |X509_free| when done.
-//
-// TODO(crbug.com/42290269): |name| should be const.
OPENSSL_EXPORT STACK_OF(X509) *X509_STORE_CTX_get1_certs(X509_STORE_CTX *ctx,
- X509_NAME *name);
+ const X509_NAME *name);
// X509_STORE_CTX_get1_crls returns a newly-allocated stack containing all
// CRLs in |ctx|'s |X509_STORE| whose subject matches |name|, or NULL on error.
// The caller must release the result with |sk_X509_CRL_pop_free| and
// |X509_CRL_free| when done.
-//
-// TODO(crbug.com/42290269): |name| should be const.
-OPENSSL_EXPORT STACK_OF(X509_CRL) *X509_STORE_CTX_get1_crls(X509_STORE_CTX *ctx,
- X509_NAME *name);
+OPENSSL_EXPORT STACK_OF(X509_CRL) *X509_STORE_CTX_get1_crls(
+ X509_STORE_CTX *ctx, const X509_NAME *name);
// X509_STORE_CTX_get_by_subject looks up an object of type |type| in |ctx|'s
// |X509_STORE| that matches |name|. |type| should be one of the |X509_LU_*|
@@ -4756,10 +4677,8 @@
// WARNING: Multiple trusted certificates or CRLs may share a name. In this
// case, this function returns an arbitrary match. Use
// |X509_STORE_CTX_get1_certs| or |X509_STORE_CTX_get1_crls| instead.
-//
-// TODO(crbug.com/42290269): |name| should be const.
OPENSSL_EXPORT int X509_STORE_CTX_get_by_subject(X509_STORE_CTX *ctx, int type,
- X509_NAME *name,
+ const X509_NAME *name,
X509_OBJECT *ret);
diff --git a/ssl/ssl_x509.cc b/ssl/ssl_x509.cc
index 4fe0da8..8dfb104 100644
--- a/ssl/ssl_x509.cc
+++ b/ssl/ssl_x509.cc
@@ -829,11 +829,7 @@
}
STACK_OF(X509_NAME) *SSL_dup_CA_list(STACK_OF(X509_NAME) *list) {
- // TODO(https://crbug.com/boringssl/407): |X509_NAME_dup| should be const.
- auto name_dup = [](const X509_NAME *name) {
- return X509_NAME_dup(const_cast<X509_NAME *>(name));
- };
- return sk_X509_NAME_deep_copy(list, name_dup, X509_NAME_free);
+ return sk_X509_NAME_deep_copy(list, X509_NAME_dup, X509_NAME_free);
}
static void set_client_CA_list(UniquePtr<STACK_OF(CRYPTO_BUFFER)> *ca_list,
