Skip to content
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.

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

[TLS 1.3] Update Migration Guide #3308

Merged
merged 1 commit into from
Feb 24, 2023
Merged

[TLS 1.3] Update Migration Guide #3308

merged 1 commit into from
Feb 24, 2023

Conversation

reneme
Copy link
Collaborator

@reneme reneme commented Feb 20, 2023

That's merely a brain dump of the current state of incompatible API changes in the TLS stack. Note that it includes the (currently unmerged) changes in #3150 and #3140 (which are ready for review, btw. 😉). Feel free to edit my English. 😅

I'll do a review of the APIs in the coming days and might amend this as necessary.

Copy link
Owner

@randombit randombit left a comment

Choose a reason for hiding this comment

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

A few minor comments, but looks good, thanks


Starting with Botan 3.0 TLS 1.3 is supported.
This development required a number of backward-incompatible changes to
accomodate the protocol differences to TLS 1.2 which is still supported.
Copy link
Owner

Choose a reason for hiding this comment

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

", which is still supported"

"""""""""""""""""""""""

The parameter `ocsp_responses` is now a vector of `std::optional<OCSP::Response>`
rather than potentially null `std::shared_ptr<OCSP::Response>`.
Copy link
Owner

Choose a reason for hiding this comment

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

This is confusingly worded IMO in that it sounds like we are guaranteeing that the std::optionals are not nullopt, which is not the case, right?

Might be easier to just strike the "rather than potentially null":

The parameter ocsp_responses, which was previously std::shared_ptr<OCSP::Response>, is now
std::optional<OCSP::Response>

"""""""""""""""""""""""""""

The new parameter `offered_by_peer` identifies the key exchange groups a peer
has sent public exchange offerings for (in TLS 1.3 handshakes only).
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe include something like "In TLS 1.2, this vector is always empty" just so it's clear that this function is still called in TLS 1.2 (IIUC), just missing this information.

@reneme
Copy link
Collaborator Author

reneme commented Feb 24, 2023

Thanks. There will be more edits re: the session management. But merging for now.

@reneme reneme merged commit 703d99c into master Feb 24, 2023
@randombit randombit deleted the tls13/migration_guide branch February 24, 2023 16:14
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.

2 participants