httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rainer Jung <rainer.j...@kippdata.de>
Subject mod_ssl: Reading dhparams and ecparams not only from the first certificate file
Date Tue, 26 May 2015 08:33:56 GMT
Current mod_ssl code tries to read embedded DH and ECC parameters only 
from the first certificate file. Although this is documented

"DH and ECDH parameters, however, are only read from the first 
SSLCertificateFile directive, as they are applied independently of the 
authentication algorithm type."

I find it questionable. I would find it more natural to embed the params 
in the cert files they apply to, so e.g. the DH params in the RSA cert 
file and the EC params in the ECDH cert file and also to not require a 
special order for the files which at the end we do not check. Since 
missing the embedded params goes unnoticed (finding them is only a DEBUG 
log line) it is not very user friendly.

Can't we simply try to read DH and ECC params from every certificate 
file and stop in each of the two cases when we have found some? That 
would tighten the user unfriendlyness to the case of having multiple 
inconsistent parameters embedded in different cert files. And even that 
could be checked and logged as a warning.

I would suggest to move the param extraction a little above in the 
certificate loop and only try to extract and use the params if a 
previous extraction for that type did not already succeed. See below for 
a patch suggestion. It moved the read code into the cert loop, removes 
the NULL check for certfile (it is done in the loop condition), adds a 
boolean dhparams_found and ecparams_found to track whether we already 
found some and adds warnings if we find them more than once or if found 
ecparams are not valid. The final "auto" behavior setting stays after 
the loop and is executed if ecparams is still NULL.

Related is handling of keys embedded in cert files. AFAIK we only try to 
read the key from the certificate file instead of a configured key file, 
if there is no key file for the same index configured. That means if you 
start mixing embedded keys and separate key files, all certificates with 
separate key files must come first, then the certificates with the 
embedded keys (and the order must match).For this I don't have a better 
solution than just documenting it.

Regards,

Rainer

Index: modules/ssl/ssl_engine_init.c
===================================================================
--- modules/ssl/ssl_engine_init.c       (revision 1681275)
+++ modules/ssl/ssl_engine_init.c       (working copy)
@@ -1039,8 +1039,10 @@
      int i;
      X509 *cert;
      DH *dhparams;
+    int dhparams_found = 0;
  #ifdef HAVE_ECC
      EC_GROUP *ecparams;
+    int ecparams_found = 0;
      int nid;
      EC_KEY *eckey = NULL;
  #endif
@@ -1174,40 +1176,67 @@
          SSL_free(ssl);
  #endif

+        /*
+         * Try to read DH parameters from the SSLCertificateFile
+         */
+        dhparams = ssl_dh_GetParamFromFile(certfile);
+        if (dhparams) {
+            if (dhparams_found) {
+                ap_log_error(APLOG_MARK, APLOG_WARNING, 0, s, 
APLOGNO(02901)
+                             "Custom DH parameters already loaded for %s,"
+                             " ignored from %s",
+                             vhost_id, certfile);
+            }
+            else {
+                SSL_CTX_set_tmp_dh(mctx->ssl_ctx, dhparams);
+                dhparams_found = 1;
+                ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02540)
+                             "Custom DH parameters (%d bits) for %s 
loaded from %s",
+                             BN_num_bits(dhparams->p), vhost_id, certfile);
+            }
+        }
+
+#ifdef HAVE_ECC
+        /*
+         * Try to read the ECDH curve name from SSLCertificateFile
+         */
+        ecparams = ssl_ec_GetParamFromFile(certfile);
+        if (ecparams) {
+            if (ecparams_found) {
+                ap_log_error(APLOG_MARK, APLOG_WARNING, 0, s, 
APLOGNO(02902)
+                             "Custom ECDH parameters already loaded for 
%s,"
+                             " ignored from %s",
+                             vhost_id, certfile);
+            }
+            else {
+                if ((nid = EC_GROUP_get_curve_name(ecparams)) &&
+                    (eckey = EC_KEY_new_by_curve_name(nid))) {
+                    SSL_CTX_set_tmp_ecdh(mctx->ssl_ctx, eckey);
+                    ecparams_found = 1;
+                    ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, 
APLOGNO(02541)
+                                 "ECDH curve %s for %s specified in %s",
+                                 OBJ_nid2sn(nid), vhost_id, certfile);
+                }
+                else {
+                    ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, 
APLOGNO(02903)
+                                 "Invalid ECDH parameters for %s 
ignored from %s",
+                                 vhost_id, certfile);
+                }
+            }
+        }
+#endif
+
          ap_log_error(APLOG_MARK, APLOG_INFO, 0, s, APLOGNO(02568)
                       "Certificate and private key %s configured from 
%s and %s",
                       key_id, certfile, keyfile);
      }

-    /*
-     * Try to read DH parameters from the (first) SSLCertificateFile
-     */
-    if ((certfile = APR_ARRAY_IDX(mctx->pks->cert_files, 0, const char 
*)) &&
-        (dhparams = ssl_dh_GetParamFromFile(certfile))) {
-        SSL_CTX_set_tmp_dh(mctx->ssl_ctx, dhparams);
-        ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02540)
-                     "Custom DH parameters (%d bits) for %s loaded from 
%s",
-                     BN_num_bits(dhparams->p), vhost_id, certfile);
-    }
-
  #ifdef HAVE_ECC
      /*
-     * Similarly, try to read the ECDH curve name from 
SSLCertificateFile...
-     */
-    if ((certfile != NULL) &&
-        (ecparams = ssl_ec_GetParamFromFile(certfile)) &&
-        (nid = EC_GROUP_get_curve_name(ecparams)) &&
-        (eckey = EC_KEY_new_by_curve_name(nid))) {
-        SSL_CTX_set_tmp_ecdh(mctx->ssl_ctx, eckey);
-        ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02541)
-                     "ECDH curve %s for %s specified in %s",
-                     OBJ_nid2sn(nid), vhost_id, certfile);
-    }
-    /*
-     * ...otherwise, enable auto curve selection (OpenSSL 1.0.2 and later)
+     * If we didn't find any ECDH params, enable auto curve selection 
(OpenSSL 1.0.2 and later)
       * or configure NIST P-256 (required to enable ECDHE for earlier 
versions)
       */
-    else {
+    if (!ecparams_found) {
  #if defined(SSL_CTX_set_ecdh_auto)
          SSL_CTX_set_ecdh_auto(mctx->ssl_ctx, 1);
  #else
Index: docs/log-message-tags/next-number
===================================================================
--- docs/log-message-tags/next-number   (revision 1681275)
+++ docs/log-message-tags/next-number   (working copy)
@@ -1 +1 @@
-2901
+2904

Mime
View raw message