[patch] serf CVE-2014-3504 patch

serf-cve-2014-3504.patch (text/plain), 10.03 KB, created by Chris Reffett (RETIRED) on 2014-08-06 12:33:18 UTC

Index: buckets/ssl_buckets.c
===================================================================
--- buckets/ssl_buckets.c	(revision 2371)
+++ buckets/ssl_buckets.c	(working copy)
@@ -202,6 +202,8 @@ struct serf_ssl_certificate_t {
 };
 
 static void disable_compression(serf_ssl_context_t *ssl_ctx);
+static char *
+    pstrdup_escape_nul_bytes(const char *buf, int len, apr_pool_t *pool);
 
 #if SSL_VERBOSE
 /* Log all ssl alerts that we receive from the server. */
@@ -427,6 +429,85 @@ static BIO_METHOD bio_file_method = {
 #endif
 };
 
+typedef enum san_copy_t {
+    EscapeNulAndCopy = 0,
+    ErrorOnNul = 1,
+} san_copy_t;
+
+
+static apr_status_t
+get_subject_alt_names(apr_array_header_t **san_arr, X509 *ssl_cert,
+                      san_copy_t copy_action, apr_pool_t *pool)
+{
+    STACK_OF(GENERAL_NAME) *names;
+
+    /* assert: copy_action == ErrorOnNul || (san_arr && pool) */
+
+    if (san_arr) {
+        *san_arr = NULL;
+    }
+
+    /* Get subjectAltNames */
+    names = X509_get_ext_d2i(ssl_cert, NID_subject_alt_name, NULL, NULL);
+    if (names) {
+        int names_count = sk_GENERAL_NAME_num(names);
+        int name_idx;
+
+        if (san_arr)
+            *san_arr = apr_array_make(pool, names_count, sizeof(char*));
+        for (name_idx = 0; name_idx < names_count; name_idx++) {
+            char *p = NULL;
+            GENERAL_NAME *nm = sk_GENERAL_NAME_value(names, name_idx);
+
+            switch (nm->type) {
+                case GEN_DNS:
+                    if (copy_action == ErrorOnNul &&
+                        strlen(nm->d.ia5->data) != nm->d.ia5->length)
+                        return SERF_ERROR_SSL_CERT_FAILED;
+                    if (san_arr && *san_arr)
+                        p = pstrdup_escape_nul_bytes((const char *)nm->d.ia5->data,
+                                                     nm->d.ia5->length,
+                                                     pool);
+                    break;
+                default:
+                    /* Don't know what to do - skip. */
+                    break;
+            }
+
+            if (p) {
+                APR_ARRAY_PUSH(*san_arr, char*) = p;
+            }
+        }
+        sk_GENERAL_NAME_pop_free(names, GENERAL_NAME_free);
+    }
+    
+    return APR_SUCCESS;
+}
+
+static apr_status_t validate_cert_hostname(X509 *server_cert, apr_pool_t *pool)
+{
+    char buf[1024];
+    int length;
+    apr_status_t ret;
+
+    ret = get_subject_alt_names(NULL, server_cert, ErrorOnNul, NULL);
+    if (ret) {
+      return ret;
+    } else {
+        /* Fail if the subject's CN field contains \0 characters. */
+        X509_NAME *subject = X509_get_subject_name(server_cert);
+        if (!subject)
+            return SERF_ERROR_SSL_CERT_FAILED;
+
+        length = X509_NAME_get_text_by_NID(subject, NID_commonName, buf, 1024);
+        if (length != -1)
+            if (strlen(buf) != length)
+                return SERF_ERROR_SSL_CERT_FAILED;
+    }
+
+    return APR_SUCCESS;
+}
+
 static int
 validate_server_certificate(int cert_valid, X509_STORE_CTX *store_ctx)
 {
@@ -435,6 +516,7 @@ validate_server_certificate(int cert_valid, X509_S
     X509 *server_cert;
     int err, depth;
     int failures = 0;
+    apr_status_t status;
 
     ssl = X509_STORE_CTX_get_ex_data(store_ctx,
                                      SSL_get_ex_data_X509_STORE_CTX_idx());
@@ -475,6 +557,11 @@ validate_server_certificate(int cert_valid, X509_S
         }
     }
 
+    /* Validate hostname */
+    status = validate_cert_hostname(server_cert, ctx->pool);
+    if (status)
+        failures |= SERF_SSL_CERT_UNKNOWN_FAILURE;
+
     /* Check certificate expiry dates. */
     if (X509_cmp_current_time(X509_get_notBefore(server_cert)) >= 0) {
         failures |= SERF_SSL_CERT_NOTYETVALID;
@@ -485,7 +572,6 @@ validate_server_certificate(int cert_valid, X509_S
 
     if (ctx->server_cert_callback &&
         (depth == 0 || failures)) {
-        apr_status_t status;
         serf_ssl_certificate_t *cert;
         apr_pool_t *subpool;
 
@@ -512,7 +598,6 @@ validate_server_certificate(int cert_valid, X509_S
 
     if (ctx->server_cert_chain_callback
         && (depth == 0 || failures)) {
-        apr_status_t status;
         STACK_OF(X509) *chain;
         const serf_ssl_certificate_t **certs;
         int certs_len;
@@ -1461,7 +1546,50 @@ serf_ssl_context_t *serf_bucket_ssl_encrypt_contex
 
 /* Functions to read a serf_ssl_certificate structure. */
 
-/* Creates a hash_table with keys (E, CN, OU, O, L, ST and C). */
+/* Takes a counted length string and escapes any NUL bytes so that
+ * it can be used as a C string.  NUL bytes are escaped as 3 characters
+ * "\00" (that's a literal backslash).
+ * The returned string is allocated in POOL.
+ */
+static char *
+pstrdup_escape_nul_bytes(const char *buf, int len, apr_pool_t *pool)
+{
+    int i, nul_count = 0;
+    char *ret;
+
+    /* First determine if there are any nul bytes in the string. */
+    for (i = 0; i < len; i++) {
+        if (buf[i] == '\0')
+            nul_count++;
+    }
+
+    if (nul_count == 0) {
+        /* There aren't so easy case to just copy the string */
+        ret = apr_pstrdup(pool, buf);
+    } else {
+        /* There are so we have to replace nul bytes with escape codes
+         * Proper length is the length of the original string, plus
+         * 2 times the number of nulls (for two digit hex code for
+         * the value) + the trailing null. */
+        char *pos;
+        ret = pos = apr_palloc(pool, len + 2 * nul_count + 1);
+        for (i = 0; i < len; i++) {
+            if (buf[i] != '\0') {
+                *(pos++) = buf[i];
+            } else {
+                *(pos++) = '\\';
+                *(pos++) = '0';
+                *(pos++) = '0';
+            }
+        }
+        *pos = '\0';
+    }
+
+    return ret;
+}
+
+/* Creates a hash_table with keys (E, CN, OU, O, L, ST and C). Any NUL bytes in
+   these fields in the certificate will be escaped as \00. */
 static apr_hash_t *
 convert_X509_NAME_to_table(X509_NAME *org, apr_pool_t *pool)
 {
@@ -1474,37 +1602,44 @@ convert_X509_NAME_to_table(X509_NAME *org, apr_poo
                                     NID_commonName,
                                     buf, 1024);
     if (ret != -1)
-        apr_hash_set(tgt, "CN", APR_HASH_KEY_STRING, apr_pstrdup(pool, buf));
+        apr_hash_set(tgt, "CN", APR_HASH_KEY_STRING,
+                     pstrdup_escape_nul_bytes(buf, ret, pool));
     ret = X509_NAME_get_text_by_NID(org,
                                     NID_pkcs9_emailAddress,
                                     buf, 1024);
     if (ret != -1)
-        apr_hash_set(tgt, "E", APR_HASH_KEY_STRING, apr_pstrdup(pool, buf));
+        apr_hash_set(tgt, "E", APR_HASH_KEY_STRING,
+                     pstrdup_escape_nul_bytes(buf, ret, pool));
     ret = X509_NAME_get_text_by_NID(org,
                                     NID_organizationalUnitName,
                                     buf, 1024);
     if (ret != -1)
-        apr_hash_set(tgt, "OU", APR_HASH_KEY_STRING, apr_pstrdup(pool, buf));
+        apr_hash_set(tgt, "OU", APR_HASH_KEY_STRING,
+                     pstrdup_escape_nul_bytes(buf, ret, pool));
     ret = X509_NAME_get_text_by_NID(org,
                                     NID_organizationName,
                                     buf, 1024);
     if (ret != -1)
-        apr_hash_set(tgt, "O", APR_HASH_KEY_STRING, apr_pstrdup(pool, buf));
+        apr_hash_set(tgt, "O", APR_HASH_KEY_STRING,
+                     pstrdup_escape_nul_bytes(buf, ret, pool));
     ret = X509_NAME_get_text_by_NID(org,
                                     NID_localityName,
                                     buf, 1024);
     if (ret != -1)
-        apr_hash_set(tgt, "L", APR_HASH_KEY_STRING, apr_pstrdup(pool, buf));
+        apr_hash_set(tgt, "L", APR_HASH_KEY_STRING,
+                     pstrdup_escape_nul_bytes(buf, ret, pool));
     ret = X509_NAME_get_text_by_NID(org,
                                     NID_stateOrProvinceName,
                                     buf, 1024);
     if (ret != -1)
-        apr_hash_set(tgt, "ST", APR_HASH_KEY_STRING, apr_pstrdup(pool, buf));
+        apr_hash_set(tgt, "ST", APR_HASH_KEY_STRING,
+                     pstrdup_escape_nul_bytes(buf, ret, pool));
     ret = X509_NAME_get_text_by_NID(org,
                                     NID_countryName,
                                     buf, 1024);
     if (ret != -1)
-        apr_hash_set(tgt, "C", APR_HASH_KEY_STRING, apr_pstrdup(pool, buf));
+        apr_hash_set(tgt, "C", APR_HASH_KEY_STRING,
+                     pstrdup_escape_nul_bytes(buf, ret, pool));
 
     return tgt;
 }
@@ -1550,7 +1685,7 @@ apr_hash_t *serf_ssl_cert_certificate(
     unsigned int md_size, i;
     unsigned char md[EVP_MAX_MD_SIZE];
     BIO *bio;
-    STACK_OF(GENERAL_NAME) *names;
+    apr_array_header_t *san_arr;
 
     /* sha1 fingerprint */
     if (X509_digest(cert->ssl_cert, EVP_sha1(), md, &md_size)) {
@@ -1595,33 +1730,9 @@ apr_hash_t *serf_ssl_cert_certificate(
     BIO_free(bio);
 
     /* Get subjectAltNames */
-    names = X509_get_ext_d2i(cert->ssl_cert, NID_subject_alt_name, NULL, NULL);
-    if (names) {
-        int names_count = sk_GENERAL_NAME_num(names);
-
-        apr_array_header_t *san_arr = apr_array_make(pool, names_count,
-                                                     sizeof(char*));
+    if (!get_subject_alt_names(&san_arr, cert->ssl_cert, EscapeNulAndCopy, pool))
         apr_hash_set(tgt, "subjectAltName", APR_HASH_KEY_STRING, san_arr);
-        for (i = 0; i < names_count; i++) {
-            char *p = NULL;
-            GENERAL_NAME *nm = sk_GENERAL_NAME_value(names, i);
 
-            switch (nm->type) {
-            case GEN_DNS:
-                p = apr_pstrmemdup(pool, (const char *)nm->d.ia5->data,
-                                   nm->d.ia5->length);
-                break;
-            default:
-                /* Don't know what to do - skip. */
-                break;
-            }
-            if (p) {
-                APR_ARRAY_PUSH(san_arr, char*) = p;
-            }
-        }
-        sk_GENERAL_NAME_pop_free(names, GENERAL_NAME_free);
-    }
-
     return tgt;
 }