Skip to content

v0.22.1 preparation #1677

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 15 commits into from
Dec 8, 2023
Merged

v0.22.1 preparation #1677

merged 15 commits into from
Dec 8, 2023

Conversation

cpu
Copy link
Member

@cpu cpu commented Dec 7, 2023 •
edited
Loading

Description

This branch targets the rel-0.22 feature branch, which was created at commit e051f5c - the last semver compatible update to main after the 0.22 release.

It brings in the following PRs since then:

It skips these PRs from main that I believe are semver incompatible:

Proposed Release Notes

  • TLS 1.2 servers now remove session tickets after observing a failure to decrypt, preventing future resumption with the same unusable ticket.
  • The rustls_pki_types crate is now re-exported as rustls::pki_types.
  • The crate examples examples have been updated to use the rustls::pki_types re-export.
  • The quic::PacketKey and quic::HeaderProtectionKey traits are now Send + Sync.
  • Performance improvements to the MessageDeframer internals.
  • Small documentation improvements.

omegablitz and others added 2 commits December 7, 2023 09:53
@omegablitz @cpu
doc: fix outdated documentation
b4e99cc
@Geal @cpu
remove the TLS 1.2 session ticket on DecryptError
652d46c 
if for some reason the recorded session ticket is invalid or decoded
incorrectly by the server, we can get into a case where the resumption
handshake happens, and right after the ChangeCipherSpec message, the
server sends an encrypted handhsake message using the invalid ticket,
and the client rejects it with the BadRecordMAC alert.
Unfortunately, if the calling code retries the connection, if it will
try again with the same ticket and obtain the same result.
This commit makes sure that if we fail to decrypt the first message, we
will remove the session ticket for this server, to start from cratch on
the next connection.
@cpu cpu self-assigned this Dec 7, 2023
@cpu cpu mentioned this pull request Dec 7, 2023
Merged
@cpu cpu changed the base branch from main to rel-0.22 December 7, 2023 15:03
@djc
Copy link
Member

djc commented Dec 7, 2023 •
edited
Loading

As mentioned in #1665, I think we should bring that in even though it is technically semver-incompatible. Also I don't think #1595 is semver-incompatible (just looked it over, I think there was also a comment from @ctz stating it only made incompatible changes to the internals API). Not sure if we want to backport it? Might improve performance a bit.

@cpu
Copy link
Member Author

cpu commented Dec 7, 2023

As mentioned in #1665, I think we should bring that in even though it is technically semver-incompatible

Oops, missed that comment. I'll pull that in.

Also I don't think #1595 is semver-incompatible (just looked it over, I think there was also a comment from @ctz stating it only made incompatible changes to the internals API).

Right again. I'll add it to the branch 👍

japaric and others added 11 commits December 7, 2023 10:30
@cpu cpu force-pushed the cpu-0.22.1-prep branch from d6bed10 to 04de127 Compare December 7, 2023 15:30
@cpu
Copy link
Member Author

cpu commented Dec 7, 2023

I've updated the branch to bring in the quic trait and message deframer changes. The PR description and proposed release notes are updated as well.

@codecov Codecov
Copy link

codecov bot commented Dec 7, 2023 •
edited
Loading

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (e051f5c) 95.90% compared to head (7b2171f) 95.90%.

Files Patch % Lines
rustls/src/msgs/deframer.rs 97.14% 6 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           rel-0.22    #1677    +/-   ##
==========================================
  Coverage     95.90%   95.90%            
==========================================
  Files            78       78            
  Lines         16083    16211   +128     
==========================================
+ Hits          15424    15547   +123     
- Misses          659      664     +5

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

@ctz
Copy link
Member

ctz commented Dec 7, 2023

As mentioned in #1665, I think we should bring that in even though it is technically semver-incompatible.

Technically, but I think it is the only way to get valid code to compile, so 🤷🏽‍♂️

I guess this makes rel-0.22 = main, but minus b2cd886?

@cpu
Copy link
Member Author

cpu commented Dec 7, 2023

I guess this makes rel-0.22 = main, but minus b2cd886?

There are a couple other changes absent from this branch (enumerated in the PR desc): The provider example no-std work (and follow-up tweaks), as well as the change marking SupportedProtocolVersion non-exhaustive.

@djc
Copy link
Member

djc commented Dec 7, 2023

Well, also #1673 (and #1672, of course).

@cpu
Copy link
Member Author

cpu commented Dec 7, 2023

#1668 might be a good candidate for a 0.21.x release too. WDYT?

@djc
Copy link
Member

djc commented Dec 7, 2023

I think we should limit 0.21.x to bugfixes only, which #1668 doesn't qualify as?

@cpu
Copy link
Member Author

cpu commented Dec 7, 2023

It smells like a bug to me (a client getting stuck in a retry loop), but I don't feel strongly and if nobody is asking for it I'm happy to skip the extra work.

@djc
Copy link
Member

djc commented Dec 7, 2023

Uh, sorry, I was confused. But you already have #1668 in your list of things that you included in the PR description?

@cpu
Copy link
Member Author

cpu commented Dec 7, 2023

But you already have #1668 in your list of things that you included in the PR description?

Right, for this 0.22.1 release. I'm suggesting it might make sense to also create a separate backport branch with just that PR for a 0.21.10 bugfix release. Does that make sense?

@djc
Copy link
Member

djc commented Dec 7, 2023

Uggh, my brain is a bit fried I guess. Yeah, I think that makes sense.

@cpu
Copy link
Member Author

cpu commented Dec 7, 2023 •
edited
Loading

Yeah, I think that makes sense.

#1678

@cpu cpu closed this Dec 7, 2023
@cpu cpu reopened this Dec 7, 2023
@cpu cpu mentioned this pull request Dec 7, 2023
Merged
djc
djc approved these changes Dec 8, 2023
View reviewed changes
Copy link
Member

@djc djc left a comment

Choose a reason for hiding this comment

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

(Including the addition of #1679.)

jsha and others added 2 commits December 8, 2023 08:47
@jsha @cpu
update examples to use pki_types re-export
8b597f8 
We now re-export the rustls-pki-types crate. I think that means
our preferred way for crates to consume pki-types is through the
re-exports.
@cpu
Cargo: version 0.22.0 -> 0.22.1
7b2171f
@cpu cpu force-pushed the cpu-0.22.1-prep branch from b5d0172 to 7b2171f Compare December 8, 2023 13:47
@cpu
Copy link
Member Author

cpu commented Dec 8, 2023

(Including the addition of #1679.)

Added. PR desc and proposed release notes updated.

@cpu cpu enabled auto-merge December 8, 2023 13:48
@cpu cpu added this pull request to the merge queue Dec 8, 2023
Merged via the queue into rustls:rel-0.22 with commit 62751aa Dec 8, 2023
@cpu cpu deleted the cpu-0.22.1-prep branch December 8, 2023 14:04
@cpu
Copy link
Member Author

cpu commented Dec 8, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@djc djc djc approved these changes

Assignees

@cpu cpu

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@cpu @djc @ctz @omegablitz @Geal @japaric @pvdrz @jsha