[SSHD-1017] Add support for chacha20-poly1305@openssh.com

[jvz]

This uses BouncyCastle due to OpenSSH using the non-RFC variant of ChaCha20 as well as having a direct implementation of Poly1305 (Java 11 does not expose the Poly1305 mac engine).

https://issues.apache.org/jira/browse/SSHD-1017

Some ideas on simplification adapted from mwiede/jsch#17

[gnodet]

@jvz What's the status of this PR ? We're planning a 2.7.0 release this week, so wondering if I should merge this PR in.

[jvz]

I'll need to revisit this PR. I've done a lot more cryptography work since then, and I probably have some simplifications now. 😁

[lgoldstein]

Don't know what the stance of the other maintainers is on having more crypto code inside sshd.

I am not opposed to it, but I am reluctant to doing this - especially if it is a lot of code. In the case of bcrypt it was one class which IMO is acceptable. Also, as mentioned we preserved the copyright notice and also cleared it with the relevant legal teams (which we should do to every piece of code we import...)

[tomaswolf]

[...] cleared it with the relevant legal teams (which we should do to every piece of code we import...)

At least in the case of ChaCha, there is indeed an undeniably public domain base in the OpenSSH sources[1] (original at [2]), and Matt is an Apache member. So this should not be necessary; including Matt's own code based on a public domain implementation is straight-forward; much simpler than including someone else's ISC-licensed code as was the case with bcrypt.

[1] https://github.com/openssh/openssh-portable/blob/master/chacha.c
[2] https://cr.yp.to/chacha.html ("merged" version)

[lgoldstein]

At least in the case of ChaCha, there is indeed an undeniably public domain base in the OpenSSH sources[1] (original at [2]), and Matt is an Apache member. So this should not be necessary; including Matt's own code based on a public domain implementation is straight-forward; much simpler than including someone else's ISC-licensed code as was the case with bcrypt.

Like I said, I am not opposed to it, and if it is as you describe then even better - let's just make sure with the legal team that there is no issue here - from what you describe it should be pretty straightforward.

[jvz]

Let me clarify the IP provenance then. I have two versions of ChaCha20 and Poly1305 in my O(1) Cryptography library. There's a Java port that I wrote based on DJB's papers about ChaCha and Salsa (which are themselves public domain and patent free) and the corresponding RFC that standardized it. There's also a C version that is copied from one of the linked implementations from https://cr.yp.to/chacha.html which is independent (and added much later). O1C is my library where I'm the only copyright holder other than some C code that's imported from elsewhere (the Java code is all pure Java, all done by reimplementing or porting public domain algorithms from other languages).

Suffice to say, I've done due diligence in the library, and I can trivially relicense any parts right now since nobody else has contributed yet.

[jvz]

I've ported in the code I mentioned earlier. Based on files from my other project (ISC licensed, but with my ICLA on file, this is dual-licensed here as Apache 2.0):

• https://github.com/o1c-dev/o1c/blob/ad51541705d117f10cfb71907a7df99028ea9925/src/main/java/dev/o1c/impl/chacha20/ChaCha20.java
• https://github.com/o1c-dev/o1c/blob/ad51541705d117f10cfb71907a7df99028ea9925/src/main/java/dev/o1c/impl/chacha20/Poly1305.java
• https://github.com/o1c-dev/o1c/blob/ad51541705d117f10cfb71907a7df99028ea9925/src/main/java/dev/o1c/util/ByteOps.java (some methods copied from here, but they're all trivial)

I tested this out interactively with OpenSSH_8.1p1, LibreSSL 2.7.3 on macOS 11.3.1.

[jvz]

Also, I tested this with Java 15 (openjdk version "15.0.2" 2021-01-19) which was the latest JDK in Homebrew.

[jvz]

I don't see why the CI build is failing. Works on my machine.

[tomaswolf]

Nice. Two little suggestions.

[tomaswolf]

I don't see why the CI build is failing. Works on my machine.

Neither do I. Maybe just an unstable test?

[tomaswolf]

LoadTest is unstable for me locally. Didn't dig deep; my suspicion is that in that test the channel gets closed sometimes before all data has been written back to the client.

[jvz]

That one hasn't failed for me locally, though I've seen the SFTP transfer test randomly hang once in a while.

[tomaswolf]

SftpTransferTest hanging is now issue SSHD-1191.

It took a while, but I finally managed to get a clean test run locally.

@jvz , is it OK for you if I squash all this and push onto this PR?

[jvz]

Go ahead!

[tomaswolf]

CI build still unstable; problems with that LoadTest. Works locally, so in it goes.

Thanks a lot, Matt!

[tomaswolf]

@jvz , I had to revert this. The hang in SftpTransportTestis caused by a KEX failure that occurs only if this chacha20-poly1305 implementation is used.

A debug log I captured is attached in SSHD-1191.

Usually KEX fails in rekeying. Something is definitely not right yet.

[tomaswolf]

And it's in again.

Chacha20-poly1305 has a block size of 8, and AbstractSession computed rekey limits for ciphers with block sizes < 16 wrongly, which caused SSHD-1191. That's fixed now. If there are remaining problems with rekeying, they're in any case not caused by the choice of cipher.

This chacha20-poly1305 implementation itself is perfectly fine.

[jvz]

Oh good to hear that because I was never able to figure out why anything wasn't working. Since I started these PRs, I've switched jobs where we use TLS instead of SSH as the secure transport behind most things, so I'm already starting to forget the protocol details from SSH. :)