Skip to content

[TLS 1.3] Signature_Scheme Class#2968

Merged
reneme merged 3 commits intodev/tls-13from
tls13/signature_scheme_class
Jun 1, 2022
Merged

[TLS 1.3] Signature_Scheme Class#2968
reneme merged 3 commits intodev/tls-13from
tls13/signature_scheme_class

Conversation

@reneme
Copy link
Copy Markdown
Collaborator

@reneme reneme commented Apr 27, 2022

Reincarnation of this pull request

@hrantzsch accidentally opened it from his private Botan fork. As this doesn't allow me to do amendments without his help (while he is enjoying his PTO), I reopened the PR here. Note that I re-posted the open discussions from the old PR.

Pull Request Dependencies

This is based on the basic TLS 1.3 implementation, should be tracked along with it and re-targeted to master once the underlying pull request is merged.

Original PR description

This PR introduces a class for signature schemes, replacing the existing Signature_Scheme enum and the free-standing functions around it (signature_algorithm_of_scheme, hash_function_of_scheme, etc).

Previously, it was not clear whether these functions would throw if an invalid or unknown scheme was provided. This has been changed: the member functions never throw. Rather, Signature_Scheme::is_available should be used to check if a scheme can be used (is implemented).

We intend to extend this class with higher level predicates, such as bool is_allowed_for(TLS::Version), or similar.

Note that this affects the public API of TLS::Policy and Signature_Scheme. Users can still create schemes as, e.g., Botan::TLS::Signature_Scheme::ECDSA_SHA256, because the enum class moved into the actual class (but is no longer an enum class).

return false;

return true;
}
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.

BoGo tests whether the curve's bit length matches the associated hash function output length. We we weren't sure how to test for this and went with a "fuzzy" range for the ECC key length. The idea was to incorporate custom curves whose bit key length don't fall exactly on predefined thesholds. Does it make sense to implement it like that? @randombit

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.

This seems ok to me. We can adjust or loosen these restrictions later if it proves necessary.

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Apr 27, 2022

This pull request introduces 1 alert when merging 6bbab1b into 00b35a6 - view on LGTM.com

new alerts:

  • 1 for Comparison result is always the same

@reneme reneme force-pushed the tls13/signature_scheme_class branch from 6bbab1b to a235137 Compare April 27, 2022 11:22
@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Apr 27, 2022

This pull request introduces 1 alert when merging a235137 into 2e01455 - view on LGTM.com

new alerts:

  • 1 for Comparison result is always the same

Hannes Rantzsch and others added 3 commits May 18, 2022 13:27
Note that this affects the public API of TLS::Policy and Signature_Scheme

Co-authored-by: René Meusel <rene.meusel@nexenio.com>
@reneme reneme force-pushed the tls13/signature_scheme_class branch from a235137 to 1d58f85 Compare May 18, 2022 11:40
@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented May 18, 2022

This pull request introduces 1 alert when merging 1d58f85 into bae64de - view on LGTM.com

new alerts:

  • 1 for Comparison result is always the same

@reneme reneme merged commit d52969f into dev/tls-13 Jun 1, 2022
@randombit randombit deleted the tls13/signature_scheme_class branch June 1, 2022 14:26
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