diff --git a/README.md b/README.md index 15b3f2935..21286d7a2 100644 --- a/README.md +++ b/README.md @@ -766,6 +766,40 @@ Notes: `-DWC_SIG_MIN_HASH_TYPE=WC_HASH_TYPE_SHA`. This re-enables a deprecated hash; prefer ECDSA unless RSA is mandated. +STRICT KEY EXCHANGE +=================== + +wolfSSH implements strict key exchange, the mitigation for the Terrapin attack +(CVE-2023-48795) described in `draft-miller-sshm-strict-kex`. It is negotiated +in the initial KEXINIT and enabled whenever the peer asks for it too, so no +configuration is needed for the usual case. + +With strict KEX in force, wolfSSH accepts nothing but the key exchange itself +and SSH_MSG_DISCONNECT until the peer's SSH_MSG_NEWKEYS arrives, and it zeroes +the packet sequence numbers at every SSH_MSG_NEWKEYS. Together those stop an +attacker splicing packets into the unauthenticated initial exchange to shift +the sequence numbers. A message that arrives out of turn ends the connection +with SSH_MSG_DISCONNECT rather than being ignored. + +Note that wolfSSH offers neither `chacha20-poly1305@openssh.com` nor the +`*-etm@openssh.com` MACs, the modes whose nonce comes from the sequence +number. The full silent-truncation form of Terrapin needs one of those, so it +was never reachable here; strict KEX closes the sequence-number shift the +attack is built on. + +A caller that has to interoperate with a peer that mishandles the marker can +turn it off, per context or per session: + + wolfSSH_CTX_SetStrictKex(ctx, 0); /* seeds every session from ctx */ + wolfSSH_SetStrictKex(ssh, 0); /* this session only */ + +Only the initial KEXINIT carries the marker, and only that exchange decides +whether the mitigation is on, so the session call has to be made before the +connection starts. A rekey neither re-advertises the marker nor revisits the +decision, and a change made to a session already keying is ignored for the +life of that session. The context call is the one that affects anything +later: it seeds the sessions made after it. + WOLFSSH APPLICATIONS ==================== diff --git a/examples/echoserver/echoserver.c b/examples/echoserver/echoserver.c index 39c240191..b50dbc00a 100644 --- a/examples/echoserver/echoserver.c +++ b/examples/echoserver/echoserver.c @@ -1915,6 +1915,10 @@ static int sftp_worker(thread_ctx_t* threadCtx) continue; } #endif + if (ret == WS_WANT_READ) { + /* Part of a packet arrived; wait for the rest. */ + continue; + } if (ret == WS_WANT_WRITE) { /* recall wolfSSH_worker here because is likely our custom * highwater callback that returned up a WS_WANT_WRITE */ diff --git a/examples/sftpclient/sftpclient.c b/examples/sftpclient/sftpclient.c index 011d1c9a9..5ccfbe908 100644 --- a/examples/sftpclient/sftpclient.c +++ b/examples/sftpclient/sftpclient.c @@ -1570,13 +1570,18 @@ static int doAutopilot(int cmd, char* local, char* remote) ret == WS_FATAL_ERROR); if (ret != WS_SUCCESS) { + /* ret is a generic failure code; the cause is in the session */ + err = wolfSSH_get_error(ssh); + if (cmd == AUTOPILOT_PUT) { - fprintf(stderr, "Unable to copy local file %s to remote file %s\n", - local, fullpath); + fprintf(stderr, "Unable to copy local file %s to remote file %s" + ": ret %d, error %d, %s\n", + local, fullpath, ret, err, wolfSSH_ErrorToName(err)); } else if (cmd == AUTOPILOT_GET) { - fprintf(stderr, "Unable to copy remote file %s to local file %s\n", - fullpath, local); + fprintf(stderr, "Unable to copy remote file %s to local file %s" + ": ret %d, error %d, %s\n", + fullpath, local, ret, err, wolfSSH_ErrorToName(err)); } } diff --git a/scripts/include.am b/scripts/include.am index 1e1c86149..abf2b4f5c 100644 --- a/scripts/include.am +++ b/scripts/include.am @@ -20,6 +20,10 @@ endif # app wasn't built. dist_noinst_SCRIPTS+= scripts/sshclient.test +# Not gated on anything: the script skips itself when OpenSSH is missing +# or predates strict KEX. +dist_noinst_SCRIPTS+= scripts/openssh-interop.test + dist_noinst_SCRIPTS+= scripts/fwd.test dist_noinst_SCRIPTS+= scripts/fwd-bulk.test endif diff --git a/scripts/openssh-interop.test b/scripts/openssh-interop.test new file mode 100755 index 000000000..2eb37dc32 --- /dev/null +++ b/scripts/openssh-interop.test @@ -0,0 +1,443 @@ +#!/bin/sh + +# OpenSSH interop test for strict key exchange, the Terrapin mitigation +# (CVE-2023-48795). +# +# Part 1 puts the OpenSSH client on the wolfSSH echoserver, part 2 the +# wolfSSH example client on OpenSSH's sshd. Each one checks that the marker +# is negotiated and that a session then runs over the connection: the +# mitigation zeroes the packet sequence numbers at SSH_MSG_NEWKEYS, and a +# peer that disagreed about the counters would fail the first packet that +# had to authenticate. +# +# The checks read OpenSSH's own debug output, so they are worded the way +# OpenSSH words them. Strict KEX arrived in OpenSSH 9.6; anything older is +# skipped. + +no_pid=-1 +server_pid=$no_pid +sshd_pid=$no_pid +work_dir="`pwd`/openssh_interop_test$$" +ready_file="$work_dir/ready" +client_out="$work_dir/client.out" +client_log="$work_dir/client.log" +sshd_log="$work_dir/sshd.log" +port=0 +counter=0 +# What the echo session sends and expects back. +probe="strictkexprobe$$" +# Seconds any one connection is given before it is killed. Nothing here +# takes more than a moment; the limit only keeps a stuck session from +# hanging make check. +run_limit=60 +killer_pid=-1 +# The test keys are not copied into the build directory, so reach them +# through srcdir the way scripts/fwd.test does, or an out-of-tree build +# and make distcheck cannot find them. +keys_dir="${srcdir:-.}/keys" + +do_cleanup() { + if [ $killer_pid != $no_pid ] + then + kill $killer_pid 2>/dev/null + killer_pid=$no_pid + fi + if [ $sshd_pid != $no_pid ] + then + kill -9 $sshd_pid 2>/dev/null + sshd_pid=$no_pid + fi + if [ $server_pid != $no_pid ] + then + kill -9 $server_pid 2>/dev/null + server_pid=$no_pid + fi + rm -rf -- "$work_dir" +} + +do_trap() { + echo "got trap" + do_cleanup + exit 1 +} + +trap do_trap INT TERM +# sshd holds a fixed port, so clean up on the way out however that happens. +trap do_cleanup EXIT + +# Wait for a backgrounded command, killing it if it outstays the limit. +# timeout(1) is GNU coreutils and is on neither stock macOS nor the BSDs, +# so the watchdog is run here, the way scripts/sshclient.test does it. +# Polls rather than sleeping the whole limit: killing a subshell that is +# waiting on a sleep leaves the sleep running. +wait_with_limit() { + watched_pid=$1 + + ( + watched=0 + while kill -0 $watched_pid 2>/dev/null; do + if [ $watched -ge $run_limit ]; then + kill -9 $watched_pid 2>/dev/null + break + fi + sleep 1 + watched=$((watched + 1)) + done + ) 2>/dev/null & + killer_pid=$! + + wait $watched_pid + watched_status=$? + + kill $killer_pid 2>/dev/null + killer_pid=$no_pid + + return $watched_status +} + +fail() { + printf '\n\n%s\n' "$1" + do_cleanup + exit 1 +} + +skip() { + printf '%s, skipping\n' "$1" + rm -rf -- "$work_dir" + exit 77 +} + +command -v ssh > /dev/null 2>&1 || skip "no ssh in the path" + +ssh_version=`ssh -V 2>&1` +case "$ssh_version" in + OpenSSH_*) ;; + *) skip "ssh is not OpenSSH ($ssh_version)" ;; +esac + +# 0 when the OpenSSH_X.Y in $1 is 9.6 or newer, 1 when it is older, and 2 +# when there is no version to read. Strict KEX arrived in 9.6. +has_strict_kex() { + hsk_major=`echo "$1" | sed -n 's/.*OpenSSH_\([0-9][0-9]*\)\..*/\1/p'` + hsk_minor=`echo "$1" | \ + sed -n 's/.*OpenSSH_[0-9][0-9]*\.\([0-9][0-9]*\).*/\1/p'` + + [ -z "$hsk_major" ] && return 2 + [ -z "$hsk_minor" ] && hsk_minor=0 + + if [ "$hsk_major" -lt 9 ] || + { [ "$hsk_major" -eq 9 ] && [ "$hsk_minor" -lt 6 ]; } + then + return 1 + fi + + return 0 +} + +has_strict_kex "$ssh_version" +case $? in + 1) skip "$ssh_version predates strict KEX" ;; + 2) skip "can't read the OpenSSH version ($ssh_version)" ;; +esac + +[ ! -x ./examples/echoserver/echoserver ] && skip "echoserver doesn't exist" +./examples/echoserver/echoserver '-?' 2>&1 | grep -q "^echoserver " \ + || skip "echoserver doesn't run" + +mkdir -p "$work_dir" || exit 1 + +# The echoserver is one shot, so start a new one per connection. It picks +# an ephemeral port and writes it to the ready file. +# +# -f keeps it in echo mode; without it a build with shell support tries to +# fork a login shell for a user who doesn't exist on this machine. Any +# argument given here is passed along, which is how the ECC sample key set +# is selected. +start_echoserver() { + if [ $server_pid != $no_pid ] + then + kill -9 $server_pid 2>/dev/null + wait $server_pid 2>/dev/null + server_pid=$no_pid + fi + + rm -f "$ready_file" + ./examples/echoserver/echoserver -1 -f "$@" -R "$ready_file" \ + > "$work_dir/server.log" 2>&1 & + server_pid=$! + + counter=0 + while [ ! -s "$ready_file" ] && [ "$counter" -lt 100 ]; do + sleep 0.1 + counter=$((counter + 1)) + done + + [ ! -s "$ready_file" ] && fail "NO ready file, the echoserver never came up" + + port=`cat "$ready_file"` +} + +# Run one echo session with the OpenSSH client. +# +# $1 - the private key to authenticate with, in a form OpenSSH reads +# rest - extra echoserver arguments +# +# The key is copied because OpenSSH refuses one that is readable by anyone +# else, and the checked-in keys are group and world readable. The 0x03 in +# the input is what tells the echoserver's worker the session is over; the +# newline before it is the line the server echoes back. +run_openssh_client() { + key_file=$1 + shift + + rm -f "$client_out" "$client_log" + cp "$key_file" "$work_dir/id" || fail "couldn't copy $key_file" + chmod 600 "$work_dir/id" + + start_echoserver "$@" + + printf '%s\n\003' "$probe" | \ + ssh -vvv -T -p $port \ + -o StrictHostKeyChecking=no \ + -o UserKnownHostsFile=/dev/null \ + -o IdentitiesOnly=yes \ + -o PreferredAuthentications=publickey \ + -i "$work_dir/id" hansel@127.0.0.1 \ + > "$client_out" 2> "$client_log" & + wait_with_limit $! +} + +echo "Test the OpenSSH client against the wolfSSH echoserver" + +# -e switches the echoserver to its ECC sample keys. A build without ECDSA +# has only the RSA set, so fall back to that rather than call it a failure. +# A build with neither has only Ed25519, and the checked-in Ed25519 key is +# PKCS#8, which an OpenSSH on LibreSSL won't load, so make a key of +# OpenSSH's own and hand the echoserver its public half. +run_openssh_client "$keys_dir/hansel-key-ecc.pem" -e +session_status=$? +if [ $session_status -ne 0 ]; then + echo "the ECC attempt failed, retrying with the RSA sample key" + run_openssh_client "$keys_dir/hansel-key-rsa.pem" + session_status=$? +fi +if [ $session_status -ne 0 ] && command -v ssh-keygen > /dev/null 2>&1; then + echo "the RSA attempt failed, retrying with an Ed25519 key" + ssh-keygen -q -t ed25519 -N '' -C hansel -f "$work_dir/ed25519" \ + < /dev/null || fail "couldn't make an Ed25519 client key" + run_openssh_client "$work_dir/ed25519" -j "$work_dir/ed25519.pub" + session_status=$? +fi +if [ $session_status -ne 0 ]; then + cat "$client_log" + fail "couldn't open a session" +fi + +# What the echoserver put in its KEXINIT. Both spellings go out; this is +# the one deployed OpenSSH matches on. +grep -q 'kex-strict-s-v00@openssh.com' "$client_log" \ + || fail "the echoserver didn't offer the strict KEX marker" + +# And what OpenSSH decided to do about it. +grep -qi 'strict KEX ordering' "$client_log" \ + || fail "the OpenSSH client didn't turn strict KEX on" + +grep -q "$probe" "$client_out" \ + || fail "the echoserver's reply didn't make it back" + +# OpenSSH logs each reset as it happens. Informational: it is the one check +# here that a reworded debug line would break, and the round trip above +# already proves the two ends agree on the sequence numbers. +if grep -q 'resetting read seqnr' "$client_log"; then + echo "the OpenSSH client reset its inbound sequence number" +else + echo "note: no sequence number reset in the client log" +fi + +echo "the OpenSSH client negotiated strict KEX with the echoserver" + +# Part 2 needs an sshd to talk to, and a host key to give it. Neither is +# worth failing the test over. +sshd_bin= +for candidate in /usr/sbin/sshd /usr/local/sbin/sshd /opt/homebrew/sbin/sshd +do + [ -x "$candidate" ] && sshd_bin="$candidate" && break +done +if [ -z "$sshd_bin" ]; then + sshd_bin=`command -v sshd 2>/dev/null` +fi + +if [ -z "$sshd_bin" ] || [ ! -x "$sshd_bin" ]; then + echo "no sshd on this machine, skipping the wolfSSH client half" + do_cleanup + exit 0 +fi +if ! command -v ssh-keygen > /dev/null 2>&1; then + echo "no ssh-keygen, skipping the wolfSSH client half" + do_cleanup + exit 0 +fi +if [ ! -x ./examples/client/client ]; then + echo "the example client wasn't built, skipping the wolfSSH client half" + do_cleanup + exit 0 +fi + +echo "Test the wolfSSH client against OpenSSH's sshd" + +# RSA and Ed25519 host keys beside the ECDSA one, for a build without +# ECDSA. A build with neither ECDSA nor Ed25519 can verify only RSA. +ssh-keygen -q -t ecdsa -N '' -f "$work_dir/hostkey" < /dev/null \ + || fail "couldn't make a host key for sshd" +ssh-keygen -q -t rsa -b 3072 -N '' -f "$work_dir/hostkey-rsa" < /dev/null \ + || fail "couldn't make an RSA host key for sshd" +ssh-keygen -q -t ed25519 -N '' -f "$work_dir/hostkey-ed25519" < /dev/null \ + || fail "couldn't make an Ed25519 host key for sshd" + +# sshd takes a fixed port, so try a few. -f /dev/null keeps the machine's +# own sshd_config out of it, and -ddd both raises the log level and keeps +# sshd in the foreground for one connection. ForceCommand stands in for the +# user's login shell: the client exits with the session's exit status, and +# whatever the user's dotfiles leave behind is not this test's business. +start_sshd() { + rm -f "$sshd_log" + "$sshd_bin" -ddd -f /dev/null -p "$1" -h "$work_dir/hostkey" \ + -h "$work_dir/hostkey-rsa" -h "$work_dir/hostkey-ed25519" \ + -o "ListenAddress=127.0.0.1" \ + -o "AuthorizedKeysFile=$work_dir/authorized_keys" \ + -o "StrictModes=no" \ + -o "PasswordAuthentication=no" \ + -o "ForceCommand=true" \ + -o "PidFile=none" > "$sshd_log" 2>&1 & + sshd_pid=$! + + # Its own counter: the loop that calls this one is counting too. A + # failed bind is checked for as well as a good one -- sshd carries on + # after one address fails, so "Server listening" alone does not mean + # the port the client dials is answering. + sshd_wait=0 + while [ "$sshd_wait" -lt 100 ]; do + grep -q 'failed: Address already in use' "$sshd_log" 2>/dev/null \ + && return 1 + grep -q 'Server listening on' "$sshd_log" 2>/dev/null && return 0 + kill -0 $sshd_pid 2>/dev/null || return 1 + sleep 0.1 + sshd_wait=$((sshd_wait + 1)) + done + + return 1 +} + +sshd_port=0 +candidate_port=$((22500 + ($$ % 1000))) +counter=0 +while [ "$counter" -lt 5 ]; do + if start_sshd $candidate_port; then + sshd_port=$candidate_port + break + fi + kill -9 $sshd_pid 2>/dev/null + sshd_pid=$no_pid + candidate_port=$((candidate_port + 1)) + counter=$((counter + 1)) +done + +if [ "$sshd_port" -eq 0 ]; then + echo "sshd wouldn't start here, skipping the wolfSSH client half" + [ -s "$sshd_log" ] && tail -n 5 "$sshd_log" + do_cleanup + exit 0 +fi + +# The version gate above covered the client. This sshd was found on its own +# and can be the older vendor one while a newer ssh sits earlier in PATH, so +# read what it says about itself before asking it for a marker it has never +# heard of. +sshd_version=`sed -n 's/.*sshd version \(OpenSSH_[^,]*\).*/\1/p' \ + "$sshd_log" | head -n 1` +has_strict_kex "$sshd_version" +if [ $? -ne 0 ]; then + echo "$sshd_bin is ${sshd_version:-an unreadable version}, no strict KEX," + echo "skipping the wolfSSH client half" + do_cleanup + exit 0 +fi + +# sshd authenticates the user running this test, with one of the canned +# client keys put in an authorized_keys file of its own. +login_user=${USER:-`id -un`} +[ -z "$login_user" ] && fail "couldn't work out who is running this" + +run_wolfssh_client() { + cat "$2" > "$work_dir/authorized_keys" \ + || fail "couldn't write the authorized_keys file" + chmod 600 "$work_dir/authorized_keys" + + # -x stops once the session is up: the KEX and the user auth are what + # this half is about, and the echo path was covered above. stdin is + # closed because the client prompts for a password when publickey is + # refused, and a prompt with nothing behind it hangs the run. + HOME="$work_dir" ./examples/client/client \ + -h 127.0.0.1 -p $sshd_port -u "$login_user" -i "$1" -j "$2" -x \ + < /dev/null > "$client_out" 2>&1 & + wait_with_limit $! +} + +# sshd handles one connection per -d run, so it needs restarting between +# attempts. Wait for the old one to go first, or the restart races it for +# the port. +restart_sshd() { + kill -9 $sshd_pid 2>/dev/null + wait $sshd_pid 2>/dev/null + sshd_pid=$no_pid + start_sshd $sshd_port || fail "couldn't restart sshd" +} + +run_wolfssh_client "$keys_dir/hansel-key-ecc.der" \ + "$keys_dir/hansel-key-ecc.pub" +if [ $? -ne 0 ]; then + echo "the ECC attempt failed, retrying with the RSA client key" + restart_sshd + run_wolfssh_client "$keys_dir/hansel-key-rsa.der" \ + "$keys_dir/hansel-key-rsa.pub" +fi +if [ $? -ne 0 ]; then + echo "the RSA attempt failed, retrying with the Ed25519 client key" + restart_sshd + run_wolfssh_client "$keys_dir/hansel-key-ed25519.der" \ + "$keys_dir/hansel-key-ed25519.pub" +fi +if [ $? -ne 0 ]; then + cat "$client_out" + tail -n 20 "$sshd_log" + fail "the wolfSSH client couldn't connect to sshd" +fi + +# Give sshd a moment to finish writing its side of the session. +counter=0 +while [ "$counter" -lt 50 ]; do + grep -q 'Accepted publickey' "$sshd_log" 2>/dev/null && break + sleep 0.1 + counter=$((counter + 1)) +done + +grep -q 'kex-strict-c-v00@openssh.com' "$sshd_log" \ + || fail "the wolfSSH client didn't offer the strict KEX marker" + +grep -qi 'strict KEX ordering' "$sshd_log" \ + || fail "sshd didn't turn strict KEX on" + +grep -q 'Accepted publickey' "$sshd_log" \ + || fail "sshd never authenticated the wolfSSH client" + +if grep -q 'resetting send seqnr' "$sshd_log"; then + echo "sshd reset its outbound sequence number" +else + echo "note: no sequence number reset in the sshd log" +fi + +echo "the wolfSSH client negotiated strict KEX with sshd" + +do_cleanup +echo "OpenSSH strict KEX interop tests passed" +exit 0 diff --git a/src/internal.c b/src/internal.c index c721b7156..188b7eb2b 100644 --- a/src/internal.c +++ b/src/internal.c @@ -585,7 +585,7 @@ const char* GetErrorString(int err) return "not a regular file"; case WS_MSGID_NOT_ALLOWED_E: - return "message not allowed before user authentication"; + return "message ID not allowed at this point in the connection"; case WS_ED25519_E: return "Ed25519 buffer error"; @@ -671,8 +671,9 @@ static INLINE int HighwaterCheck(WOLFSSH* ssh, byte side) /* RFC 4344 Sec 3.1: rekey before the 32-bit SSH sequence number wraps * to prevent MAC/nonce reuse. Counter is per-key (resets on rekey), - * not the absolute ssh->seq (which does not reset); default 2^31 - * keeps each epoch comfortably under the 2^32 wrap. */ + * not the absolute ssh->seq (which resets only at a strict KEX + * NEWKEYS); default 2^31 keeps each epoch comfortably under the + * 2^32 wrap. */ if (!ssh->msgHighwaterFlag && ssh->msgHighwaterMark && (ssh->txMsgCount >= ssh->msgHighwaterMark || ssh->rxMsgCount >= ssh->msgHighwaterMark)) { @@ -725,6 +726,27 @@ static HandshakeInfo* HandshakeInfoNew(void* heap) } +/* The strict KEX setting this handshake runs on. Frozen the first time it + * is read -- at our KEXINIT or at the peer's, whichever comes first -- so + * that a caller toggling the setting mid-handshake cannot leave one side + * advertising the marker and the other refusing to enforce it. A later + * toggle takes effect on the next handshake, which allocates its own + * HandshakeInfo. */ +static byte HandshakeStrictKex(WOLFSSH* ssh) +{ + if (ssh->handshake == NULL) { + return ssh->sendStrictKex; + } + + if (!ssh->handshake->strictKexSet) { + ssh->handshake->strictKex = ssh->sendStrictKex ? 1 : 0; + ssh->handshake->strictKexSet = 1; + } + + return ssh->handshake->strictKex; +} + + static void HandshakeInfoFree(HandshakeInfo* hs, void* heap) { WOLFSSH_UNUSED(heap); @@ -1019,6 +1041,27 @@ INLINE static int IsMessageAllowedClient(WOLFSSH *ssh, byte msg) * send some of those ids the other way. */ INLINE static int IsMessageAllowed(WOLFSSH *ssh, byte msg, byte state) { + /* Strict KEX (Terrapin mitigation). Nothing in the initial KEX is + * authenticated, so any packet spliced into it shifts the receiver's + * sequence number, and that shift is the attack. Take an allow list + * rather than name the messages to refuse: until the peer's NEWKEYS + * lands, the only things it can legitimately send are DISCONNECT and + * the key exchange itself. IGNORE, DEBUG, UNIMPLEMENTED, EXT_INFO and + * the unassigned transport IDs are all otherwise accepted here, and + * every one of them counts against peerSeq. DISCONNECT stays allowed + * so a peer can still tear the connection down. */ + if (state == WS_MSG_RECV && ssh->strictKexEnabled && + !ssh->initialKexDone) { + if (msg != MSGID_DISCONNECT && msg != MSGID_KEXINIT && + msg != MSGID_NEWKEYS && !MSGIDLIMIT_TRANS_KEX(msg)) { + WLOG(WS_LOG_DEBUG, + "Message ID %u not allowed during the initial strict KEX", + msg); + ssh->error = WS_MSGID_NOT_ALLOWED_E; + return 0; + } + } + #ifndef NO_WOLFSSH_SERVER if (ssh->ctx->side == WOLFSSH_ENDPOINT_SERVER) { return IsMessageAllowedServer(ssh, msg); @@ -1399,6 +1442,7 @@ WOLFSSH_CTX* CtxInit(WOLFSSH_CTX* ctx, byte side, void* heap) ctx->windowSz = DEFAULT_WINDOW_SZ; ctx->maxPacketSz = DEFAULT_MAX_PACKET_SZ; ctx->maxAuthAttempts = DEFAULT_MAX_AUTH_ATTEMPTS; + ctx->sendStrictKex = 1; /* default-enabled, callers can opt out */ ctx->sshProtoIdStr = sshProtoIdStr; ctx->sshProtoIdStrSz = (word32)(sizeof(sshProtoIdStr) - 1); ctx->algoListKex = cannedKexAlgoNames; @@ -1795,6 +1839,7 @@ WOLFSSH* SshInit(WOLFSSH* ssh, WOLFSSH_CTX* ctx) ssh->acceptState = ACCEPT_BEGIN; ssh->clientState = CLIENT_BEGIN; ssh->isKeying = 0; /* initial state of not keying yet */ + ssh->sendStrictKex = ctx->sendStrictKex; ssh->authId = ID_USERAUTH_PUBLICKEY; ssh->supportedAuth[0] = ID_USERAUTH_PUBLICKEY; ssh->supportedAuth[1] = ID_USERAUTH_PASSWORD; @@ -3787,8 +3832,16 @@ static const NameIdPair NameIdMap[] = { { ID_CURVE25519_SHA256, TYPE_KEX, "curve25519-sha256" }, { ID_CURVE25519_SHA256_LIBSSH, TYPE_KEX, "curve25519-sha256@libssh.org" }, #endif - { ID_EXTINFO_S, TYPE_OTHER, "ext-info-s" }, - { ID_EXTINFO_C, TYPE_OTHER, "ext-info-c" }, + { ID_EXT_INFO_S, TYPE_OTHER, "ext-info-s" }, + { ID_EXT_INFO_C, TYPE_OTHER, "ext-info-c" }, + /* Strict KEX marker. draft-miller-sshm-strict-kex defines the + * unprefixed names for eventual IETF standardization; deployed + * OpenSSH only sends the -v00@openssh.com ones. Advertise and accept + * both spellings. */ + { ID_EXT_STRICT_KEX_S, TYPE_OTHER, "kex-strict-s" }, + { ID_EXT_STRICT_KEX_C, TYPE_OTHER, "kex-strict-c" }, + { ID_EXT_PRE_STRICT_KEX_S, TYPE_OTHER, "kex-strict-s-v00@openssh.com" }, + { ID_EXT_PRE_STRICT_KEX_C, TYPE_OTHER, "kex-strict-c-v00@openssh.com" }, /* Public Key IDs */ #ifndef WOLFSSH_NO_RSA @@ -6651,12 +6704,56 @@ static int DoKexInit(WOLFSSH* ssh, byte* buf, word32 len, word32* idx) /* Match the peer accepts extInfo. */ algoId = (side == WOLFSSH_ENDPOINT_SERVER) - ? ID_EXTINFO_C : ID_EXTINFO_S; + ? ID_EXT_INFO_C : ID_EXT_INFO_S; extInfo = MatchIdLists(side, list, listSz, &algoId, 1); ssh->sendExtInfo = extInfo == algoId; } } + /* Strict KEX marker (Terrapin mitigation). Only valid in the initial + * KEXINIT (sessionIdSz == 0); if the peer offers it during a rekey, + * ignore the marker per draft-miller-sshm-strict-kex. */ + if (ret == WS_SUCCESS) { + if (ssh->sessionIdSz == 0) { + /* OpenSSH only ever sends the -v00@openssh.com name, so both + * spellings have to be accepted for the marker to negotiate + * against a real peer. */ + byte expectedStrict[2]; + byte matched; + + if (side == WOLFSSH_ENDPOINT_SERVER) { + expectedStrict[0] = ID_EXT_STRICT_KEX_C; + expectedStrict[1] = ID_EXT_PRE_STRICT_KEX_C; + } + else { + expectedStrict[0] = ID_EXT_STRICT_KEX_S; + expectedStrict[1] = ID_EXT_PRE_STRICT_KEX_S; + } + matched = MatchIdLists(side, list, listSz, expectedStrict, 2); + ssh->peerStrictKex = (matched != ID_UNKNOWN); + ssh->strictKexEnabled = + ssh->peerStrictKex && HandshakeStrictKex(ssh); + if (ssh->strictKexEnabled) { + WLOG(WS_LOG_DEBUG, "DKI: strict KEX negotiated"); + } + + /* Strict KEX requires KEXINIT to be the peer's first packet. + * The allow list is not armed until now. peerSeq is still + * this packet's number here. */ + if (ssh->strictKexEnabled && ssh->peerSeq != 0) { + WLOG(WS_LOG_DEBUG, + "DKI: strict KEX, KEXINIT was not the first packet"); + (void)SendDisconnect(ssh, + WOLFSSH_DISCONNECT_KEY_EXCHANGE_FAILED); + ret = WS_MSGID_NOT_ALLOWED_E; + } + } + else { + WLOG(WS_LOG_DEBUG, + "DKI: rekey, ignoring any peer strict KEX marker"); + } + } + /* Server Host Key Algorithms */ if (ret == WS_SUCCESS) { WLOG(WS_LOG_DEBUG, "DKI: Server Host Key Algorithms"); @@ -13917,6 +14014,20 @@ static int DoPacket(WOLFSSH* ssh, byte* bufferConsumed) msgAllowed = IsMessageAllowed(ssh, msg, WS_MSG_RECV); + if (!msgAllowed && ssh->strictKexEnabled && !ssh->initialKexDone) { + /* Strict KEX calls for terminating the connection, not just + * dropping the packet, so tell the peer why on the way out. Every + * refused id ends it here, the unassigned ones an UNIMPLEMENTED + * would otherwise answer included. The error is relatched because + * the send path can overwrite it. */ + if (!ssh->disconnected) { + (void)SendDisconnect(ssh, + WOLFSSH_DISCONNECT_KEY_EXCHANGE_FAILED); + ssh->error = WS_MSGID_NOT_ALLOWED_E; + } + return WS_MSGID_NOT_ALLOWED_E; + } + if (!msgAllowed && (MsgIdKnown(msg) || MSGIDLIMIT_POST_USERAUTH(msg))) { /* RFC 4252 section 6: disconnect on a known id at the wrong time, * and on any id of 80 or higher, which IsMessageAllowed() refuses @@ -14168,7 +14279,23 @@ static int DoPacket(WOLFSSH* ssh, byte* bufferConsumed) idx = len; } ssh->inputBuffer.idx = idx; - ssh->peerSeq++; + if (msg == MSGID_NEWKEYS) { + /* Strict KEX (Terrapin mitigation): once negotiated, every + * SSH_MSG_NEWKEYS resets the incoming sequence number so the + * next inbound packet starts at zero under the new keys. */ + if (ssh->strictKexEnabled) { + ssh->peerSeq = 0; + } + else { + ssh->peerSeq++; + } + /* The peer's initial KEX is over, so IGNORE, DEBUG, and + * UNIMPLEMENTED are legal from it again. */ + ssh->initialKexDone = 1; + } + else { + ssh->peerSeq++; + } ssh->rxMsgCount++; *bufferConsumed = 1; @@ -14756,7 +14883,9 @@ int DoReceive(WOLFSSH* ssh) ssh->inputBuffer.idx += peerMacSz; WLOG(WS_LOG_DEBUG, "PR4: Shrinking input buffer"); - ShrinkBuffer(&ssh->inputBuffer, 1); + /* Keep bytes past this packet; DoProtoId() can leave some. */ + ShrinkBuffer(&ssh->inputBuffer, + ssh->inputBuffer.idx >= ssh->inputBuffer.length); ssh->processReplyState = PROCESS_INIT; } @@ -15260,12 +15389,38 @@ int SendKexInit(WOLFSSH* ssh) } if (ret == WS_SUCCESS) { + /* The strict-kex marker is only advertised during the initial KEX + * (RFC draft-miller-sshm-strict-kex), distinguished here by an + * empty session id. The sendStrictKex flag lets callers opt out + * at runtime; the handshake freezes it so this and the enforcement + * decision in DoKexInit() cannot read different values. The + * -v00@openssh.com marker goes last: Paramiko lets the last + * kex-strict-* name it sees decide, and knows only that one. */ + int includeStrictKex = + (ssh->sessionIdSz == 0) && HandshakeStrictKex(ssh); + if (ssh->ctx->side == WOLFSSH_ENDPOINT_CLIENT) { - kexAlgoNamesPlus = ",ext-info-c"; + if (includeStrictKex) { + kexAlgoNamesPlus = + ",ext-info-c" + ",kex-strict-c" + ",kex-strict-c-v00@openssh.com"; + } + else { + kexAlgoNamesPlus = ",ext-info-c"; + } kexAlgoNamesPlusSz = (word32)WSTRLEN(kexAlgoNamesPlus); } else { - kexAlgoNamesPlus = ",ext-info-s"; + if (includeStrictKex) { + kexAlgoNamesPlus = + ",ext-info-s" + ",kex-strict-s" + ",kex-strict-s-v00@openssh.com"; + } + else { + kexAlgoNamesPlus = ",ext-info-s"; + } kexAlgoNamesPlusSz = (word32)WSTRLEN(kexAlgoNamesPlus); } @@ -18717,6 +18872,16 @@ int SendNewKeys(WOLFSSH* ssh) ssh->outputBuffer.length = idx; ret = BundlePacket(ssh); + + /* Strict KEX (Terrapin mitigation): once negotiated, every + * SSH_MSG_NEWKEYS resets the outgoing sequence number so the next + * outbound packet starts at zero under the new keys. This has to + * happen here, before SendPendingChannelWindowAdjust() below can + * bundle a packet, or that packet goes out at the old sequence + * number while the peer MACs it at zero. */ + if (ret == WS_SUCCESS && ssh->strictKexEnabled) { + ssh->seq = 0; + } } if (ret == WS_SUCCESS) { diff --git a/src/ssh.c b/src/ssh.c index fa75c4177..6e729eeaa 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -3465,6 +3465,56 @@ int wolfSSH_CheckAlgoName(const char* name) } +int wolfSSH_CTX_SetStrictKex(WOLFSSH_CTX* ctx, byte enable) +{ + int ret = WS_BAD_ARGUMENT; + + if (ctx) { + ctx->sendStrictKex = enable ? 1 : 0; + ret = WS_SUCCESS; + } + + return ret; +} + + +int wolfSSH_CTX_GetStrictKex(WOLFSSH_CTX* ctx) +{ + int ret = WS_BAD_ARGUMENT; + + if (ctx) { + ret = ctx->sendStrictKex ? 1 : 0; + } + + return ret; +} + + +int wolfSSH_SetStrictKex(WOLFSSH* ssh, byte enable) +{ + int ret = WS_SSH_NULL_E; + + if (ssh) { + ssh->sendStrictKex = enable ? 1 : 0; + ret = WS_SUCCESS; + } + + return ret; +} + + +int wolfSSH_GetStrictKex(WOLFSSH* ssh) +{ + int ret = WS_SSH_NULL_E; + + if (ssh) { + ret = ssh->sendStrictKex ? 1 : 0; + } + + return ret; +} + + const char* wolfSSH_QueryKex(word32* idx) { return NameByIndexType(TYPE_KEX, idx); @@ -5699,7 +5749,7 @@ size_t wolfSSH_GetText(WOLFSSH *ssh, WS_Text id, char *str, size_t strSz) break; #endif /* !WOLFSSH_NO_DH */ - case ID_EXTINFO_S: + case ID_EXT_INFO_S: #if defined(__CCRX__) ret = WSNPRINTF0(str, strSz, "Server extensions KEX"); #else @@ -5707,7 +5757,7 @@ size_t wolfSSH_GetText(WOLFSSH *ssh, WS_Text id, char *str, size_t strSz) #endif break; - case ID_EXTINFO_C: + case ID_EXT_INFO_C: #if defined(__CCRX__) ret = WSNPRINTF0(str, strSz, "Client extensions KEX"); #else diff --git a/tests/api.c b/tests/api.c index 916180037..42e27bf05 100644 --- a/tests/api.c +++ b/tests/api.c @@ -7320,6 +7320,69 @@ static void test_wolfSSH_RealPath(void) { ; } #endif +/* Strict KEX (Terrapin mitigation, CVE-2023-48795) is on by default; the + * runtime controls exist so a caller stuck with a broken peer can turn the + * marker off. */ +static void test_wolfSSH_StrictKex(void) +{ + WOLFSSH_CTX* ctx; + WOLFSSH* ssh; + + AssertIntEQ(wolfSSH_SetStrictKex(NULL, 1), WS_SSH_NULL_E); + AssertIntEQ(wolfSSH_GetStrictKex(NULL), WS_SSH_NULL_E); + AssertIntEQ(wolfSSH_CTX_SetStrictKex(NULL, 1), WS_BAD_ARGUMENT); + AssertIntEQ(wolfSSH_CTX_GetStrictKex(NULL), WS_BAD_ARGUMENT); + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL); + AssertNotNull(ctx); + ssh = wolfSSH_new(ctx); + AssertNotNull(ssh); + + /* Default-enabled: a caller has to ask to be vulnerable. */ + AssertIntEQ(wolfSSH_CTX_GetStrictKex(ctx), 1); + AssertIntEQ(wolfSSH_GetStrictKex(ssh), 1); + + AssertIntEQ(wolfSSH_SetStrictKex(ssh, 0), WS_SUCCESS); + AssertIntEQ(wolfSSH_GetStrictKex(ssh), 0); + + AssertIntEQ(wolfSSH_SetStrictKex(ssh, 1), WS_SUCCESS); + AssertIntEQ(wolfSSH_GetStrictKex(ssh), 1); + + /* Any non-zero enables, and the getter normalizes to 1. */ + AssertIntEQ(wolfSSH_SetStrictKex(ssh, 200), WS_SUCCESS); + AssertIntEQ(wolfSSH_GetStrictKex(ssh), 1); + + /* Nothing has been negotiated yet, so the peer and session flags are + * still clear. */ + AssertIntEQ(ssh->peerStrictKex, 0); + AssertIntEQ(ssh->strictKexEnabled, 0); + AssertIntEQ(ssh->initialKexDone, 0); + + /* The CTX setting seeds the sessions made after it and leaves the ones + * already made alone, so the session above keeps what it was born + * with across the change. */ + AssertIntEQ(wolfSSH_CTX_SetStrictKex(ctx, 0), WS_SUCCESS); + AssertIntEQ(wolfSSH_CTX_GetStrictKex(ctx), 0); + AssertIntEQ(wolfSSH_GetStrictKex(ssh), 1); + + wolfSSH_free(ssh); + + ssh = wolfSSH_new(ctx); + AssertNotNull(ssh); + AssertIntEQ(wolfSSH_GetStrictKex(ssh), 0); + + /* The session setting overrides it for that session alone. */ + AssertIntEQ(wolfSSH_SetStrictKex(ssh, 1), WS_SUCCESS); + AssertIntEQ(wolfSSH_GetStrictKex(ssh), 1); + AssertIntEQ(wolfSSH_CTX_GetStrictKex(ctx), 0); + + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); + + printf("\tstrict KEX runtime controls.\n"); +} + + static void test_wolfSSH_SetMaxAuthAttempts(void) { WOLFSSH_CTX* ctx; @@ -8361,6 +8424,7 @@ int wolfSSH_ApiTest(int argc, char** argv) test_wolfSSH_ReadKey_sshNoComment(); test_wolfSSH_QueryAlgoList(); test_wolfSSH_SetMaxAuthAttempts(); + test_wolfSSH_StrictKex(); test_wolfSSH_AlgoListKeyInSync(); test_wolfSSH_SetAlgoList(); test_wolfSSH_DefaultAlgoListsExcludeWeak(); diff --git a/tests/regress.c b/tests/regress.c index bb6de33d0..bed34bab1 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -682,6 +682,11 @@ static word32 LoadFileBuffer(const char* path, byte* buf, word32 bufSz) } #endif /* KEXDH_REPLY_REGRESS_KEX_ALGO || WOLFSSH_TEST_INTERNAL */ +/* A transport-layer message id with no meaning assigned to it. Anything in + * 8..19 will do; the receiver has no handler for it and answers with + * UNIMPLEMENTED, having already counted the packet. */ +#define REGRESS_UNASSIGNED_TRANS_MSGID 9 + #ifdef KEXDH_REPLY_REGRESS_KEX_ALGO #define REGRESS_DUPLEX_QUEUE_SZ 32768U @@ -733,6 +738,25 @@ static word32 LoadFileBuffer(const char* path, byte* buf, word32 bufSz) #define REGRESS_MUTATE_E_EMPTY 5 #define REGRESS_MUTATE_GEX_GROUP_SHRINK 6 #define REGRESS_MUTATE_GEX_GEN_BAD 7 +#define REGRESS_MUTATE_TERRAPIN 8 +/* The same splice aimed the other way: the packet goes in ahead of the + * client's NEWKEYS, so the server is the one asked to accept it. */ +#define REGRESS_MUTATE_TERRAPIN_C2S 9 +/* The splice ahead of the server's KEXINIT, before the allow list arms. */ +#define REGRESS_MUTATE_TERRAPIN_PRE_KEXINIT 10 + +/* 4 (len) + 1 (padLen) + 1 (msgId) + 4 (empty string payload) + 6 (pad) */ +#define REGRESS_INJECT_PACKET_SZ 16U + +/* An injected DISCONNECT carries a reason code and two strings, RFC 4253 + * section 11.1, so it decodes and the receiver's answer is the gate's + * doing rather than a short payload's. + * 4 (len) + 1 (padLen) + 1 (msgId) + 4 (reason) + 4 + 4 (empty strings) + * + 6 (pad) */ +#define REGRESS_INJECT_DISCONNECT_SZ 24U + +/* Every injected packet fits in a buffer of this size. */ +#define REGRESS_INJECT_MAX_SZ REGRESS_INJECT_DISCONNECT_SZ typedef struct { byte data[REGRESS_DUPLEX_QUEUE_SZ]; @@ -748,6 +772,9 @@ typedef struct { byte scratch[REGRESS_MUTATION_SCRATCH_SZ]; word32 scratchSz; byte mode; + /* Terrapin bookkeeping */ + byte injectMsgId; + word32 injectedPackets; } KexReplyMutator; typedef struct DuplexEndpoint { @@ -757,6 +784,9 @@ typedef struct DuplexEndpoint { word32 disconnectReason; byte isServer; byte sawDisconnect; + /* What this endpoint put in its own plaintext KEXINIT */ + byte sentStrictKexMarker; + byte sentExtInfoMarker; } DuplexEndpoint; typedef struct { @@ -1186,11 +1216,194 @@ static int RewriteSingleKexDhGexGroupPacket(const byte* packet, } #endif /* REGRESS_GEX_KEX_ALGO */ -/* SIG_*, F_TRUNC and the GEX_* modes rewrite the server's messages; - * E_TRUNC and E_EMPTY the client's init. */ +/* Substring search over a wire buffer, which is not NUL terminated. */ +static int BufferContainsString(const byte* buf, word32 bufSz, const char* str) +{ + word32 strSz = (word32)WSTRLEN(str); + word32 i; + + if (strSz == 0 || bufSz < strSz) { + return 0; + } + + for (i = 0; i + strSz <= bufSz; i++) { + if (WMEMCMP(buf + i, str, strSz) == 0) { + return 1; + } + } + + return 0; +} + +/* Note the pseudo-KEX markers this endpoint advertises. Only the initial + * KEXINIT travels in the clear, which is the one the strict KEX marker is + * allowed on, so a rekey that wrongly re-advertises it is caught by the + * marker never appearing rather than by reading the encrypted packet. */ +static void NoteOutboundKexInitMarkers(DuplexEndpoint* endpoint, + const byte* packet, word32 packetSz) +{ + const char* strictName = endpoint->isServer + ? "kex-strict-s" : "kex-strict-c"; + const char* extInfoName = endpoint->isServer ? "ext-info-s" : "ext-info-c"; + + if (BufferContainsString(packet, packetSz, strictName)) { + endpoint->sentStrictKexMarker = 1; + } + if (BufferContainsString(packet, packetSz, extInfoName)) { + endpoint->sentExtInfoMarker = 1; + } +} + +/* Build the packet a Terrapin attacker splices into the initial KEX. The + * payload is four zero bytes, which reads as an empty string for IGNORE + * (RFC 4253 section 11.2, and DoIgnore() only skips it), as a sequence + * number for UNIMPLEMENTED, and as an empty extension list for EXT_INFO. + * An unassigned id has no handler to read it at all. Sized to a multiple + * of MIN_BLOCK_SZ because the packet travels in the clear. */ +static void BuildInjectedPacketPlain(byte* out, word32 outSz, byte msgId) +{ + const byte padLen = 6; + /* padLen field + msg id + empty string + padding */ + const word32 packetLen = PAD_LENGTH_SZ + MSG_ID_SZ + UINT32_SZ + padLen; + + AssertIntEQ(outSz, REGRESS_INJECT_PACKET_SZ); + AssertIntEQ(UINT32_SZ + packetLen, REGRESS_INJECT_PACKET_SZ); + + (void)AppendUint32(out, outSz, 0, packetLen); + out[UINT32_SZ] = padLen; + out[UINT32_SZ + PAD_LENGTH_SZ] = msgId; + /* empty string payload, then zero padding */ + WMEMSET(out + UINT32_SZ + PAD_LENGTH_SZ + MSG_ID_SZ, 0, + UINT32_SZ + padLen); +} + +/* The DISCONNECT form of the same splice. Strict KEX lets this one through + * on purpose, so unlike the packets above it has to decode: DoDisconnect() + * reads the reason code and both strings before it latches WS_DISCONNECT. */ +static void BuildInjectedDisconnectPlain(byte* out, word32 outSz) +{ + const byte padLen = 6; + /* padLen field + msg id + reason + two empty strings + padding */ + const word32 packetLen = PAD_LENGTH_SZ + MSG_ID_SZ + (3 * UINT32_SZ) + + padLen; + word32 idx; + + AssertIntEQ(outSz, REGRESS_INJECT_DISCONNECT_SZ); + AssertIntEQ(UINT32_SZ + packetLen, REGRESS_INJECT_DISCONNECT_SZ); + + idx = AppendUint32(out, outSz, 0, packetLen); + out[idx++] = padLen; + out[idx++] = MSGID_DISCONNECT; + idx = AppendUint32(out, outSz, idx, WOLFSSH_DISCONNECT_BY_APPLICATION); + /* empty description and language strings, then zero padding */ + WMEMSET(out + idx, 0, outSz - idx); +} + +/* Lay down the packet to splice in, reporting how long it came out. */ +static word32 BuildInjectedPacket(byte* out, word32 outSz, byte msgId) +{ + AssertIntEQ(outSz, REGRESS_INJECT_MAX_SZ); + + if (msgId == MSGID_DISCONNECT) { + BuildInjectedDisconnectPlain(out, REGRESS_INJECT_DISCONNECT_SZ); + return REGRESS_INJECT_DISCONNECT_SZ; + } + + BuildInjectedPacketPlain(out, REGRESS_INJECT_PACKET_SZ, msgId); + return REGRESS_INJECT_PACKET_SZ; +} + +/* Walk a plaintext write looking for the packet carrying msgId, reporting + * the offset it starts at. Only sound before NEWKEYS, where the framing is + * readable; the Terrapin splice point is by definition in that window. */ +static int FindPlainPacketOffset(const byte* packet, word32 packetSz, + byte msgId, word32* offsetOut) +{ + word32 offset = 0; + + while (packetSz - offset >= UINT32_SZ + PAD_LENGTH_SZ + MSG_ID_SZ) { + word32 curPacketSz = ReadUint32(packet + offset) + UINT32_SZ; + + if (curPacketSz > packetSz - offset || + curPacketSz < UINT32_SZ + PAD_LENGTH_SZ + MSG_ID_SZ + + MIN_PAD_LENGTH) { + return 0; + } + + if (packet[offset + UINT32_SZ + PAD_LENGTH_SZ] == msgId) { + *offsetOut = offset; + return 1; + } + + offset += curPacketSz; + } + + return 0; +} + +/* The Terrapin prefix-truncation attack (CVE-2023-48795). Nothing in the + * initial KEX is authenticated, so a man in the middle can splice an extra + * SSH_MSG_IGNORE in ahead of the victim's NEWKEYS. The victim accepts it, + * and from then on its inbound sequence number runs one ahead of what the + * peer is MACing against. + * + * The published attack pairs that with deleting a packet after NEWKEYS to + * put the counters back in step, silently truncating the stream. That step + * only works when the cipher takes its nonce from the sequence number -- + * chacha20-poly1305@openssh.com or the *-etm@openssh.com MACs. wolfSSH + * implements neither: its AES-GCM, CTR, and CBC modes all carry chained + * cipher state, so a deletion breaks decryption outright. The injection + * alone is what these tests exercise, and refusing it is what the + * mitigation has to do. */ +static void TerrapinInject(DuplexEndpoint* endpoint, + const byte** output, word32* outputSz) +{ + KexReplyMutator* mutator = endpoint->mutator; + byte injectPkt[REGRESS_INJECT_MAX_SZ]; + word32 injectPktSz; + word32 anchorOffset; + byte anchorMsgId = (mutator->mode == REGRESS_MUTATE_TERRAPIN_PRE_KEXINIT) + ? MSGID_KEXINIT : MSGID_NEWKEYS; + + if (mutator->injectedPackets > 0) { + return; + } + if (!FindPlainPacketOffset(*output, *outputSz, anchorMsgId, + &anchorOffset)) { + return; + } + + injectPktSz = BuildInjectedPacket(injectPkt, (word32)sizeof(injectPkt), + mutator->injectMsgId); + + /* Pass through what precedes the anchor, then the forged packet; the + * caller forwards the rest. */ + if (anchorOffset > 0) { + AssertIntEQ(QueueAppend(&endpoint->peer->inbound, *output, + anchorOffset), WS_SUCCESS); + } + AssertIntEQ(QueueAppend(&endpoint->peer->inbound, injectPkt, + injectPktSz), WS_SUCCESS); + + mutator->injectedPackets++; + *output += anchorOffset; + *outputSz -= anchorOffset; +} + +/* Each splices a packet into the initial KEX ahead of a different one. */ +static int IsTerrapinMode(byte mode) +{ + return mode == REGRESS_MUTATE_TERRAPIN || + mode == REGRESS_MUTATE_TERRAPIN_C2S || + mode == REGRESS_MUTATE_TERRAPIN_PRE_KEXINIT; +} + +/* E_TRUNC, E_EMPTY and TERRAPIN_C2S rewrite the client's messages; the + * rest rewrite the server's. */ static int MutatorTargetsEndpoint(byte mode, byte isServer) { - if (mode == REGRESS_MUTATE_E_TRUNC || mode == REGRESS_MUTATE_E_EMPTY) { + if (mode == REGRESS_MUTATE_E_TRUNC || mode == REGRESS_MUTATE_E_EMPTY || + mode == REGRESS_MUTATE_TERRAPIN_C2S) { return !isServer; } return isServer != 0; @@ -1276,6 +1489,18 @@ static int DuplexSend(WOLFSSH* ssh, void* buf, word32 sz, void* ctx) if (endpoint->mutator != NULL && endpoint->mutator->enabled && + IsTerrapinMode(endpoint->mutator->mode) && + MutatorTargetsEndpoint(endpoint->mutator->mode, + endpoint->isServer) && + !(outputSz >= REGRESS_SSH_PROTO_PREFIX_SZ && + WMEMCMP(output, REGRESS_SSH_PROTO_PREFIX, + REGRESS_SSH_PROTO_PREFIX_SZ) == 0)) { + TerrapinInject(endpoint, &output, &outputSz); + } + + if (endpoint->mutator != NULL && + endpoint->mutator->enabled && + !IsTerrapinMode(endpoint->mutator->mode) && MutatorTargetsEndpoint(endpoint->mutator->mode, endpoint->isServer) && endpoint->mutator->mutatedPackets == 0 && @@ -1329,6 +1554,7 @@ static int DuplexSend(WOLFSSH* ssh, void* buf, word32 sz, void* ctx) WMEMCMP(output, REGRESS_SSH_PROTO_PREFIX, REGRESS_SSH_PROTO_PREFIX_SZ) == 0)) { NoteOutboundDisconnect(endpoint, output, outputSz); + NoteOutboundKexInitMarkers(endpoint, output, outputSz); } ret = QueueAppend(&endpoint->peer->inbound, output, outputSz); @@ -2100,6 +2326,558 @@ static void TestKexDhGexGroupBadGeneratorSendsDisconnect(void) } #endif /* REGRESS_GEX_KEX_ALGO */ +/* ---- Strict KEX, the Terrapin mitigation (CVE-2023-48795) ---- */ + +/* KEXINIT, KEXDH_INIT/KEXDH_REPLY, NEWKEYS. Each side sends exactly three + * packets before its NEWKEYS, so strict KEX zeroing the counters there + * leaves both of them three short of an unmitigated run. */ +#define REGRESS_PRE_NEWKEYS_PACKETS 3 + +/* injectMsgId is the message a man in the middle splices in ahead of the + * NEWKEYS the mode names, or MSGID_NONE for a clean run. */ +static void InitStrictKexHarnessMode(KexReplyHarness* harness, + byte injectMsgId, byte clientStrict, byte serverStrict, byte mode) +{ + InitKexReplyHarnessEx(harness, REGRESS_DEFAULT_KEY_ALGO, + REGRESS_DEFAULT_KEY_PATH, injectMsgId != MSGID_NONE, + mode, NULL, 0); + harness->mutator.injectMsgId = injectMsgId; + AssertIntEQ(wolfSSH_SetStrictKex(harness->client, clientStrict), + WS_SUCCESS); + AssertIntEQ(wolfSSH_SetStrictKex(harness->server, serverStrict), + WS_SUCCESS); +} + +/* The injection the client has to answer: spliced in ahead of the server's + * NEWKEYS. */ +static void InitStrictKexHarness(KexReplyHarness* harness, byte injectMsgId, + byte clientStrict, byte serverStrict) +{ + InitStrictKexHarnessMode(harness, injectMsgId, clientStrict, serverStrict, + REGRESS_MUTATE_TERRAPIN); +} + +/* Two wolfSSH peers with the default settings must come out of the initial + * KEX with strict KEX on at both ends. */ +static void TestStrictKexNegotiatedByDefault(void) +{ + KexReplyHarness harness; + KexReplyRunResult result; + + InitStrictKexHarness(&harness, MSGID_NONE, 1, 1); + AssertIntEQ(wolfSSH_GetStrictKex(harness.client), 1); + AssertIntEQ(wolfSSH_GetStrictKex(harness.server), 1); + + RunKexReplyHandshake(&harness, &result); + + AssertTrue(result.clientSuccess); + AssertTrue(result.serverSuccess); + AssertIntEQ(harness.client->peerStrictKex, 1); + AssertIntEQ(harness.client->strictKexEnabled, 1); + AssertIntEQ(harness.server->peerStrictKex, 1); + AssertIntEQ(harness.server->strictKexEnabled, 1); + + /* Both sides saw the peer's NEWKEYS, so the initial-KEX message + * restriction has lifted. */ + AssertIntEQ(harness.client->initialKexDone, 1); + AssertIntEQ(harness.server->initialKexDone, 1); + + /* Both markers really went out on the initial KEXINIT. */ + AssertIntEQ(harness.clientIo.sentStrictKexMarker, 1); + AssertIntEQ(harness.serverIo.sentStrictKexMarker, 1); + + /* Control for the Terrapin tests below: an unattacked client does + * receive the server's EXT_INFO. */ + AssertTrue(harness.client->peerSigIdSz > 0); + + FreeKexReplyHarness(&harness); +} + +/* The marker rides on the KEXINIT algorithm list, so a peer that opts out + * never advertises it and neither side turns the mitigation on. The + * handshake still has to complete -- an old peer must stay interoperable. */ +static void AssertStrictKexOptOut(byte clientStrict, byte serverStrict) +{ + KexReplyHarness harness; + KexReplyRunResult result; + + InitStrictKexHarness(&harness, MSGID_NONE, clientStrict, serverStrict); + RunKexReplyHandshake(&harness, &result); + + AssertTrue(result.clientSuccess); + AssertTrue(result.serverSuccess); + + /* The opted-out side left the marker off its KEXINIT, and the peer + * agrees it never arrived. */ + AssertIntEQ(harness.clientIo.sentStrictKexMarker, clientStrict); + AssertIntEQ(harness.serverIo.sentStrictKexMarker, serverStrict); + AssertIntEQ(harness.client->peerStrictKex, serverStrict); + AssertIntEQ(harness.server->peerStrictKex, clientStrict); + AssertIntEQ(harness.client->strictKexEnabled, 0); + AssertIntEQ(harness.server->strictKexEnabled, 0); + + /* Only the strict KEX marker is conditional; ext-info still goes out. */ + AssertIntEQ(harness.clientIo.sentExtInfoMarker, 1); + AssertIntEQ(harness.serverIo.sentExtInfoMarker, 1); + + FreeKexReplyHarness(&harness); +} + +static void TestStrictKexClientOptOut(void) +{ + AssertStrictKexOptOut(0, 1); +} + +static void TestStrictKexServerOptOut(void) +{ + AssertStrictKexOptOut(1, 0); +} + +/* A handshake freezes the strict KEX setting when it starts, so the + * decision to advertise the marker and the decision to enforce it cannot + * read different values. + * + * The server is where the two can be prised apart: its KEXINIT goes out on + * the version exchange, in a different wolfSSH_accept() call from the one + * that reads the client's KEXINIT and decides whether to enforce. Turn the + * setting off in between. Without the freeze the server would advertise a + * mitigation it then declined to apply, leaving it counting sequence + * numbers its peer had already zeroed. */ +static void TestStrictKexSettingFrozenForHandshake(void) +{ + KexReplyHarness harness; + KexReplyRunResult result; + word32 step; + + InitStrictKexHarness(&harness, MSGID_NONE, 1, 1); + + /* The client's version string, which is all the server needs to answer + * with its own version and its KEXINIT. */ + (void)wolfSSH_connect(harness.client); + + /* Drive the server alone: the client sends its KEXINIT on its next + * call, so the server cannot reach DoKexInit() while this runs. */ + for (step = 0; step < REGRESS_MAX_HANDSHAKE_STEPS && + !harness.serverIo.sentStrictKexMarker; step++) { + (void)wolfSSH_accept(harness.server); + } + AssertIntEQ(harness.serverIo.sentStrictKexMarker, 1); + + /* Advertised, but nothing has been negotiated yet. */ + AssertIntEQ(harness.server->peerStrictKex, 0); + AssertIntEQ(harness.server->strictKexEnabled, 0); + + /* The caller changes its mind with the handshake already running. */ + AssertIntEQ(wolfSSH_SetStrictKex(harness.server, 0), WS_SUCCESS); + AssertIntEQ(wolfSSH_GetStrictKex(harness.server), 0); + + RunKexReplyHandshake(&harness, &result); + + AssertTrue(result.clientSuccess); + AssertTrue(result.serverSuccess); + + /* Both ends came out of it mitigated, on the value the handshake + * started with rather than the one the session now holds. */ + AssertIntEQ(harness.server->strictKexEnabled, 1); + AssertIntEQ(harness.client->strictKexEnabled, 1); + AssertIntEQ(wolfSSH_GetStrictKex(harness.server), 0); + + FreeKexReplyHarness(&harness); +} + +/* draft-miller-sshm-strict-kex puts the marker in the first KEXINIT only. + * A rekey KEXINIT is encrypted, so rather than read it off the wire, force + * SendKexInit down its rekey path -- a session id is already established -- + * and check the marker never appears in the clear. The handshake itself is + * expected to fall over on the planted session id; only what went out + * before that matters. */ +static void TestStrictKexMarkerNotSentOnRekey(void) +{ + KexReplyHarness harness; + KexReplyRunResult result; + + InitStrictKexHarness(&harness, MSGID_NONE, 1, 1); + + /* A non-empty session id is what SendKexInit reads as "this is a + * rekey", the same test DoKexInit makes on the receiving side. */ + WMEMSET(harness.client->sessionId, 0x5A, WC_SHA256_DIGEST_SIZE); + harness.client->sessionIdSz = WC_SHA256_DIGEST_SIZE; + + RunKexReplyHandshake(&harness, &result); + + /* The client did send a KEXINIT, and it carried ext-info-c but not the + * strict KEX marker. */ + AssertIntEQ(harness.clientIo.sentExtInfoMarker, 1); + AssertIntEQ(harness.clientIo.sentStrictKexMarker, 0); + + /* The server saw no marker either, so it cannot have turned the + * mitigation on off the back of a rekey. */ + AssertIntEQ(harness.server->peerStrictKex, 0); + AssertIntEQ(harness.server->strictKexEnabled, 0); + + FreeKexReplyHarness(&harness); +} + +/* NEWKEYS must zero both sequence numbers once strict KEX is negotiated. + * Run the same handshake with and without it and compare the counters: the + * mitigated run has to be short by exactly the packets each side sent + * before its NEWKEYS. */ +static void TestStrictKexResetsSequenceNumbers(void) +{ + KexReplyHarness strict; + KexReplyHarness plain; + KexReplyRunResult result; + + InitStrictKexHarness(&strict, MSGID_NONE, 1, 1); + RunKexReplyHandshake(&strict, &result); + AssertTrue(result.clientSuccess); + AssertTrue(result.serverSuccess); + AssertIntEQ(strict.client->strictKexEnabled, 1); + + InitStrictKexHarness(&plain, MSGID_NONE, 0, 0); + RunKexReplyHandshake(&plain, &result); + AssertTrue(result.clientSuccess); + AssertTrue(result.serverSuccess); + AssertIntEQ(plain.client->strictKexEnabled, 0); + + AssertIntEQ(strict.client->seq + REGRESS_PRE_NEWKEYS_PACKETS, + plain.client->seq); + AssertIntEQ(strict.client->peerSeq + REGRESS_PRE_NEWKEYS_PACKETS, + plain.client->peerSeq); + AssertIntEQ(strict.server->seq + REGRESS_PRE_NEWKEYS_PACKETS, + plain.server->seq); + AssertIntEQ(strict.server->peerSeq + REGRESS_PRE_NEWKEYS_PACKETS, + plain.server->peerSeq); + + FreeKexReplyHarness(&plain); + FreeKexReplyHarness(&strict); +} + +/* The injection, with the mitigation off. Nothing rejects the forged + * SSH_MSG_IGNORE: the client processes it, accepts the server's NEWKEYS, + * and finishes the handshake with its inbound counter one ahead of what + * the server is sending against. Nothing notices, now or later -- the + * harness negotiates aes256-gcm@openssh.com, and CreateMac()/VerifyMac() + * are reached only on the non-AEAD paths, so under an AEAD cipher the + * sequence number is not an authentication input at all. The counters + * simply stay apart. That undetected shift is what Terrapin is built on. */ +static void TestTerrapinInjectionAcceptedWithoutStrictKex(void) +{ + KexReplyHarness harness; + KexReplyRunResult result; + + InitStrictKexHarness(&harness, MSGID_IGNORE, 0, 0); + RunKexReplyHandshake(&harness, &result); + + AssertIntEQ(harness.mutator.injectedPackets, 1); + AssertIntEQ(harness.mutator.parseError, 0); + AssertIntEQ(harness.client->strictKexEnabled, 0); + + /* The client accepted the injected packet and kept going all the way + * through the server's NEWKEYS. Nothing in the KEX objected, and the + * handshake came up. */ + AssertTrue(result.clientSuccess); + AssertTrue(result.serverSuccess); + AssertIntEQ(harness.client->initialKexDone, 1); + AssertTrue(harness.client->error != WS_MSGID_NOT_ALLOWED_E); + + /* The shift: the client has counted one more inbound packet than the + * server has sent. */ + AssertIntEQ(harness.client->peerSeq, harness.server->seq + 1); + + FreeKexReplyHarness(&harness); +} + +/* The same injection with strict KEX negotiated, for each message a man in + * the middle could reach for. All of them are refused for as long as the + * initial KEX is running, so the sequence number is never shifted and the + * connection dies loudly instead. */ +static void AssertTerrapinInjectionRejected(byte injectMsgId) +{ + KexReplyHarness harness; + KexReplyRunResult result; + + InitStrictKexHarness(&harness, injectMsgId, 1, 1); + RunKexReplyHandshake(&harness, &result); + + AssertIntEQ(harness.mutator.injectedPackets, 1); + AssertIntEQ(harness.mutator.parseError, 0); + AssertIntEQ(harness.client->strictKexEnabled, 1); + + AssertFalse(result.clientSuccess); + AssertIntEQ(result.clientErr, WS_MSGID_NOT_ALLOWED_E); + + /* Rejected where it was injected: ahead of the server's NEWKEYS, so + * the client counted only the server's KEXINIT and KEXDH_REPLY and its + * sequence number was never shifted. */ + AssertIntEQ(harness.client->initialKexDone, 0); + AssertIntEQ(harness.client->peerSeq, 2); + + /* Strict KEX calls for ending the connection, not just dropping the + * packet, so the client tells the server why before it goes. */ + AssertIntEQ(harness.client->disconnected, 1); + + /* The runner stops as soon as the client fails, so give the server one + * more turn to read what the client sent on its way out. */ + (void)wolfSSH_accept(harness.server); + AssertIntEQ(harness.server->disconnected, 1); + + /* The server decoded it as a DISCONNECT rather than as a broken + * packet, which is as far as this can be checked from here: the client + * sent its own NEWKEYS before it read the forgery, so the message goes + * out encrypted and the plaintext sniffer never sees the reason code + * itself. */ + AssertIntEQ(harness.server->error, WS_DISCONNECT); + + FreeKexReplyHarness(&harness); +} + +static void TestTerrapinInjectionRejectedWithStrictKex(void) +{ + /* IGNORE, DEBUG and UNIMPLEMENTED are the messages the attack is + * written up with. EXT_INFO and an unassigned transport id are here + * because the receiver used to take those during the initial KEX just + * as readily, and each one buys the attacker the same step of the + * sequence number. */ + static const byte injectable[] = { + MSGID_IGNORE, MSGID_DEBUG, MSGID_UNIMPLEMENTED, MSGID_EXT_INFO, + REGRESS_UNASSIGNED_TRANS_MSGID + }; + word32 i; + + for (i = 0; i < (word32)(sizeof(injectable) / sizeof(injectable[0])); i++) { + AssertTerrapinInjectionRejected(injectable[i]); + } +} + +/* The same splice aimed at the server. The attack does not care which end + * it desynchronizes, and the two sides run the gate off their own state, + * so the server has to refuse an injected message just as the client does. + * The client's NEWKEYS is the splice point here, by which time the server + * has counted the client's KEXINIT and KEXDH_INIT and nothing else. */ +static void TestTerrapinInjectionRejectedByServer(void) +{ + KexReplyHarness harness; + KexReplyRunResult result; + + InitStrictKexHarnessMode(&harness, MSGID_IGNORE, 1, 1, + REGRESS_MUTATE_TERRAPIN_C2S); + RunKexReplyHandshake(&harness, &result); + + AssertIntEQ(harness.mutator.injectedPackets, 1); + AssertIntEQ(harness.mutator.parseError, 0); + AssertIntEQ(harness.server->strictKexEnabled, 1); + + AssertFalse(result.serverSuccess); + AssertIntEQ(result.serverErr, WS_MSGID_NOT_ALLOWED_E); + + AssertIntEQ(harness.server->initialKexDone, 0); + AssertIntEQ(harness.server->peerSeq, 2); + AssertIntEQ(harness.server->disconnected, 1); + + FreeKexReplyHarness(&harness); +} + +/* DISCONNECT is the one message the gate lets through mid-KEX, so that a + * peer can still give up on the exchange. The client takes the spliced-in + * one and ends the session on it: the error is the peer's disconnect, not + * the gate's refusal, and the packet is counted because it was accepted. */ +static void TestStrictKexTakesInjectedDisconnect(void) +{ + KexReplyHarness harness; + KexReplyRunResult result; + + InitStrictKexHarness(&harness, MSGID_DISCONNECT, 1, 1); + RunKexReplyHandshake(&harness, &result); + + AssertIntEQ(harness.mutator.injectedPackets, 1); + AssertIntEQ(harness.mutator.parseError, 0); + AssertIntEQ(harness.client->strictKexEnabled, 1); + + AssertFalse(result.clientSuccess); + AssertIntEQ(result.clientErr, WS_DISCONNECT); + AssertIntEQ(harness.client->disconnected, 1); + + /* The server's KEXINIT and KEXDH_REPLY, then the forgery. */ + AssertIntEQ(harness.client->peerSeq, 3); + + /* It landed ahead of the server's NEWKEYS, so the initial KEX never + * finished and the gate was still up when the message went through. */ + AssertIntEQ(harness.client->initialKexDone, 0); + + FreeKexReplyHarness(&harness); +} + +/* A packet ahead of the server's KEXINIT passes the unarmed allow list, + * so the KEXINIT arrives at a nonzero sequence number and the client + * disconnects. */ +static void AssertPreKexInitInjectionRejected(byte injectMsgId) +{ + KexReplyHarness harness; + KexReplyRunResult result; + + InitStrictKexHarnessMode(&harness, injectMsgId, 1, 1, + REGRESS_MUTATE_TERRAPIN_PRE_KEXINIT); + RunKexReplyHandshake(&harness, &result); + + AssertIntEQ(harness.mutator.injectedPackets, 1); + AssertIntEQ(harness.mutator.parseError, 0); + AssertIntEQ(harness.client->strictKexEnabled, 1); + + AssertFalse(result.clientSuccess); + AssertIntEQ(result.clientErr, WS_MSGID_NOT_ALLOWED_E); + AssertIntEQ(harness.client->initialKexDone, 0); + AssertIntEQ(harness.client->disconnected, 1); + + /* Still plaintext, so the reason is readable. */ + AssertIntEQ(harness.clientIo.sawDisconnect, 1); + AssertIntEQ(harness.clientIo.disconnectReason, + WOLFSSH_DISCONNECT_KEY_EXCHANGE_FAILED); + + FreeKexReplyHarness(&harness); +} + +static void TestPreKexInitInjectionRejectedWithStrictKex(void) +{ + /* EXT_INFO would replace the client's server-sig-algs list. DEBUG is + * omitted: it gets decoded here, and the payload is too short. */ + static const byte injectable[] = { + MSGID_IGNORE, MSGID_UNIMPLEMENTED, MSGID_EXT_INFO, + REGRESS_UNASSIGNED_TRANS_MSGID + }; + word32 i; + + for (i = 0; i < (word32)(sizeof(injectable) / sizeof(injectable[0])); i++) { + AssertPreKexInitInjectionRejected(injectable[i]); + } +} + +/* Without strict KEX the same splice is accepted. */ +static void TestPreKexInitInjectionAcceptedWithoutStrictKex(void) +{ + KexReplyHarness harness; + KexReplyRunResult result; + + InitStrictKexHarnessMode(&harness, MSGID_IGNORE, 0, 0, + REGRESS_MUTATE_TERRAPIN_PRE_KEXINIT); + RunKexReplyHandshake(&harness, &result); + + AssertIntEQ(harness.mutator.injectedPackets, 1); + AssertIntEQ(harness.mutator.parseError, 0); + AssertIntEQ(harness.client->strictKexEnabled, 0); + AssertTrue(result.clientSuccess); + AssertTrue(result.serverSuccess); + + FreeKexReplyHarness(&harness); +} + +/* What a worker call is allowed to come back with while a rekey runs: an + * empty queue, a packet handed to the application, or the rekey itself + * still in progress. Anything else is a real failure. */ +static int IsPumpRetryable(int err) +{ + return IsHandshakeRetryable(err) || err == WS_CHAN_RXD || + err == WS_REKEYING; +} + +/* Run both ends until their queues are empty and neither is keying. Only + * sound once the handshake is done, where the only traffic left is what + * the test asked for. */ +static void PumpDuplexPair(KexReplyHarness* harness) +{ + word32 step; + + for (step = 0; step < REGRESS_MAX_HANDSHAKE_STEPS; step++) { + int ret; + + ret = wolfSSH_worker(harness->client, NULL); + if (ret < WS_SUCCESS) { + int err = wolfSSH_get_error(harness->client); + AssertTrue(IsPumpRetryable(err)); + } + + ret = wolfSSH_worker(harness->server, NULL); + if (ret < WS_SUCCESS) { + int err = wolfSSH_get_error(harness->server); + AssertTrue(IsPumpRetryable(err)); + } + + if (harness->clientIo.inbound.len == 0 && + harness->serverIo.inbound.len == 0 && + harness->client->isKeying == 0 && + harness->server->isKeying == 0) { + return; + } + } + + Fail(("the rekey to finish"), ("it ran out of steps")); +} + +/* draft-miller-sshm-strict-kex resets the sequence numbers at every + * NEWKEYS, not just the initial one. Establish a session, note where the + * counters have got to, then rekey: with the mitigation on they fall back + * to count only what followed the new NEWKEYS, and with it off they carry + * on climbing. A counter only ever goes up on its own, so a smaller number + * after the rekey is the reset and nothing else. */ +static void AssertRekeyResetsSequenceNumbers(byte strict) +{ + KexReplyHarness harness; + KexReplyRunResult result; + word32 clientSeq, clientPeerSeq, serverSeq, serverPeerSeq; + + InitStrictKexHarness(&harness, MSGID_NONE, strict, strict); + RunKexReplyHandshake(&harness, &result); + + AssertTrue(result.clientSuccess); + AssertTrue(result.serverSuccess); + AssertIntEQ(harness.client->strictKexEnabled, strict); + AssertIntEQ(harness.server->strictKexEnabled, strict); + + clientSeq = harness.client->seq; + clientPeerSeq = harness.client->peerSeq; + serverSeq = harness.server->seq; + serverPeerSeq = harness.server->peerSeq; + + /* Userauth and the session channel put every counter well clear of + * zero, so a reset is visible as a drop. */ + AssertTrue(clientSeq > REGRESS_PRE_NEWKEYS_PACKETS); + AssertTrue(clientPeerSeq > REGRESS_PRE_NEWKEYS_PACKETS); + AssertTrue(serverSeq > REGRESS_PRE_NEWKEYS_PACKETS); + AssertTrue(serverPeerSeq > REGRESS_PRE_NEWKEYS_PACKETS); + + AssertIntEQ(wolfSSH_TriggerKeyExchange(harness.client), WS_SUCCESS); + PumpDuplexPair(&harness); + + /* The rekey ran to the end: both sides are done keying and neither + * fell over on a packet it could not authenticate. */ + AssertIntEQ(harness.client->isKeying, 0); + AssertIntEQ(harness.server->isKeying, 0); + AssertIntEQ(harness.client->disconnected, 0); + AssertIntEQ(harness.server->disconnected, 0); + + if (strict) { + AssertTrue(harness.client->seq < clientSeq); + AssertTrue(harness.client->peerSeq < clientPeerSeq); + AssertTrue(harness.server->seq < serverSeq); + AssertTrue(harness.server->peerSeq < serverPeerSeq); + } + else { + AssertTrue(harness.client->seq > clientSeq); + AssertTrue(harness.client->peerSeq > clientPeerSeq); + AssertTrue(harness.server->seq > serverSeq); + AssertTrue(harness.server->peerSeq > serverPeerSeq); + } + + FreeKexReplyHarness(&harness); +} + +static void TestStrictKexResetsSequenceNumbersOnRekey(void) +{ + AssertRekeyResetsSequenceNumbers(1); + /* The control: the same rekey with the mitigation off. */ + AssertRekeyResetsSequenceNumbers(0); +} + #endif /* KEXDH_REPLY_REGRESS_KEX_ALGO */ /* Shared with the client-side forwarding tests below. */ @@ -2491,6 +3269,102 @@ static void TestAuthMessageBlockedDuringKeying(WOLFSSH* ssh) AssertIntEQ(ssh->handshake->expectMsgId, MSGID_NONE); } +/* Strict KEX gates the messages an attacker can inject for free during the + * initial KEX. They are otherwise legal at any time, which is exactly what + * makes them useful for shifting the peer's sequence number + * (CVE-2023-48795). */ +static void TestStrictKexBlocksInjectableMessages(WOLFSSH* ssh) +{ + /* Every message id the receiver would otherwise take during the initial + * KEX. IGNORE, DEBUG and UNIMPLEMENTED are the ones the attack is + * usually described with, but EXT_INFO and the unassigned transport ids + * are just as good: MSGIDLIMIT_TRANS_GEN() covers 1 through 19 and used + * to wave all of them through, and any one of them steps peerSeq. */ + static const byte injectable[] = { + MSGID_IGNORE, MSGID_DEBUG, MSGID_UNIMPLEMENTED, MSGID_EXT_INFO, + REGRESS_UNASSIGNED_TRANS_MSGID, MSGIDLIMIT_TRANS_GEN_MAX + }; + word32 i; + + for (i = 0; i < (word32)(sizeof(injectable) / sizeof(injectable[0])); i++) { + byte msg = injectable[i]; + + /* Baseline: allowed with the mitigation off, so the rejections + * below are the strict KEX gate and not some other state check. */ + ResetSession(ssh); + ssh->strictKexEnabled = 0; + ssh->initialKexDone = 0; + AssertTrue(wolfSSH_TestIsMessageAllowed(ssh, msg, WS_MSG_RECV)); + AssertIntEQ(ssh->error, 0); + + /* Negotiated, peer's NEWKEYS not yet seen: refused. */ + ResetSession(ssh); + ssh->strictKexEnabled = 1; + ssh->initialKexDone = 0; + AssertFalse(wolfSSH_TestIsMessageAllowed(ssh, msg, WS_MSG_RECV)); + AssertIntEQ(ssh->error, WS_MSGID_NOT_ALLOWED_E); + + /* The gate is on the receive path only; sending one is our own + * business and does not desynchronize anything. */ + ResetSession(ssh); + ssh->strictKexEnabled = 1; + ssh->initialKexDone = 0; + AssertTrue(wolfSSH_TestIsMessageAllowed(ssh, msg, WS_MSG_SEND)); + + /* Peer's NEWKEYS arrived, so the restriction lifts. */ + ResetSession(ssh); + ssh->strictKexEnabled = 1; + ssh->initialKexDone = 1; + AssertTrue(wolfSSH_TestIsMessageAllowed(ssh, msg, WS_MSG_RECV)); + AssertIntEQ(ssh->error, 0); + } + + ResetSession(ssh); + ssh->strictKexEnabled = 0; + ssh->initialKexDone = 0; +} + +/* A peer has to be able to tear the connection down mid-KEX, so DISCONNECT + * stays allowed even while the injectable messages are refused. */ +static void TestStrictKexAllowsDisconnectDuringInitialKex(WOLFSSH* ssh) +{ + ResetSession(ssh); + ssh->strictKexEnabled = 1; + ssh->initialKexDone = 0; + + AssertTrue(wolfSSH_TestIsMessageAllowed(ssh, MSGID_DISCONNECT, + WS_MSG_RECV)); + AssertIntEQ(ssh->error, 0); + + /* The KEX itself must still run, too. NEWKEYS is what lifts the gate, + * and the algorithm-specific exchange in between has to reach its + * handler or nothing ever completes. */ + AssertTrue(wolfSSH_TestIsMessageAllowed(ssh, MSGID_KEXINIT, WS_MSG_RECV)); + AssertIntEQ(ssh->error, 0); + + { + static const byte kexMsgs[] = { + MSGID_KEXDH_REPLY, MSGID_KEXDH_GEX_REPLY, MSGID_NEWKEYS + }; + word32 i; + + for (i = 0; i < (word32)(sizeof(kexMsgs) / sizeof(kexMsgs[0])); i++) { + ResetSession(ssh); + ssh->strictKexEnabled = 1; + ssh->initialKexDone = 0; + ssh->isKeying = WOLFSSH_PEER_IS_KEYING; + ssh->handshake = AllocHandshake(ssh); + ssh->handshake->expectMsgId = kexMsgs[i]; + AssertTrue(wolfSSH_TestIsMessageAllowed(ssh, kexMsgs[i], + WS_MSG_RECV)); + AssertIntEQ(ssh->error, 0); + } + } + + ResetSession(ssh); + ssh->strictKexEnabled = 0; +} + /* Reject USERAUTH_FAILURE with password list during keying (password-leak PoC). */ static void TestUserauthFailureDuringKeying(WOLFSSH* ssh) { @@ -13731,15 +14605,13 @@ static void TestKeyboardResponseNullCtx(WOLFSSH* ssh) #endif /* WOLFSSH_KEYBOARD_INTERACTIVE */ -#if !defined(WOLFSSH_NO_ECDH_SHA2_NISTP256) \ - && !defined(WOLFSSH_NO_RSA) \ - && !defined(WOLFSSH_NO_CURVE25519_SHA256) \ - && !defined(WOLFSSH_NO_RSA_SHA2_256) +/* KEXINIT payload builders. Used by the first_packet_follows coverage and by + * the strict KEX marker cases below, so they are guarded by what they need + * rather than by either caller's narrower conditions. */ +#if !defined(WOLFSSH_NO_ECDH_SHA2_NISTP256) && !defined(WOLFSSH_NO_RSA) -#define FPF_KEX_GOOD "ecdh-sha2-nistp256" -#define FPF_KEX_BAD "curve25519-sha256" -#define FPF_KEY_GOOD "ssh-rsa" -#define FPF_KEY_BAD "rsa-sha2-256" +#define REGRESS_KEXINIT_KEX_ALGO "ecdh-sha2-nistp256" +#define REGRESS_KEXINIT_KEY_ALGO "ssh-rsa" /* AppendString for one of the library's own canned algorithm lists. Those are * built by concatenating "name," fragments, so they carry a trailing comma @@ -13788,6 +14660,18 @@ static word32 BuildKexInitPayload(WOLFSSH* ssh, const char* kexList, return idx; } +#endif /* KEXINIT payload builder guard */ + +#if !defined(WOLFSSH_NO_ECDH_SHA2_NISTP256) \ + && !defined(WOLFSSH_NO_RSA) \ + && !defined(WOLFSSH_NO_CURVE25519_SHA256) \ + && !defined(WOLFSSH_NO_RSA_SHA2_256) + +#define FPF_KEX_GOOD "ecdh-sha2-nistp256" +#define FPF_KEX_BAD "curve25519-sha256" +#define FPF_KEY_GOOD "ssh-rsa" +#define FPF_KEY_BAD "rsa-sha2-256" + #if !defined(WOLFSSH_NO_AES_CBC) && !defined(WOLFSSH_NO_AES_CTR) \ && !defined(WOLFSSH_NO_HMAC_SHA1) && !defined(WOLFSSH_NO_HMAC_SHA2_256) /* Like BuildKexInitPayload but with explicit per-direction cipher/MAC lists. */ @@ -15592,8 +16476,180 @@ static void TestDoKexInitRejectsWhenPeerIsKeying(void) wolfSSH_CTX_free(ctx); } + #endif /* first_packet_follows coverage guard */ +#if !defined(WOLFSSH_NO_ECDH_SHA2_NISTP256) && !defined(WOLFSSH_NO_RSA) + +/* ---- Strict KEX marker negotiation (CVE-2023-48795) ---- */ + +#define SK_OPENSSH_S "kex-strict-s-v00@openssh.com" +#define SK_OPENSSH_C "kex-strict-c-v00@openssh.com" +#define SK_DRAFT_S "kex-strict-s" +#define SK_DRAFT_C "kex-strict-c" + +typedef struct { + const char* description; + const char* peerKexList; /* what the peer put in its KEXINIT */ + byte side; /* endpoint under test */ + byte sendStrictKex; /* local opt-in */ + byte rekey; /* pretend a session id is already established */ + byte expectPeerStrictKex; + byte expectStrictKexEnabled; +} StrictKexMarkerCase; + +static const StrictKexMarkerCase strictKexMarkerCases[] = { + /* A peer that says nothing leaves the mitigation off. */ + { "no marker", + REGRESS_KEXINIT_KEX_ALGO, WOLFSSH_ENDPOINT_SERVER, 1, 0, 0, 0 }, + + /* Deployed OpenSSH only ever sends the -v00@openssh.com spelling, so + * accepting it is what makes the mitigation work against a real peer. */ + { "openssh marker only", + REGRESS_KEXINIT_KEX_ALGO "," SK_OPENSSH_C, + WOLFSSH_ENDPOINT_SERVER, 1, 0, 1, 1 }, + + /* draft-miller-sshm-strict-kex names it without the vendor suffix. */ + { "draft marker only", + REGRESS_KEXINIT_KEX_ALGO "," SK_DRAFT_C, + WOLFSSH_ENDPOINT_SERVER, 1, 0, 1, 1 }, + + { "both spellings", + REGRESS_KEXINIT_KEX_ALGO "," SK_OPENSSH_C "," SK_DRAFT_C, + WOLFSSH_ENDPOINT_SERVER, 1, 0, 1, 1 }, + + /* The marker is directional. A server must ignore the server-side + * names, which are its own to send, not the client's. */ + { "server-side names from a client", + REGRESS_KEXINIT_KEX_ALGO "," SK_OPENSSH_S "," SK_DRAFT_S, + WOLFSSH_ENDPOINT_SERVER, 1, 0, 0, 0 }, + + /* Both sides have to ask for it. */ + { "peer asks, local opted out", + REGRESS_KEXINIT_KEX_ALGO "," SK_OPENSSH_C, + WOLFSSH_ENDPOINT_SERVER, 0, 0, 1, 0 }, + + /* draft-miller-sshm-strict-kex: the marker is meaningful only in the + * first KEXINIT, and a rekey carrying it must not turn anything on. */ + { "marker in a rekey KEXINIT", + REGRESS_KEXINIT_KEX_ALGO "," SK_OPENSSH_C "," SK_DRAFT_C, + WOLFSSH_ENDPOINT_SERVER, 1, 1, 0, 0 }, + + /* The client side of the same negotiation. */ + { "client sees openssh marker", + REGRESS_KEXINIT_KEX_ALGO "," SK_OPENSSH_S, + WOLFSSH_ENDPOINT_CLIENT, 1, 0, 1, 1 }, + { "client sees draft marker", + REGRESS_KEXINIT_KEX_ALGO "," SK_DRAFT_S, + WOLFSSH_ENDPOINT_CLIENT, 1, 0, 1, 1 }, + { "client-side names from a server", + REGRESS_KEXINIT_KEX_ALGO "," SK_OPENSSH_C "," SK_DRAFT_C, + WOLFSSH_ENDPOINT_CLIENT, 1, 0, 0, 0 }, +}; + +static void RunStrictKexMarkerCase(const StrictKexMarkerCase* tc) +{ + WOLFSSH_CTX* ctx; + WOLFSSH* ssh; + byte payload[768]; + word32 payloadSz; + word32 idx = 0; + + ctx = wolfSSH_CTX_new(tc->side, NULL); + AssertNotNull(ctx); + + ssh = wolfSSH_new(ctx); + AssertNotNull(ssh); + + AssertIntEQ(wolfSSH_SetAlgoListKex(ssh, REGRESS_KEXINIT_KEX_ALGO), + WS_SUCCESS); + AssertIntEQ(wolfSSH_SetAlgoListKey(ssh, REGRESS_KEXINIT_KEY_ALGO), + WS_SUCCESS); + AssertIntEQ(wolfSSH_SetStrictKex(ssh, tc->sendStrictKex), WS_SUCCESS); + + if (tc->rekey) { + /* DoKexInit reads a non-empty session id as "this is a rekey". */ + WMEMSET(ssh->sessionId, 0x5A, WC_SHA256_DIGEST_SIZE); + ssh->sessionIdSz = WC_SHA256_DIGEST_SIZE; + } + + payloadSz = BuildKexInitPayload(ssh, tc->peerKexList, + REGRESS_KEXINIT_KEY_ALGO, 0, + payload, (word32)sizeof(payload)); + + /* DoKexInit's tail hashes and answers the KEXINIT, which fails on a + * stripped-down WOLFSSH with no host key loaded. Only the negotiation + * this test asserts on happens before that. */ + (void)wolfSSH_TestDoKexInit(ssh, payload, payloadSz, &idx); + + if (ssh->peerStrictKex != tc->expectPeerStrictKex) { + Fail(("peerStrictKex == %u (%s)", + tc->expectPeerStrictKex, tc->description), + ("%u", ssh->peerStrictKex)); + } + if (ssh->strictKexEnabled != tc->expectStrictKexEnabled) { + Fail(("strictKexEnabled == %u (%s)", + tc->expectStrictKexEnabled, tc->description), + ("%u", ssh->strictKexEnabled)); + } + + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); +} + +static void TestStrictKexMarkerNegotiation(void) +{ + word32 i; + + for (i = 0; i < (word32)(sizeof(strictKexMarkerCases) / + sizeof(strictKexMarkerCases[0])); i++) { + RunStrictKexMarkerCase(&strictKexMarkerCases[i]); + } +} + +/* A rekey must not clear a mitigation the initial KEX turned on, either. */ +static void TestStrictKexSurvivesRekeyKexInit(void) +{ + WOLFSSH_CTX* ctx; + WOLFSSH* ssh; + byte payload[768]; + word32 payloadSz; + word32 idx = 0; + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL); + AssertNotNull(ctx); + + ssh = wolfSSH_new(ctx); + AssertNotNull(ssh); + + AssertIntEQ(wolfSSH_SetAlgoListKex(ssh, REGRESS_KEXINIT_KEX_ALGO), + WS_SUCCESS); + AssertIntEQ(wolfSSH_SetAlgoListKey(ssh, REGRESS_KEXINIT_KEY_ALGO), + WS_SUCCESS); + + /* Established session, strict KEX already negotiated. */ + WMEMSET(ssh->sessionId, 0x5A, WC_SHA256_DIGEST_SIZE); + ssh->sessionIdSz = WC_SHA256_DIGEST_SIZE; + ssh->peerStrictKex = 1; + ssh->strictKexEnabled = 1; + + /* A rekey KEXINIT that no longer carries the marker, which is what a + * conforming peer sends. */ + payloadSz = BuildKexInitPayload(ssh, REGRESS_KEXINIT_KEX_ALGO, + REGRESS_KEXINIT_KEY_ALGO, 0, + payload, (word32)sizeof(payload)); + (void)wolfSSH_TestDoKexInit(ssh, payload, payloadSz, &idx); + + AssertIntEQ(ssh->peerStrictKex, 1); + AssertIntEQ(ssh->strictKexEnabled, 1); + + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); +} + +#endif /* strict KEX marker guard */ + + /* Regression coverage for issue 5575: the documented ssh://hostname form must * set the hostname even without an explicit port, and a malformed destination @@ -16212,6 +17268,8 @@ int main(int argc, char** argv) #endif #ifndef NO_WOLFSSH_CLIENT TestAuthMessageBlockedDuringKeying(ssh); + TestStrictKexBlocksInjectableMessages(ssh); + TestStrictKexAllowsDisconnectDuringInitialKex(ssh); TestUserauthFailureDuringKeying(ssh); TestPasswordLeakAborts(ssh); TestPrematureUserauthSuccess(ssh); @@ -16455,6 +17513,10 @@ int main(int argc, char** argv) TestKexInitLanguageLengthOverflow(); TestDoKexInitRejectsWhenPeerIsKeying(); #endif +#if !defined(WOLFSSH_NO_ECDH_SHA2_NISTP256) && !defined(WOLFSSH_NO_RSA) + TestStrictKexMarkerNegotiation(); + TestStrictKexSurvivesRekeyKexInit(); +#endif #if !defined(WOLFSSH_NO_ECDH_SHA2_NISTP256) && !defined(WOLFSSH_NO_RSA) \ && !defined(WOLFSSH_NO_CURVE25519_SHA256) \ && !defined(WOLFSSH_NO_RSA_SHA2_256) \ @@ -16568,6 +17630,20 @@ int main(int argc, char** argv) TestKexDhGexGroupShrunkPrimeSendsDisconnect(); TestKexDhGexGroupBadGeneratorSendsDisconnect(); #endif + /* Strict KEX, the Terrapin mitigation (CVE-2023-48795) */ + TestStrictKexNegotiatedByDefault(); + TestStrictKexClientOptOut(); + TestStrictKexServerOptOut(); + TestStrictKexSettingFrozenForHandshake(); + TestStrictKexMarkerNotSentOnRekey(); + TestStrictKexResetsSequenceNumbers(); + TestStrictKexResetsSequenceNumbersOnRekey(); + TestTerrapinInjectionAcceptedWithoutStrictKex(); + TestTerrapinInjectionRejectedWithStrictKex(); + TestTerrapinInjectionRejectedByServer(); + TestStrictKexTakesInjectedDisconnect(); + TestPreKexInitInjectionRejectedWithStrictKex(); + TestPreKexInitInjectionAcceptedWithoutStrictKex(); #endif #ifdef WOLFSSH_SFTP diff --git a/wolfssh/internal.h b/wolfssh/internal.h index c44bccf5a..c1ca45f61 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -529,8 +529,12 @@ enum { ID_CURVE25519_SHA256, ID_CURVE25519_SHA256_LIBSSH, #endif - ID_EXTINFO_S, /* Pseudo-KEX to indicate server extensions. */ - ID_EXTINFO_C, /* Pseudo-KEX to indicate client extensions. */ + ID_EXT_INFO_S, /* Pseudo-KEX to indicate server extensions. */ + ID_EXT_INFO_C, /* Pseudo-KEX to indicate client extensions. */ + ID_EXT_STRICT_KEX_S, /* Pseudo-KEX to indicate server strict KEX. */ + ID_EXT_STRICT_KEX_C, /* Pseudo-KEX to indicate client strict KEX. */ + ID_EXT_PRE_STRICT_KEX_S, /* OpenSSH -v00 spelling of the server's. */ + ID_EXT_PRE_STRICT_KEX_C, /* OpenSSH -v00 spelling of the client's. */ /* Public Key IDs */ ID_SSH_RSA, @@ -947,6 +951,7 @@ struct WOLFSSH_CTX { word32 windowSz; word32 maxPacketSz; word32 maxAuthAttempts; /* server cap on failed userauth */ + byte sendStrictKex; /* offer the strict KEX marker */ byte side; /* client or server */ byte showBanner; byte appChannels; /* app drives channels, see ssh.h */ @@ -980,6 +985,8 @@ typedef struct Keys { typedef struct HandshakeInfo { byte expectMsgId; + byte strictKex; /* the strict KEX setting this handshake runs on */ + byte strictKexSet; /* strictKex has been read from the session */ byte kexId; byte kexHashId; byte pubKeyId; @@ -1351,6 +1358,10 @@ struct WOLFSSH { byte userAuthPkDone; byte sendExtInfo; byte extInfoSent; /* track if the ext info has already been sent */ + byte sendStrictKex; /* offer the strict KEX marker on initial KEXINIT */ + byte peerStrictKex; /* peer offered the strict KEX marker (initial KEX) */ + byte strictKexEnabled; /* both sides negotiated strict KEX for this session */ + byte initialKexDone; /* peer's initial KEX finished (its NEWKEYS arrived) */ byte* peerSigId; word32 peerSigIdSz; diff --git a/wolfssh/ssh.h b/wolfssh/ssh.h index d1e2d10b0..9f0cc6f5d 100644 --- a/wolfssh/ssh.h +++ b/wolfssh/ssh.h @@ -203,6 +203,21 @@ WOLFSSH_API const char* wolfSSH_GetAlgoListKeyAccepted(WOLFSSH* ssh); WOLFSSH_API int wolfSSH_CheckAlgoName(const char* name); +/* Strict KEX (Terrapin mitigation, CVE-2023-48795) controls. Defaults to + * enabled. The CTX setting seeds every session made from it; the session + * setting overrides it for that session alone. Only the initial KEXINIT + * carries the marker and only that exchange decides whether the mitigation + * is on, so the session call has to be made before the connection starts: + * once a session is keying, its setting is fixed for the life of that + * session and a rekey does not revisit it. The CTX calls report + * WS_BAD_ARGUMENT on a NULL ctx, the session calls WS_SSH_NULL_E on a NULL + * ssh; the getters otherwise report the setting, 1 for enabled and 0 for + * disabled, not whether strict KEX was negotiated. */ +WOLFSSH_API int wolfSSH_CTX_SetStrictKex(WOLFSSH_CTX* ctx, byte enable); +WOLFSSH_API int wolfSSH_CTX_GetStrictKex(WOLFSSH_CTX* ctx); +WOLFSSH_API int wolfSSH_SetStrictKex(WOLFSSH* ssh, byte enable); +WOLFSSH_API int wolfSSH_GetStrictKex(WOLFSSH* ssh); + WOLFSSH_API const char* wolfSSH_QueryKex(word32* idx); WOLFSSH_API const char* wolfSSH_QueryKey(word32* idx); WOLFSSH_API const char* wolfSSH_QueryCipher(word32* idx);