Skip to content

getinfo: repair CURLINFO_TLS_SESSION#21290

Closed
bagder wants to merge 9 commits into
masterfrom
bagder/TLS-info
Closed

getinfo: repair CURLINFO_TLS_SESSION#21290
bagder wants to merge 9 commits into
masterfrom
bagder/TLS-info

Conversation

@bagder
Copy link
Copy Markdown
Member

@bagder bagder commented Apr 10, 2026

This should return a SSL_CTX pointer but it was accidentally broken.

Follow-up to 2db8ae4

Spotted by Codex Security

This should return a SSL_CTX pointer but it was accidentally broken.

Follow-up to 2db8ae4

Spotted by Codex Security
@bagder bagder requested a review from icing April 10, 2026 13:56
@bagder bagder requested a review from Copilot April 10, 2026 14:00
@bagder bagder marked this pull request as ready for review April 10, 2026 14:04
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

Fixes curl_easy_getinfo() handling of CURLINFO_TLS_SESSION so it again returns the legacy TLS “session” internal pointer (e.g., SSL_CTX* for OpenSSL / WOLFSSL_CTX* for wolfSSL), which had regressed and started returning the same pointer as CURLINFO_TLS_SSL_PTR.

Changes:

  • Select the appropriate cfilter query (CF_QUERY_SSL_INFO vs CF_QUERY_SSL_CTX_INFO) depending on whether CURLINFO_TLS_SSL_PTR or CURLINFO_TLS_SESSION is requested.
  • Extend Curl_conn_get_ssl_info() to accept a query parameter and pass it through to the connection filter chain.

Reviewed changes

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

File Description
lib/getinfo.c Uses CF_QUERY_SSL_CTX_INFO for CURLINFO_TLS_SESSION to restore SSL_CTX/WOLFSSL_CTX semantics.
lib/cfilters.h Updates Curl_conn_get_ssl_info() declaration to accept a query selector.
lib/cfilters.c Updates Curl_conn_get_ssl_info() definition to forward the requested query type to cft->query().

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

Comment thread lib/cfilters.h
Comment thread lib/getinfo.c
@github-actions github-actions Bot added the tests label Apr 10, 2026
@bagder
Copy link
Copy Markdown
Member Author

bagder commented Apr 10, 2026

augment review

@augmentcode
Copy link
Copy Markdown

augmentcode Bot commented Apr 10, 2026

🤖 Augment PR Summary

Summary: Repairs the legacy behavior of CURLINFO_TLS_SESSION so it again returns an SSL_CTX* (instead of the active SSL*) for OpenSSL-family backends, matching the documented contract.

Changes:

  • Extends Curl_conn_get_ssl_info() to accept a cfilter query selector and forwards it to the filter chain.
  • Updates getinfo_slist() to request CF_QUERY_SSL_CTX_INFO for CURLINFO_TLS_SESSION and CF_QUERY_SSL_INFO for CURLINFO_TLS_SSL_PTR.
  • Documents the new query parameter in lib/cfilters.h.
  • Adds regression test 1587 (data file + libtest) that exercises both info values on OpenSSL by calling SSL_CTX_* vs SSL_* APIs on the returned pointer.
  • Wires the new test into the tests Makefiles.

Technical Notes: Uses the existing cfilter query() mechanism (CF_QUERY_SSL_INFO/CF_QUERY_SSL_CTX_INFO) to select between per-connection TLS handle vs TLS context when the backend differentiates.

🤖 Was this summary useful? React with 👍 or 👎

Copy link
Copy Markdown

@augmentcode augmentcode Bot left a comment

Choose a reason for hiding this comment

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

Review completed. 1 suggestion posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

Comment thread tests/libtest/lib1587.c
@bagder bagder closed this in 3e40ccb Apr 11, 2026
@bagder bagder deleted the bagder/TLS-info branch April 11, 2026 14:56
dkarpov1970 pushed a commit to dkarpov1970/curl that referenced this pull request Apr 13, 2026
This should return a SSL_CTX pointer but it was accidentally broken.

Verify with test 1587

Follow-up to 2db8ae4

Spotted by Codex Security

Closes curl#21290
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.

2 participants