kvspb / nginx-auth-ldap

LDAP authentication module for nginx
BSD 2-Clause "Simplified" License
735 stars 252 forks source link

fix segfault in ngx_http_auth_ldap_ssl_handshake_handler and add "chain" option for ssl_check_cert #237

Open mmguero opened 4 years ago

mmguero commented 4 years ago

This pull request addresses two things relating to ngx_http_auth_ldap_ssl_handshake_handler:

  1. Fixes kvspb/nginx-auth-ldap#236 by not assuming conn->sockaddr is non-NULL. Instead, it tries to get it from conn->sockaddr, c->conn.sockaddr, and finally c->server->parsed_url.sockaddr.sockaddr, in that order:
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;

Additionally, it uses the correct AF_INET and AF_INET6 constants for checking sa_family for IPv4 vs. IPv6, rather than the incorrect 4 and 6 integer literals.

  1. ssl_check_cert now accepts chain in addition to on and off:
    • on - full certificate verification in ngx_http_auth_ldap_ssl_handshake_handler, meaning checking the certificate chain with SSL_get_verify_result and X509_check_host or X509_check_ip (same as previous on behavior; full can also be used to mean the same thing as on)
    • off - no certificate verification (same as previous off behavior)
    • chain - perform certificate chain verification with SSL_get_verify_result, but don't do host/IP verification (i.e., don't call X509_check_host or X509_check_ip)

This should not break backwards compatibility. I've been testing it in conjunction with an issue in my project (idaholab/Malcolm#128) and it addresses my needs. I've left the checks in place for OPENSSL_VERSION_NUMBER to leave the warnings in place if compiled with too old a version of openssl.

Let me know if you've got any issues or questions.

mmguero commented 2 years ago

ping... project still active?