From 49f6078a217841f053c4cb02010ef10373116f04 Mon Sep 17 00:00:00 2001 From: Jan Vcelak Date: Wed, 24 Aug 2011 18:40:37 +0200 Subject: [PATCH] incorrect behavior of allow/try options of VerifyCert and TLS_REQCERT Resolves: #725819 --- openldap-nss-reqcert-hostname.patch | 28 ++++ openldap-nss-verifycert.patch | 209 ++++++++++++++++++++++++++++ openldap.spec | 5 + 3 files changed, 242 insertions(+) create mode 100644 openldap-nss-reqcert-hostname.patch create mode 100644 openldap-nss-verifycert.patch diff --git a/openldap-nss-reqcert-hostname.patch b/openldap-nss-reqcert-hostname.patch new file mode 100644 index 0000000..325319c --- /dev/null +++ b/openldap-nss-reqcert-hostname.patch @@ -0,0 +1,28 @@ +Do not check server hostname when TLS_REQCERT is 'allow'. + +If server certificate hostname does not match the server hostname, +connection is closed even if client has set TLS_REQCERT to 'allow'. This +is wrong - the documentation says, that bad certificates are being +ignored when TLS_REQCERT is set to 'allow'. + +Author: Jan Vcelak +Upstream ITS: #7014 +Resolves: #725819 + +diff --git a/libraries/libldap/tls2.c b/libraries/libldap/tls2.c +index f38db27..3f05c1e 100644 +--- a/libraries/libldap/tls2.c ++++ b/libraries/libldap/tls2.c +@@ -838,7 +838,8 @@ ldap_int_tls_start ( LDAP *ld, LDAPConn *conn, LDAPURLDesc *srv ) + /* + * compare host with name(s) in certificate + */ +- if (ld->ld_options.ldo_tls_require_cert != LDAP_OPT_X_TLS_NEVER) { ++ if (ld->ld_options.ldo_tls_require_cert != LDAP_OPT_X_TLS_NEVER && ++ ld->ld_options.ldo_tls_require_cert != LDAP_OPT_X_TLS_ALLOW) { + ld->ld_errno = ldap_pvt_tls_check_hostname( ld, ssl, host ); + if (ld->ld_errno != LDAP_SUCCESS) { + return ld->ld_errno; +-- +1.7.6 + diff --git a/openldap-nss-verifycert.patch b/openldap-nss-verifycert.patch new file mode 100644 index 0000000..9504eff --- /dev/null +++ b/openldap-nss-verifycert.patch @@ -0,0 +1,209 @@ +Fix server side VerifyCert allow/try behavior + +If the olcTLSVerifyClient is set to a value other than "never", the server +should request that the client send a client certificate for possible use +with client cert auth (e.g. SASL/EXTERNAL). +If set to "allow", if the client sends a cert, and there are problems with +it, the server will warn about problems, but will allow the SSL session to +proceed without a client cert. +If set to "try", if the client sends a cert, and there are problems with +it, the server will warn about those problems, and shutdown the SSL session. +If set to "demand" or "hard", the client must send a cert, and the server +will shutdown the SSL session if there are problems. +I added a new member of the tlsm context structure - tc_warn_only - if this +is set, tlsm_verify_cert will only warn about errors, and only if TRACE +level debug is set. This allows the server to warn but allow bad certs +if "allow" is set, and warn and fail if "try" is set. + +Author: Rich Megginson +Upstream ITS: #7002 +Upstream commit: 210b156ece28a71cb625283fa5c30ee76d639cdc +Resolves: #725819 + +diff --git a/libraries/libldap/tls_m.c b/libraries/libldap/tls_m.c +index 72fdf49..997b3eb 100644 +--- a/libraries/libldap/tls_m.c ++++ b/libraries/libldap/tls_m.c +@@ -96,6 +96,7 @@ typedef struct tlsm_ctx { + #endif + PK11GenericObject **tc_pem_objs; /* array of objects to free */ + int tc_n_pem_objs; /* number of objects */ ++ PRBool tc_warn_only; /* only warn of errors in validation */ + #ifdef LDAP_R_COMPILE + ldap_pvt_thread_mutex_t tc_refmutex; + #endif +@@ -945,6 +946,11 @@ tlsm_verify_cert(CERTCertDBHandle *handle, CERTCertificate *cert, void *pinarg, + CERTVerifyLog verifylog; + SECStatus ret = SECSuccess; + const char *name; ++ int debug_level = LDAP_DEBUG_ANY; ++ ++ if ( errorToIgnore == -1 ) { ++ debug_level = LDAP_DEBUG_TRACE; ++ } + + /* the log captures information about every cert in the chain, so we can tell + which cert caused the problem and what the problem was */ +@@ -965,7 +971,7 @@ tlsm_verify_cert(CERTCertDBHandle *handle, CERTCertificate *cert, void *pinarg, + /* it is possible for CERT_VerifyCertificate return with an error with no logging */ + if ( ret != SECSuccess ) { + PRErrorCode errcode = PR_GetError(); +- Debug( LDAP_DEBUG_ANY, ++ Debug( debug_level, + "TLS: certificate [%s] is not valid - error %d:%s.\n", + name ? name : "(unknown)", + errcode, PR_ErrorToString( errcode, PR_LANGUAGE_I_DEFAULT ) ); +@@ -995,17 +1001,17 @@ tlsm_verify_cert(CERTCertDBHandle *handle, CERTCertificate *cert, void *pinarg, + "please fix your certs if possible\n", name, 0, 0 ); + } else { /* does not have basicconstraint, or some other error */ + ret = SECFailure; +- Debug( LDAP_DEBUG_ANY, ++ Debug( debug_level, + "TLS: certificate [%s] is not valid - CA cert is not valid\n", + name, 0, 0 ); + } + } else if ( errorToIgnore && ( node->error == errorToIgnore ) ) { +- Debug( LDAP_DEBUG_ANY, ++ Debug( debug_level, + "TLS: Warning: ignoring error for certificate [%s] - error %ld:%s.\n", + name, node->error, PR_ErrorToString( node->error, PR_LANGUAGE_I_DEFAULT ) ); + } else { + ret = SECFailure; +- Debug( LDAP_DEBUG_ANY, ++ Debug( debug_level, + "TLS: certificate [%s] is not valid - error %ld:%s.\n", + name, node->error, PR_ErrorToString( node->error, PR_LANGUAGE_I_DEFAULT ) ); + } +@@ -1020,7 +1026,9 @@ tlsm_verify_cert(CERTCertDBHandle *handle, CERTCertificate *cert, void *pinarg, + if ( ret == SECSuccess ) { + Debug( LDAP_DEBUG_TRACE, + "TLS: certificate [%s] is valid\n", name, 0, 0 ); +- } ++ } else if ( errorToIgnore == -1 ) { ++ ret = SECSuccess; ++ } + + return ret; + } +@@ -1032,10 +1040,15 @@ tlsm_auth_cert_handler(void *arg, PRFileDesc *fd, + SECCertificateUsage certUsage = isServer ? certificateUsageSSLClient : certificateUsageSSLServer; + SECStatus ret = SECSuccess; + CERTCertificate *peercert = SSL_PeerCertificate( fd ); ++ int errorToIgnore = 0; ++ tlsm_ctx *ctx = (tlsm_ctx *)arg; ++ ++ if (ctx && ctx->tc_warn_only ) ++ errorToIgnore = -1; + +- ret = tlsm_verify_cert( (CERTCertDBHandle *)arg, peercert, ++ ret = tlsm_verify_cert( ctx->tc_certdb, peercert, + SSL_RevealPinArg( fd ), +- checksig, certUsage, 0 ); ++ checksig, certUsage, errorToIgnore ); + CERT_DestroyCertificate( peercert ); + + return ret; +@@ -1758,6 +1771,8 @@ tlsm_find_and_verify_cert_key(tlsm_ctx *ctx, PRFileDesc *fd, const char *certnam + SECCertificateUsage certUsage; + PRBool checkSig = PR_TRUE; + SECStatus status; ++ /* may not have a CA cert - ok - ignore SEC_ERROR_UNKNOWN_ISSUER */ ++ int errorToIgnore = SEC_ERROR_UNKNOWN_ISSUER; + + if ( pRetKey ) { + *pRetKey = key; /* caller will deal with this */ +@@ -1774,9 +1789,11 @@ tlsm_find_and_verify_cert_key(tlsm_ctx *ctx, PRFileDesc *fd, const char *certnam + } else { + checkSig = PR_FALSE; + } +- /* may not have a CA cert - ok - ignore SEC_ERROR_UNKNOWN_ISSUER */ ++ if ( ctx->tc_warn_only ) { ++ errorToIgnore = -1; ++ } + status = tlsm_verify_cert( ctx->tc_certdb, cert, pin_arg, +- checkSig, certUsage, SEC_ERROR_UNKNOWN_ISSUER ); ++ checkSig, certUsage, errorToIgnore ); + if ( status == SECSuccess ) { + rc = 0; + } +@@ -1803,10 +1820,14 @@ tlsm_get_client_auth_data( void *arg, PRFileDesc *fd, + { + tlsm_ctx *ctx = (tlsm_ctx *)arg; + int rc; ++ PRBool saveval; + + /* don't need caNames - this function will call CERT_VerifyCertificateNow + which will verify the cert against the known CAs */ ++ saveval = ctx->tc_warn_only; ++ ctx->tc_warn_only = PR_TRUE; + rc = tlsm_find_and_verify_cert_key( ctx, fd, ctx->tc_certname, 0, pRetCert, pRetKey ); ++ ctx->tc_warn_only = saveval; + if ( rc ) { + Debug( LDAP_DEBUG_ANY, + "TLS: error: unable to perform client certificate authentication for " +@@ -1837,8 +1858,12 @@ tlsm_clientauth_init( tlsm_ctx *ctx ) + { + SECStatus status = SECFailure; + int rc; ++ PRBool saveval; + ++ saveval = ctx->tc_warn_only; ++ ctx->tc_warn_only = PR_TRUE; + rc = tlsm_find_and_verify_cert_key( ctx, ctx->tc_model, ctx->tc_certname, 0, NULL, NULL ); ++ ctx->tc_warn_only = saveval; + if ( rc ) { + Debug( LDAP_DEBUG_ANY, + "TLS: error: unable to set up client certificate authentication for " +@@ -1887,6 +1912,7 @@ tlsm_ctx_new ( struct ldapoptions *lo ) + #endif /* HAVE_NSS_INITCONTEXT */ + ctx->tc_pem_objs = NULL; + ctx->tc_n_pem_objs = 0; ++ ctx->tc_warn_only = PR_FALSE; + } + return (tls_ctx *)ctx; + } +@@ -2048,7 +2074,9 @@ tlsm_deferred_ctx_init( void *arg ) + return -1; + } + +- if ( ctx->tc_require_cert ) { ++ if ( !ctx->tc_require_cert ) { ++ ctx->tc_verify_cert = PR_FALSE; ++ } else if ( !ctx->tc_is_server ) { + request_cert = PR_TRUE; + require_cert = SSL_REQUIRE_NO_ERROR; + if ( ctx->tc_require_cert == LDAP_OPT_X_TLS_DEMAND || +@@ -2057,8 +2085,22 @@ tlsm_deferred_ctx_init( void *arg ) + } + if ( ctx->tc_require_cert != LDAP_OPT_X_TLS_ALLOW ) + ctx->tc_verify_cert = PR_TRUE; +- } else { +- ctx->tc_verify_cert = PR_FALSE; ++ } else { /* server */ ++ /* server does not request certs by default */ ++ /* if allow - client may send cert, server will ignore if errors */ ++ /* if try - client may send cert, server will error if bad cert */ ++ /* if hard or demand - client must send cert, server will error if bad cert */ ++ request_cert = PR_TRUE; ++ require_cert = SSL_REQUIRE_NO_ERROR; ++ if ( ctx->tc_require_cert == LDAP_OPT_X_TLS_DEMAND || ++ ctx->tc_require_cert == LDAP_OPT_X_TLS_HARD ) { ++ require_cert = SSL_REQUIRE_ALWAYS; ++ } ++ if ( ctx->tc_require_cert != LDAP_OPT_X_TLS_ALLOW ) { ++ ctx->tc_verify_cert = PR_TRUE; ++ } else { ++ ctx->tc_warn_only = PR_TRUE; ++ } + } + + if ( SECSuccess != SSL_OptionSet( ctx->tc_model, SSL_REQUEST_CERTIFICATE, request_cert ) ) { +@@ -2193,7 +2235,7 @@ tlsm_deferred_ctx_init( void *arg ) + + /* Callback for authenticating certificate */ + if ( SSL_AuthCertificateHook( ctx->tc_model, tlsm_auth_cert_handler, +- ctx->tc_certdb ) != SECSuccess ) { ++ ctx ) != SECSuccess ) { + PRErrorCode err = PR_GetError(); + Debug( LDAP_DEBUG_ANY, + "TLS: error: could not set auth cert handler for moznss - error %d:%s\n", diff --git a/openldap.spec b/openldap.spec index 2228687..22226b4 100644 --- a/openldap.spec +++ b/openldap.spec @@ -31,6 +31,8 @@ Patch5: openldap-ldaprc-currentdir.patch Patch6: openldap-userconfig-setgid.patch Patch7: openldap-nss-free-peer-cert.patch Patch8: openldap-nss-init-threadsafe.patch +Patch9: openldap-nss-reqcert-hostname.patch +Patch10: openldap-nss-verifycert.patch # patches for the evolution library (see README.evolution) Patch200: openldap-evolution-ntlm.patch @@ -132,6 +134,8 @@ pushd openldap-%{version} %patch6 -p1 -b .userconfig-setgid %patch7 -p1 -b .nss-free-peer-cert %patch8 -p1 -b .nss-init-threadsafe +%patch9 -p1 -b .nss-reqcert-hostname +%patch10 -p1 -b .nss-verifycert cp %{_datadir}/libtool/config/config.{sub,guess} build/ @@ -658,6 +662,7 @@ exit 0 * Wed Aug 24 2011 Jan Vcelak 2.4.26-2 - security hardening: library needs partial RELRO support added (#733071) - fix: NSS_Init* functions are not thread safe (#731112) +- fix: incorrect behavior of allow/try options of VerifyCert and TLS_REQCERT (#725819) * Sun Aug 14 2011 Rex Dieter - 2.4.26-1.1 - Rebuilt for rpm (#728707)