From: Joe Orton <jorton@apache.org>
Date: Fri, 4 May 2018 17:56:32 +0000 (+0000)
Subject: Simplify the ssl_asn1_table API, remove abstraction (it is used only
X-Git-Tag: 2.5.0-alpha2-ci-test-only~2630
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9e4ad2f685bfde0cdb47c7a9cd95edec40c2e35b;p=thirdparty%2Fapache%2Fhttpd.git

Simplify the ssl_asn1_table API, remove abstraction (it is used only
to cache serialized EVP_PKEYs not any char * blobs), and document.

* modules/ssl/ssl_util.c (ssl_asn1_table_set): Take the EVP_PKEY and
  serialize internally.  Use ap_realloc.  Return the ssl_asn1_t *
  pointer.  Don't call apr_hash_set() for unchanged pointer case.

* modules/ssl/ssl_engine_pphrase.c (ssl_load_encrypted_pkey):
  Adjust for the above.

* modules/ssl/ssl_private.h: Adjust as above, add docs.


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1830927 13f79535-47bb-0310-9956-ffa450edef68
---

diff --git a/modules/ssl/ssl_engine_pphrase.c b/modules/ssl/ssl_engine_pphrase.c
index 193f3f9e89c..4b74cc589e6 100644
--- a/modules/ssl/ssl_engine_pphrase.c
+++ b/modules/ssl/ssl_engine_pphrase.c
@@ -137,8 +137,6 @@ apr_status_t ssl_load_encrypted_pkey(server_rec *s, apr_pool_t *p, int idx,
     const char *key_id = asn1_table_vhost_key(mc, p, sc->vhost_id, idx);
     EVP_PKEY *pPrivateKey = NULL;
     ssl_asn1_t *asn1;
-    unsigned char *ucp;
-    long int length;
     int nPassPhrase = (*pphrases)->nelts;
     int nPassPhraseRetry = 0;
     apr_time_t pkey_mtime = 0;
@@ -345,19 +343,12 @@ apr_status_t ssl_load_encrypted_pkey(server_rec *s, apr_pool_t *p, int idx,
         nPassPhrase++;
     }
 
-    /*
-     * Insert private key into the global module configuration
-     * (we convert it to a stand-alone DER byte sequence
-     * because the SSL library uses static variables inside a
-     * RSA structure which do not survive DSO reloads!)
-     */
-    length = i2d_PrivateKey(pPrivateKey, NULL);
-    ucp = ssl_asn1_table_set(mc->tPrivateKey, key_id, length);
-    (void)i2d_PrivateKey(pPrivateKey, &ucp); /* 2nd arg increments */
+    /* Cache the private key in the global module configuration so it
+     * can be used after subsequent reloads. */
+    asn1 = ssl_asn1_table_set(mc->tPrivateKey, key_id, pPrivateKey);
 
     if (ppcb_arg.nPassPhraseDialogCur != 0) {
         /* remember mtime of encrypted keys */
-        asn1 = ssl_asn1_table_get(mc->tPrivateKey, key_id);
         asn1->source_mtime = pkey_mtime;
     }
 
diff --git a/modules/ssl/ssl_private.h b/modules/ssl/ssl_private.h
index c43556fb02a..8836b100930 100644
--- a/modules/ssl/ssl_private.h
+++ b/modules/ssl/ssl_private.h
@@ -1007,15 +1007,14 @@ DH           *ssl_dh_GetParamFromFile(const char *);
 EC_GROUP     *ssl_ec_GetParamFromFile(const char *);
 #endif
 
-unsigned char *ssl_asn1_table_set(apr_hash_t *table,
-                                  const char *key,
-                                  long int length);
-
-ssl_asn1_t *ssl_asn1_table_get(apr_hash_t *table,
-                               const char *key);
-
-void ssl_asn1_table_unset(apr_hash_t *table,
-                          const char *key);
+/* Store the EVP_PKEY key (serialized into DER) in the hash table with
+ * key, returning the ssl_asn1_t structure pointer. */
+ssl_asn1_t *ssl_asn1_table_set(apr_hash_t *table, const char *key,
+                               EVP_PKEY *pkey);
+/* Retrieve the ssl_asn1_t structure with given key from the hash. */
+ssl_asn1_t *ssl_asn1_table_get(apr_hash_t *table, const char *key);
+/* Remove and free the ssl_asn1_t structure with given key. */
+void ssl_asn1_table_unset(apr_hash_t *table, const char *key);
 
 /**  Mutex Support  */
 int          ssl_mutex_init(server_rec *, apr_pool_t *);
diff --git a/modules/ssl/ssl_util.c b/modules/ssl/ssl_util.c
index ebc36051291..c8cab33c43d 100644
--- a/modules/ssl/ssl_util.c
+++ b/modules/ssl/ssl_util.c
@@ -192,45 +192,37 @@ BOOL ssl_util_path_check(ssl_pathcheck_t pcm, const char *path, apr_pool_t *p)
     return TRUE;
 }
 
-/*
- * certain key data needs to survive restarts,
- * which are stored in the user data table of s->process->pool.
- * to prevent "leaking" of this data, we use malloc/free
- * rather than apr_palloc and these wrappers to help make sure
- * we do not leak the malloc-ed data.
- */
-unsigned char *ssl_asn1_table_set(apr_hash_t *table,
-                                  const char *key,
-                                  long int length)
+/* Decrypted private keys are cached to survive restarts.  The cached
+ * data must have lifetime of the process (hence malloc/free rather
+ * than pools), and uses raw DER since the EVP_PKEY structure
+ * internals may not survive across a module reload. */
+ssl_asn1_t *ssl_asn1_table_set(apr_hash_t *table, const char *key,
+                               EVP_PKEY *pkey)
 {
     apr_ssize_t klen = strlen(key);
     ssl_asn1_t *asn1 = apr_hash_get(table, key, klen);
+    apr_size_t length = i2d_PrivateKey(pkey, NULL);
+    unsigned char *p;
 
-    /*
-     * if a value for this key already exists,
-     * reuse as much of the already malloc-ed data
-     * as possible.
-     */
+    /* Re-use structure if cached previously. */
     if (asn1) {
         if (asn1->nData != length) {
-            free(asn1->cpData); /* XXX: realloc? */
-            asn1->cpData = NULL;
+            asn1->cpData = ap_realloc(asn1->cpData, length);
         }
     }
     else {
         asn1 = ap_malloc(sizeof(*asn1));
         asn1->source_mtime = 0; /* used as a note for encrypted private keys */
-        asn1->cpData = NULL;
-    }
-
-    asn1->nData = length;
-    if (!asn1->cpData) {
         asn1->cpData = ap_malloc(length);
+
+        apr_hash_set(table, key, klen, asn1);
     }
 
-    apr_hash_set(table, key, klen, asn1);
+    asn1->nData = length;
+    p = asn1->cpData;
+    i2d_PrivateKey(pkey, &p); /* increases p by length */
 
-    return asn1->cpData; /* caller will assign a value to this */
+    return asn1;
 }
 
 ssl_asn1_t *ssl_asn1_table_get(apr_hash_t *table,