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>

include/openssl/base.h

diff --git a/include/openssl/base.h b/include/openssl/base.h
index f0981b8..9a47321 100644
--- a/include/openssl/base.h
+++ b/include/openssl/base.h
@@ -109,7 +109,7 @@
 // A consumer may use this symbol in the preprocessor to temporarily build
 // against multiple revisions of BoringSSL at the same time. It is not
 // recommended to do so for longer than is necessary.
-#define BORINGSSL_API_VERSION 28
+#define BORINGSSL_API_VERSION 29
 
 #if defined(BORINGSSL_SHARED_LIBRARY)
 

include/openssl/ssl.h

diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index f1d4eb0..ab6225e 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -2601,13 +2601,13 @@
 OPENSSL_EXPORT void SSL_set_hostflags(SSL *ssl, unsigned flags);
 
 // SSL_CTX_set_verify_depth sets the maximum depth of a certificate chain
-// accepted in verification. This number does not include the leaf, so a depth
-// of 1 allows the leaf and one CA certificate.
+// accepted in verification. This count excludes both the target certificate and
+// the trust anchor (root certificate).
 OPENSSL_EXPORT void SSL_CTX_set_verify_depth(SSL_CTX *ctx, int depth);
 
 // SSL_set_verify_depth sets the maximum depth of a certificate chain accepted
-// in verification. This number does not include the leaf, so a depth of 1
-// allows the leaf and one CA certificate.
+// in verification. This count excludes both the target certificate and the
+// trust anchor (root certificate).
 OPENSSL_EXPORT void SSL_set_verify_depth(SSL *ssl, int depth);
 
 // SSL_CTX_get_verify_depth returns the maximum depth of a certificate accepted

include/openssl/x509.h

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