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
