Skip to content

vtls_scache: include cert_blob independently of verifypeer#21222

Closed
bagder wants to merge 3 commits into
masterfrom
bagder/ssl-session-blob
Closed

vtls_scache: include cert_blob independently of verifypeer#21222
bagder wants to merge 3 commits into
masterfrom
bagder/ssl-session-blob

Conversation

@bagder
Copy link
Copy Markdown
Member

@bagder bagder commented Apr 4, 2026

The making of the TLS session cache key should use the cert blob independently of verifypeer on/off.

Follow-up to fa0ccd9

Spotted by Codex Security

@bagder bagder requested review from Copilot and icing April 4, 2026 16:24
@github-actions github-actions Bot added the TLS label Apr 4, 2026
@bagder bagder marked this pull request as ready for review April 4, 2026 16:25
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

Adjusts TLS session cache peer key generation so that a configured client-certificate blob contributes to the cache key regardless of verifypeer being enabled, preventing unintended session reuse across differing client cert inputs.

Changes:

  • Moves inclusion of ssl->cert_blob into the peer key outside the ssl->verifypeer conditional.
  • Ensures the cert blob hash is always incorporated when present, aligning cache partitioning with actual TLS client-auth configuration.

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

Comment thread lib/vtls/vtls_scache.c
Copy link
Copy Markdown
Contributor

@icing icing left a comment

Choose a reason for hiding this comment

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

Makes sense. Maybe the name cert_blob did not connect in my brain to "client cert" at the time.

bagder added 2 commits April 8, 2026 09:34
The making of the TLS session cache key should use the cert blob
independently of verifypeer on/off.

Follow-up to fa0ccd9

Spotted by Codex Security
@bagder bagder force-pushed the bagder/ssl-session-blob branch from 7dd14ff to 0ac312f Compare April 8, 2026 07:48
@bagder bagder requested a review from icing April 8, 2026 07:52
@bagder
Copy link
Copy Markdown
Member Author

bagder commented Apr 8, 2026

augment review

@augmentcode
Copy link
Copy Markdown

augmentcode Bot commented Apr 8, 2026

🤖 Augment PR Summary

Summary: Updates TLS session cache peer key generation to always include the client certificate blob, even when peer verification is disabled.
Rationale: Prevents unintended session-cache key sharing across different in-memory client certificates when verifypeer is off.

🤖 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 lib/vtls/vtls_scache.c
@bagder bagder closed this in 698eee1 Apr 8, 2026
@bagder bagder deleted the bagder/ssl-session-blob branch April 8, 2026 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

3 participants