Skip to content

Conversation

@solarispika
Copy link
Contributor

@solarispika
Copy link
Contributor Author

I noticed that there is verify_result_ for storing OpenSSL result and is retrieved by get_openssl_verify_result(), but I have no idea how to set it. Please advise.

@yhirose
Copy link
Owner

yhirose commented Jun 24, 2025

@solarispika sorry for the delay. According to this comment #1978 (comment), you mentioned you ended up bypassing CRL in your production server.

Do you think that the current pull request which doesn't have the bypassing code will affect a number of Windows users? If not many, I don't mind merging this code. But it has a risk to affect many users, I would like you to implement a feature flag like CPPHTTPLIB_USE_WINDOWS_AUTOMATIC_ROOT_CERTIFICATES_UPDATE. If it's not set, the original code will be used.

@solarispika
Copy link
Contributor Author

Hi @yhirose

I am not sure how many of them will be, possibly the number being proportional to users located in China.
I can add a toggle for this feature.
It looks like you prefer users to enable this feature, but not users to disable it, right?

@yhirose
Copy link
Owner

yhirose commented Jul 2, 2025

@solarispika , (1) If a number of users will be affected by this, I prefer making it an opt-in feature with CPPHTTPLIB_USE_WINDOWS_AUTOMATIC_ROOT_CERTIFICATES_UPDATE.

(2) But if we expect only few users will be affected, we can enable this feature by the default and uses can disable it with CPPHTTPLIB_DISABLE_WINDOWS_AUTOMATIC_ROOT_CERTIFICATES_UPDATE.

I prefer #2.

@solarispika solarispika force-pushed the default-using-windows-schannel branch from f97e72c to 4661630 Compare July 7, 2025 03:32
@solarispika
Copy link
Contributor Author

@yhirose
I agree with you on enabling the feature by default.
I also update CMake which enables the feature HTTPLIB_USE_WINDOWS_AUTOMATIC_ROOT_CERTIFICATES_UPDATE by default, and if disabled, define CPPHTTPLIB_DISABLE_WINDOWS_AUTOMATIC_ROOT_CERTIFICATES_UPDATE.

@solarispika solarispika force-pushed the default-using-windows-schannel branch from 4661630 to bb7c085 Compare July 7, 2025 05:13
@yhirose
Copy link
Owner

yhirose commented Jul 8, 2025

Could you please take a look at unit test errors on 'test / windows with SSL (pull_request)'?

@solarispika
Copy link
Contributor Author

Sure, it looks like #2169 saves openssl errors which I didn't notice.
I'll try to fix it.

@solarispika
Copy link
Contributor Author

@yhirose

I found that it is hard to map errors between Win32 API and OpenSSL.

What do you recommend? Is it proper to mask those checks when Schannel is used?

@yhirose
Copy link
Owner

yhirose commented Jul 8, 2025

@solarispika I actually don't know what do to. Could you please investigate why those errors occur before making any change?

@solarispika
Copy link
Contributor Author

@yhirose
These errors are due to #2169 adding ssl_error and ssl_openssl_error member functions to class Result for retrieving OpenSSL errors, and some tests being extended for these errors.

As those member functions are defined for OpenSSL, it is inappropriate for me to use it directly for errors coming from Windows API.


if (verification_status == SSLVerifierResponse::NoDecisionMade) {
#if !defined(_WIN32) || \
defined(CPPHTTPLIB_DISABLE_WINDOWS_AUTOMATIC_ROOT_CERTIFICATES_UPDATE)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@solarispika why does this check need to be skipped when CPPHTTPLIB_DISABLE_WINDOWS_AUTOMATIC_ROOT_CERTIFICATES_UPDATE is defined?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this check and check from Windows are both checking certificate, so it is a either-or behavior.

So, if Windows automatic root certificate updates is disabled, then using old behavior; otherwise, use new one.

Am I missing something?

@yhirose yhirose added the windows Windows specific topic label Aug 28, 2025
@yhirose
Copy link
Owner

yhirose commented Aug 29, 2025

@solarispika it looks that the Windows code returns only SSLServerVerification and SSLServerHostnameVerification which are kind of general errors. However, it seems like some of wincrypt APIs can tell us more about errors, and they could be helpful for users. How about to make wincrypt_verify_result_ and get_wincrypt_verify_result() to store such a wincrypt specific error? Is it too much?

@solarispika
Copy link
Contributor Author

@solarispika it looks that the Windows code returns only SSLServerVerification and SSLServerHostnameVerification which are kind of general errors. However, it seems like some of wincrypt APIs can tell us more about errors, and they could be helpful for users. How about to make wincrypt_verify_result_ and get_wincrypt_verify_result() to store such a wincrypt specific error? Is it too much?

@yhirose I have no experience using those APIs, so I can't give any suggestions on it now.
It needs further investigations, and I think they might be useful.

@BLesnau
Copy link

BLesnau commented Nov 17, 2025

@solarispika @yhirose I have a project that I think would benefit from these changes. I may have some time to help out if there is anything else that is needed to get this merged. Is there anything specific that you have in mind? I did some testing with this change and updating "normal" certificates on my machine. I saw it use updated certificates automatically as expected. I did not specifically test with root certificates yet though, so I did not yet see that working, but it seems like it should.

I see that a couple of the checks failed, but the logs are now gone, so I can't tell what went wrong. I didn't see a way to manually re-run those actions. I was thinking of making a commit change to have the actions run again, but I didn't want to mess with this PR without warning or make my own fork and PR just to see the logs.

@ponie419-max
Copy link

ponie419-max commented Nov 18, 2025 via email

@BLesnau
Copy link

BLesnau commented Nov 18, 2025

I was trying to test some of this myself, and I think I may not know enough about how to test it properly. I tried with self-signed certs. I tried reproducing the problem first, but the certificate was always available as long and I closed and reopened my app that uses cpp-httplib after I imported the cert. I tried importing it specifically as a root cert for LocalMachine, but it is always being propogated to both LocalMachine and CurrentUser no matter how I try to import the cert. Maybe I am going about the testing all wrong. I'm pretty new to this with the certs.

Any tips on how to test it would be great. Even if testing isn't straightforward, I'm still good with helping to get this PR moving closer to being merged.

@ponie419-max
Copy link

ponie419-max commented Nov 18, 2025 via email

@solarispika
Copy link
Contributor Author

@BLesnau The error come from ssl and openssl errors are not set correspondingly if I remember correctly.
@yhirose Per your suggestion, I think recording SChannel errors is great.
SChannel errors can be retrieved by GetLastError() and can be stored separately, wincrypt_error_ and wincrypt_chain_error_.
SSL errors are cleared to 0 before starting SChannel's verification.
Users can check for Error::SSLServerVerification and use those error for debug.

@solarispika solarispika force-pushed the default-using-windows-schannel branch from bb7c085 to 30669cc Compare November 19, 2025 11:41
solarispika and others added 2 commits November 19, 2025 19:48
This commit enables Windows Schannel for certificate verification on Windows
platforms, providing automatic root certificate updates from Windows Update.

Key changes:
- Added Windows Schannel certificate verification using CertGetCertificateChain
  and CertVerifyCertificateChainPolicy APIs
- Implemented wincrypt_error() and wincrypt_chain_error() to expose Windows
  certificate errors alongside existing OpenSSL error fields
- Clear ssl_openssl_error before Windows verification to provide unambiguous
  error source indication (0 means Windows error, non-zero means OpenSSL error)
- Updated tests with platform-specific assertions for Windows vs OpenSSL errors
- Added comprehensive Windows certificate error documentation to README

Certificate verification flow on Windows:
1. OpenSSL performs TLS handshake (can set ssl_error)
2. Windows Schannel verifies certificate chain (sets wincrypt_error and
   wincrypt_chain_error)
3. Users check wincrypt_error() for Windows-specific error codes like
   CERT_E_EXPIRED, CERT_E_UNTRUSTEDROOT, CERT_E_REVOKED, etc.

Feature can be disabled with CPPHTTPLIB_DISABLE_WINDOWS_AUTOMATIC_ROOT_CERTIFICATES_UPDATE
or by setting HTTPLIB_USE_WINDOWS_AUTOMATIC_ROOT_CERTIFICATES_UPDATE=OFF in CMake.

Fixes yhirose#1978

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@solarispika solarispika force-pushed the default-using-windows-schannel branch from 30669cc to b8e2cba Compare November 19, 2025 11:54
@BLesnau
Copy link

BLesnau commented Nov 20, 2025

@solarispika @yhirose As part of looking at this PR, I was trying to understand more about how cpp-httplib does certain things (very new to me). Feel free to tell me to bring this discussion elsewhere if it's not related enough to this PR.

Without these Schannel changes, does cpp-httplib use the certs on Windows already or is it only the certs that are part of OpenSSL when it was compiled? Based on my testing, it does look like the Windows certs are retrieved when the client initializes. If cpp-httplib does use the Windows certs at init time, is there any specific code I should look at to understand it better?

Edit: I think I found the relevant code with the load_system_certs_on_windows() method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

windows Windows specific topic

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Proposal: Default to Windows Automatic Root Certificates Update for Improved User Experience

4 participants