Skip to content

[TLS 1.3] Extend CredentialsManager for upcoming TLS 1.3 PSK#3622

Merged
randombit merged 3 commits intorandombit:masterfrom
Rohde-Schwarz:tls13/creds_mgr_for_psk
Jul 18, 2023
Merged

[TLS 1.3] Extend CredentialsManager for upcoming TLS 1.3 PSK#3622
randombit merged 3 commits intorandombit:masterfrom
Rohde-Schwarz:tls13/creds_mgr_for_psk

Conversation

@reneme
Copy link
Copy Markdown
Collaborator

@reneme reneme commented Jul 11, 2023

This attempts to step away from the generic Credentials_Manager::psk() method that is parameterized with type and context. Instead, the class now has dedicated getter-methods for the specific keys it manages.

Before After
type:"tls-server", context:"session-ticket" Credentials_Manager::session_ticket_key()
type:"tls-server", context:"dtls-cookie-secret" Credentials_Manager::dtls_cookie_secret()
type"tls-server"/"tls-client", context:"my.host.name" Credentials_Manager::find_preshared_keys()

The TLS 1.2 implementation keeps using the ::psk() method as before -- so existing applications that override it will continue to work as before. We think, dedicated methods for specific artefacts is much more on-par with the other TLS dependency classes (e.g. Callbacks and Policy). Additionally, its easier to understand the API like that.

New applications can make use of the added default implementation of ::psk() that orchestrates the new dedicated methods as shown in the table above. With Botan 4.0 the ::psk() method should probably disappear entirely. (And Credentials_Manager should perhaps move into the TLS namespace.)

The new methods find_preshared_keys() and choose_preshared_key() take into account that one can offer multiple PSKs in a TLS 1.3 Client Hello. The server then gets to choose which PSK it wants to negotiate with.

Given that TLS 1.2 pre-negotiates a singular PSK identity prior to actually requesting the actual PSK-value, it can benefit from the same find_preshared_keys() (through the legacy psk()). If exactly one PSK identity is requested, it should be guaranteed that at most one PSK is returned by find_preshared_keys().

The actual TLS 1.3 PSK implementation will come in a follow-up.

@reneme reneme mentioned this pull request Jul 11, 2023
Copy link
Copy Markdown
Collaborator Author

@reneme reneme left a comment

Choose a reason for hiding this comment

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

Self-review for tomorrow.

@coveralls
Copy link
Copy Markdown

coveralls commented Jul 11, 2023

Coverage Status

coverage: 91.706% (-0.03%) from 91.735% when pulling c575c31 on Rohde-Schwarz:tls13/creds_mgr_for_psk into 054e13f on randombit:master.

This introduces new explicit methods for the session ticket key, the
DTLS hello cookie and a more generic PSK search method.

The existing psk() method now has a default implementation that
orchestrates the new methods via the `context` and `type` parameters
just like before.

Co-Authored-By: Fabian Albert <fabian.albert@rohde-schwarz.com>
@reneme reneme force-pushed the tls13/creds_mgr_for_psk branch from 21efe5b to 8d1c89c Compare July 12, 2023 06:28
Copy link
Copy Markdown
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.

LGTM


// New applications should use the appropriate credentials methods. This is a
// retrofit of the behaviour before Botan 3.2.0 and will be removed in a
// future major release.
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

We should note this in doc/deprecated.rst

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done: c575c31

Please merge at will, if the text in doc/deprecated.rst is sufficient.

@randombit randombit merged commit 65b7548 into randombit:master Jul 18, 2023
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