From: Kaspar Brand Date: Tue, 7 Jan 2014 17:52:34 +0000 (+0000) Subject: Backport r1544784 from trunk: X-Git-Tag: 2.4.8~256 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=1278feaa72d7b5580aff1627dd88f25d2d3a3a96;p=apache Backport r1544784 from trunk: Remove SSLPKCS7CertificateFile support: - was never documented, so very unlikely that it was ever used - adds complexity without apparent benefit; PKCS#7 files can be trivially converted to a file for use with SSLCertificateChainFile (concatenated X509 CERTIFICATE chunks, openssl pkcs7 -print_certs...) - only supports PKCS7 files with PEM encoding, i.e. relies on a non-standardized PEM header (cf. RFC 2315 and draft-josefsson-pkix-textual) - issues pointed out in http://mail-archives.apache.org/mod_mbox/httpd-dev/200607.mbox/%3C20060723093125.GA19423@redhat.com%3E were never fully addressed (cf. r424707 and r424735) - has never worked in vhost context due to a cfgMergeString call missing from modssl_ctx_cfg_merge Proposed by: kbrand Reviewed by: covener, druggeri git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1556290 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/STATUS b/STATUS index 09302c57ed..3a11f52559 100644 --- a/STATUS +++ b/STATUS @@ -153,12 +153,6 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK: 2.4.x patch: trunk patch works +1: kbrand - * mod_ssl: drop the (undocumented) SSLPKCS7CertificateFile directive, - for the reasons listed in the trunk commit message - trunk patch: https://svn.apache.org/r1544784 - 2.4.x patch: https://people.apache.org/~kbrand/mod_ssl-2.4.x-drop-pkcs7.diff - +1: kbrand, covener, druggeri - * prefork: PR: 54852. Only use a dummy_connection for idle processes trunk patch: http://svn.apache.org/viewvc?view=revision&revision=1542379 2.4.x patch: trunk patch works mod CHANGES diff --git a/modules/ssl/mod_ssl.c b/modules/ssl/mod_ssl.c index 2afca28194..6e632a3803 100644 --- a/modules/ssl/mod_ssl.c +++ b/modules/ssl/mod_ssl.c @@ -91,9 +91,6 @@ static const command_rec ssl_config_cmds[] = { SSL_CMD_SRV(CertificateChainFile, TAKE1, "SSL Server CA Certificate Chain file " "('/path/to/file' - PEM encoded)") - SSL_CMD_SRV(PKCS7CertificateFile, TAKE1, - "PKCS#7 file containing server certificate and chain" - " certificates ('/path/to/file' - PEM encoded)") #ifdef HAVE_TLS_SESSION_TICKETS SSL_CMD_SRV(SessionTicketKeyFile, TAKE1, "TLS session ticket encryption/decryption key file (RFC 5077) " diff --git a/modules/ssl/ssl_engine_config.c b/modules/ssl/ssl_engine_config.c index c323ece3f3..d843ce546c 100644 --- a/modules/ssl/ssl_engine_config.c +++ b/modules/ssl/ssl_engine_config.c @@ -116,7 +116,6 @@ static void modssl_ctx_init(modssl_ctx_t *mctx, apr_pool_t *p) mctx->pphrase_dialog_type = SSL_PPTYPE_UNSET; mctx->pphrase_dialog_path = NULL; - mctx->pkcs7 = NULL; mctx->cert_chain = NULL; mctx->crl_path = NULL; @@ -841,22 +840,6 @@ const char *ssl_cmd_SSLCertificateChainFile(cmd_parms *cmd, return NULL; } -const char *ssl_cmd_SSLPKCS7CertificateFile(cmd_parms *cmd, - void *dcfg, - const char *arg) -{ - SSLSrvConfigRec *sc = mySrvConfig(cmd->server); - const char *err; - - if ((err = ssl_cmd_check_file(cmd, &arg))) { - return err; - } - - sc->server->pkcs7 = arg; - - return NULL; -} - #ifdef HAVE_TLS_SESSION_TICKETS const char *ssl_cmd_SSLSessionTicketKeyFile(cmd_parms *cmd, void *dcfg, diff --git a/modules/ssl/ssl_engine_init.c b/modules/ssl/ssl_engine_init.c index 8d3cb4e4d2..a07b5737e7 100644 --- a/modules/ssl/ssl_engine_init.c +++ b/modules/ssl/ssl_engine_init.c @@ -292,7 +292,7 @@ static void ssl_init_server_check(server_rec *s, * check for important parameters and the * possibility that the user forgot to set them. */ - if (!mctx->pks->cert_files[0] && !mctx->pkcs7) { + if (!mctx->pks->cert_files[0]) { ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s, APLOGNO(01891) "No SSL Certificate set [hint: SSLCertificateFile]"); ssl_die(s); @@ -720,23 +720,6 @@ static void ssl_init_ctx_crl(server_rec *s, } } -static void ssl_init_ctx_pkcs7_cert_chain(server_rec *s, modssl_ctx_t *mctx) -{ - STACK_OF(X509) *certs = ssl_read_pkcs7(s, mctx->pkcs7); - int n; - STACK_OF(X509) *extra_certs = NULL; - -#ifdef OPENSSL_NO_SSL_INTERN - SSL_CTX_get_extra_chain_certs(mctx->ssl_ctx, &extra_certs); -#else - extra_certs = mctx->ssl_ctx->extra_certs; -#endif - - if (!extra_certs) - for (n = 1; n < sk_X509_num(certs); ++n) - SSL_CTX_add_extra_chain_cert(mctx->ssl_ctx, sk_X509_value(certs, n)); -} - static void ssl_init_ctx_cert_chain(server_rec *s, apr_pool_t *p, apr_pool_t *ptemp, @@ -746,11 +729,6 @@ static void ssl_init_ctx_cert_chain(server_rec *s, int i, n; const char *chain = mctx->cert_chain; - if (mctx->pkcs7) { - ssl_init_ctx_pkcs7_cert_chain(s, mctx); - return; - } - /* * Optionally configure extra server certificate chain certificates. * This is usually done by OpenSSL automatically when one of the diff --git a/modules/ssl/ssl_engine_pphrase.c b/modules/ssl/ssl_engine_pphrase.c index ca8e130fb9..d3d2a3a618 100644 --- a/modules/ssl/ssl_engine_pphrase.c +++ b/modules/ssl/ssl_engine_pphrase.c @@ -190,8 +190,7 @@ void ssl_pphrase_Handle(server_rec *s, apr_pool_t *p) * Read in server certificate(s): This is the easy part * because this file isn't encrypted in any way. */ - if (sc->server->pks->cert_files[0] == NULL - && sc->server->pkcs7 == NULL) { + if (sc->server->pks->cert_files[0] == NULL) { ap_log_error(APLOG_MARK, APLOG_EMERG, 0, pServ, APLOGNO(02240) "Server should be SSL-aware but has no certificate " "configured [Hint: SSLCertificateFile] (%s:%d)", @@ -207,37 +206,29 @@ void ssl_pphrase_Handle(server_rec *s, apr_pool_t *p) /* Iterate through configured certificate files for this * server. */ for (i = 0, j = 0; i < SSL_AIDX_MAX - && (sc->server->pks->cert_files[i] != NULL - || sc->server->pkcs7); i++) { + && (sc->server->pks->cert_files[i] != NULL); i++) { const char *key_id; int using_cache = 0; - if (sc->server->pkcs7) { - STACK_OF(X509) *certs = ssl_read_pkcs7(pServ, - sc->server->pkcs7); - pX509Cert = sk_X509_value(certs, 0); - i = SSL_AIDX_MAX; - } else { - apr_cpystrn(szPath, sc->server->pks->cert_files[i], - sizeof(szPath)); - if ((rv = exists_and_readable(szPath, p, NULL)) - != APR_SUCCESS) { - ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(02201) - "Init: Can't open server certificate file %s", - szPath); - ssl_die(s); - } - if ((pX509Cert = SSL_read_X509(szPath, NULL, NULL)) == NULL) { - ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s, APLOGNO(02241) - "Init: Unable to read server certificate from" - " file %s", szPath); - ssl_log_ssl_error(SSLLOG_MARK, APLOG_EMERG, s); - ssl_die(s); - } - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02202) - "Init: Read server certificate from '%s'", + apr_cpystrn(szPath, sc->server->pks->cert_files[i], + sizeof(szPath)); + if ((rv = exists_and_readable(szPath, p, NULL)) + != APR_SUCCESS) { + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(02201) + "Init: Can't open server certificate file %s", szPath); + ssl_die(s); + } + if ((pX509Cert = SSL_read_X509(szPath, NULL, NULL)) == NULL) { + ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s, APLOGNO(02241) + "Init: Unable to read server certificate from" + " file %s", szPath); + ssl_log_ssl_error(SSLLOG_MARK, APLOG_EMERG, s); + ssl_die(s); } + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02202) + "Init: Read server certificate from '%s'", + szPath); /* * check algorithm type of certificate and make * sure only one certificate per type is used. diff --git a/modules/ssl/ssl_private.h b/modules/ssl/ssl_private.h index af873ec634..f95f2fc6de 100644 --- a/modules/ssl/ssl_private.h +++ b/modules/ssl/ssl_private.h @@ -609,7 +609,6 @@ typedef struct { const char *pphrase_dialog_path; const char *cert_chain; - const char *pkcs7; /** certificate revocation list */ const char *crl_path; @@ -718,7 +717,6 @@ const char *ssl_cmd_SSLCipherSuite(cmd_parms *, void *, const char *); const char *ssl_cmd_SSLCertificateFile(cmd_parms *, void *, const char *); const char *ssl_cmd_SSLCertificateKeyFile(cmd_parms *, void *, const char *); const char *ssl_cmd_SSLCertificateChainFile(cmd_parms *, void *, const char *); -const char *ssl_cmd_SSLPKCS7CertificateFile(cmd_parms *, void *, const char *); const char *ssl_cmd_SSLCACertificatePath(cmd_parms *, void *, const char *); const char *ssl_cmd_SSLCACertificateFile(cmd_parms *, void *, const char *); const char *ssl_cmd_SSLCADNRequestPath(cmd_parms *, void *, const char *); @@ -903,8 +901,6 @@ const char *ssl_asn1_table_keyfmt(apr_pool_t *p, const char *id, int keytype); -STACK_OF(X509) *ssl_read_pkcs7(server_rec *s, const char *pkcs7); - /** Mutex Support */ int ssl_mutex_init(server_rec *, apr_pool_t *); int ssl_mutex_reinit(server_rec *, apr_pool_t *); diff --git a/modules/ssl/ssl_util.c b/modules/ssl/ssl_util.c index d21227844d..3a03e032c1 100644 --- a/modules/ssl/ssl_util.c +++ b/modules/ssl/ssl_util.c @@ -277,57 +277,6 @@ const char *ssl_asn1_table_keyfmt(apr_pool_t *p, return apr_pstrcat(p, id, ":", keystr, NULL); } -STACK_OF(X509) *ssl_read_pkcs7(server_rec *s, const char *pkcs7) -{ - PKCS7 *p7; - STACK_OF(X509) *certs = NULL; - FILE *f; - - f = fopen(pkcs7, "r"); - if (!f) { - ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s, APLOGNO(02212) "Can't open %s", pkcs7); - ssl_die(s); - } - - p7 = PEM_read_PKCS7(f, NULL, NULL, NULL); - if (!p7) { - ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(02274) - "Can't read PKCS7 object %s", pkcs7); - ssl_log_ssl_error(SSLLOG_MARK, APLOG_CRIT, s); - exit(1); - } - - switch (OBJ_obj2nid(p7->type)) { - case NID_pkcs7_signed: - certs = p7->d.sign->cert; - p7->d.sign->cert = NULL; - PKCS7_free(p7); - break; - - case NID_pkcs7_signedAndEnveloped: - certs = p7->d.signed_and_enveloped->cert; - p7->d.signed_and_enveloped->cert = NULL; - PKCS7_free(p7); - break; - - default: - ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s, APLOGNO(02213) - "Don't understand PKCS7 file %s", pkcs7); - ssl_die(s); - } - - if (!certs) { - ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s, APLOGNO(02214) - "No certificates in %s", pkcs7); - ssl_die(s); - } - - fclose(f); - - return certs; -} - - #if APR_HAS_THREADS /* * To ensure thread-safetyness in OpenSSL - work in progress