From febae0ad0dcb67f89ad05ed08f0de2e603a88ce4 Mon Sep 17 00:00:00 2001 From: Tobiasz Laskowski Date: Sat, 23 Nov 2024 11:58:26 +0000 Subject: [PATCH 01/10] Use windows api to verify ssl certs Taken from: https://github.com/Apprentice-Alchemist/hashlink/commit/4d590124f91e3b142724c215468892481598e804 --- libs/mbedtls/mbedtls_stubs.c | 45 ++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/libs/mbedtls/mbedtls_stubs.c b/libs/mbedtls/mbedtls_stubs.c index 9d8132d9e07..d58870f49af 100644 --- a/libs/mbedtls/mbedtls_stubs.c +++ b/libs/mbedtls/mbedtls_stubs.c @@ -302,12 +302,57 @@ static struct custom_operations ssl_config_ops = { .deserialize = custom_deserialize_default, }; +#ifdef _WIN32 +static int verify_callback(void* param, mbedtls_x509_crt *crt, int depth, uint32_t *flags) { + if(depth == 0) { + HCERTSTORE store = CertOpenStore(CERT_STORE_PROV_MEMORY, 0, 0, CERT_STORE_DEFER_CLOSE_UNTIL_LAST_FREE_FLAG, NULL); + if(store == NULL) { + // handle error + } + PCCERT_CONTEXT primary_context = {0}; + if(!CertAddEncodedCertificateToStore(store, X509_ASN_ENCODING, crt->raw.p, crt->raw.len, CERT_STORE_ADD_ALWAYS, &primary_context)) { + return MBEDTLS_ERR_X509_FATAL_ERROR; + } + while(crt->next) { + crt = crt->next; + PCCERT_CONTEXT ctx = {0}; + if (!CertAddEncodedCertificateToStore(store, X509_ASN_ENCODING, crt->raw.p, crt->raw.len, CERT_STORE_ADD_ALWAYS, &ctx)) + { + return MBEDTLS_ERR_X509_FATAL_ERROR; + } + CertFreeCertificateContext(ctx); + } + PCCERT_CHAIN_CONTEXT chain_context = {0}; + PCERT_CHAIN_PARA parameters = {0}; + if(!CertGetCertificateChain(NULL, primary_context, NULL, store, parameters, 0, NULL, &chain_context)) { + return MBEDTLS_ERR_X509_FATAL_ERROR; + } + PCERT_CHAIN_POLICY_PARA policy_parameters = {0}; + CERT_CHAIN_POLICY_STATUS policy_status = {0}; + if(!CertVerifyCertificateChainPolicy(CERT_CHAIN_POLICY_SSL, chain_context, policy_parameters, &policy_status)) { + return MBEDTLS_ERR_X509_FATAL_ERROR; + } + if(policy_status.dwError != 0) { + // TODO: properly map errors + *flags |= MBEDTLS_X509_BADCERT_OTHER; + } + CertFreeCertificateChain(chain_context); + CertFreeCertificateContext(primary_context); + CertCloseStore(store, 0); + } + return 0; +} +#endif + CAMLprim value ml_mbedtls_ssl_config_init(void) { CAMLparam0(); CAMLlocal1(obj); obj = caml_alloc_custom(&ssl_config_ops, sizeof(mbedtls_ssl_config*), 0, 1); mbedtls_ssl_config* ssl_config = malloc(sizeof(mbedtls_ssl_config)); mbedtls_ssl_config_init(ssl_config); + #ifdef _WIN32 + mbedtls_ssl_conf_verify(ssl_config, verify_callback, NULL); + #endif Config_val(obj) = ssl_config; CAMLreturn(obj); } From 5d611893708474de6d66876e88898359266d0368 Mon Sep 17 00:00:00 2001 From: Tobiasz Laskowski Date: Sat, 23 Nov 2024 14:38:44 +0000 Subject: [PATCH 02/10] Handle error if cert store fails to open --- libs/mbedtls/mbedtls_stubs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/mbedtls/mbedtls_stubs.c b/libs/mbedtls/mbedtls_stubs.c index d58870f49af..fd5b6072e95 100644 --- a/libs/mbedtls/mbedtls_stubs.c +++ b/libs/mbedtls/mbedtls_stubs.c @@ -307,7 +307,7 @@ static int verify_callback(void* param, mbedtls_x509_crt *crt, int depth, uint32 if(depth == 0) { HCERTSTORE store = CertOpenStore(CERT_STORE_PROV_MEMORY, 0, 0, CERT_STORE_DEFER_CLOSE_UNTIL_LAST_FREE_FLAG, NULL); if(store == NULL) { - // handle error + return MBEDTLS_ERR_X509_FATAL_ERROR; } PCCERT_CONTEXT primary_context = {0}; if(!CertAddEncodedCertificateToStore(store, X509_ASN_ENCODING, crt->raw.p, crt->raw.len, CERT_STORE_ADD_ALWAYS, &primary_context)) { From 9281c6a7b46f044c0ff70a94c7d18806d6c46234 Mon Sep 17 00:00:00 2001 From: Tobiasz Laskowski Date: Sat, 23 Nov 2024 14:39:17 +0000 Subject: [PATCH 03/10] Fix mscv warnings about invalid arguments Warning C6387 'parameters' could be '0': this does not adhere to the specification for the function 'CertGetCertificateChain'. Warning C6387 'policy_parameters' could be '0': this does not adhere to the specification for the function 'CertVerifyCertificateChainPolicy'. This also fixes an "incorrect parameter" runtime error. --- libs/mbedtls/mbedtls_stubs.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libs/mbedtls/mbedtls_stubs.c b/libs/mbedtls/mbedtls_stubs.c index fd5b6072e95..8924533827e 100644 --- a/libs/mbedtls/mbedtls_stubs.c +++ b/libs/mbedtls/mbedtls_stubs.c @@ -323,13 +323,13 @@ static int verify_callback(void* param, mbedtls_x509_crt *crt, int depth, uint32 CertFreeCertificateContext(ctx); } PCCERT_CHAIN_CONTEXT chain_context = {0}; - PCERT_CHAIN_PARA parameters = {0}; - if(!CertGetCertificateChain(NULL, primary_context, NULL, store, parameters, 0, NULL, &chain_context)) { + CERT_CHAIN_PARA parameters = {0}; + if(!CertGetCertificateChain(NULL, primary_context, NULL, store, ¶meters, 0, NULL, &chain_context)) { return MBEDTLS_ERR_X509_FATAL_ERROR; } - PCERT_CHAIN_POLICY_PARA policy_parameters = {0}; + CERT_CHAIN_POLICY_PARA policy_parameters = {0}; CERT_CHAIN_POLICY_STATUS policy_status = {0}; - if(!CertVerifyCertificateChainPolicy(CERT_CHAIN_POLICY_SSL, chain_context, policy_parameters, &policy_status)) { + if(!CertVerifyCertificateChainPolicy(CERT_CHAIN_POLICY_SSL, chain_context, &policy_parameters, &policy_status)) { return MBEDTLS_ERR_X509_FATAL_ERROR; } if(policy_status.dwError != 0) { From 1a293cfb852c066523481fb6b5a14ecb3fd269d8 Mon Sep 17 00:00:00 2001 From: Tobiasz Laskowski Date: Sun, 24 Nov 2024 00:56:57 +0000 Subject: [PATCH 04/10] Clear errors if certificate loading succeeded --- libs/mbedtls/mbedtls_stubs.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libs/mbedtls/mbedtls_stubs.c b/libs/mbedtls/mbedtls_stubs.c index 8924533827e..2b155b24f63 100644 --- a/libs/mbedtls/mbedtls_stubs.c +++ b/libs/mbedtls/mbedtls_stubs.c @@ -332,7 +332,9 @@ static int verify_callback(void* param, mbedtls_x509_crt *crt, int depth, uint32 if(!CertVerifyCertificateChainPolicy(CERT_CHAIN_POLICY_SSL, chain_context, &policy_parameters, &policy_status)) { return MBEDTLS_ERR_X509_FATAL_ERROR; } - if(policy_status.dwError != 0) { + if(policy_status.dwError == 0) { + *flags = 0; + } else { // TODO: properly map errors *flags |= MBEDTLS_X509_BADCERT_OTHER; } From d8ad7da021825d2fba9cd168038b07cc18be6e8e Mon Sep 17 00:00:00 2001 From: Tobiasz Laskowski Date: Sun, 24 Nov 2024 02:29:04 +0000 Subject: [PATCH 05/10] Perform checks for all calls of verify_callback We need to do this every time, because if any callback call returns a non zero flags then the entire verification fails, see: https://github.com/Mbed-TLS/mbedtls/blob/3aefa5b705846c5d4466ae8747160ae9e5054ea8/library/x509_crt.c#L3031 We don't need to loop through the chain, since mbedtls already loops through and calls the callback on every certificate in the chain. --- libs/mbedtls/mbedtls_stubs.c | 63 +++++++++++++++--------------------- 1 file changed, 26 insertions(+), 37 deletions(-) diff --git a/libs/mbedtls/mbedtls_stubs.c b/libs/mbedtls/mbedtls_stubs.c index 2b155b24f63..3c8557366b4 100644 --- a/libs/mbedtls/mbedtls_stubs.c +++ b/libs/mbedtls/mbedtls_stubs.c @@ -304,44 +304,33 @@ static struct custom_operations ssl_config_ops = { #ifdef _WIN32 static int verify_callback(void* param, mbedtls_x509_crt *crt, int depth, uint32_t *flags) { - if(depth == 0) { - HCERTSTORE store = CertOpenStore(CERT_STORE_PROV_MEMORY, 0, 0, CERT_STORE_DEFER_CLOSE_UNTIL_LAST_FREE_FLAG, NULL); - if(store == NULL) { - return MBEDTLS_ERR_X509_FATAL_ERROR; - } - PCCERT_CONTEXT primary_context = {0}; - if(!CertAddEncodedCertificateToStore(store, X509_ASN_ENCODING, crt->raw.p, crt->raw.len, CERT_STORE_ADD_ALWAYS, &primary_context)) { - return MBEDTLS_ERR_X509_FATAL_ERROR; - } - while(crt->next) { - crt = crt->next; - PCCERT_CONTEXT ctx = {0}; - if (!CertAddEncodedCertificateToStore(store, X509_ASN_ENCODING, crt->raw.p, crt->raw.len, CERT_STORE_ADD_ALWAYS, &ctx)) - { - return MBEDTLS_ERR_X509_FATAL_ERROR; - } - CertFreeCertificateContext(ctx); - } - PCCERT_CHAIN_CONTEXT chain_context = {0}; - CERT_CHAIN_PARA parameters = {0}; - if(!CertGetCertificateChain(NULL, primary_context, NULL, store, ¶meters, 0, NULL, &chain_context)) { - return MBEDTLS_ERR_X509_FATAL_ERROR; - } - CERT_CHAIN_POLICY_PARA policy_parameters = {0}; - CERT_CHAIN_POLICY_STATUS policy_status = {0}; - if(!CertVerifyCertificateChainPolicy(CERT_CHAIN_POLICY_SSL, chain_context, &policy_parameters, &policy_status)) { - return MBEDTLS_ERR_X509_FATAL_ERROR; - } - if(policy_status.dwError == 0) { - *flags = 0; - } else { - // TODO: properly map errors - *flags |= MBEDTLS_X509_BADCERT_OTHER; - } - CertFreeCertificateChain(chain_context); - CertFreeCertificateContext(primary_context); - CertCloseStore(store, 0); + HCERTSTORE store = CertOpenStore(CERT_STORE_PROV_MEMORY, 0, 0, CERT_STORE_DEFER_CLOSE_UNTIL_LAST_FREE_FLAG, NULL); + if(store == NULL) { + return MBEDTLS_ERR_X509_FATAL_ERROR; + } + PCCERT_CONTEXT primary_context = {0}; + if(!CertAddEncodedCertificateToStore(store, X509_ASN_ENCODING, crt->raw.p, crt->raw.len, CERT_STORE_ADD_ALWAYS, &primary_context)) { + return MBEDTLS_ERR_X509_FATAL_ERROR; + } + PCCERT_CHAIN_CONTEXT chain_context = {0}; + CERT_CHAIN_PARA parameters = {0}; + if(!CertGetCertificateChain(NULL, primary_context, NULL, store, ¶meters, 0, NULL, &chain_context)) { + return MBEDTLS_ERR_X509_FATAL_ERROR; + } + CERT_CHAIN_POLICY_PARA policy_parameters = {0}; + CERT_CHAIN_POLICY_STATUS policy_status = {0}; + if(!CertVerifyCertificateChainPolicy(CERT_CHAIN_POLICY_SSL, chain_context, &policy_parameters, &policy_status)) { + return MBEDTLS_ERR_X509_FATAL_ERROR; + } + if(policy_status.dwError == 0) { + *flags = 0; + } else { + // TODO: properly map errors + *flags |= MBEDTLS_X509_BADCERT_OTHER; } + CertFreeCertificateChain(chain_context); + CertFreeCertificateContext(primary_context); + CertCloseStore(store, 0); return 0; } #endif From 13f2061841412b2e353bf138ed302de85200e268 Mon Sep 17 00:00:00 2001 From: Tobiasz Laskowski Date: Sat, 23 Nov 2024 14:42:48 +0000 Subject: [PATCH 06/10] Free handles on certificate verification errors --- libs/mbedtls/mbedtls_stubs.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/libs/mbedtls/mbedtls_stubs.c b/libs/mbedtls/mbedtls_stubs.c index 3c8557366b4..39427f6f8d7 100644 --- a/libs/mbedtls/mbedtls_stubs.c +++ b/libs/mbedtls/mbedtls_stubs.c @@ -310,16 +310,22 @@ static int verify_callback(void* param, mbedtls_x509_crt *crt, int depth, uint32 } PCCERT_CONTEXT primary_context = {0}; if(!CertAddEncodedCertificateToStore(store, X509_ASN_ENCODING, crt->raw.p, crt->raw.len, CERT_STORE_ADD_ALWAYS, &primary_context)) { + CertCloseStore(store, 0); return MBEDTLS_ERR_X509_FATAL_ERROR; } PCCERT_CHAIN_CONTEXT chain_context = {0}; CERT_CHAIN_PARA parameters = {0}; if(!CertGetCertificateChain(NULL, primary_context, NULL, store, ¶meters, 0, NULL, &chain_context)) { + CertFreeCertificateContext(primary_context); + CertCloseStore(store, 0); return MBEDTLS_ERR_X509_FATAL_ERROR; } CERT_CHAIN_POLICY_PARA policy_parameters = {0}; CERT_CHAIN_POLICY_STATUS policy_status = {0}; if(!CertVerifyCertificateChainPolicy(CERT_CHAIN_POLICY_SSL, chain_context, &policy_parameters, &policy_status)) { + CertFreeCertificateChain(chain_context); + CertFreeCertificateContext(primary_context); + CertCloseStore(store, 0); return MBEDTLS_ERR_X509_FATAL_ERROR; } if(policy_status.dwError == 0) { From cb339c9ee3864e38ad13b0f8ba75ef9ea2498283 Mon Sep 17 00:00:00 2001 From: Tobiasz Laskowski Date: Sat, 23 Nov 2024 16:26:58 +0000 Subject: [PATCH 07/10] Replace existing certificates if they exist This avoids duplicate certificates in the store --- libs/mbedtls/mbedtls_stubs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/mbedtls/mbedtls_stubs.c b/libs/mbedtls/mbedtls_stubs.c index 39427f6f8d7..eb83b02a948 100644 --- a/libs/mbedtls/mbedtls_stubs.c +++ b/libs/mbedtls/mbedtls_stubs.c @@ -309,7 +309,7 @@ static int verify_callback(void* param, mbedtls_x509_crt *crt, int depth, uint32 return MBEDTLS_ERR_X509_FATAL_ERROR; } PCCERT_CONTEXT primary_context = {0}; - if(!CertAddEncodedCertificateToStore(store, X509_ASN_ENCODING, crt->raw.p, crt->raw.len, CERT_STORE_ADD_ALWAYS, &primary_context)) { + if(!CertAddEncodedCertificateToStore(store, X509_ASN_ENCODING, crt->raw.p, crt->raw.len, CERT_STORE_ADD_REPLACE_EXISTING, &primary_context)) { CertCloseStore(store, 0); return MBEDTLS_ERR_X509_FATAL_ERROR; } From e94350e4d58cc5ad4cef6dd698aa883848b6e3e0 Mon Sep 17 00:00:00 2001 From: Tobiasz Laskowski Date: Mon, 25 Nov 2024 01:09:22 +0000 Subject: [PATCH 08/10] Propagate CN_MISMATCH ssl cert error The windows api functions won't check this automatically for us without further modifications, so it's easiest to just respect mbedtls' judgement and propagate this error. --- libs/mbedtls/mbedtls_stubs.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libs/mbedtls/mbedtls_stubs.c b/libs/mbedtls/mbedtls_stubs.c index eb83b02a948..60778328ae4 100644 --- a/libs/mbedtls/mbedtls_stubs.c +++ b/libs/mbedtls/mbedtls_stubs.c @@ -304,6 +304,10 @@ static struct custom_operations ssl_config_ops = { #ifdef _WIN32 static int verify_callback(void* param, mbedtls_x509_crt *crt, int depth, uint32_t *flags) { + if (*flags & MBEDTLS_X509_BADCERT_CN_MISMATCH) { + return 0; + } + HCERTSTORE store = CertOpenStore(CERT_STORE_PROV_MEMORY, 0, 0, CERT_STORE_DEFER_CLOSE_UNTIL_LAST_FREE_FLAG, NULL); if(store == NULL) { return MBEDTLS_ERR_X509_FATAL_ERROR; From 5eeaab9eb004009a15c1597f77987108c6b3ce29 Mon Sep 17 00:00:00 2001 From: Tobiasz Laskowski Date: Mon, 25 Nov 2024 01:51:34 +0000 Subject: [PATCH 09/10] Clarify comment regarding error mapping --- libs/mbedtls/mbedtls_stubs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libs/mbedtls/mbedtls_stubs.c b/libs/mbedtls/mbedtls_stubs.c index 60778328ae4..102b1c03c30 100644 --- a/libs/mbedtls/mbedtls_stubs.c +++ b/libs/mbedtls/mbedtls_stubs.c @@ -335,7 +335,8 @@ static int verify_callback(void* param, mbedtls_x509_crt *crt, int depth, uint32 if(policy_status.dwError == 0) { *flags = 0; } else { - // TODO: properly map errors + // if we ever want to read the verification result, + // we need to properly map dwError to flags *flags |= MBEDTLS_X509_BADCERT_OTHER; } CertFreeCertificateChain(chain_context); From 6b84ade1bcdc5df2c668791e78574fef717aaeda Mon Sep 17 00:00:00 2001 From: Tobiasz Laskowski Date: Mon, 25 Nov 2024 01:58:33 +0000 Subject: [PATCH 10/10] Skip verification callback if no errors were found --- libs/mbedtls/mbedtls_stubs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/mbedtls/mbedtls_stubs.c b/libs/mbedtls/mbedtls_stubs.c index 102b1c03c30..f66484ccd48 100644 --- a/libs/mbedtls/mbedtls_stubs.c +++ b/libs/mbedtls/mbedtls_stubs.c @@ -304,7 +304,7 @@ static struct custom_operations ssl_config_ops = { #ifdef _WIN32 static int verify_callback(void* param, mbedtls_x509_crt *crt, int depth, uint32_t *flags) { - if (*flags & MBEDTLS_X509_BADCERT_CN_MISMATCH) { + if (*flags == 0 || *flags & MBEDTLS_X509_BADCERT_CN_MISMATCH) { return 0; }