Browse Source

schannel: fix ordering of cert chain info

- Use CERT_CONTEXT's pbCertEncoded to determine chain order.

CERT_CONTEXT from SECPKG_ATTR_REMOTE_CERT_CONTEXT contains
end-entity/server certificate in pbCertEncoded. We can use this pointer
to determine the order of certificates when enumerating hCertStore using
CertEnumCertificatesInStore.

This change is to help ensure that the ordering of the certificate chain
requested by the user via CURLINFO_CERTINFO has the same ordering on all
versions of Windows.

Prior to this change Schannel certificate order was reversed in 8986df80
but that was later reverted in f540a39b when it was discovered that
Windows 11 22H2 does the reversal on its own.

Ref: https://github.com/curl/curl/issues/9706

Closes https://github.com/curl/curl/pull/11632
Nathan Moinvaziri 9 months ago
parent
commit
f6700c744b
5 changed files with 224 additions and 8 deletions
  1. 27 6
      lib/vtls/schannel.c
  2. 1 1
      tests/data/Makefile.inc
  3. 51 0
      tests/data/test3102
  4. 4 1
      tests/libtest/Makefile.inc
  5. 141 0
      tests/libtest/lib3102.c

+ 27 - 6
lib/vtls/schannel.c

@@ -1656,7 +1656,8 @@ valid_cert_encoding(const CERT_CONTEXT *cert_context)
     (cert_context->cbCertEncoded > 0);
 }
 
-typedef bool(*Read_crt_func)(const CERT_CONTEXT *ccert_context, void *arg);
+typedef bool(*Read_crt_func)(const CERT_CONTEXT *ccert_context,
+                             bool reverse_order, void *arg);
 
 static void
 traverse_cert_store(const CERT_CONTEXT *context, Read_crt_func func,
@@ -1664,19 +1665,32 @@ traverse_cert_store(const CERT_CONTEXT *context, Read_crt_func func,
 {
   const CERT_CONTEXT *current_context = NULL;
   bool should_continue = true;
+  bool first = true;
+  bool reverse_order = false;
   while(should_continue &&
         (current_context = CertEnumCertificatesInStore(
           context->hCertStore,
-          current_context)) != NULL)
-    should_continue = func(current_context, arg);
+          current_context)) != NULL) {
+    /* Windows 11 22H2 OS Build 22621.674 or higher enumerates certificates in
+       leaf-to-root order while all previous versions of Windows enumerate
+       certificates in root-to-leaf order. Determine the order of enumeration
+       by comparing SECPKG_ATTR_REMOTE_CERT_CONTEXT's pbCertContext with the
+       first certificate's pbCertContext. */
+    if(first && context->pbCertEncoded != current_context->pbCertEncoded)
+      reverse_order = true;
+    should_continue = func(current_context, reverse_order, arg);
+    first = false;
+  }
 
   if(current_context)
     CertFreeCertificateContext(current_context);
 }
 
 static bool
-cert_counter_callback(const CERT_CONTEXT *ccert_context, void *certs_count)
+cert_counter_callback(const CERT_CONTEXT *ccert_context, bool reverse_order,
+                      void *certs_count)
 {
+  (void)reverse_order; /* unused */
   if(valid_cert_encoding(ccert_context))
     (*(int *)certs_count)++;
   return true;
@@ -1687,17 +1701,23 @@ struct Adder_args
   struct Curl_easy *data;
   CURLcode result;
   int idx;
+  int certs_count;
 };
 
 static bool
-add_cert_to_certinfo(const CERT_CONTEXT *ccert_context, void *raw_arg)
+add_cert_to_certinfo(const CERT_CONTEXT *ccert_context, bool reverse_order,
+                     void *raw_arg)
 {
   struct Adder_args *args = (struct Adder_args*)raw_arg;
   args->result = CURLE_OK;
   if(valid_cert_encoding(ccert_context)) {
     const char *beg = (const char *) ccert_context->pbCertEncoded;
     const char *end = beg + ccert_context->cbCertEncoded;
-    args->result = Curl_extract_certinfo(args->data, (args->idx)++, beg, end);
+    int insert_index = reverse_order ? (args->certs_count - 1) - args->idx :
+                       args->idx;
+    args->result = Curl_extract_certinfo(args->data, insert_index,
+                                         beg, end);
+    args->idx++;
   }
   return args->result == CURLE_OK;
 }
@@ -1831,6 +1851,7 @@ schannel_connect_step3(struct Curl_cfilter *cf, struct Curl_easy *data)
       struct Adder_args args;
       args.data = data;
       args.idx = 0;
+      args.certs_count = certs_count;
       traverse_cert_store(ccert_context, add_cert_to_certinfo, &args);
       result = args.result;
     }

+ 1 - 1
tests/data/Makefile.inc

@@ -256,6 +256,6 @@ test3008 test3009 test3010 test3011 test3012 test3013 test3014 test3015 \
 test3016 test3017 test3018 test3019 test3020 test3021 test3022 test3023 \
 test3024 test3025 test3026 test3027 test3028 test3029 test3030 \
 \
-test3100 test3101 \
+test3100 test3101 test3102 \
 test3200 \
 test3201 test3202

+ 51 - 0
tests/data/test3102

@@ -0,0 +1,51 @@
+<testcase>
+<info>
+<keywords>
+HTTPS
+HTTP GET
+</keywords>
+</info>
+
+#
+# Server-side
+<reply>
+<data>
+</data>
+</reply>
+
+#
+# Client-side
+<client>
+# SSL with libraries supporting CURLOPT_CERTINFO
+<features>
+SSL
+!bearssl
+!mbedtls
+!rustls
+!wolfssl
+</features>
+<server>
+https
+</server>
+<tool>
+lib%TESTNUMBER
+</tool>
+ <name>
+verify certificate chain order with simple HTTPS GET
+ </name>
+ <command>
+https://%HOSTIP:%HTTPSPORT/%TESTNUMBER
+</command>
+</client>
+
+#
+# Verify data after the test has been "shot"
+<verify>
+<protocol>
+GET /%TESTNUMBER HTTP/1.1
+Host: %HOSTIP:%HTTPSPORT
+Accept: */*
+
+</protocol>
+</verify>
+</testcase>

+ 4 - 1
tests/libtest/Makefile.inc

@@ -75,7 +75,7 @@ noinst_PROGRAMS = chkhostname libauthretry libntlmconnect libprereq      \
  lib2402 lib2404 \
  lib2502 \
  lib3010 lib3025 lib3026 lib3027 \
- lib3100 lib3101
+ lib3100 lib3101 lib3102
 
 chkhostname_SOURCES = chkhostname.c ../../lib/curl_gethostname.c
 chkhostname_LDADD = @CURL_NETWORK_LIBS@
@@ -686,3 +686,6 @@ lib3100_LDADD = $(TESTUTIL_LIBS)
 
 lib3101_SOURCES = lib3101.c $(SUPPORTFILES) $(TESTUTIL) $(WARNLESS)
 lib3101_LDADD = $(TESTUTIL_LIBS)
+
+lib3102_SOURCES = lib3102.c $(SUPPORTFILES) $(TESTUTIL) $(WARNLESS)
+lib3102_LDADD = $(TESTUTIL_LIBS)

+ 141 - 0
tests/libtest/lib3102.c

@@ -0,0 +1,141 @@
+/***************************************************************************
+ *                                  _   _ ____  _
+ *  Project                     ___| | | |  _ \| |
+ *                             / __| | | | |_) | |
+ *                            | (__| |_| |  _ <| |___
+ *                             \___|\___/|_| \_\_____|
+ *
+ * Copyright (C) Daniel Stenberg, <daniel@haxx.se>, et al.
+ *
+ * This software is licensed as described in the file COPYING, which
+ * you should have received as part of this distribution. The terms
+ * are also available at https://curl.se/docs/copyright.html.
+ *
+ * You may opt to use, copy, modify, merge, publish, distribute and/or sell
+ * copies of the Software, and permit persons to whom the Software is
+ * furnished to do so, under the terms of the COPYING file.
+ *
+ * This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY
+ * KIND, either express or implied.
+ *
+ * SPDX-License-Identifier: curl
+ *
+ ***************************************************************************/
+#include "test.h"
+
+#include "memdebug.h"
+
+/*
+ * Verify correct order of certificates in the chain by comparing the
+ * subject and issuer attributes of each certificate.
+ */
+static bool is_chain_in_order(struct curl_certinfo *cert_info)
+{
+  char *last_issuer = NULL;
+  int cert;
+
+  /* Chains with only a single certificate are always in order */
+  if(cert_info->num_of_certs <= 1)
+    return 1;
+
+  /* Enumerate each certificate in the chain */
+  for(cert = 0; cert < cert_info->num_of_certs; cert++) {
+    struct curl_slist *slist = cert_info->certinfo[cert];
+    char *issuer = NULL;
+    char *subject = NULL;
+
+    /* Find the certificate issuer and subject by enumerating each field */
+    for(; slist && (!issuer || !subject); slist = slist->next) {
+      const char issuer_prefix[] = "Issuer:";
+      const char subject_prefix[] = "Subject:";
+
+      if(!strncmp(slist->data, issuer_prefix, sizeof(issuer_prefix)-1)) {
+        issuer = slist->data + sizeof(issuer_prefix)-1;
+      }
+      if(!strncmp(slist->data, subject_prefix, sizeof(subject_prefix)-1)) {
+        subject = slist->data + sizeof(subject_prefix)-1;
+      }
+    }
+
+    if(subject && issuer) {
+      printf("cert %d\n", cert);
+      printf("  subject: %s\n", subject);
+      printf("  issuer: %s\n", issuer);
+
+      if(last_issuer) {
+        /* If the last certificate's issuer matches the current certificate's
+         * subject, then the chain is in order */
+        if(strcmp(last_issuer, subject) != 0) {
+          fprintf(stderr, "cert %d issuer does not match cert %d subject\n",
+                  cert - 1, cert);
+          fprintf(stderr, "certificate chain is not in order\n");
+          return false;
+        }
+      }
+    }
+
+    last_issuer = issuer;
+  }
+
+  printf("certificate chain is in order\n");
+  return true;
+}
+
+static size_t wrfu(void *ptr,  size_t  size,  size_t  nmemb,  void *stream)
+{
+  (void)stream;
+  (void)ptr;
+  return size * nmemb;
+}
+
+int test(char *URL)
+{
+  CURL *curl;
+  CURLcode res = CURLE_OK;
+
+  if(curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK) {
+    fprintf(stderr, "curl_global_init() failed\n");
+    return TEST_ERR_MAJOR_BAD;
+  }
+
+  curl = curl_easy_init();
+  if(!curl) {
+    fprintf(stderr, "curl_easy_init() failed\n");
+    curl_global_cleanup();
+    return TEST_ERR_MAJOR_BAD;
+  }
+
+  /* Set the HTTPS url to retrieve. */
+  test_setopt(curl, CURLOPT_URL, URL);
+
+  /* Capture certificate information */
+  test_setopt(curl, CURLOPT_CERTINFO, 1L);
+
+  /* Ignore output */
+  test_setopt(curl, CURLOPT_WRITEFUNCTION, wrfu);
+
+  /* No peer verify */
+  test_setopt(curl, CURLOPT_SSL_VERIFYPEER, 0L);
+  test_setopt(curl, CURLOPT_SSL_VERIFYHOST, 0L);
+
+  /* Perform the request, res will get the return code */
+  res = curl_easy_perform(curl);
+  if(!res || res == CURLE_GOT_NOTHING) {
+    struct curl_certinfo *cert_info = NULL;
+    /* Get the certificate information */
+    res = curl_easy_getinfo(curl, CURLINFO_CERTINFO, &cert_info);
+    if(!res) {
+      /* Check to see if the certificate chain is ordered correctly */
+      if(!is_chain_in_order(cert_info))
+        res = TEST_ERR_FAILURE;
+    }
+  }
+
+test_cleanup:
+
+  /* always cleanup */
+  curl_easy_cleanup(curl);
+  curl_global_cleanup();
+
+  return res;
+}