Conversation
This was referenced Feb 20, 2023
randombit
approved these changes
Feb 23, 2023
Owner
randombit
left a comment
There was a problem hiding this comment.
A few minor comments, but looks good, thanks
doc/migration_guide.rst
Outdated
|
|
||
| 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. |
doc/migration_guide.rst
Outdated
| """"""""""""""""""""""" | ||
|
|
||
| The parameter `ocsp_responses` is now a vector of `std::optional<OCSP::Response>` | ||
| rather than potentially null `std::shared_ptr<OCSP::Response>`. |
Owner
There was a problem hiding this comment.
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 previouslystd::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). |
Owner
There was a problem hiding this comment.
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.
ff4c72f to
7708cab
Compare
Collaborator
Author
|
Thanks. There will be more edits re: the session management. But merging for now. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.