Can you fix up CI? Let's drop the separate examples for now.

Can you fix up CI?

Sure thing. I fixed bogo locally, just haven't pushed it yet. For semver should I bump the version in the cargo manifest?

Let's drop the separate examples for now.

👍

@djc okay, I've verified that bogo passes locally. For some reason runme exits with 0 (on my machine) even when tests fail.

I don't think we're in a state where we want to merge semver-incompatible changes. As such:

• We can't add a HandshakeKind variant right now
• We can't remove a PeerMisbehaved variant right now (but we can #[deprecate] it and stop emitting it)

I don't think we're in a state where we want to merge semver-incompatible changes. As such:

• We can't add a HandshakeKind variant right now

This is probably fine since rustls doesn't make any important decisions with HandshakeKind, but if we don't add a new variant then it's unclear which variant we should return when external PSKs are used. HandshakeKind::Resumed is the closest, but external PSKs are different from session resumption. Specifically, you can use an external PSK without ever having previously performed a handshake.

Okay, that is a lot of stuff...

I think this needs a bunch of work to clean it up. There should be a clean commit history (potentially as separate PRs so we can merge some of it sooner). I also think there should be a bunch of refactoring here so that the PSK/resumption code is more isolated from the rest of the handshake.

Let me know if you'd like me to roll up the commit history. I typically wait to squash commits before committing so it doesn't disrupt code review.

Let me know which parts you'd like moved to separate PRs, if any. I tried to make this PR as minimal as possible, but some refactoring (particularly of resumption handling) was necessary, otherwise the code would've been terribly difficult to follow, which is unacceptable for security critical code.

Codecov Report

Attention: Patch coverage is 90.00876% with 114 lines in your changes missing coverage. Please review.

Project coverage is 95.62%. Comparing base (47ed0c6) to head (648a025).
Report is 7 commits behind head on main.

@@            Coverage Diff             @@
##             main    #2424      +/-   ##
==========================================
- Coverage   95.96%   95.62%   -0.34%     
==========================================
  Files          94       94              
  Lines       22634    23349     +715     
==========================================
+ Hits        21721    22328     +607     
- Misses        913     1021     +108     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

I don't think we're in a state where we want to merge semver-incompatible changes. As such:

• We can't add a HandshakeKind variant right now

This is probably fine since rustls doesn't make any important decisions with HandshakeKind, but if we don't add a new variant then it's unclear which variant we should return when external PSKs are used. HandshakeKind::Resumed is the closest, but external PSKs are different from session resumption. Specifically, you can use an external PSK without ever having previously performed a handshake.

I think using Resumed is probably fine, if you add documentation on the variant that it can be used this way -- and file an issue to make HandshakeKind #[non_exhaustive] in the future.

Let me know if you'd like me to roll up the commit history. I typically wait to squash commits before committing so it doesn't disrupt code review.

Let me know which parts you'd like moved to separate PRs, if any. I tried to make this PR as minimal as possible, but some refactoring (particularly of resumption handling) was necessary, otherwise the code would've been terribly difficult to follow, which is unacceptable for security critical code.

We typically work with smallish commits that each compile and can pass CI (though we don't usually test whether exactly each commit does). I suggest you break this up into smaller pieces, especially by (1) splitting refactoring into separate commits, (2) splitting client from server. Also try to minimize unrelated changes (like renaming secret to psk). We don't usually incrementally review, so keeping all WIP commits around just makes things harder.

This is a pretty large amount of code to review. Maybe your organization could consider sponsoring the integration effort?

This is a pretty large amount of code to review. Maybe your organization could consider sponsoring the integration effort?

What would this cost/involve?

This is a pretty large amount of code to review. Maybe your organization could consider sponsoring the integration effort?

What would this cost/involve?

Send me an email? You can trivially find my address from my homepage. Feel free to include a proposal for what you would consider reasonable.

Going to close this for now, pending future work on PSK support.