Skip to content

Commit

Permalink
Merge r1825120 from trunk:
Browse files Browse the repository at this point in the history
* modules/ssl/ssl_engine_init.c (ssl_init_PushCAList): Remove function.
  (ssl_init_ca_cert_path): Use SSL_add_file_cert_subjects_to_stack()
  instead.

[Edit: This does change behaviour: the acceptable client CA list is now 
 always be sent in sorted order rather than configured/file order.
 In the case of SSLCACertificatePath and SSLCADNRequestPath, the
 order will be stable rather than non-determistic as previously.]

PR: 61574
Github: closes #406
Reviewed by: jorton, jfclere, covener


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1915740 13f79535-47bb-0310-9956-ffa450edef68
  • Loading branch information
notroj committed Feb 12, 2024
1 parent 7e3634b commit d02cfc9
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 42 deletions.
4 changes: 4 additions & 0 deletions changes-entries/pr61574.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
*) mod_ssl: Use OpenSSL-standard functions to assemble CA
name lists for SSLCACertificatePath/SSLCADNRequestPath.
Names will now be consistently sorted. PR 61574.
[Joe Orton]
44 changes: 2 additions & 42 deletions modules/ssl/ssl_engine_init.c
Original file line number Diff line number Diff line change
Expand Up @@ -2248,46 +2248,6 @@ static int ssl_init_FindCAList_X509NameCmp(const X509_NAME * const *a,
return(X509_NAME_cmp(*a, *b));
}

static void ssl_init_PushCAList(STACK_OF(X509_NAME) *ca_list,
server_rec *s, apr_pool_t *ptemp,
const char *file)
{
int n;
STACK_OF(X509_NAME) *sk;

sk = (STACK_OF(X509_NAME) *)
SSL_load_client_CA_file(file);

if (!sk) {
return;
}

for (n = 0; n < sk_X509_NAME_num(sk); n++) {
X509_NAME *name = sk_X509_NAME_value(sk, n);

ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02209)
"CA certificate: %s",
modssl_X509_NAME_to_string(ptemp, name, 0));

/*
* note that SSL_load_client_CA_file() checks for duplicates,
* but since we call it multiple times when reading a directory
* we must also check for duplicates ourselves.
*/

if (sk_X509_NAME_find(ca_list, name) < 0) {
/* this will be freed when ca_list is */
sk_X509_NAME_push(ca_list, name);
}
else {
/* need to free this ourselves, else it will leak */
X509_NAME_free(name);
}
}

sk_X509_NAME_free(sk);
}

static apr_status_t ssl_init_ca_cert_path(server_rec *s,
apr_pool_t *ptemp,
const char *path,
Expand All @@ -2310,7 +2270,7 @@ static apr_status_t ssl_init_ca_cert_path(server_rec *s,
}
file = apr_pstrcat(ptemp, path, "/", direntry.name, NULL);
if (ca_list) {
ssl_init_PushCAList(ca_list, s, ptemp, file);
SSL_add_file_cert_subjects_to_stack(ca_list, file);
}
if (xi_list) {
load_x509_info(ptemp, xi_list, file);
Expand Down Expand Up @@ -2339,7 +2299,7 @@ STACK_OF(X509_NAME) *ssl_init_FindCAList(server_rec *s,
* Process CA certificate bundle file
*/
if (ca_file) {
ssl_init_PushCAList(ca_list, s, ptemp, ca_file);
SSL_add_file_cert_subjects_to_stack(ca_list, ca_file);
/*
* If ca_list is still empty after trying to load ca_file
* then the file failed to load, and users should hear about that.
Expand Down

0 comments on commit d02cfc9

Please sign in to comment.