| ------------------------------------------------------------------------ |
| r1699925 | breser | 2014-08-04 11:04:00 -0700 (Mon, 04 Aug 2014) | 5 lines |
| |
| On the 1.3.x branch: |
| |
| Merge changes from trunk: |
| - r2392: Handle NUL bytes in fields of X.509 certs. |
| |
| ------------------------------------------------------------------------ |
| Index: misc/build/serf-1.2.1/buckets/ssl_buckets.c |
| =================================================================== |
| --- misc/serf-1.2.1/buckets/ssl_buckets.c (revision 1699924) |
| +++ misc/build/serf-1.2.1/buckets/ssl_buckets.c (revision 1699925) |
| @@ -202,6 +202,8 @@ |
| }; |
| |
| 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,81 @@ |
| #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) */ |
| + |
| + /* 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 +512,7 @@ |
| 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 +553,11 @@ |
| } |
| } |
| |
| + /* 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 +568,6 @@ |
| |
| if (ctx->server_cert_callback && |
| (depth == 0 || failures)) { |
| - apr_status_t status; |
| serf_ssl_certificate_t *cert; |
| apr_pool_t *subpool; |
| |
| @@ -512,7 +594,6 @@ |
| |
| 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 +1542,50 @@ |
| |
| /* 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 +1598,44 @@ |
| 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 +1681,7 @@ |
| 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 +1726,9 @@ |
| 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; |
| } |
| |
| ------------------------------------------------------------------------ |
| r1699931 | breser | 2014-08-05 19:24:00 -0700 (Tue, 05 Aug 2014) | 6 lines |
| |
| On the 1.3.x branch: |
| |
| Merge changes from trunk: |
| - r2398: Initialize san_arr when we're expected to fill it. |
| |
| |
| ------------------------------------------------------------------------ |
| Index: misc/build/serf-1.2.1/buckets/ssl_buckets.c |
| =================================================================== |
| --- misc/serf-1.2.1/buckets/ssl_buckets.c (revision 1699930) |
| +++ misc/build/serf-1.2.1/buckets/ssl_buckets.c (revision 1699931) |
| @@ -443,6 +443,10 @@ |
| |
| /* 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) { |
