Skip to content

schannel: fix revoke_best_effort reading wrong ssl config#21683

Closed
MegaManSec wants to merge 1 commit into
curl:masterfrom
MegaManSec:schannel-revoke-best-effort-fix
Closed

schannel: fix revoke_best_effort reading wrong ssl config#21683
MegaManSec wants to merge 1 commit into
curl:masterfrom
MegaManSec:schannel-revoke-best-effort-fix

Conversation

@MegaManSec

Copy link
Copy Markdown
Contributor

schannel_verify.c line 808 reads data->set.ssl.revoke_best_effort unconditionally, ignoring whether the connection being verified is an origin or proxy connection.

ssl_config is already set at line 660 via Curl_ssl_cf_get_config(), which returns &data->set.proxy_ssl for proxy cfilters and &data->set.ssl for origin cfilters. Lines 752 and 792 in the same function already use ssl_config correctly; this makes line 808 consistent.

The impact is policy bleed: when CURLSSLOPT_REVOKE_BEST_EFFORT differs between CURLOPT_SSL_OPTIONS and CURLOPT_PROXY_SSL_OPTIONS, the wrong revocation policy is applied during proxy certificate verification.

Line 808 was reading from data->set.ssl.revoke_best_effort
unconditionally, bypassing the context-aware ssl_config pointer
already established at line 660 via Curl_ssl_cf_get_config().
Lines 752 and 792 in the same function already use ssl_config
correctly; make 808 consistent.
@github-actions github-actions Bot added TLS Windows Windows-specific labels May 19, 2026
@jay jay closed this in cce4d3b May 20, 2026
@jay

jay commented May 20, 2026

Copy link
Copy Markdown
Member

Thanks

outcast36 pushed a commit to greearb/curl that referenced this pull request Jun 3, 2026
- Fix revoke_best_effort reading wrong ssl config.

Prior to this change the revoke_best_effort setting for the proxy was
wrongly ignored in favor of the same setting for the destination host.

In other words, CURLSSLOPT_REVOKE_BEST_EFFORT set via
CURLOPT_PROXY_SSL_OPTIONS did not apply to the proxy and
CURLSSLOPT_REVOKE_BEST_EFFORT set via CURLOPT_SSL_OPTIONS wrongly
applied to the proxy.

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

Labels

TLS Windows Windows-specific

Development

Successfully merging this pull request may close these issues.

2 participants