From 98ba209ef027dddb71976796be98ff57a247430b Mon Sep 17 00:00:00 2001 From: Nathan Erickson Date: Tue, 11 Aug 2026 12:46:21 -0400 Subject: [PATCH] fix(server): properly unwrap signature blob per RFC 8332 RFC 8332 Section 3 allows the signature blob's algorithm identifier to differ from the packet-level algorithm field. For example, a client may put 'ssh-rsa' in the packet algorithm field but sign with 'rsa-sha2-256', embedding that algorithm name in the signature blob. Previously, the signature header was only stripped when the embedded algorithm exactly matched keyAlgo. This caused signature verification failures for clients (e.g., TablePlus/libssh) that advertise 'ssh-rsa' in the packet but sign with 'rsa-sha2-256'. The fix reads the actual algorithm from the signature blob and strips the header for any known SSH algorithm. It also derives hashAlgo from the signature's embedded algorithm when the packet didn't set one. Applies to both publickey and hostbased authentication methods. --- lib/protocol/handlers.misc.js | 59 ++++++++++++++--- test/test-userauth-rfc8332.js | 115 ++++++++++++++++++++++++++++++++++ 2 files changed, 166 insertions(+), 8 deletions(-) create mode 100644 test/test-userauth-rfc8332.js diff --git a/lib/protocol/handlers.misc.js b/lib/protocol/handlers.misc.js index 24580bec..8475f4b5 100644 --- a/lib/protocol/handlers.misc.js +++ b/lib/protocol/handlers.misc.js @@ -256,10 +256,34 @@ module.exports = { const blobEnd = bufferParser.pos(); let signature = bufferParser.readString(); if (signature !== undefined) { - if (signature.length > (4 + keyAlgo.length + 4) - && signature.utf8Slice(4, 4 + keyAlgo.length) === keyAlgo) { - // Skip algoLen + algo + sigLen - signature = bufferSlice(signature, 4 + keyAlgo.length + 4); + // RFC 8332: The signature blob contains its own algorithm + // identifier which may differ from keyAlgo. For example, a client + // may put 'ssh-rsa' in the packet algorithm field but sign with + // 'rsa-sha2-256'. Read the embedded algorithm to strip the header + // correctly and derive the hash algorithm. + if (signature.length > 8) { + const sigAlgoLen = signature.readUInt32BE(0); + if (sigAlgoLen > 0 && sigAlgoLen < 64 + && signature.length > (4 + sigAlgoLen + 4)) { + const sigAlgo = signature.utf8Slice(4, 4 + sigAlgoLen); + if (sigAlgo === keyAlgo + || sigAlgo === 'rsa-sha2-256' + || sigAlgo === 'rsa-sha2-512' + || sigAlgo === 'ssh-rsa' + || sigAlgo === 'ssh-dss' + || sigAlgo === 'ssh-ed25519' + || sigAlgo.startsWith('ecdsa-sha2-')) { + signature = bufferSlice(signature, 4 + sigAlgoLen + 4); + // Derive hashAlgo from the signature's algorithm if the + // packet-level algorithm didn't already set it + if (!hashAlgo) { + switch (sigAlgo) { + case 'rsa-sha2-256': hashAlgo = 'sha256'; break; + case 'rsa-sha2-512': hashAlgo = 'sha512'; break; + } + } + } + } } signature = sigSSHToASN1(signature, realKeyAlgo); @@ -318,10 +342,29 @@ module.exports = { const blobEnd = bufferParser.pos(); let signature = bufferParser.readString(); if (signature !== undefined) { - if (signature.length > (4 + keyAlgo.length + 4) - && signature.utf8Slice(4, 4 + keyAlgo.length) === keyAlgo) { - // Skip algoLen + algo + sigLen - signature = bufferSlice(signature, 4 + keyAlgo.length + 4); + // RFC 8332: Same unwrapping logic as publickey auth — the signature + // blob may contain a different algorithm than the packet-level keyAlgo. + if (signature.length > 8) { + const sigAlgoLen = signature.readUInt32BE(0); + if (sigAlgoLen > 0 && sigAlgoLen < 64 + && signature.length > (4 + sigAlgoLen + 4)) { + const sigAlgo = signature.utf8Slice(4, 4 + sigAlgoLen); + if (sigAlgo === keyAlgo + || sigAlgo === 'rsa-sha2-256' + || sigAlgo === 'rsa-sha2-512' + || sigAlgo === 'ssh-rsa' + || sigAlgo === 'ssh-dss' + || sigAlgo === 'ssh-ed25519' + || sigAlgo.startsWith('ecdsa-sha2-')) { + signature = bufferSlice(signature, 4 + sigAlgoLen + 4); + if (!hashAlgo) { + switch (sigAlgo) { + case 'rsa-sha2-256': hashAlgo = 'sha256'; break; + case 'rsa-sha2-512': hashAlgo = 'sha512'; break; + } + } + } + } } signature = sigSSHToASN1(signature, realKeyAlgo); diff --git a/test/test-userauth-rfc8332.js b/test/test-userauth-rfc8332.js new file mode 100644 index 00000000..d29af097 --- /dev/null +++ b/test/test-userauth-rfc8332.js @@ -0,0 +1,115 @@ +'use strict'; + +// Test for RFC 8332 Section 3 compliance: server correctly handles clients +// that put 'ssh-rsa' in the packet algorithm field but 'rsa-sha2-256' or +// 'rsa-sha2-512' in the signature blob's algorithm identifier. +// +// This tests the fix in handlers.misc.js that properly unwraps the signature +// blob regardless of whether its embedded algorithm matches the packet-level +// algorithm field. +// +// NOTE: The true mismatch scenario (packet algo='ssh-rsa', sig blob +// algo='rsa-sha2-256') cannot be easily reproduced with the ssh2 client since +// it always writes the same algorithm in both places. The mismatch case has +// been verified with real clients (TablePlus/libssh) that exhibit this +// behavior per RFC 8332 Section 3. These tests serve as regression tests to +// ensure the unwrapping logic doesn't break the normal paths. + +const assert = require('assert'); + +const { + fixtureKey, + mustCall, + setup, +} = require('./common.js'); + +const serverCfg = { hostKeys: [fixtureKey('ssh_host_rsa_key').raw] }; +const debug = false; + +// Test 1: Normal RSA auth still works (baseline regression test) +// The ssh2 client uses rsa-sha2-256 when supported, putting 'rsa-sha2-256' +// in both the packet algo field and the signature blob. This should still work. +{ + const clientKey = fixtureKey('openssh_new_rsa'); + const username = 'RFC8332 Baseline'; + + const { server } = setup( + 'RFC 8332 - baseline: rsa-sha2-256 in both packet and sig blob', + { + client: { username, privateKey: clientKey.raw }, + server: serverCfg, + debug, + } + ); + + server.on('connection', mustCall((conn) => { + conn.on('authentication', mustCall((ctx) => { + if (ctx.method === 'none') + return ctx.reject(); + + assert(ctx.method === 'publickey', + `Wrong auth method: ${ctx.method}`); + + if (ctx.signature) { + const result = + clientKey.key.verify(ctx.blob, ctx.signature, ctx.hashAlgo); + assert(result === true, + `Could not verify publickey signature (hashAlgo: ${ctx.hashAlgo})`); + } + ctx.accept(); + }, 3)).on('ready', mustCall(() => { + conn.end(); + })); + })); +} + +// Test 2: RSA auth with ssh-rsa packet algo (SHA-1 signature) +// Forces the client to use plain ssh-rsa (SHA-1), testing that the +// unwrapping still works when packet algo and sig blob algo match as 'ssh-rsa'. +{ + const clientKey = fixtureKey('openssh_new_rsa'); + const username = 'RFC8332 SSH-RSA'; + + const { server } = setup( + 'RFC 8332 - ssh-rsa in both packet and sig blob (SHA-1)', + { + client: { + username, + privateKey: clientKey.raw, + algorithms: { + // Exclude rsa-sha2-* from server host key algos to prevent the + // client from negotiating them + serverHostKey: [ + 'ssh-rsa', + 'ecdsa-sha2-nistp256', + 'ecdsa-sha2-nistp384', + 'ecdsa-sha2-nistp521', + 'ssh-ed25519', + ], + }, + }, + server: serverCfg, + debug, + } + ); + + server.on('connection', mustCall((conn) => { + conn.on('authentication', mustCall((ctx) => { + if (ctx.method === 'none') + return ctx.reject(); + + assert(ctx.method === 'publickey', + `Wrong auth method: ${ctx.method}`); + + if (ctx.signature) { + const result = + clientKey.key.verify(ctx.blob, ctx.signature, ctx.hashAlgo); + assert(result === true, + `Could not verify publickey signature (hashAlgo: ${ctx.hashAlgo})`); + } + ctx.accept(); + }, 3)).on('ready', mustCall(() => { + conn.end(); + })); + })); +}