Skip to content

Conversation

@jhoyla
Copy link
Contributor

@jhoyla jhoyla commented May 6, 2025

Add support for sending and receiving TLS Flags, and support for the request mTLS flag.

@jhoyla jhoyla force-pushed the jhoyla/tls-flags-rebase branch from e05cbae to 5324115 Compare May 6, 2025 15:35
@bwesterb
Copy link
Member

bwesterb commented May 6, 2025

Confirmed except for the new commit it's a rebase.

$ git range-diff cf/jhoyla/tls-flags^..cf/jhoyla/tls-flags  cf/cf..jonathan/jhoyla/tls-flags-rebase 
1:  4092d3f479 = 1:  f46052fa94 Implement TLS Flags and the request mTLS flag.
-:  ---------- > 2:  5324115d20 Fix nits

@jhoyla jhoyla force-pushed the jhoyla/tls-flags-rebase branch from 5324115 to e13e0de Compare May 6, 2025 16:10
@jhoyla jhoyla force-pushed the jhoyla/tls-flags-rebase branch 2 times, most recently from d64905b to 08059b3 Compare May 6, 2025 16:24
Copy link
Contributor

@cjpatton cjpatton left a comment

Choose a reason for hiding this comment

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

Looks good! I have some comments on style, plus some improvements for testing I'd strongly recommend. Feel free to merge when you're happy.

Comment on lines +945 to +949
for _, flag := range hs.tlsFlags {
if flag == ExperimentalFlagSupportMTLS {
return true
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion, feel free to ignore: I feel like it ought to be possible to do this without iterating over the set flags. Can't we just do some bit fiddling on the the encoded TLS flags to see if the req mTLS flag is set?

@jhoyla jhoyla force-pushed the jhoyla/tls-flags-rebase branch from e5a732d to 7d3a695 Compare May 7, 2025 09:23
@bwesterb bwesterb merged commit 37bc41c into cloudflare:cf May 7, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants