Change certificate depth limit to match OpenSSL and document
OpenSSL 1.1.0 included d9b8b89bec4480de3a10bdaf9425db371c19145b, which a
cleanup change to X509_verify_cert. This cleanup changed the semanitcs
of the depth limit. Previously, the depth limit omitted the leaf but
included the trust anchor. Now it omits both.
We forked a little before 1.0.2, so we still had the old behavior. Now
that the new behavior is well-established, switch to new one. Bump
BORINGSSL_API_VERSION so callers can detect one or the other as needed.
Document the new semantics. Also fix up some older docs where I implied
-1 was unlimited depth. Negative numbers were actually enforced as
negative numbers (which means only explicitly-trusted self-signed certs
worked).
Update-Note: The new semantics increase the limit by 1 compared to the
old ones. Thus this change should only accept more chains than
previously and be relatively safe. It also makes us more
OpenSSL-compatible. Envoy will need a tweak because they unit test the
boundary condition for the depth limit.
Bug: 426
Fixed: 459
Change-Id: Ifaa108b8135ea3d875f2ac1f2a3b2cd8a22aa323
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64707
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/include/openssl/x509.h b/include/openssl/x509.h
index 852a96f..b27c97d 100644
--- a/include/openssl/x509.h
+++ b/include/openssl/x509.h
@@ -3522,8 +3522,14 @@
X509 *x);
typedef int (*X509_STORE_CTX_check_crl_fn)(X509_STORE_CTX *ctx, X509_CRL *crl);
+// X509_STORE_set_depth configures |store| to, by default, limit certificate
+// chains to |depth| intermediate certificates. This count excludes both the
+// target certificate and the trust anchor (root certificate).
OPENSSL_EXPORT int X509_STORE_set_depth(X509_STORE *store, int depth);
+// X509_STORE_CTX_set_depth configures |ctx| to, by default, limit certificate
+// chains to |depth| intermediate certificates. This count excludes both the
+// target certificate and the trust anchor (root certificate).
OPENSSL_EXPORT void X509_STORE_CTX_set_depth(X509_STORE_CTX *ctx, int depth);
#define X509_STORE_CTX_set_app_data(ctx, data) \
@@ -3734,14 +3740,12 @@
//
// As of writing these late defaults are a depth limit (see
// |X509_VERIFY_PARAM_set_depth|) and the |X509_V_FLAG_TRUSTED_FIRST| flag. This
-// warning does not apply if the parameters were set in |store|. That is,
-// callers may safely set a concrete depth limit in |store|, but unlimited depth
-// must be configured at |X509_STORE_CTX|.
+// warning does not apply if the parameters were set in |store|.
//
// TODO(crbug.com/boringssl/441): This behavior is very surprising. Can we
-// remove this notion of late defaults? A depth limit of 100 can probably be
-// applied unconditionally. |X509_V_FLAG_TRUSTED_FIRST| is mostly a workaround
-// for poor path-building.
+// remove this notion of late defaults? The unsettable value at |X509_STORE| is
+// -1, which rejects everything but explicitly-trusted self-signed certificates.
+// |X509_V_FLAG_TRUSTED_FIRST| is mostly a workaround for poor path-building.
OPENSSL_EXPORT X509_VERIFY_PARAM *X509_STORE_get0_param(X509_STORE *store);
// X509_STORE_set_verify_cb acts like |X509_STORE_CTX_set_verify_cb| but sets
@@ -3963,6 +3967,10 @@
int purpose);
OPENSSL_EXPORT int X509_VERIFY_PARAM_set_trust(X509_VERIFY_PARAM *param,
int trust);
+
+// X509_VERIFY_PARAM_set_depth configures |param| to limit certificate chains to
+// |depth| intermediate certificates. This count excludes both the target
+// certificate and the trust anchor (root certificate).
OPENSSL_EXPORT void X509_VERIFY_PARAM_set_depth(X509_VERIFY_PARAM *param,
int depth);
@@ -4043,6 +4051,8 @@
OPENSSL_EXPORT int X509_VERIFY_PARAM_set1_ip_asc(X509_VERIFY_PARAM *param,
const char *ipasc);
+// X509_VERIFY_PARAM_get_depth returns the maximum depth configured in |param|.
+// See |X509_VERIFY_PARAM_set_depth|.
OPENSSL_EXPORT int X509_VERIFY_PARAM_get_depth(const X509_VERIFY_PARAM *param);
typedef void *(*X509V3_EXT_NEW)(void);
