jvcelak@redhat.com wrote:

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.

Sounds like a simple sequencing bug then. Just initialize the global options before the first ldap_initialize() call.

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