Hello! On Wed, Jul 20, 2022 at 05:58:26PM +0400, Sergey Kandaurov wrote:
> # HG changeset patch > # User Sergey Kandaurov <pluk...@nginx.com> > # Date 1658325446 -14400 > # Wed Jul 20 17:57:26 2022 +0400 > # Node ID 5a9cc2a846c9ea4c0af03109ab186af1ac28e222 > # Parent 069a4813e8d6d7ec662d282a10f5f7062ebd817f > SSL: consistent certificate verification depth. > > Originally, certificate verification depth was used to limit the number > of signatures to validate, that is, to limit chains with intermediate > certificates one less. The semantics was changed in OpenSSL 1.1.0, and > instead it limits now the number of intermediate certificates allowed. > This makes it not possible to limit certificate checking to self-signed > certificates with verify depth 0 in OpenSSL 1.1.0+, and is inconsistent > compared with former behaviour in BoringSSL and older OpenSSL versions. > > This change restores former verification logic when using OpenSSL 1.1.0+. > The verify callback is adjusted to emit the "certificate chain too long" > error when the certificate chain exceeds the verification depth. It has > no effect to other SSL libraries, where this is limited by other means. > Also, this fixes verification checks when using LibreSSL 3.4.0+, where > a chain depth is not limited except by X509_VERIFY_MAX_CHAIN_CERTS (32). This (highly questionable) OpenSSL behaviour seems to be status quo for a while, also recorded in tests (see ssl_verify_depth.t). Any specific reasons for the nginx-side workaround? > > diff -r 069a4813e8d6 -r 5a9cc2a846c9 src/event/ngx_event_openssl.c > --- a/src/event/ngx_event_openssl.c Tue Jul 19 17:05:27 2022 +0300 > +++ b/src/event/ngx_event_openssl.c Wed Jul 20 17:57:26 2022 +0400 > @@ -997,16 +997,26 @@ ngx_ssl_crl(ngx_conf_t *cf, ngx_ssl_t *s > static int > ngx_ssl_verify_callback(int ok, X509_STORE_CTX *x509_store) > { > + int depth, verify_depth; > + ngx_ssl_conn_t *ssl_conn; > + > + ssl_conn = X509_STORE_CTX_get_ex_data(x509_store, > + > SSL_get_ex_data_X509_STORE_CTX_idx()); > + > + depth = X509_STORE_CTX_get_error_depth(x509_store); > + verify_depth = SSL_CTX_get_verify_depth(SSL_get_SSL_CTX(ssl_conn)); s/verify_depth/limit/? Also, using SSL_get_verify_depth() instead of going through SSL context might be easier. > + > + if (depth > verify_depth) { > + X509_STORE_CTX_set_error(x509_store, X509_V_ERR_CERT_CHAIN_TOO_LONG); This is going to overwrite earlier errors, if any. Does not look like a good idea. > + } > + > #if (NGX_DEBUG) > + > + int err; This is not going to work, variables have to be defined at the start of a block unless you assume C99. At least MSVC 2010 won't be able to compile this. > char *subject, *issuer; > - int err, depth; > X509 *cert; > X509_NAME *sname, *iname; > ngx_connection_t *c; > - ngx_ssl_conn_t *ssl_conn; > - > - ssl_conn = X509_STORE_CTX_get_ex_data(x509_store, > - > SSL_get_ex_data_X509_STORE_CTX_idx()); > > c = ngx_ssl_get_connection(ssl_conn); > > @@ -1016,7 +1026,6 @@ ngx_ssl_verify_callback(int ok, X509_STO > > cert = X509_STORE_CTX_get_current_cert(x509_store); > err = X509_STORE_CTX_get_error(x509_store); > - depth = X509_STORE_CTX_get_error_depth(x509_store); > > sname = X509_get_subject_name(cert); > > @@ -1058,6 +1067,7 @@ ngx_ssl_verify_callback(int ok, X509_STO > if (issuer) { > OPENSSL_free(issuer); > } > + > #endif > > return 1; > -- Maxim Dounin http://mdounin.ru/ _______________________________________________ nginx-devel mailing list -- nginx-devel@nginx.org To unsubscribe send an email to nginx-devel-le...@nginx.org