From 60a8ed26f3120356b2d3e6639ffc932eb0cb8721 Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Sat, 20 Feb 2021 18:02:49 +0300 Subject: SSL: X509_NAME_oneline() error handling. --- src/event/ngx_event_openssl.c | 42 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 37 insertions(+), 5 deletions(-) (limited to 'src/event') diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c index 93a6ae46e..b03c7ce86 100644 --- a/src/event/ngx_event_openssl.c +++ b/src/event/ngx_event_openssl.c @@ -1019,21 +1019,43 @@ ngx_ssl_verify_callback(int ok, X509_STORE_CTX *x509_store) depth = X509_STORE_CTX_get_error_depth(x509_store); sname = X509_get_subject_name(cert); - subject = sname ? X509_NAME_oneline(sname, NULL, 0) : "(none)"; + + if (sname) { + subject = X509_NAME_oneline(sname, NULL, 0); + if (subject == NULL) { + ngx_ssl_error(NGX_LOG_ALERT, c->log, 0, + "X509_NAME_oneline() failed"); + } + + } else { + subject = NULL; + } iname = X509_get_issuer_name(cert); - issuer = iname ? X509_NAME_oneline(iname, NULL, 0) : "(none)"; + + if (iname) { + issuer = X509_NAME_oneline(iname, NULL, 0); + if (issuer == NULL) { + ngx_ssl_error(NGX_LOG_ALERT, c->log, 0, + "X509_NAME_oneline() failed"); + } + + } else { + issuer = NULL; + } ngx_log_debug5(NGX_LOG_DEBUG_EVENT, c->log, 0, "verify:%d, error:%d, depth:%d, " "subject:\"%s\", issuer:\"%s\"", - ok, err, depth, subject, issuer); + ok, err, depth, + subject ? subject : "(none)", + issuer ? issuer : "(none)"); - if (sname) { + if (subject) { OPENSSL_free(subject); } - if (iname) { + if (issuer) { OPENSSL_free(issuer); } #endif @@ -4900,6 +4922,11 @@ ngx_ssl_get_subject_dn_legacy(ngx_connection_t *c, ngx_pool_t *pool, } p = X509_NAME_oneline(name, NULL, 0); + if (p == NULL) { + ngx_ssl_error(NGX_LOG_ALERT, c->log, 0, "X509_NAME_oneline() failed"); + X509_free(cert); + return NGX_ERROR; + } for (len = 0; p[len]; len++) { /* void */ } @@ -4943,6 +4970,11 @@ ngx_ssl_get_issuer_dn_legacy(ngx_connection_t *c, ngx_pool_t *pool, } p = X509_NAME_oneline(name, NULL, 0); + if (p == NULL) { + ngx_ssl_error(NGX_LOG_ALERT, c->log, 0, "X509_NAME_oneline() failed"); + X509_free(cert); + return NGX_ERROR; + } for (len = 0; p[len]; len++) { /* void */ } -- cgit From 7ae100407c9b771e1067396baf1d013490b488f5 Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Sat, 20 Feb 2021 18:02:54 +0300 Subject: SSL: added missed error reporting during variables evaluation. --- src/event/ngx_event_openssl.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) (limited to 'src/event') diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c index b03c7ce86..48b3192b1 100644 --- a/src/event/ngx_event_openssl.c +++ b/src/event/ngx_event_openssl.c @@ -83,7 +83,7 @@ static time_t ngx_ssl_parse_time( #if OPENSSL_VERSION_NUMBER > 0x10100000L const #endif - ASN1_TIME *asn1time); + ASN1_TIME *asn1time, ngx_log_t *log); static void *ngx_openssl_create_conf(ngx_cycle_t *cycle); static char *ngx_openssl_engine(ngx_conf_t *cf, ngx_command_t *cmd, void *conf); @@ -4817,11 +4817,13 @@ ngx_ssl_get_subject_dn(ngx_connection_t *c, ngx_pool_t *pool, ngx_str_t *s) bio = BIO_new(BIO_s_mem()); if (bio == NULL) { + ngx_ssl_error(NGX_LOG_ALERT, c->log, 0, "BIO_new() failed"); X509_free(cert); return NGX_ERROR; } if (X509_NAME_print_ex(bio, name, 0, XN_FLAG_RFC2253) < 0) { + ngx_ssl_error(NGX_LOG_ALERT, c->log, 0, "X509_NAME_print_ex() failed"); goto failed; } @@ -4869,11 +4871,13 @@ ngx_ssl_get_issuer_dn(ngx_connection_t *c, ngx_pool_t *pool, ngx_str_t *s) bio = BIO_new(BIO_s_mem()); if (bio == NULL) { + ngx_ssl_error(NGX_LOG_ALERT, c->log, 0, "BIO_new() failed"); X509_free(cert); return NGX_ERROR; } if (X509_NAME_print_ex(bio, name, 0, XN_FLAG_RFC2253) < 0) { + ngx_ssl_error(NGX_LOG_ALERT, c->log, 0, "X509_NAME_print_ex() failed"); goto failed; } @@ -5011,6 +5015,7 @@ ngx_ssl_get_serial_number(ngx_connection_t *c, ngx_pool_t *pool, ngx_str_t *s) bio = BIO_new(BIO_s_mem()); if (bio == NULL) { + ngx_ssl_error(NGX_LOG_ALERT, c->log, 0, "BIO_new() failed"); X509_free(cert); return NGX_ERROR; } @@ -5049,6 +5054,7 @@ ngx_ssl_get_fingerprint(ngx_connection_t *c, ngx_pool_t *pool, ngx_str_t *s) } if (!X509_digest(cert, EVP_sha1(), buf, &len)) { + ngx_ssl_error(NGX_LOG_ALERT, c->log, 0, "X509_digest() failed"); X509_free(cert); return NGX_ERROR; } @@ -5122,6 +5128,7 @@ ngx_ssl_get_client_v_start(ngx_connection_t *c, ngx_pool_t *pool, ngx_str_t *s) bio = BIO_new(BIO_s_mem()); if (bio == NULL) { + ngx_ssl_error(NGX_LOG_ALERT, c->log, 0, "BIO_new() failed"); X509_free(cert); return NGX_ERROR; } @@ -5166,6 +5173,7 @@ ngx_ssl_get_client_v_end(ngx_connection_t *c, ngx_pool_t *pool, ngx_str_t *s) bio = BIO_new(BIO_s_mem()); if (bio == NULL) { + ngx_ssl_error(NGX_LOG_ALERT, c->log, 0, "BIO_new() failed"); X509_free(cert); return NGX_ERROR; } @@ -5208,9 +5216,9 @@ ngx_ssl_get_client_v_remain(ngx_connection_t *c, ngx_pool_t *pool, ngx_str_t *s) } #if OPENSSL_VERSION_NUMBER > 0x10100000L - end = ngx_ssl_parse_time(X509_get0_notAfter(cert)); + end = ngx_ssl_parse_time(X509_get0_notAfter(cert), c->log); #else - end = ngx_ssl_parse_time(X509_get_notAfter(cert)); + end = ngx_ssl_parse_time(X509_get_notAfter(cert), c->log); #endif if (end == (time_t) NGX_ERROR) { @@ -5245,7 +5253,7 @@ ngx_ssl_parse_time( #if OPENSSL_VERSION_NUMBER > 0x10100000L const #endif - ASN1_TIME *asn1time) + ASN1_TIME *asn1time, ngx_log_t *log) { BIO *bio; char *value; @@ -5261,6 +5269,7 @@ ngx_ssl_parse_time( bio = BIO_new(BIO_s_mem()); if (bio == NULL) { + ngx_ssl_error(NGX_LOG_ALERT, log, 0, "BIO_new() failed"); return NGX_ERROR; } -- cgit From ef44627852ce831407d54c09d7a100e3ca41b9a6 Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Sat, 20 Feb 2021 18:03:04 +0300 Subject: SSL: added check for debugging. If debugging is not enabled, there is no need to do extra work in ngx_ssl_verify_callback() and ngx_ssl_handshake_log(). --- src/event/ngx_event_openssl.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'src/event') diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c index 48b3192b1..d762d6b7f 100644 --- a/src/event/ngx_event_openssl.c +++ b/src/event/ngx_event_openssl.c @@ -1014,6 +1014,10 @@ ngx_ssl_verify_callback(int ok, X509_STORE_CTX *x509_store) c = ngx_ssl_get_connection(ssl_conn); + if (!(c->log->log_level & NGX_LOG_DEBUG_EVENT)) { + return 1; + } + 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); @@ -1970,6 +1974,10 @@ ngx_ssl_handshake_log(ngx_connection_t *c) #endif SSL_CIPHER *cipher; + if (!(c->log->log_level & NGX_LOG_DEBUG_EVENT)) { + return; + } + cipher = SSL_get_current_cipher(c->ssl->connection); if (cipher) { -- cgit From d5a31fdad50dbd28974cb0eb7f23948241a7559e Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Fri, 5 Mar 2021 17:16:15 +0300 Subject: Events: fixed eventport handling in ngx_handle_read_event(). The "!rev->ready" test seems to be a typo, introduced in the original commit (719:f30b1a75fd3b). The ngx_handle_write_event() code properly tests for "rev->ready" instead. Due to this typo, read events might be unexpectedly removed during proxying after an event on the other part of the proxied connection. Catched by mail proxying tests. --- src/event/ngx_event.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/event') diff --git a/src/event/ngx_event.c b/src/event/ngx_event.c index 402a7f5e2..ed7b30bf3 100644 --- a/src/event/ngx_event.c +++ b/src/event/ngx_event.c @@ -318,7 +318,7 @@ ngx_handle_read_event(ngx_event_t *rev, ngx_uint_t flags) return NGX_OK; } - if (rev->oneshot && !rev->ready) { + if (rev->oneshot && rev->ready) { if (ngx_del_event(rev, NGX_READ_EVENT, 0) == NGX_ERROR) { return NGX_ERROR; } -- cgit