Skip to content

Draft: kex: Add sntrup761x25519-sha512@openssh.com.

Simon Josefsson requested to merge jas/libssh-mirror:jas/sntrup761 into master

This is work in progress -- but it does work, and is ready to be improved by review. What do you think?

Known issues:

  • The pkd_hello_i1 self-test fails randomly. This seems to have been an historic problem, and the self-test looks fragile, but it does work reliably on 'master' so it should be possible to fix with sntrup enabled too. It fails randomly with (TCP?) connection closing, usually for non-sntrup761 KEX's, and retrying the CICD job often leads it to succeeding. Any ideas?
  • There seems to be some general problem with having SHA-512-based KEX's as the default in libssh. A few self-tests fail if you put one of the existing SHA-512 KEX's before our current Curve25519 default. This is the reason sntrup761 isn't the new preferred default, but only second after the existing Curve25519. It seems to happen when the same session is reused for two KEX's, and I guess the default key lengths are not restored properly between sessions.
  • Code indentation. I can't really understand how to indent libssh code properly. The Emacs recommendation in CONTRIBUTING.md does not result in indentation consistent with existing code. Could we add a 'make indent' target to avoid having manually curated indentation? I don't care what style.
  • The low-level crypto code should be synced up with the reference code in the IETF draft. There are some minor hacks in there now.
  • There is overlap between sntrup761.c and existing curve25519.c since this is a hybrid mode with curve25519. Possibly curve25519.c could be abstracted a bit to allow sntrup761.c to re-use some of it instead of cut'n'pasting it. It may just make things more complex though.

Here are notes if you want to interop test against OpenSSH:

mkdir -p ~/src
cd ~/src
git clone https://github.com/openssh/openssh-portable.git
cd openssh-portable
git checkout V_9_3_P2
autoreconf
./configure
make CC="cc -DDEBUG_KEX=1 -DDEBUG_KEXDH=1 -DDEBUG_KEXECDH=1"

cd ~/src
git checkout https://gitlab.com/jas/libssh-mirror.git
cd libssh
git checkout jas/sntrup761 
mkdir build
cd build
cmake -DUNIT_TESTING=ON -DWITH_SERVER=ON -DSERVER_TESTING=ON -DCLIENT_TESTING=ON -DPICKY_DEVELOPER=ON -DWITH_DEBUG_CRYPTO=ON -DCMAKE_BUILD_TYPE=Debug -DCMAKE_VERBOSE_MAKEFILE=ON .. # -DWITH_GCRYPT=ON
make
make test
make test ARGS="--rerun-failed --output-on-failure"

examples/keygen2 -t ed25519 -f ~/host_ed25519

make ssh_server_fork && examples/ssh_server_fork -k ~/host_ed25519 -p 2222 0.0.0.0 -a ~/.ssh/authorized_keys -v -v
~/src/openssh-portable/ssh -v -p 2222 localhost -oKexAlgorithms=sntrup761x25519-sha512@openssh.com

while true; do ~/src/openssh-portable/sshd -p 2222 -f /dev/null -h ~/host_ed25519 -oKexAlgorithms=sntrup761x25519-sha512@openssh.com -d; done
make ssh-client && examples/ssh-client -p 2222 localhost -v -v -v

Checklist

  • Commits have Signed-off-by: with name/author being identical to the commit author
  • Code modified for feature
  • Test suite updated with functionality tests
  • Test suite updated with negative tests
  • Documentation updated

Reviewer's checklist:

  • Any issues marked for closing are addressed
  • There is a test suite reasonably covering new functionality or modifications
  • Function naming, parameters, return values, types, etc., are consistent and according to CONTRIBUTING.md
  • This feature/change has adequate documentation added
  • No obvious mistakes in the code
Edited by Simon Josefsson

Merge request reports