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

Key exchange parameters subtest: check for the exact ECDHE curves #972

Open
baknu opened this issue May 4, 2023 · 6 comments
Open

Key exchange parameters subtest: check for the exact ECDHE curves #972

baknu opened this issue May 4, 2023 · 6 comments
Assignees
Labels
bug Unexpected or unwanted behaviour of current implementations tls
Milestone

Comments

@baknu
Copy link
Contributor

baknu commented May 4, 2023

The Key exchange parameters subtest does not seem to check for the exact curve but only for the length. Therefore, for example, the cruve sect283k1 (length >=224) now passes the test, while it should fail because it is considered to be "Insufficient" according to the NCSC's TLS guidelines (table 9).

@baknu baknu added the bug Unexpected or unwanted behaviour of current implementations label May 4, 2023
@baknu baknu added this to the v1.9 milestone May 4, 2023
@baknu
Copy link
Contributor Author

baknu commented May 10, 2023

@gthess Can you remember if there was a certain reason to not test for the exact curve?

@gthess
Copy link
Collaborator

gthess commented May 10, 2023

No, looking at the code I believe that this part was probably not updated for the new guidelines. From a quick look I don't see the length requirement in the current guidelines.

@baknu
Copy link
Contributor Author

baknu commented May 10, 2023

In the first version of NCSC's TLS guidelines footnote 9 clarified the term "Andere krommen" with status "Onvoldoende" as follows:

Bedoeld worden de krommen sect163k1, sect163r1, sect163r2, sect193r1, sect193r2, sect233k1,
sect233r1, sect239k1, sect283k1, sect283r1, sect409k1, sect409r1, sect571k1, sect571r1, secp160k1,
secp160r1, secp160r2, secp192k1, secp192r1, secp224k1, secp256k1 en de mogelijkheid om zelf
krommen te kiezen.

See: https://www.ncsc.nl/binaries/ncsc/documenten/publicaties/2014/november/1/ict-beveiligingsrichtlijnen-voor-transport-layer-security-1.0/ICT-beveiligingsrichtlijnen-voor-Transport-Layer-Security-leesversie.pdf

@gthess
Copy link
Collaborator

gthess commented May 11, 2023

It seems that this earlier footnote was not used in the earlier code and I believe the check there was wrong and it bled through to the current version.
So my suggestion would be to take out the length check and only pass the explicitly named curves.
But now I see the following in the test description:

ECDHE: The security of elliptic curve Diffie-Hellman (ECDHE) ephemeral key exchange depends on the used elliptic curve. We check if the bit-length of the used elliptic curves is a least 224 bits. Currently we are not able to check the elliptic curve name.

For the last sentence specifically I sadly have no recollection; the code seems to suggest otherwise unless we don't get any values from nassl.

@baknu
Copy link
Contributor Author

baknu commented May 12, 2023

For the last sentence specifically I sadly have no recollection; the code seems to suggest otherwise unless we don't get any values from nassl.

Found some mail conversations between GT and BK from1 Nov 2019 when the we implemented version 2 of NCSC's TLS guidelines in Internet.nl. Below some relevant quotes:

The catch is that the EC check for that is only based on bit length as
we can't get the name of the curve from the library. The length we use
is 224. This is based on the previous guidelines and the current ones
that are only based on curve names (the bit length is included in the name).


In addition to the previous: I assume B5-1 and table 9 are covered by "Public key
of certificate" (and not by "Key exchange parameters").

They should be covered by both but note my previous email about the
catch for the EC check on the key exchange parameters.

So in the "Key exchange parameters" subtest, besides DHE finite fields, we check if the bit length for ECDHE > 224 (but we do/can not check what exact Elliptic curve type is used. So it could still be one of the insufficient "other curves" from table 9?)

Correct but unfortunately we can't do anything more about it.

What else is tested for ECDHE in the "Public key of certificate" subtest? Or is the same tested there?

Curve names and bit length (224).

@mxsasha
Copy link
Collaborator

mxsasha commented Mar 19, 2024

This will almost certainly be outdated by #1218 as that gets us exact curve names.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Unexpected or unwanted behaviour of current implementations tls
Development

No branches or pull requests

3 participants