usnistgov
Add support for X25519 and X448
This pull request adds support for the X25519 and X448 to the server (also known as Curve25519 / Curve448 Diffie-Hellman). Three algorithms are implemented: KeyGen, KeyVer, and SSC (Shared Secret Computation). The different commits modify different layers of the server for easier review.
Some points that were considered while implementing this code:
- RFC 7748 is the only reference used for this implementation.
- All existing ECC code was part of "NIST.CVP.ACVTS.Libraries.Crypto/DSA.ECC" or "NIST.CVP.ACVTS.Libraries.Crypto/DSA.Ed". Because X25519 and X448 are not DSA algorithms, this is not a right fit for these algorithms. To avoid a very large refactor, the code was separated into a separate directory "NIST.CVP.ACVTS.Libraries.Crypto/XECDH".
- As a result, there are also no general-purpose classes or interfaces for Montgomery curves or points on Montgomery curves. They were initially implemented, but cumbersome to work with as X25519/X448 are much more specialized. The algorithms rely on the theory, but use several shortcuts. For example, X25519/X448 only use the u coordinate (Montgomery points are (u, v)) as an input to the Diffie-Hellman process.
- Initially, I considered an "ECX_Component" similar to the existing "ECC_Component", that was fully integrated in the existing KAS code. However, as KeyGen and KeyVer algorithms (similar to EdDSA) were added later, I decided to pull out the code and completely separate it from KAS.
- The cryptographic code uses BitString but does not fully use its capabilities. For example, converting a BitString to an integer in a little-endian manner is explicitly implemented following the RFC. This was to a) Make sure the code clearly maps to the RFC b) Avoid any potential endianness issues since the BitString internally uses big-endian
- For X25519/X448, the RFC requires that the implementation accepts basically any 32-byte (resp. 48-byte) string as a valid public key. Therefore, the KeyVer testing is very limited. As the private key is currently not provided to the IUT, pair-wise consistency is not in scope (similar to EdDSA). It could be considered to expand this, as long as compliance with the RFC is maintained.
- Test cases for the cryptographic code are taken from RFC 7748. In particular, the iterated test cases are using only 1,000 iterations at this point. ~1,000,000 should be possible but may take much longer (perhaps 30 minutes per test, on my hardware)~ 1,000,000 does not provide too much extra value compared to 1,000.
I am also very open to code style changes, I tried to follow the existing style but didn't see anything explicitly documented (e.g., line limit / wrapping)
No formal code style guidelines. We use basic C# styling though I see several syntactic sugars on that page that didn't exist when we started the project.
It is strongly preferred for unit tests to finish in a matter of minutes or seconds. It is OK to define longer running tests but they should be identified with the [LongRunningCryptoTest] attribute. The iterated tests on 1000 iterations make sense here.
Note if/when we accept this PR, it will not be merged here. This is an external repository used for our production code once published. We maintain an internal repository where this would be merged. Just a heads up if anyone looks at this PR later to see why it was "closed" versus "merged".
It is strongly preferred for unit tests to finish in a matter of minutes or seconds. It is OK to define longer running tests but they should be identified with the
[LongRunningCryptoTest]attribute. The iterated tests on 1000 iterations make sense here.
Right now the 1,000 iterations finish in about 5 seconds on my machine. That test case is identified using [LongRunningCryptoTest] though, I just didn't have time to test the full 1,000,000 iterations yet. A naive extrapolation of the 1,000 iterations runtime indicates it should take about 1.4 hours, which would make [LongRunningCryptoTest] on that test case justified.
@celic additional tests were implemented in https://github.com/usnistgov/ACVP-Server/pull/391/commits/a2d331415dd717dd26e66355b2b77fdffb62001e
@livebe01 due to the force push, your commit to add the xecdh solution was overwritten. I can try to add it again, or maybe you can add it after this PR is merged?
@jvdsn @celic I'm thinking it may make sense to remove the XECDH KeyGen and KeyVer testing. KeyGen effectively gets tested by the SSC tests.
@celic @jvdsn right now, the XECDH SSC testing provides the server's public key in the prompt for each test case. In the response, the IUT provides its public key along with the value it computed for z. What do you think about instead having the prompt include both the server's public key and the private key that should be used by the IUT? The response would then only contain the value computed for k by the IUT. I believe this would be better testing as it would allow ACVTS to vary the private keys used by the IUT in performing the SSC.
@celic @jvdsn right now, the XECDH SSC testing provides the server's public key in the prompt for each test case. In the response, the IUT provides its public key along with the value it computed for z. What do you think about instead having the prompt include both the server's public key and the private key that should be used by the IUT? The response would then only contain the value computed for k by the IUT. I believe this would be better testing as it would allow ACVTS to vary the private keys used by the IUT in performing the SSC.
@livebe01 it makes sense to me, right now the XECDH SSC testing was modeled after the ECC CDH component, but I'm open to changing this. The only constraint may be that some IUTs may not accept a private key from the outside?
@livebe01 in that case, KeyGen and KeyVer would remain though, right? Since then KeyGen wouldn't be covered by SSC anymore
@celic @jvdsn right now, the XECDH SSC testing provides the server's public key in the prompt for each test case. In the response, the IUT provides its public key along with the value it computed for z. What do you think about instead having the prompt include both the server's public key and the private key that should be used by the IUT? The response would then only contain the value computed for k by the IUT. I believe this would be better testing as it would allow ACVTS to vary the private keys used by the IUT in performing the SSC.
@livebe01 it makes sense to me, right now the XECDH SSC testing was modeled after the ECC CDH component, but I'm open to changing this. The only constraint may be that some IUTs may not accept a private key from the outside?
@jvdsn I'd thought of that as well. @celic what are your thoughts?
@livebe01 in that case, KeyGen and KeyVer would remain though, right? Since then KeyGen wouldn't be covered by SSC anymore
I guess I don't understand why you implemented KeyGen and KeyVer and could use your help to understand. Full disclosure, I don't know a lot about X25519 and X448. I read RFC 7748 yesterday and that's about it.
It seemed to me that RFC 7748 is describing a key agreement scheme. If you look through the RFC, it doesn't seem to break out key generation and key verification functions. What it does define are the X25519 and X448 functions (Section 5). Section 6 specifies how the X25519 and X448 functions can be used in an ECDH protocol. Performing the ECDH protocol requires each party to execute the X25519 or X448 function two times. The first time a party executes the X____ function, they are computing their public key. The second time they are executing the X____ function they are computing the shared secret. My thought is that having XECDH SSC testing alone is sufficient. The prompt provides the IUT private key and the server public key. I suppose it would make sense to require the IUT to provide both its public key and the computed shared secret in the response. This would require the IUT to exercise the X____ function both times. The first time to produce its public key and the second time to produce the shared secret.
If the purpose of adding XECDH testing to ACVTS is to allow testing to RFC7748, then I don't think there's a point to implementing an XECDH KeyGen algorithm. We'd only want to implement XECDH KeyGen algorithm testing if there were some use for X25519 and X448 key pairs outside of the key agreement scheme defined in RFC7748. For example, if it were useful to have XECDH KeyGen algorithm testing because X25519 and X448 key pairs are used separately from RFC7748 in digital signature schemes.
@livebe01 keys used by X25519 and X448 are not (should not) be used for any other cryptographic schemes. The reason why I added KeyGen and KeyVer was just for consistency with EdDSA (after all, keys used by EdDSA are not and should not be used by any other cryptographic schemes either).
KeyVer could probably be dropped without any issue. KeyGen may still be useful because it is specifically defined as X25519 using the private key as the scalar and the base point as the u coordinate. I can imagine some cryptographic libraries / modules have a separate function for that. That is Section 6 of RFC7748 as you pointed out:
Alice generates 32 random bytes in a[0] to a[31] and transmits K_A = X25519(a, 9) to Bob, where 9 is the u-coordinate of the base point and is encoded as a byte with value 9, followed by 31 zero bytes.
@livebe01https://github.com/livebe01 keys used by X25519 and X448 are not (should not) be used for any other cryptographic schemes. The reason why I added KeyGen and KeyVer was just for consistency with EdDSA (after all, keys used by EdDSA are not and should not be used by any other cryptographic schemes either).
Sure, that’s part of what I was getting at. You modeled XECDH after EdDSA, but EdDSA is a digital signature algorithm whereas XECDH is a key agreement scheme.
KeyVer could probably be dropped without any issue. KeyGen may still be useful because it is specifically defined as X25519 using the private key as the scalar and the base point as the u coordinate. I can imagine some cryptographic libraries / modules have a separate function for that. That is Section 6 of RFC7748 as you pointed out:
Alice generates 32 random bytes in a[0] to a[31] and transmits K_A = X25519(a, 9) to Bob, where 9 is the u-coordinate of the base point and is encoded as a byte with value 9, followed by 31 zero bytes.
I'm not sure that we would want two certificates (both keygen and SSC) for a module that implements XECDH? I’m trying to think what would normally show up on a CAVP certificate that includes a KAS * SSC * algorithm. Do keyGen entries normally accompany SSC entries on certs?
I'm not sure that we would want two certificates (both keygen and SSC) for a module that implements XECDH? I’m trying to think what would normally show up on a CAVP certificate that includes a KAS * SSC * algorithm. Do keyGen entries normally accompany SSC entries on certs?
@livebe01 I would say so, yes.
For KAS-ECC-SSC, ECDSA KeyGen is generally repurposed. For KAS-FFC-SSC, Safe Primes KeyGen generally used. I don't think this is mandatory (it's certainly not required here https://csrc.nist.gov/Projects/cryptographic-algorithm-validation-program/prerequisites) but I do see it pretty often. Especially since it is mandatory for ephemeral key exchanges, which a very common claim for cryptographic modules.
Furthermore, for NIAP (Common Criteria) evaluations, key generation is almost always a prerequisite for SSC. E.g., from PP_MDF_V3.3:
FCS_CKM.2/LOCKED Cryptographic Key Establishment
The TSF shall perform cryptographic key establishment in accordance with a
specified cryptographic key establishment method: [selection:
...
[Elliptic curve-based key establishment schemes] that meet the following:
[selection:
...
RFC 7748, "Elliptic Curves for Security"
]
...] for the purposes of encrypting sensitive data received while the device
is locked.
The elliptic curves used for the key establishment scheme must correlate with
the curves specified in FCS_CKM.1.1.
TSS
The evaluator shall ensure that the supported key establishment schemes correspond to the key
generation schemes identified in FCS_CKM.1.1. If the ST specifies more than one scheme, the
evaluator shall examine the TSS to verify that it identifies the usage for each scheme.
FCS_CKM.1 Cryptographic Key Generation
The TSF shall generate asymmetric cryptographic keys in accordance with a
specified cryptographic key generation algorithm [selection:
...
ECC schemes using: [selection:
...
Curve25519 schemes that meet the following: [RFC 7748]
]
...
].
Application Note: The ST author must select all key generation schemes used
for key establishment and entity authentication
As to the usefulness for KeyVer, I would agree, but then I would also question the usefulness of ECDSA, EdDSA, and Safe Primes KeyVer :)
RFC 7748 doesn't talk about key generation. Is there a separate RFC for that?
@celic it is Section 6 of RFC7748:
Alice generates 32 random bytes in a[0] to a[31] and transmits K_A = X25519(a, 9) to Bob, where 9 is the u-coordinate of the base point and is encoded as a byte with value 9, followed by 31 zero bytes.
The 32 random bytes are the private key and K_A is the public key
Ah I see. The more involved process I'm thinking of is the EdDSA key pairs. The key pairs for X25519 and X448 are very simple.
In practice, because this would typically be used in TLS, are the public keys computed ahead of time as part of a certificate? Or are they essentially ephemeral for the computation?
Context: I'm seeing a slight discrepancy between the testing and the RFC and that could be because you are fitting the RFC into a mold of SP 800-56A. I don't have any preference one way or another, but it may affect how the tests are used. Normally we get to target an SP or FIPS that defines the algorithms that need to be tested. Targeting an RFC leaves it a bit ambiguous due to the flexibility a protocol could take when implementing the RFC. That may be more of a philosophical discussion than we need though.
@celic as far as I know, they will always be ephemeral in the context of TLS. I'm not sure if there's other non-ephemeral users. Apple uses it for their file protection (https://help.apple.com/pdf/security/en_US/apple-platform-security-guide.pdf page 105) but that's probably ephemeral.
There are parties asking for KeyGen testing though, e.g. NIAP as shown above.
Hello, It's great to see this development. Does it mean it will be possible to do a FIPS 140-3 certification for x25519/x448 in a future?
Hello, It's great to see this development. Does it mean it will be possible to do a FIPS 140-3 certification for x25519/x448 in a future?
@kriskwiatkowski I am not affiliated with NIST nor CMVP, so I would not read too much into this PR. If you're interested in how/why I worked on this you're welcome to attend my ICMC talk https://icmconference.org/?post_type=session&p=23836
Hello, It's great to see this development. Does it mean it will be possible to do a FIPS 140-3 certification for x25519/x448 in a future?
No it would not. The CAVP who manages ACVTS is interested in providing testing for non-approved algorithms if there is a need for it. @jvdsn was kind enough to offer code for X25519/X448. It has no relation to the NIST cryptographic standards.
@jvdsn, @livebe01 I'd say leave it as it is then.
Edit: I'm referring to keeping KeyGen, KeyVer, and SSC as distinct algorithms. There may be some improvements that could be made to SSC to smooth over things we've had users point out to us in the past.
@celic @livebe01 I took a stab at adding hashFunctionZ as discussed here: https://github.com/usnistgov/ACVP/pull/1564#discussion_r2027887611
The main roadblock right now is the type used to return the result from the NIST.CVP.ACVTS.Libraries.Generation.XECDH.RFC7748.SSC.DeferredTestCaseResolver to the NIST.CVP.ACVTS.Libraries.Generation.XECDH.RFC7748.SSC.TestCaseValidator. Right now, that transition uses NIST.CVP.ACVTS.Libraries.Crypto.Common.KES.SharedSecretResponse, which does not have a HashZ field. I could add the field, but SharedSecret is used by other algorithms as well so I don't want to mess anything up.
Other than that, I have code to add the HashZ and HashFunctionZ fields to the relevant classes (e.g., parameters, test groups, oracle input/output types, etc.)
Should be fine to add the field in. It can be helpful to add a // Only used in XECDH over the property to help. A caller isn't required to populate the entire object and it should be fine to leave fields as null or default.