From 691cd6203fd37e47b2a0041c6b4a179e6fea7abb Mon Sep 17 00:00:00 2001 From: nindanaoto Date: Mon, 23 Feb 2026 15:17:51 +0900 Subject: [PATCH 1/2] FIDO2: check SkPublicKey before Ed25519 in authenticatePublicKey Move the `instanceof SkPublicKey` check before the `isEd25519Key()` check in AuthenticationManager.authenticatePublicKey(). SK Ed25519 keys (sk-ssh-ed25519@openssh.com) match isEd25519Key() because their algorithm is "Ed25519", causing them to enter the generic Ed25519 path which tries DER encoding and fails with InvalidKeySpecException. Co-Authored-By: Claude Opus 4.6 --- .../ssh2/auth/AuthenticationManager.java | 51 ++++++++++--------- 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/src/main/java/com/trilead/ssh2/auth/AuthenticationManager.java b/src/main/java/com/trilead/ssh2/auth/AuthenticationManager.java index 76410cf..71c5116 100644 --- a/src/main/java/com/trilead/ssh2/auth/AuthenticationManager.java +++ b/src/main/java/com/trilead/ssh2/auth/AuthenticationManager.java @@ -317,34 +317,11 @@ else if (publicKey instanceof ECPublicKey) tm.sendMessage(ua.getPayload()); } - else if (PublicKeyUtils.isEd25519Key(publicKey)) - { - final String algo = Ed25519Verify.ED25519_ID; - - byte[] pk_enc = Ed25519Verify.get().encodePublicKey(publicKey); - - byte[] msg = this.generatePublicKeyUserAuthenticationRequest(user, algo, pk_enc); - - byte[] ed_sig_enc; - if (signatureProxy != null) - { - ed_sig_enc = signatureProxy.sign(msg, SignatureProxy.SHA512); - } - else - { - Ed25519PrivateKey pk = Ed25519Verify.convertPrivateKey(privateKey); - ed_sig_enc = Ed25519Verify.get().generateSignature(msg, pk, rnd); - } - - PacketUserauthRequestPublicKey ua = new PacketUserauthRequestPublicKey("ssh-connection", user, - algo, pk_enc, ed_sig_enc); - - tm.sendMessage(ua.getPayload()); - } else if (publicKey instanceof SkPublicKey) { // FIDO2 Security Key (SK) authentication - // SK keys require external signing via SignatureProxy + // This check must come before the Ed25519 check because + // SK Ed25519 keys match isEd25519Key() but require SK-specific encoding. if (signatureProxy == null) { throw new IOException("SK key authentication requires a SignatureProxy for signing."); @@ -380,6 +357,30 @@ else if (publicKey instanceof SkPublicKey) tm.sendMessage(ua.getPayload()); } + else if (PublicKeyUtils.isEd25519Key(publicKey)) + { + final String algo = Ed25519Verify.ED25519_ID; + + byte[] pk_enc = Ed25519Verify.get().encodePublicKey(publicKey); + + byte[] msg = this.generatePublicKeyUserAuthenticationRequest(user, algo, pk_enc); + + byte[] ed_sig_enc; + if (signatureProxy != null) + { + ed_sig_enc = signatureProxy.sign(msg, SignatureProxy.SHA512); + } + else + { + Ed25519PrivateKey pk = Ed25519Verify.convertPrivateKey(privateKey); + ed_sig_enc = Ed25519Verify.get().generateSignature(msg, pk, rnd); + } + + PacketUserauthRequestPublicKey ua = new PacketUserauthRequestPublicKey("ssh-connection", user, + algo, pk_enc, ed_sig_enc); + + tm.sendMessage(ua.getPayload()); + } else { throw new IOException("Unknown public key type."); From a474d1301cc51908c5cc2e766c36c84e6b75a998 Mon Sep 17 00:00:00 2001 From: nindanaoto Date: Mon, 23 Feb 2026 15:18:37 +0900 Subject: [PATCH 2/2] FIDO2: add regression tests for SK key algorithm dispatch Add tests that use realistic algorithm names ("Ed25519" and "EC") matching what real SkEd25519PublicKey and SkEcdsaPublicKey return. These reproduce the bug where isEd25519Key() matched SK Ed25519 keys before the instanceof SkPublicKey check, causing DER encoding failure. Co-Authored-By: Claude Opus 4.6 --- .../auth/AuthenticationManagerSkKeyTest.java | 47 ++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/src/test/java/com/trilead/ssh2/auth/AuthenticationManagerSkKeyTest.java b/src/test/java/com/trilead/ssh2/auth/AuthenticationManagerSkKeyTest.java index 8900309..7e15469 100644 --- a/src/test/java/com/trilead/ssh2/auth/AuthenticationManagerSkKeyTest.java +++ b/src/test/java/com/trilead/ssh2/auth/AuthenticationManagerSkKeyTest.java @@ -53,11 +53,17 @@ public class AuthenticationManagerSkKeyTest { */ static class TestSkPublicKey implements SkPublicKey { private final String keyType; + private final String algorithm; private final String application; private final byte[] keyData; TestSkPublicKey(String keyType, String application, byte[] keyData) { + this(keyType, keyType, application, keyData); + } + + TestSkPublicKey(String keyType, String algorithm, String application, byte[] keyData) { this.keyType = keyType; + this.algorithm = algorithm; this.application = application; this.keyData = keyData.clone(); } @@ -79,7 +85,7 @@ public byte[] getKeyData() { @Override public String getAlgorithm() { - return keyType; + return algorithm; } @Override @@ -238,6 +244,45 @@ public void authenticateSkKey_SendsAuthenticationRequest() throws Exception { "Authentication should send at least one message"); } + /** + * Regression test: SK Ed25519 key with "Ed25519" algorithm must use the SK + * code path, not the generic Ed25519 path. The real SkEd25519PublicKey returns + * "Ed25519" from getAlgorithm(), which matches isEd25519Key(). Without the + * fix, this causes Ed25519Verify.encodePublicKey() to attempt DER decoding + * and fail with InvalidKeySpecException. + */ + @Test + public void authenticateSkEd25519Key_WithEd25519Algorithm_UsesSkPath() throws Exception { + TestSkPublicKey skKey = new TestSkPublicKey(SK_ED25519_KEY_TYPE, "Ed25519", DEFAULT_APPLICATION, TEST_KEY_DATA); + TestSignatureProxy signatureProxy = new TestSignatureProxy(skKey, TEST_SIGNATURE); + + setupMocksForAuthentication(); + setupMockForAuthSuccess(); + + authManager.authenticatePublicKey(TEST_USER, signatureProxy); + + assertEquals(SignatureProxy.SHA512, signatureProxy.getLastHashAlgorithm(), + "SK Ed25519 key with Ed25519 algorithm should use SK path with SHA512"); + } + + /** + * Regression test: SK ECDSA key with "EC" algorithm must use the SK + * code path, not the generic ECDSA path. + */ + @Test + public void authenticateSkEcdsaKey_WithEcAlgorithm_UsesSkPath() throws Exception { + TestSkPublicKey skKey = new TestSkPublicKey(SK_ECDSA_KEY_TYPE, "EC", DEFAULT_APPLICATION, TEST_KEY_DATA); + TestSignatureProxy signatureProxy = new TestSignatureProxy(skKey, TEST_SIGNATURE); + + setupMocksForAuthentication(); + setupMockForAuthSuccess(); + + authManager.authenticatePublicKey(TEST_USER, signatureProxy); + + assertEquals(SignatureProxy.SHA256, signatureProxy.getLastHashAlgorithm(), + "SK ECDSA key with EC algorithm should use SK path with SHA256"); + } + private void setupMocksForAuthentication() throws IOException { lenient().when(tm.getSessionIdentifier()).thenReturn(TEST_SESSION_ID); lenient().when(tm.getExtensionInfo()).thenReturn(extensionInfo);