[Date Prev][Date Next] [Chronological] [Thread] [Top]

(ITS#7240) [PATCH] MozNSS: skip hostname check if peer certificate was not requested



Full_Name: Jan Vcelak
Version: git master
OS: Linux
URL: ftp://ftp.openldap.org/incoming/jvcelak-120412-moznss-hostname-check.patch
Submission from: (NULL) (209.132.186.34)


Hello.

With Mozilla NSS crypto backend, the 'tls_checkpeer no' option in 'sudo' tool
does not work. I have uploaded a patch which fixes the bug.

But some explanation is necessary:

sudo initially creates a ldap handle using ldap_initialize() function. Then it
uses ldap_set_option() to set all the settings. The TLS configuration is applied
without passing a LDAP handle, which means the the global options are changed.

In ldap_int_start_tls():

- The ldap_int_tls_connect() is called and the global TLS context is used
(LDAP_INT_GLOBAL_OPT()->ldo_tls_ctx). Which is correct.

- The ldap_pvt_tls_check_hostname() is called always. "sudo" will set the
ldo_tls_require_cert in global options to LDAP_OPT_X_TLS_NEVER, but the handle
options (ld->ld_options) are used to check whether the verification is supposed
to happen. And ldo_tls_require_cert in handle options is LDAP_OPT_X_TLS_DEMAND
(the default), because it was copied from the global options at the time when
ldap_initialize() was called.

- With OpenSSL, hostname check (tlso_session_chkhost) will succeed because
tlso_get_cert() will return NULL. This is true because no certificate was
requested when connecting. We do not request the certificate as well in Mozilla
NSS, but it seems that the peer can send it anyway and the library will accept
and store it. In this case, hostname check (tlsm_session_chkhost) might fail
because SSL_PeerCertificate() will return it.

I've added an additional check, which skips the hostname verification within
tlsm_session_chkhost if we do not request the certificate validation. With this
patch, the behavior is consistent with OpenSSL.

(Maybe ldap_pvt_tls_check_hostname() should not just be called, because the
global context was used and therefore we should use global options. Not sure
what is expected, but I think that changing this might break some applications.
My fix is safer.)

Jan