segmentation fault in ngx_http_auth_ldap_ssl_handshake_handler with ssl_check_cert and ssl_ca_dir

[mmguero]

My config file looks like this:

ldap_server ad_server {
  url "ldaps://192.168.0.73:636/DC=local,DC=lan?uid?sub?(objectClass=posixAccount)";
  ssl_check_cert on;
  ssl_ca_dir /var/run/ca-trust;

  binddn "xxxxxxxxxx";
  binddn_passwd "xxxxxxxxxx";

  require valid_user;
}

When I remove the ssl_ stuff, it works fine. With it in, however, I get:

...
[alert] 360#360: worker process 405 exited on signal 11
...

Putting things up in GDB, I see the segfault here, in ngx_http_auth_ldap_ssl_handshake_handler:

1340│           if (!addr_verified) { // domain not in cert? try IP
1341│             size_t len; // get IP length
1342├───────────> if (conn->sockaddr->sa_family == 4) len = 4;
1343│             else if (conn->sockaddr->sa_family == 6) len = 16;
1344│             else { // very unlikely indeed
1345│               ngx_http_auth_ldap_close_connection(c);
1346│               return;
1347│             }
1348│             addr_verified = X509_check_ip(cert, (const unsigned char*)conn->sockaddr->sa_data, len, 0);
1349│           }

(gdb) print *conn
$3 = {data = 0x55bdc4c27ff0, read = 0x55bdc4c2c0a0, write = 0x55bdc4c440c0, fd = 21, recv = 0x55bdc45e2078 <ngx_ssl_recv>, send = 0x55bdc45e26f5 <ngx_ssl_write>, recv_chain = 0x55bdc45e2647 <
ngx_ssl_recv_chain>, send_chain = 0x55bdc45e3046 <ngx_ssl_send_chain>, listening = 0x0, sent = 0, log = 0x55bdc4b70ec8, pool = 0x55bdc4b70e60, type = 1, sockaddr = 0x0, socklen = 0, addr_text
 = {len = 0, data = 0x0}, proxy_protocol = 0x0, ssl = 0x55bdc4c28228, udp = 0x0, local_sockaddr = 0x0, local_socklen = 0, buffer = 0x0, queue = {prev = 0x0, next = 0x0}, number = 1, requests
= 0, buffered = 0, log_error = 1, timedout = 0, error = 0, destroyed = 0, idle = 0, reusable = 0, close = 0, shared = 0, sendfile = 1, sndlowat = 0, tcp_nodelay = 0, tcp_nopush = 0, need_last
_buf = 0, busy_count = 0, sendfile_task = 0x0}

So conn->sockaddr is 0x0, hence the segfault.

Let me know if you need more information. This is running in a docker container (Dockerfile) and was discovered as I was working on an enhancement for my project, idaholab/Malcolm#128

Here's the backtrace into the function:

(gdb) bt
#0  ngx_http_auth_ldap_ssl_handshake_handler (conn=0x7f9260474590, validate=validate@entry=1) at /usr/src/nginx-auth-ldap/ngx_http_auth_ldap_module.c:1326
#1  0x000055aefc148ecf in ngx_http_auth_ldap_ssl_handshake_validating_handler (conn=<optimized out>) at /usr/src/nginx-auth-ldap/ngx_http_auth_ldap_module.c:1383
#2  0x000055aefc09dccf in ngx_ssl_handshake_handler (ev=0x55aefcf530a0) at src/event/ngx_event_openssl.c:1890
#3  0x000055aefc0983ee in ngx_epoll_process_events (cycle=0x55aefce97eb0, timer=<optimized out>, flags=<optimized out>) at src/event/modules/ngx_epoll_module.c:901
#4  0x000055aefc08c5e9 in ngx_process_events_and_timers (cycle=cycle@entry=0x55aefce97eb0) at src/event/ngx_event.c:247
#5  0x000055aefc096104 in ngx_worker_process_cycle (cycle=0x55aefce97eb0, data=<optimized out>) at src/os/unix/ngx_process_cycle.c:750
#6  0x000055aefc09443d in ngx_spawn_process (cycle=cycle@entry=0x55aefce97eb0, proc=proc@entry=0x55aefc095fdd <ngx_worker_process_cycle>, data=data@entry=0x0, name=name@entry=0x55aefc14e746 "
worker process", respawn=respawn@entry=-3) at src/os/unix/ngx_process.c:199
#7  0x000055aefc095094 in ngx_start_worker_processes (cycle=cycle@entry=0x55aefce97eb0, n=1, type=type@entry=-3) at src/os/unix/ngx_process_cycle.c:359
#8  0x000055aefc096a17 in ngx_master_process_cycle (cycle=0x55aefce97eb0) at src/os/unix/ngx_process_cycle.c:131
#9  0x000055aefc06b097 in main (argc=<optimized out>, argv=<optimized out>) at src/core/nginx.c:382

[mmguero]

Just spitballing, perhaps the same value could be pulled from c->conn->sockaddr or c->server->parsed_url->sockaddr?

[mmguero]

Looking at this a little bit closer, here's what I think would be really neat:

Right now the code in ngx_http_auth_ldap_ssl_handshake_handler verifies the chain ( long chain_verified = SSL_get_verify_result(conn->ssl->connection)) and then verifies the hostname (addr_verified = X509_check_host(cert, hostname, 0, 0, 0)) or IP address (X509_check_ip(cert, (const unsigned char*)conn->sockaddr->sa_data, len, 0)).

It would be nice to have the ability to verify the chain without checking the host/ip. stunnel does this by allowing you to specify a checkHost or checkIP value. If unspecified, the chain is verified without the host/IP being checked.

Maybe ssl_check_cert could have, instead of on and off, on, off, and chain or something like that? That would allow checking against the CA without going to the host/ip check.

[mmguero]

I'm working on something in my fork of this plugin to handle the case of the last comment:

compare kvspb/nginx-auth-ldap/master ... mmguero-dev/nginx-auth-ldap/master

This does't fix the bug, but it would allow a partial verification of the file while maintaining backwards compatibility. I'm going to do some more testing and do a pull request if you think it's okay and it works for my case.

[mmguero]

I should also note, although it's obvious from looking at the code: this only fails if the call to X509_check_host fails, and it drops down into the domain not in cert? try IP code.

[mmguero]

I'm pretty this check is wrong anyway, even if it werent' getting the segfault:

              if (conn->sockaddr->sa_family == 4) len = 4;
              else if (conn->sockaddr->sa_family == 6) len = 16;

sa_family should be compared against the AF_INET or AF_INET6 constants (from sys/socket.h), not a literal 4 or 6. These may vary from platform to platform.

[mmguero]

Created PR #237 to address this bug.