ssl config: adjust to origin#21695
Open
icing wants to merge 4 commits into
Open
Conversation
When a transfer goes against another origin than the initial one, do not add the following to the ssl configuration: client cert, client key, srp user/pass, pinned key.
Member
|
Replaces #21690 as well, right? |
Contributor
Author
Once I get it working, yes. |
There was a problem hiding this comment.
Pull request overview
Adjusts how libcurl derives per-connection SSL “primary” configuration from an easy handle so that credentials and pinning material are not implicitly carried over when a transfer targets a different origin than the initial request (e.g., redirects to another host).
Changes:
- Refactors SSL config cloning/matching/cleanup helpers out of
vtls.cinto a newvtls_config.[ch]module. - Changes
Curl_ssl_easy_config_complete()to take anoriginand omit client cert/key, SRP user/pass, and pinned public key whenorigin != data->state.initial_origin. - Updates call sites and unit test wiring to pass/construct an origin.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/unit3303.c | Adapts unit test to pass an origin into SSL config completion. |
| lib/vtls/vtls.h | Removes SSL config helper declarations (moved to new header). |
| lib/vtls/vtls.c | Removes SSL config helper implementations (moved to new source file). |
| lib/vtls/vtls_config.h | New header for SSL config structs and helper APIs. |
| lib/vtls/vtls_config.c | New implementation of SSL config init/complete/clone/match/cleanup logic, including origin-based credential omission. |
| lib/urldata.h | Moves SSL config struct definitions to vtls_config.h via include. |
| lib/url.c | Initializes/cleans up SSL primary configs and passes origin into SSL config completion before connection reuse. |
| lib/Makefile.inc | Adds new vtls_config.c/h to the build inputs. |
Comments suppressed due to low confidence (1)
tests/unit/unit3303.c:75
- The PR’s core behavior change (clearing mTLS/SRP/pinned-key settings when
origin != initial_origin) is not exercised by this unit test: it only setsinitial_origin == origin. Consider adding a second peer with a different origin, callingCurl_ssl_easy_config_complete(..., other_origin), and asserting that the resultingdata->set.ssl.primary(and clonedconn->ssl_config) has client cert/key/SRP/pinned-key cleared and does not match a connection configured with those credentials.
result = Curl_peer_create((struct Curl_easy *)curl,
&Curl_scheme_https,
"test.curl.se", 1234, &origin);
if(result) {
curl_easy_cleanup(curl);
curl_global_cleanup();
goto unit_test_abort;
}
Curl_peer_link(&((struct Curl_easy *)curl)->state.initial_origin, origin);
curl_easy_setopt(curl, CURLOPT_SSLCERT, "client.pem");
curl_easy_setopt(curl, CURLOPT_SSLKEY, "client.key");
curl_easy_setopt(curl, CURLOPT_KEYPASSWD, "secret");
curl_easy_setopt(curl, CURLOPT_SSLCERTTYPE, "PEM");
curl_easy_setopt(curl, CURLOPT_SSLKEYTYPE, "PEM");
if(Curl_ssl_easy_config_complete((struct Curl_easy *)curl, origin)) {
Curl_peer_unlink(&origin);
curl_easy_cleanup(curl);
curl_global_cleanup();
goto unit_test_abort;
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a transfer goes against another origin than the initial one, do not add the following to the ssl configuration: client cert, client key, srp user/pass, pinned key.