-
-
Notifications
You must be signed in to change notification settings - Fork 10.5k
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
Fully Pluggable TLSv1.3 Key Exchange #11914
Conversation
test/tls-provider.c
Outdated
* Top secret. This algorithm only works if no one knows what this number is. | ||
* Please don't tell anyone what it is. | ||
*/ | ||
const unsigned char private_constant[XOR_KEY_SIZE] = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
static
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please also fix the comment to make it obvious that it is humorous.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed and added an extra warning about not really using this.
* implementation you will get back. Theoretically this could be | ||
* different every time...we assume here that you'll always get the | ||
* same one back if you repeat the exact same fetch. Is this a reasonable | ||
* assumption to make (in which case perhaps we should document this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If its a pluggable provider that had priority then I dont think that assumption would be a good one - but maybe in that case it should error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably not :(
A PKCS #11 provider could offer something one second and not the next.
I could add code to randomise the returns. That would be interesting...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A PKCS #11 provider could offer something one second and not the next.
Libssl just won't work with such a provider at the moment. That's a much more significant overahaul that I don't plan to do for 3.0.
9c04e5d
to
c91f150
Compare
I've rebased this (which was a nightmare), and taken it out of WIP since all its dependencies are now in. I've not yet addressed the feedback. |
Pushed...addressing feedback comments above. |
Fixups pushed to address some Travis issues. Ping? |
b8fa324
to
5502752
Compare
This pull request is ready to merge |
Pushed! Thanks! |
With capabilities we can query a provider about what it can do. Initially we support a "TLS-GROUP" capability. Reviewed-by: Shane Lontis <[email protected]> (Merged from #11914)
Provide a function to applications to query the capabilities that a provider can perform. Reviewed-by: Shane Lontis <[email protected]> (Merged from #11914)
Now that we have added the TLS-GROUP capability to the default provider we can use that to discover the supported group list based on the loaded providers. Reviewed-by: Shane Lontis <[email protected]> (Merged from #11914)
We rename these function to EVP_PKEY_CTX_get_group_name and EVP_PKEY_CTX_set_group_name so that they can be used for other algorithms other than EC. Reviewed-by: Shane Lontis <[email protected]> (Merged from #11914)
The previous commit added the EVP_PKEY_CTX_[get|set]_group_name functions to work with EC groups. We now extend that to also work for DH. Reviewed-by: Shane Lontis <[email protected]> (Merged from #11914)
The previous commits made EVP_PKEY_CTX_[get|set]_group_name work for EC and DH keys. We now extend this to ECX. Even though that keys with these key types only have one group we still allow it to be explicitly set so that we have only one codepath for all keys. Setting the group name for these types of keys is optional, but if you do so it must have the correct name. Additionally we enable parameter generation for these keys. Parameters aren't actually needed for this key type, but for the same reasons as above (to ensure a single codepath for users of these algorithms) we enable it anyway. Reviewed-by: Shane Lontis <[email protected]> (Merged from #11914)
Document the OSSL_PROVIDER_get_capabilities() function as well as the provider side support for capabilities. Reviewed-by: Shane Lontis <[email protected]> (Merged from #11914)
Reviewed-by: Shane Lontis <[email protected]> (Merged from #11914)
A number of these functions returned a NID or an array of NIDs for the groups. Now that groups can come from the providers we do not necessarily know the NID. Therefore we need to handle this in a clean way. Reviewed-by: Shane Lontis <[email protected]> (Merged from #11914)
Reviewed-by: Shane Lontis <[email protected]> (Merged from #11914)
If a provider had a "copy" function in the its keymgmt definition we were ignoring it. Reviewed-by: Shane Lontis <[email protected]> (Merged from #11914)
If EVP_PKEY_copy_parameters() failed in libssl we did not provide a very helpful error message. We provide a better one. Reviewed-by: Shane Lontis <[email protected]> (Merged from #11914)
The supported_groups extension only supported EC groups in DTLS. Therefore we shouldn't send it in a no-ec build. Reviewed-by: Shane Lontis <[email protected]> (Merged from #11914)
Reviewed-by: Shane Lontis <[email protected]> (Merged from #11914)
@mattcaswell this seems like quite a significant new feature for |
@@ -332,6 +344,72 @@ pointing at the string "foo,bar" | |||
For more information on handling parameters, see L<OSSL_PARAM(3)> as | |||
L<OSSL_PARAM_int(3)>. | |||
|
|||
=head1 CAPABILITIES | |||
|
|||
Capabilties describe some of the services that a provider can offer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: Capabilties
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As previously mentioned to @slontis, this is entirely the fault of my keyboard:
I think there's a problem with my keyboard. It can't seem to spell that word.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
singer instead of signer is one of my favorites.
Yes - it should have that. I'd actually like to blog about it at some point with some kind of tutorial type approach to explain how to use it. |
This PR started life as an attempt to ensure that we are actually able to support all the groups that a client offers or a server accepts in a TLS handshake. If the default provider is not loaded it is possible that we have a different set of groups available. This turned out to be quite tricky and the easiest solution was to ask the available providers what groups they can support. From there it was a short step to having support for fully pluggable (TLSv1.3) key exchange. Pluggable TLSv1.2 key exchange is quite a bit trickier so I've not attempted that.
With this PR it is possible to write a provider that adds new TLS groups and TLSv1.3 key exchange algorithms. It is marked as WIP because it is dependent on #11898 and includes the commits from there.
This PR also adds the concept of "Capabilities". This is information a provider can supply about things that it can do. At the moment there is support for just one capability (the "TLS-GROUP" capability), but I envisage it could be easily extended to other things in the future.