Add support for mlkem768x25519-sha256

[alexandrejbr]

Adds support for the mlkem768x25519-sha256 SSH key exchange, defined in draft-kampanakis-curdle-ssh-pq-ke and shipped by OpenSSH 9.9+ and Erlang/OTP 28.4+. It combines classical curve25519-sha256 with NIST FIPS-203 ML-KEM-768, providing post-quantum confidentiality (with the KEM provided by BouncyCastle) without giving up the security of X25519 if either primitive is later broken.

To keep compatibility with older versions I didn't take the JCA approach and instead created a bouncy castle wrapper which meant that I couldn't get for free things like the SecureRandom so one needs to be created explicitly. There is a Random factory but that is not guaranteed to be a SecureRandom as far as I understand.

EDIT: there's no direct dependency with bouncy castle in the updated version, but it's instead used as a fallback using reflection.

The changes of this PR were tested with openssh 10+ and Erlang OTP 28.4.1.

I kept groovy and spock-core upgrade in a separate commit as I am unsure that is wanted in this project but I needed it to test and may be nice to do anyway. The integration tests are as well in a separate commit as they require a different base image.

Fixes #1017

[exceptionfactory]

Thanks for working on support for this PQC algorithm @alexandrejbr!

As you mentioned in the description, the MLKEM768 class makes direct use of the Bouncy Castle primitives, as opposed to the JCA.

I have a separate open pull request #1006 that would make Bouncy Castle an optional dependency.

In light of other work to remove explicit use of Bouncy Castle classes, it would be helpful to consider an implementation approach that supports making Bouncy Castle optional.

[alexandrejbr]

Thanks for working on support for this PQC algorithm @alexandrejbr!

As you mentioned in the description, the MLKEM768 class makes direct use of the Bouncy Castle primitives, as opposed to the JCA.

I have a separate open pull request #1006 that would make Bouncy Castle an optional dependency.

In light of other work to remove explicit use of Bouncy Castle classes, it would be helpful to consider an implementation approach that supports making Bouncy Castle optional.

I can try that route again, but I am quite certain that having a higher java version would be necessary, and from memory it was higher than 21. Do you think that is a reasonable approach @exceptionfactory ?

[exceptionfactory]

Thanks for the quick reply @alexandrejbr.

I agree that going with the JCA-based approach could involve issues with Java versions. However, it would be helpful to confirm if it works when Bouncy Castle is registered as a Security Provider.

One possibility is testing for the availability of specific algorithms available to the JCA, which could support an approach based on feature detection.

Along similar lines, another option could be to test for the presence of a specific Bouncy Castle class before registering the new MLKEM implementation factory.

[alexandrejbr]

Thanks for the quick reply @alexandrejbr.

I agree that going with the JCA-based approach could involve issues with Java versions. However, it would be helpful to confirm if it works when Bouncy Castle is registered as a Security Provider.

One possibility is testing for the availability of specific algorithms available to the JCA, which could support an approach based on feature detection.

Along similar lines, another option could be to test for the presence of a specific Bouncy Castle class before registering the new MLKEM implementation factory.

Sounds good @exceptionfactory, I will work on it and report back with an updated implementation.

[alexandrejbr]

Hi @exceptionfactory,

The implementation is more complicated than what I would want. I would say there's 2 main problems:

• The JCA keys obtained via SecurityUtils.getKeyPairGenerator("ML-KEM-768").generateKeyPair() include an algorithm identifier while the BouncyCastle MLKEMPublicKeyParameters operates on the raw key directly which is more suitable since that is what is going to be used in the wire format. Managing this difference is a unwanted complexity.

• The other problem is that the decapsulation/encapsulation needed for this hybrid key exchange needs is only part of JDK 21 API (JEP 452) , so in order for this to compile with older version I had to use reflection not only for the usage of BouncyCastle but as well for when JCA is used.

What do you think?

[exceptionfactory]

Hi @exceptionfactory,

The implementation is more complicated than what I would want. I would say there's 2 main problems:

* The JCA keys obtained via SecurityUtils.getKeyPairGenerator("ML-KEM-768").generateKeyPair() include an algorithm identifier while the BouncyCastle MLKEMPublicKeyParameters operates on the raw key directly which is more suitable since that is what is going to be used in the wire format. Managing this difference is a unwanted complexity.

* The other problem is that the decapsulation/encapsulation needed for this hybrid key exchange needs is only part of JDK 21 API (JEP 452) , so in order for this to compile with older version I had to use reflection not only for the usage of BouncyCastle but as well for when JCA is used.

What do you think?

I would defer to @hierynomus as the project maintainer for particular direction, but this does raise some fundamental implementation questions.

Although going in the JCA direction has appeal, the use of reflection adds quite a bit of complexity. Some other JCA-based implementations include similar prefixed algorithm identifiers, so removing those as part of the implementation is not out of the question, as long as it is documented.

The JDK 21 API level seems to be the more problematic concern. Possible solutions include raising the minimum version, although that is more involved. Another possibility is providing this functionality in an independent module, requiring Java 21.

Taking a different path forward, it would also be possible to keep the Bouncy Castle-specific implementation classes, but use Class-availability detection to determine whether to load the corresponding SSHJ classes. This is not trivial, but it is probably cleaner to maintain than reflection for JCA methods.

[alexandrejbr]

Hi @exceptionfactory,

The implementation is more complicated than what I would want. I would say there's 2 main problems:

* The JCA keys obtained via SecurityUtils.getKeyPairGenerator("ML-KEM-768").generateKeyPair() include an algorithm identifier while the BouncyCastle MLKEMPublicKeyParameters operates on the raw key directly which is more suitable since that is what is going to be used in the wire format. Managing this difference is a unwanted complexity.

* The other problem is that the decapsulation/encapsulation needed for this hybrid key exchange needs is only part of JDK 21 API (JEP 452) , so in order for this to compile with older version I had to use reflection not only for the usage of BouncyCastle but as well for when JCA is used.

What do you think?

I would defer to @hierynomus as the project maintainer for particular direction, but this does raise some fundamental implementation questions.

Although going in the JCA direction has appeal, the use of reflection adds quite a bit of complexity. Some other JCA-based implementations include similar prefixed algorithm identifiers, so removing those as part of the implementation is not out of the question, as long as it is documented.

The JDK 21 API level seems to be the more problematic concern. Possible solutions include raising the minimum version, although that is more involved. Another possibility is providing this functionality in an independent module, requiring Java 21.

Taking a different path forward, it would also be possible to keep the Bouncy Castle-specific implementation classes, but use Class-availability detection to determine whether to load the corresponding SSHJ classes. This is not trivial, but it is probably cleaner to maintain than reflection for JCA methods.

Thanks for the feedback. I am fine putting the time to get the implementation to where it needs to be. So, some guidance from @hierynomus would be greatly appreciated.