Skip to content

transfer secure check#20951

Closed
icing wants to merge 7 commits intocurl:masterfrom
icing:xfer-secure-check
Closed

transfer secure check#20951
icing wants to merge 7 commits intocurl:masterfrom
icing:xfer-secure-check

Conversation

@icing
Copy link
Copy Markdown
Contributor

@icing icing commented Mar 17, 2026

Introduce Curl_xfer_is_secure(data) that returns TRUE for transfers that happen(ed) over a end-to-end secured connection, e.g. SSL.

Add test1586 to verify behaviour for http: transfers via a https: proxy.

Introduce `Curl_xfer_is_secure(data)` that returns TRUE for transfers
that happen(ed) over a end-to-end secured connection, e.g. SSL.

Add test1586 to verify behaviour for http: transfers via a https: proxy.
@icing icing added the TLS label Mar 17, 2026
@github-actions github-actions bot added the tests label Mar 17, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces an internal helper to consistently decide whether a transfer is end-to-end secure (based on the target scheme/TLS state), and uses it to gate security-sensitive behaviors (HSTS/Alt-Svc, channel binding, STARTTLS decisions). It also adds a regression test for the key scenario of http:// targets fetched via an https:// proxy.

Changes:

  • Add Curl_xfer_is_secure(data) and expose it via lib/transfer.h.
  • Switch several call sites (HTTP HSTS/Alt-Svc, HTTP Negotiate CBT, IMAP STARTTLS decision, HTTP/2 pseudo :scheme) to use the new helper.
  • Add test1586 and wire it into the test suite to verify HSTS is ignored for http:// over an HTTPS proxy.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/data/test1586 New test ensuring HSTS headers are ignored for http:// URLs when using an https:// proxy.
tests/data/Makefile.am Adds test1586 to the autotools test data list.
lib/transfer.h Declares the new Curl_xfer_is_secure() helper.
lib/transfer.c Implements Curl_xfer_is_secure() based on connection scheme and TLS state.
lib/imap.c Uses the helper when deciding whether to initiate STARTTLS.
lib/http_negotiate.c Uses the helper to decide whether to collect TLS channel binding data for Negotiate auth.
lib/http.c Uses the helper to gate HSTS/Alt-Svc processing and to derive HTTP/2 pseudo :scheme.
Comments suppressed due to low confidence (1)

lib/http_negotiate.c:132

  • In proxy Negotiate auth mode (proxy == true), Curl_xfer_is_secure(data) can be FALSE for an http:// URL even when the proxy connection itself is TLS-secured (eg -x https://...). That would skip CBT/channel-binding generation for proxy authentication, which should be based on the security of the authenticated peer (the proxy), not the target URL scheme. Consider using Curl_conn_is_ssl(conn, FIRSTSOCKET) when proxy is true, and Curl_xfer_is_secure(data) only for origin-server authentication.
  curlx_dyn_init(&neg_ctx->channel_binding_data, SSL_CB_MAX_SIZE + 1);
  if(Curl_conn_is_ssl(conn, FIRSTSOCKET)) {
    result = Curl_ssl_get_channel_binding(data, FIRSTSOCKET,
                                          &neg_ctx->channel_binding_data);
    if(result) {
      http_auth_nego_reset(conn, neg_ctx, proxy);
      return result;
    }
  }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

@bagder bagder closed this in adda113 Mar 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

4 participants