From 4e6f698d07ff7353dea00fd20b15873563a0941f Mon Sep 17 00:00:00 2001 From: SG <13872653+mmguero@users.noreply.github.com> Date: Mon, 20 Apr 2020 14:54:55 -0600 Subject: [PATCH 1/4] Revert "Merge pull request #199 from AVENTER-UG/issue_180" This reverts commit bf64cf217abbe79917f9d44a651c2ecbb82ec993, reversing changes made to f022103e31e71e70a4bb64cd4c7792fa3f238b4a. This change isn't right -- it an LDAP setup when `group_attribute_is_dn on` is enabled, which is what this section of code (https://github.com/kvspb/nginx-auth-ldap/commit/bf64cf217abbe79917f9d44a651c2ecbb82ec993#diff-c05c0daefb48996cbf510b81002b49bcR2230) is conditionally targeting. This original PR #199 changed the underlying LDAP query (eg `user_val`) from looking up the user's DN as a group attribute in LDAP (eg set via the `group_attribute` directive in nginx) to looking up the _group's_ DN, which isn't right and won't work. This PR reverts the previous change to make this work correctly again. Fwiw, the originally-referenced issue #180 seems to be a completely different issue, relating to escaping and parentheses. --- ngx_http_auth_ldap_module.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/ngx_http_auth_ldap_module.c b/ngx_http_auth_ldap_module.c index 37d0718..2f4e592 100644 --- a/ngx_http_auth_ldap_module.c +++ b/ngx_http_auth_ldap_module.c @@ -2216,7 +2216,6 @@ ngx_http_auth_ldap_check_group(ngx_http_request_t *r, ngx_http_auth_ldap_ctx_t * ngx_memcpy(gr, val.data, val.len); gr[val.len] = '\0'; tail_gr = ngx_strchr(gr, ','); - if (tail_gr == NULL) { ngx_log_error(NGX_LOG_ERR, r->connection->log, 0, "http_auth_ldap: Incorrect group DN: \"%s\"", gr); ctx->outcome = OUTCOME_ERROR; @@ -2230,9 +2229,9 @@ ngx_http_auth_ldap_check_group(ngx_http_request_t *r, ngx_http_auth_ldap_ctx_t * if (ctx->server->group_attribute_dn == 1) { user_val = ngx_pcalloc( r->pool, - ctx->dn.len + 1); - ngx_memcpy(user_val, ctx->dn.data, ctx->dn.len); - user_val[ctx->dn.len] = '\0'; + ctx->user_dn.len + 1); + ngx_memcpy(user_val, ctx->user_dn.data, ctx->user_dn.len); + user_val[ctx->user_dn.len] = '\0'; } else { user_val = ngx_pcalloc( r->pool, From 456514a8771d96bc2a8aea2198b56ae8ed6f2194 Mon Sep 17 00:00:00 2001 From: SG <13872653+mmguero@users.noreply.github.com> Date: Mon, 4 May 2020 16:12:01 -0600 Subject: [PATCH 2/4] Allow ssl_check_cert to be on (full verification), off (no verification) or chain (verify cert chain but not hostname/IP) --- ngx_http_auth_ldap_module.c | 75 +++++++++++++++++++++++-------------- 1 file changed, 46 insertions(+), 29 deletions(-) diff --git a/ngx_http_auth_ldap_module.c b/ngx_http_auth_ldap_module.c index 2f4e592..f123924 100644 --- a/ngx_http_auth_ldap_module.c +++ b/ngx_http_auth_ldap_module.c @@ -69,6 +69,10 @@ extern int ldap_init_fd(ber_socket_t fd, int proto, const char *url, LDAP **ld); #define NGX_HTTP_AUTH_LDAP_MAX_SERVERS_SIZE 7 +#define SSL_CERT_VERIFY_OFF 0 +#define SSL_CERT_VERIFY_FULL 1 +#define SSL_CERT_VERIFY_CHAIN 2 + typedef struct { LDAPURLDesc *ludpp; @@ -431,18 +435,24 @@ ngx_http_auth_ldap_ldap_server(ngx_conf_t *cf, ngx_command_t *dummy, void *conf) return NGX_CONF_ERROR; } server->connections = i; - } else if (ngx_strcmp(value[0].data, "ssl_check_cert") == 0 && ngx_strcmp(value[1].data, "on") == 0) { - #if OPENSSL_VERSION_NUMBER >= 0x10002000 - server->ssl_check_cert = 1; - #else - #if GNUC > 4 - #warning "http_auth_ldap: Compiling with OpenSSL < 1.0.2, certificate verification will be unavailable. OPENSSL_VERSION_NUMBER == " XSTR(OPENSSL_VERSION_NUMBER) - #endif - ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, - "http_auth_ldap: 'ssl_cert_check': cannot verify remote certificate's domain name because " - "your version of OpenSSL is too old. " - "Please install OpenSSL >= 1.02 and recompile nginx."); - #endif + } else if (ngx_strcmp(value[0].data, "ssl_check_cert") == 0) { + #if OPENSSL_VERSION_NUMBER >= 0x10002000 + if (ngx_strcmp(value[1].data, "on") == 0) { + server->ssl_check_cert = SSL_CERT_VERIFY_FULL; + } else if (ngx_strcmp(value[1].data, "chain") == 0) { + server->ssl_check_cert = SSL_CERT_VERIFY_CHAIN; + } else { + server->ssl_check_cert = SSL_CERT_VERIFY_OFF; + } + #else + #if GNUC > 4 + #warning "http_auth_ldap: Compiling with OpenSSL < 1.0.2, certificate verification will be unavailable. OPENSSL_VERSION_NUMBER == " XSTR(OPENSSL_VERSION_NUMBER) + #endif + ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, + "http_auth_ldap: 'ssl_cert_check': cannot verify remote certificate's domain name because " + "your version of OpenSSL is too old. " + "Please install OpenSSL >= 1.0.2 and recompile nginx."); + #endif } else if (ngx_strcmp(value[0].data, "ssl_ca_dir") == 0) { server->ssl_ca_dir = value[1]; } else if (ngx_strcmp(value[0].data, "ssl_ca_file") == 0) { @@ -1334,18 +1344,25 @@ ngx_http_auth_ldap_ssl_handshake_handler(ngx_connection_t *conn, ngx_flag_t vali long chain_verified = SSL_get_verify_result(conn->ssl->connection); int addr_verified; - char *hostname = c->server->ludpp->lud_host; - addr_verified = X509_check_host(cert, hostname, 0, 0, 0); - - if (!addr_verified) { // domain not in cert? try IP - size_t len; // get IP length - if (conn->sockaddr->sa_family == 4) len = 4; - else if (conn->sockaddr->sa_family == 6) len = 16; - else { // very unlikely indeed - ngx_http_auth_ldap_close_connection(c); - return; + if (c->server->ssl_check_cert == SSL_CERT_VERIFY_CHAIN) { + // chain_verified is enough, not requiring full name/IP verification + addr_verified = 1; + + } else { + // verify hostname/IP + char *hostname = c->server->ludpp->lud_host; + addr_verified = X509_check_host(cert, hostname, 0, 0, 0); + + if (!addr_verified) { // domain not in cert? try IP + size_t len; // get IP length + if (conn->sockaddr->sa_family == 4) len = 4; + else if (conn->sockaddr->sa_family == 6) len = 16; + else { // very unlikely indeed + ngx_http_auth_ldap_close_connection(c); + return; + } + addr_verified = X509_check_ip(cert, (const unsigned char*)conn->sockaddr->sa_data, len, 0); } - addr_verified = X509_check_ip(cert, (const unsigned char*)conn->sockaddr->sa_data, len, 0); } // Find anything fishy? @@ -1528,11 +1545,11 @@ ngx_http_auth_ldap_read_handler(ngx_event_t *rev) // if LDAP_SERVER_DOWN (usually timeouts or server disconnects) if (rc == LDAP_SERVER_DOWN && \ - c->server->max_down_retries_count < c->server->max_down_retries) { - /** - update counter (this is always reset in - ngx_http_auth_ldap_connect() for a successful ldap - connection + c->server->max_down_retries_count < c->server->max_down_retries) { + /** + update counter (this is always reset in + ngx_http_auth_ldap_connect() for a successful ldap + connection **/ c->server->max_down_retries_count++; ngx_log_error(NGX_LOG_ERR, c->log, 0, "http_auth_ldap: LDAP_SERVER_DOWN: retry count: %d", @@ -1542,7 +1559,7 @@ ngx_http_auth_ldap_read_handler(ngx_event_t *rev) // timer call to this read handler again ngx_http_auth_ldap_reconnect_handler(rev); return; - } + } return; } From 3686bd0859310c54ebe7bda50b872be6f81731b4 Mon Sep 17 00:00:00 2001 From: SG <13872653+mmguero@users.noreply.github.com> Date: Tue, 5 May 2020 10:55:35 -0600 Subject: [PATCH 3/4] testing fix for kvspb/nginx-auth-ldap#236 segmentation fault in ngx_http_auth_ldap_ssl_handshake_handler with ssl_check_cert and ssl_ca_dir --- ngx_http_auth_ldap_module.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/ngx_http_auth_ldap_module.c b/ngx_http_auth_ldap_module.c index f123924..9dd1dcb 100644 --- a/ngx_http_auth_ldap_module.c +++ b/ngx_http_auth_ldap_module.c @@ -26,6 +26,7 @@ * SUCH DAMAGE. */ +#include #include #include #include @@ -437,7 +438,7 @@ ngx_http_auth_ldap_ldap_server(ngx_conf_t *cf, ngx_command_t *dummy, void *conf) server->connections = i; } else if (ngx_strcmp(value[0].data, "ssl_check_cert") == 0) { #if OPENSSL_VERSION_NUMBER >= 0x10002000 - if (ngx_strcmp(value[1].data, "on") == 0) { + if ((ngx_strcmp(value[1].data, "on") == 0) || (ngx_strcmp(value[1].data, "full") == 0)) { server->ssl_check_cert = SSL_CERT_VERIFY_FULL; } else if (ngx_strcmp(value[1].data, "chain") == 0) { server->ssl_check_cert = SSL_CERT_VERIFY_CHAIN; @@ -1355,13 +1356,19 @@ ngx_http_auth_ldap_ssl_handshake_handler(ngx_connection_t *conn, ngx_flag_t vali if (!addr_verified) { // domain not in cert? try IP size_t len; // get IP length - if (conn->sockaddr->sa_family == 4) len = 4; - else if (conn->sockaddr->sa_family == 6) len = 16; + + struct sockaddr *conn_sockaddr = NULL; + if (conn->sockaddr != NULL) conn_sockaddr = conn->sockaddr; + else if (c->conn->sockaddr != NULL) conn_sockaddr = c->conn->sockaddr; + else conn_sockaddr = &c->server->parsed_url->sockaddr.sockaddr; + + if (conn_sockaddr->sa_family == AF_INET) len = 4; + else if (conn_sockaddr->sa_family == AF_INET6) len = 16; else { // very unlikely indeed ngx_http_auth_ldap_close_connection(c); return; } - addr_verified = X509_check_ip(cert, (const unsigned char*)conn->sockaddr->sa_data, len, 0); + addr_verified = X509_check_ip(cert, (const unsigned char*)conn_sockaddr->sa_data, len, 0); } } From 61b70e4282cf61db50f6369ef059d2a71763f667 Mon Sep 17 00:00:00 2001 From: SG <13872653+mmguero@users.noreply.github.com> Date: Tue, 5 May 2020 11:11:27 -0600 Subject: [PATCH 4/4] fix compiler error --- ngx_http_auth_ldap_module.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ngx_http_auth_ldap_module.c b/ngx_http_auth_ldap_module.c index 9dd1dcb..341c9f8 100644 --- a/ngx_http_auth_ldap_module.c +++ b/ngx_http_auth_ldap_module.c @@ -1359,8 +1359,8 @@ ngx_http_auth_ldap_ssl_handshake_handler(ngx_connection_t *conn, ngx_flag_t vali struct sockaddr *conn_sockaddr = NULL; if (conn->sockaddr != NULL) conn_sockaddr = conn->sockaddr; - else if (c->conn->sockaddr != NULL) conn_sockaddr = c->conn->sockaddr; - else conn_sockaddr = &c->server->parsed_url->sockaddr.sockaddr; + else if (c->conn.sockaddr != NULL) conn_sockaddr = c->conn.sockaddr; + else conn_sockaddr = &c->server->parsed_url.sockaddr.sockaddr; if (conn_sockaddr->sa_family == AF_INET) len = 4; else if (conn_sockaddr->sa_family == AF_INET6) len = 16;