You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by kb...@apache.org on 2014/01/07 18:52:35 UTC

svn commit: r1556290 - in /httpd/httpd/branches/2.4.x: ./ STATUS modules/ssl/mod_ssl.c modules/ssl/ssl_engine_config.c modules/ssl/ssl_engine_init.c modules/ssl/ssl_engine_pphrase.c modules/ssl/ssl_private.h modules/ssl/ssl_util.c

Author: kbrand
Date: Tue Jan  7 17:52:34 2014
New Revision: 1556290

URL: http://svn.apache.org/r1556290
Log:
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

Modified:
    httpd/httpd/branches/2.4.x/   (props changed)
    httpd/httpd/branches/2.4.x/STATUS
    httpd/httpd/branches/2.4.x/modules/ssl/mod_ssl.c
    httpd/httpd/branches/2.4.x/modules/ssl/ssl_engine_config.c
    httpd/httpd/branches/2.4.x/modules/ssl/ssl_engine_init.c
    httpd/httpd/branches/2.4.x/modules/ssl/ssl_engine_pphrase.c
    httpd/httpd/branches/2.4.x/modules/ssl/ssl_private.h
    httpd/httpd/branches/2.4.x/modules/ssl/ssl_util.c

Propchange: httpd/httpd/branches/2.4.x/
------------------------------------------------------------------------------
  Merged /httpd/httpd/trunk:r1544784

Modified: httpd/httpd/branches/2.4.x/STATUS
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1556290&r1=1556289&r2=1556290&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/STATUS (original)
+++ httpd/httpd/branches/2.4.x/STATUS Tue Jan  7 17:52:34 2014
@@ -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

Modified: httpd/httpd/branches/2.4.x/modules/ssl/mod_ssl.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/ssl/mod_ssl.c?rev=1556290&r1=1556289&r2=1556290&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/modules/ssl/mod_ssl.c (original)
+++ httpd/httpd/branches/2.4.x/modules/ssl/mod_ssl.c Tue Jan  7 17:52:34 2014
@@ -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) "

Modified: httpd/httpd/branches/2.4.x/modules/ssl/ssl_engine_config.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/ssl/ssl_engine_config.c?rev=1556290&r1=1556289&r2=1556290&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/modules/ssl/ssl_engine_config.c (original)
+++ httpd/httpd/branches/2.4.x/modules/ssl/ssl_engine_config.c Tue Jan  7 17:52:34 2014
@@ -116,7 +116,6 @@ static void modssl_ctx_init(modssl_ctx_t
     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_SSLCertificateChainF
     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,

Modified: httpd/httpd/branches/2.4.x/modules/ssl/ssl_engine_init.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/ssl/ssl_engine_init.c?rev=1556290&r1=1556289&r2=1556290&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/modules/ssl/ssl_engine_init.c (original)
+++ httpd/httpd/branches/2.4.x/modules/ssl/ssl_engine_init.c Tue Jan  7 17:52:34 2014
@@ -292,7 +292,7 @@ static void ssl_init_server_check(server
      * 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 
     }
 }
 
-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(serv
     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

Modified: httpd/httpd/branches/2.4.x/modules/ssl/ssl_engine_pphrase.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/ssl/ssl_engine_pphrase.c?rev=1556290&r1=1556289&r2=1556290&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/modules/ssl/ssl_engine_pphrase.c (original)
+++ httpd/httpd/branches/2.4.x/modules/ssl/ssl_engine_pphrase.c Tue Jan  7 17:52:34 2014
@@ -190,8 +190,7 @@ void ssl_pphrase_Handle(server_rec *s, a
          * 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, a
         /* 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.

Modified: httpd/httpd/branches/2.4.x/modules/ssl/ssl_private.h
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/ssl/ssl_private.h?rev=1556290&r1=1556289&r2=1556290&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/modules/ssl/ssl_private.h (original)
+++ httpd/httpd/branches/2.4.x/modules/ssl/ssl_private.h Tue Jan  7 17:52:34 2014
@@ -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_
 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_po
                                   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 *);

Modified: httpd/httpd/branches/2.4.x/modules/ssl/ssl_util.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/ssl/ssl_util.c?rev=1556290&r1=1556289&r2=1556290&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/modules/ssl/ssl_util.c (original)
+++ httpd/httpd/branches/2.4.x/modules/ssl/ssl_util.c Tue Jan  7 17:52:34 2014
@@ -277,57 +277,6 @@ const char *ssl_asn1_table_keyfmt(apr_po
     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