From 4210c3e5b746e3837ccb194428875d3c23aca411 Mon Sep 17 00:00:00 2001 From: nindanaoto Date: Thu, 9 Apr 2026 03:07:08 +0000 Subject: [PATCH 1/3] Add tests proving host key rotation crash (connectbot/connectbot#2023) When the server sends hostkeys-00@openssh.com after auth, the code compares stored algorithm names (e.g. "rsa-sha2-512") against key blob algorithm identifiers ("ssh-rsa"). This mismatch causes removeServerHostKey to be called with a null hostKey parameter, which crashes Kotlin callers (like ConnectBot) that declare the parameter as non-nullable ByteArray. These tests demonstrate: 1. removeServerHostKey is called with null due to RSA algo name mismatch 2. The resulting NPE propagates out of handleMessage uncaught 3. Baseline: matching algorithm names cause no removal Co-Authored-By: Claude Opus 4.6 (1M context) --- .../ssh2/channel/ChannelManagerTest.java | 144 ++++++++++++++++++ 1 file changed, 144 insertions(+) diff --git a/src/test/java/com/trilead/ssh2/channel/ChannelManagerTest.java b/src/test/java/com/trilead/ssh2/channel/ChannelManagerTest.java index 3452d3e9..b0847985 100644 --- a/src/test/java/com/trilead/ssh2/channel/ChannelManagerTest.java +++ b/src/test/java/com/trilead/ssh2/channel/ChannelManagerTest.java @@ -1,19 +1,25 @@ package com.trilead.ssh2.channel; import com.trilead.ssh2.ChannelCondition; +import com.trilead.ssh2.ConnectionInfo; import com.trilead.ssh2.ExtendedServerHostKeyVerifier; import com.trilead.ssh2.packets.PacketGlobalHostkeys; import com.trilead.ssh2.packets.Packets; import com.trilead.ssh2.packets.TypesWriter; +import com.trilead.ssh2.signature.RSASHA1Verify; +import com.trilead.ssh2.signature.RSASHA512Verify; import com.trilead.ssh2.transport.ITransportConnection; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import java.io.IOException; import java.util.Arrays; +import java.util.Collections; +import java.util.List; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -22,9 +28,13 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; import static org.mockito.Mockito.any; +import static org.mockito.Mockito.anyInt; +import static org.mockito.Mockito.anyString; import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.nullable; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -778,4 +788,138 @@ public void testMsgChannelOpenWithUnknownType() throws IOException { channelManager.msgChannelOpen(msg, offset); verify(mockTransportConnection).sendAsynchronousMessage(any(byte[].class)); } + + // ---- Host key rotation tests ---- + + /** + * Build an SSH key blob whose algorithm identifier is the given string. + * The blob is: uint32(len) + algorithm_name + uint32(len) + dummy_data. + * This is enough for extractKeyAlgorithm() which only reads the first string. + */ + private byte[] buildKeyBlob(String algorithm) { + TypesWriter tw = new TypesWriter(); + tw.writeString(algorithm); + // Append a dummy "key data" field so it looks like a plausible key blob + tw.writeString(new byte[]{0x00, 0x01, 0x02, 0x03}, 0, 4); + return tw.getBytes(); + } + + /** + * Build an SSH_MSG_GLOBAL_REQUEST message for hostkeys-00@openssh.com + * containing the given host key blobs. + */ + private byte[] buildHostkeysGlobalRequest(String requestName, boolean wantReply, byte[]... keyBlobs) { + TypesWriter tw = new TypesWriter(); + tw.writeByte(Packets.SSH_MSG_GLOBAL_REQUEST); + tw.writeString(requestName); + tw.writeBoolean(wantReply); + for (byte[] blob : keyBlobs) { + tw.writeString(blob, 0, blob.length); + } + return tw.getBytes(); + } + + /** + * Simulates the scenario from GitHub issue connectbot/connectbot#2023: + * + * 1. ConnectBot stores the host key algorithm as "rsa-sha2-512" (negotiated algo). + * 2. Server sends hostkeys-00@openssh.com advertising its RSA key blob + * which contains "ssh-rsa" as the key format identifier. + * 3. processHostkeysAdvertisement sees "rsa-sha2-512" NOT in the advertised set + * {"ssh-rsa"} and calls removeServerHostKey(hostname, port, "rsa-sha2-512", null). + * 4. A Kotlin implementation (like ConnectBot's) declares hostKey as non-nullable + * ByteArray, so passing null throws NullPointerException, crashing the app. + * + * This test verifies the bug: removeServerHostKey is called with null hostKey. + */ + @Test + public void testHostkeysAdvertisement_rsaAlgoMismatch_callsRemoveWithNull() throws Exception { + // Set up an ExtendedServerHostKeyVerifier that reports "rsa-sha2-512" as known + ExtendedServerHostKeyVerifier mockVerifier = mock(ExtendedServerHostKeyVerifier.class); + when(mockVerifier.getKnownKeyAlgorithmsForHost(anyString(), anyInt())) + .thenReturn(Collections.singletonList(RSASHA512Verify.ID_RSA_SHA_2_512)); + + // Mock getConnectionInfo so requestHostkeysProve doesn't NPE on ConnectionInfo + ConnectionInfo connInfo = new ConnectionInfo(); + connInfo.serverHostKeyAlgorithm = RSASHA512Verify.ID_RSA_SHA_2_512; + when(mockTransportConnection.getConnectionInfo(anyInt())).thenReturn(connInfo); + + when(mockTransportConnection.getServerHostKeyVerifier()).thenReturn(mockVerifier); + when(mockTransportConnection.getHostname()).thenReturn("esxi.example.com"); + when(mockTransportConnection.getPort()).thenReturn(22); + + // Server advertises an RSA key blob (algorithm in blob = "ssh-rsa") + byte[] rsaKeyBlob = buildKeyBlob(RSASHA1Verify.ID_SSH_RSA); + byte[] msg = buildHostkeysGlobalRequest( + "hostkeys-00@openssh.com", false, rsaKeyBlob); + + channelManager.handleMessage(msg, msg.length); + + // BUG: removeServerHostKey is called with null hostKey because + // "rsa-sha2-512" (known) is not in {"ssh-rsa"} (advertised) + ArgumentCaptor hostKeyCaptor = ArgumentCaptor.forClass(byte[].class); + verify(mockVerifier).removeServerHostKey( + anyString(), anyInt(), anyString(), hostKeyCaptor.capture()); + + // This proves the bug: the hostKey argument is null + assertNull(hostKeyCaptor.getValue(), + "removeServerHostKey should NOT be called with null hostKey, " + + "but currently it is due to RSA algorithm name mismatch"); + } + + /** + * Simulates the crash: if removeServerHostKey throws when called with null + * (as Kotlin's non-nullable ByteArray check does), the exception propagates + * uncaught through handleMessage, killing the SSH receiver thread and + * crashing the app. + */ + @Test + public void testHostkeysAdvertisement_rsaAlgoMismatch_crashesReceiverThread() throws Exception { + // Set up an ExtendedServerHostKeyVerifier that throws NPE on null hostKey + // (simulating Kotlin's non-nullable parameter check) + ExtendedServerHostKeyVerifier mockVerifier = mock(ExtendedServerHostKeyVerifier.class); + when(mockVerifier.getKnownKeyAlgorithmsForHost(anyString(), anyInt())) + .thenReturn(Collections.singletonList(RSASHA512Verify.ID_RSA_SHA_2_512)); + doThrow(new NullPointerException("Parameter specified as non-null is null: parameter hostKey")) + .when(mockVerifier).removeServerHostKey(anyString(), anyInt(), anyString(), nullable(byte[].class)); + + when(mockTransportConnection.getServerHostKeyVerifier()).thenReturn(mockVerifier); + when(mockTransportConnection.getHostname()).thenReturn("esxi.example.com"); + when(mockTransportConnection.getPort()).thenReturn(22); + + byte[] rsaKeyBlob = buildKeyBlob(RSASHA1Verify.ID_SSH_RSA); + byte[] msg = buildHostkeysGlobalRequest( + "hostkeys-00@openssh.com", false, rsaKeyBlob); + + // The NPE propagates out of handleMessage — this kills the receiver thread + // and crashes the Android app + assertThrows(NullPointerException.class, () -> + channelManager.handleMessage(msg, msg.length)); + } + + /** + * Verify that when the stored algorithm matches the advertised key blob + * algorithm (both "ssh-rsa"), removeServerHostKey is NOT called. + * This is the baseline: no mismatch, no problem. + */ + @Test + public void testHostkeysAdvertisement_matchingAlgo_noRemoval() throws Exception { + ExtendedServerHostKeyVerifier mockVerifier = mock(ExtendedServerHostKeyVerifier.class); + when(mockVerifier.getKnownKeyAlgorithmsForHost(anyString(), anyInt())) + .thenReturn(Collections.singletonList(RSASHA1Verify.ID_SSH_RSA)); + + when(mockTransportConnection.getServerHostKeyVerifier()).thenReturn(mockVerifier); + when(mockTransportConnection.getHostname()).thenReturn("esxi.example.com"); + when(mockTransportConnection.getPort()).thenReturn(22); + + byte[] rsaKeyBlob = buildKeyBlob(RSASHA1Verify.ID_SSH_RSA); + byte[] msg = buildHostkeysGlobalRequest( + "hostkeys-00@openssh.com", false, rsaKeyBlob); + + channelManager.handleMessage(msg, msg.length); + + // No algorithm mismatch, so removeServerHostKey should not be called + verify(mockVerifier, never()).removeServerHostKey( + anyString(), anyInt(), anyString(), any()); + } } From f791f805a0f044920c0f7299d4c46352166d7865 Mon Sep 17 00:00:00 2001 From: nindanaoto Date: Thu, 9 Apr 2026 03:12:24 +0000 Subject: [PATCH 2/3] Fix host key rotation crash with RSA algorithm name mismatch Fixes connectbot/connectbot#2023. When the server sends hostkeys-00@openssh.com after auth, processHostkeysAdvertisement compared stored algorithm names (e.g. "rsa-sha2-512") against key blob identifiers ("ssh-rsa"). Since these are the same RSA key with different signature algorithms, the mismatch caused removeServerHostKey to be called with a null hostKey, crashing Kotlin callers. Changes: 1. Add normalizeKeyAlgorithm() to treat rsa-sha2-256/512 as ssh-rsa for key identity comparison. This prevents the false mismatch. 2. Catch Exception (not just IOException) around processHostkeysAdvertisement so RuntimeExceptions from verifier callbacks don't kill the receiver thread. 3. Move hostkeys-prove check before globalSuccessCounter increment in msgGlobalSuccess to prevent counter interference with other global requests. 4. Send REQUEST_SUCCESS (not FAILURE) for hostkeys requests we handle. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../trilead/ssh2/channel/ChannelManager.java | 79 +++++++---- .../ssh2/channel/ChannelManagerTest.java | 125 ++++++++++++------ 2 files changed, 137 insertions(+), 67 deletions(-) diff --git a/src/main/java/com/trilead/ssh2/channel/ChannelManager.java b/src/main/java/com/trilead/ssh2/channel/ChannelManager.java index e097b6fa..f0678c14 100644 --- a/src/main/java/com/trilead/ssh2/channel/ChannelManager.java +++ b/src/main/java/com/trilead/ssh2/channel/ChannelManager.java @@ -1676,24 +1676,12 @@ public void msgChannelOpenFailure(byte[] msg, int msglen) throws IOException public void msgGlobalRequest(byte[] msg, int msglen) throws IOException { - /* Currently we do not support any kind of global request */ - TypesReader tr = new TypesReader(msg, 0, msglen); tr.readByte(); // skip packet type String requestName = tr.readString(); boolean wantReply = tr.readBoolean(); - if (wantReply) - { - byte[] reply_failure = new byte[1]; - reply_failure[0] = Packets.SSH_MSG_REQUEST_FAILURE; - - tm.sendAsynchronousMessage(reply_failure); - } - - /* We do not clean up the requestName String - that is OK for debug */ - if (log.isEnabled()) log.log(80, "Got SSH_MSG_GLOBAL_REQUEST (" + requestName + ")"); @@ -1702,20 +1690,33 @@ public void msgGlobalRequest(byte[] msg, int msglen) throws IOException try { PacketGlobalHostkeys hostkeys = new PacketGlobalHostkeys(msg, 0, msglen); processHostkeysAdvertisement(hostkeys, requestName); - } catch (IOException e) { + } catch (Exception e) { if (log.isEnabled()) - log.log(20, "Failed to parse hostkeys advertisement: " + e.getMessage()); + log.log(20, "Failed to process hostkeys advertisement: " + e.getMessage()); } + // hostkeys-00@openssh.com typically has wantReply=false, but if the + // server does request a reply, acknowledge it since we processed it. + if (wantReply) + { + byte[] reply_success = new byte[1]; + reply_success[0] = Packets.SSH_MSG_REQUEST_SUCCESS; + tm.sendAsynchronousMessage(reply_success); + } + return; } - } - public void msgGlobalSuccess(byte[] msg, int msglen) throws IOException { - synchronized (channels) + if (wantReply) { - globalSuccessCounter++; - channels.notifyAll(); + byte[] reply_failure = new byte[1]; + reply_failure[0] = Packets.SSH_MSG_REQUEST_FAILURE; + + tm.sendAsynchronousMessage(reply_failure); } + } + public void msgGlobalSuccess(byte[] msg, int msglen) throws IOException { + // Check for pending hostkeys-prove BEFORE incrementing the global counter, + // so the hostkeys-prove response doesn't interfere with other global request tracking. synchronized (hostkeysProveLock) { if (pendingHostkeysProve != null && !pendingHostkeysProve.completed) { try { @@ -1739,6 +1740,12 @@ public void msgGlobalSuccess(byte[] msg, int msglen) throws IOException { } } + synchronized (channels) + { + globalSuccessCounter++; + channels.notifyAll(); + } + if (log.isEnabled()) log.log(80, "Got SSH_MSG_REQUEST_SUCCESS"); } @@ -1882,19 +1889,28 @@ private void processHostkeysAdvertisement(PacketGlobalHostkeys hostkeys, String } List knownAlgos = extVerifier.getKnownKeyAlgorithmsForHost(hostname, port); - Set knownAlgoSet = (knownAlgos != null) ? new HashSet<>(knownAlgos) : new HashSet<>(); + + // Normalize algorithm names so RSA signature variants (rsa-sha2-256, + // rsa-sha2-512) are treated as the same key type as ssh-rsa. + Set normalizedKnownAlgoSet = new HashSet<>(); + if (knownAlgos != null) { + for (String algo : knownAlgos) { + normalizedKnownAlgoSet.add(normalizeKeyAlgorithm(algo)); + } + } List newKeys = new ArrayList<>(); - Set advertisedAlgoSet = new HashSet<>(); + Set normalizedAdvertisedAlgoSet = new HashSet<>(); for (byte[] keyBlob : advertisedKeys) { String keyAlgo = extractKeyAlgorithm(keyBlob); if (keyAlgo == null) continue; - advertisedAlgoSet.add(keyAlgo); + String normalizedAlgo = normalizeKeyAlgorithm(keyAlgo); + normalizedAdvertisedAlgoSet.add(normalizedAlgo); - if (!knownAlgoSet.contains(keyAlgo)) { + if (!normalizedKnownAlgoSet.contains(normalizedAlgo)) { newKeys.add(keyBlob); } } @@ -1903,7 +1919,7 @@ private void processHostkeysAdvertisement(PacketGlobalHostkeys hostkeys, String for (String knownAlgo : knownAlgos) { if (knownAlgo == null) continue; - if (!advertisedAlgoSet.contains(knownAlgo)) { + if (!normalizedAdvertisedAlgoSet.contains(normalizeKeyAlgorithm(knownAlgo))) { extVerifier.removeServerHostKey(hostname, port, knownAlgo, null); if (log.isEnabled()) log.log(50, "Removed hostkey algorithm no longer advertised: " + knownAlgo); @@ -2050,6 +2066,21 @@ private String extractKeyAlgorithm(byte[] keyBlob) throws IOException return tr.readString(); } + /** + * Normalizes RSA algorithm variants to a canonical form for comparison. + * In SSH, rsa-sha2-256 and rsa-sha2-512 use the same RSA key as ssh-rsa + * (the difference is only the signature hash algorithm). Key blobs always + * identify as ssh-rsa regardless of which signature algorithm was negotiated. + */ + static String normalizeKeyAlgorithm(String algorithm) + { + if (RSASHA256Verify.ID_RSA_SHA_2_256.equals(algorithm) || + RSASHA512Verify.ID_RSA_SHA_2_512.equals(algorithm)) { + return RSASHA1Verify.ID_SSH_RSA; + } + return algorithm; + } + private SSHSignature getSignatureVerifier(String algorithm) { if (RSASHA1Verify.ID_SSH_RSA.equals(algorithm)) diff --git a/src/test/java/com/trilead/ssh2/channel/ChannelManagerTest.java b/src/test/java/com/trilead/ssh2/channel/ChannelManagerTest.java index b0847985..2262d2a8 100644 --- a/src/test/java/com/trilead/ssh2/channel/ChannelManagerTest.java +++ b/src/test/java/com/trilead/ssh2/channel/ChannelManagerTest.java @@ -1,12 +1,12 @@ package com.trilead.ssh2.channel; import com.trilead.ssh2.ChannelCondition; -import com.trilead.ssh2.ConnectionInfo; import com.trilead.ssh2.ExtendedServerHostKeyVerifier; import com.trilead.ssh2.packets.PacketGlobalHostkeys; import com.trilead.ssh2.packets.Packets; import com.trilead.ssh2.packets.TypesWriter; import com.trilead.ssh2.signature.RSASHA1Verify; +import com.trilead.ssh2.signature.RSASHA256Verify; import com.trilead.ssh2.signature.RSASHA512Verify; import com.trilead.ssh2.transport.ITransportConnection; import org.junit.jupiter.api.BeforeEach; @@ -19,7 +19,6 @@ import java.io.IOException; import java.util.Arrays; import java.util.Collections; -import java.util.List; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -820,66 +819,50 @@ private byte[] buildHostkeysGlobalRequest(String requestName, boolean wantReply, } /** - * Simulates the scenario from GitHub issue connectbot/connectbot#2023: + * Regression test for GitHub issue connectbot/connectbot#2023: * - * 1. ConnectBot stores the host key algorithm as "rsa-sha2-512" (negotiated algo). - * 2. Server sends hostkeys-00@openssh.com advertising its RSA key blob - * which contains "ssh-rsa" as the key format identifier. - * 3. processHostkeysAdvertisement sees "rsa-sha2-512" NOT in the advertised set - * {"ssh-rsa"} and calls removeServerHostKey(hostname, port, "rsa-sha2-512", null). - * 4. A Kotlin implementation (like ConnectBot's) declares hostKey as non-nullable - * ByteArray, so passing null throws NullPointerException, crashing the app. + * Before the fix, when the stored algorithm was "rsa-sha2-512" and the + * key blob contained "ssh-rsa", processHostkeysAdvertisement would call + * removeServerHostKey with null hostKey (crashing Kotlin callers). * - * This test verifies the bug: removeServerHostKey is called with null hostKey. + * After the fix, RSA algorithm variants are normalized so "rsa-sha2-512" + * and "ssh-rsa" are recognized as the same key type. removeServerHostKey + * should NOT be called, since the key is still present. */ @Test - public void testHostkeysAdvertisement_rsaAlgoMismatch_callsRemoveWithNull() throws Exception { - // Set up an ExtendedServerHostKeyVerifier that reports "rsa-sha2-512" as known + public void testHostkeysAdvertisement_rsaAlgoMismatch_noRemovalAfterFix() throws Exception { ExtendedServerHostKeyVerifier mockVerifier = mock(ExtendedServerHostKeyVerifier.class); when(mockVerifier.getKnownKeyAlgorithmsForHost(anyString(), anyInt())) .thenReturn(Collections.singletonList(RSASHA512Verify.ID_RSA_SHA_2_512)); - // Mock getConnectionInfo so requestHostkeysProve doesn't NPE on ConnectionInfo - ConnectionInfo connInfo = new ConnectionInfo(); - connInfo.serverHostKeyAlgorithm = RSASHA512Verify.ID_RSA_SHA_2_512; - when(mockTransportConnection.getConnectionInfo(anyInt())).thenReturn(connInfo); - when(mockTransportConnection.getServerHostKeyVerifier()).thenReturn(mockVerifier); when(mockTransportConnection.getHostname()).thenReturn("esxi.example.com"); when(mockTransportConnection.getPort()).thenReturn(22); - // Server advertises an RSA key blob (algorithm in blob = "ssh-rsa") byte[] rsaKeyBlob = buildKeyBlob(RSASHA1Verify.ID_SSH_RSA); byte[] msg = buildHostkeysGlobalRequest( "hostkeys-00@openssh.com", false, rsaKeyBlob); channelManager.handleMessage(msg, msg.length); - // BUG: removeServerHostKey is called with null hostKey because - // "rsa-sha2-512" (known) is not in {"ssh-rsa"} (advertised) - ArgumentCaptor hostKeyCaptor = ArgumentCaptor.forClass(byte[].class); - verify(mockVerifier).removeServerHostKey( - anyString(), anyInt(), anyString(), hostKeyCaptor.capture()); - - // This proves the bug: the hostKey argument is null - assertNull(hostKeyCaptor.getValue(), - "removeServerHostKey should NOT be called with null hostKey, " + - "but currently it is due to RSA algorithm name mismatch"); + // After fix: rsa-sha2-512 is normalized to ssh-rsa, so the algorithm + // is recognized as still advertised. removeServerHostKey must NOT be called. + verify(mockVerifier, never()).removeServerHostKey( + anyString(), anyInt(), anyString(), any()); } /** - * Simulates the crash: if removeServerHostKey throws when called with null - * (as Kotlin's non-nullable ByteArray check does), the exception propagates - * uncaught through handleMessage, killing the SSH receiver thread and - * crashing the app. + * Regression test: even if removeServerHostKey throws (e.g., Kotlin's + * non-nullable parameter check), it must not propagate out of handleMessage + * and kill the SSH receiver thread. */ @Test - public void testHostkeysAdvertisement_rsaAlgoMismatch_crashesReceiverThread() throws Exception { - // Set up an ExtendedServerHostKeyVerifier that throws NPE on null hostKey - // (simulating Kotlin's non-nullable parameter check) + public void testHostkeysAdvertisement_removeThrows_doesNotCrashReceiverThread() throws Exception { + // Use a non-RSA algorithm so normalization doesn't prevent the removal call. + // Pretend the client knows "ssh-dss" but server no longer advertises it. ExtendedServerHostKeyVerifier mockVerifier = mock(ExtendedServerHostKeyVerifier.class); when(mockVerifier.getKnownKeyAlgorithmsForHost(anyString(), anyInt())) - .thenReturn(Collections.singletonList(RSASHA512Verify.ID_RSA_SHA_2_512)); + .thenReturn(Collections.singletonList("ssh-dss")); doThrow(new NullPointerException("Parameter specified as non-null is null: parameter hostKey")) .when(mockVerifier).removeServerHostKey(anyString(), anyInt(), anyString(), nullable(byte[].class)); @@ -887,14 +870,17 @@ public void testHostkeysAdvertisement_rsaAlgoMismatch_crashesReceiverThread() th when(mockTransportConnection.getHostname()).thenReturn("esxi.example.com"); when(mockTransportConnection.getPort()).thenReturn(22); - byte[] rsaKeyBlob = buildKeyBlob(RSASHA1Verify.ID_SSH_RSA); + // Server advertises only an ed25519 key (no DSS) + byte[] ed25519KeyBlob = buildKeyBlob("ssh-ed25519"); byte[] msg = buildHostkeysGlobalRequest( - "hostkeys-00@openssh.com", false, rsaKeyBlob); + "hostkeys-00@openssh.com", false, ed25519KeyBlob); + + // Even if removeServerHostKey throws, handleMessage must NOT propagate it + channelManager.handleMessage(msg, msg.length); - // The NPE propagates out of handleMessage — this kills the receiver thread - // and crashes the Android app - assertThrows(NullPointerException.class, () -> - channelManager.handleMessage(msg, msg.length)); + // The call did happen (and threw), but the exception was caught + verify(mockVerifier).removeServerHostKey( + anyString(), anyInt(), anyString(), nullable(byte[].class)); } /** @@ -922,4 +908,57 @@ public void testHostkeysAdvertisement_matchingAlgo_noRemoval() throws Exception verify(mockVerifier, never()).removeServerHostKey( anyString(), anyInt(), anyString(), any()); } + + // ---- normalizeKeyAlgorithm tests ---- + + @Test + public void testNormalizeKeyAlgorithm_rsaSha2_512() { + assertEquals(RSASHA1Verify.ID_SSH_RSA, + ChannelManager.normalizeKeyAlgorithm(RSASHA512Verify.ID_RSA_SHA_2_512)); + } + + @Test + public void testNormalizeKeyAlgorithm_rsaSha2_256() { + assertEquals(RSASHA1Verify.ID_SSH_RSA, + ChannelManager.normalizeKeyAlgorithm(RSASHA256Verify.ID_RSA_SHA_2_256)); + } + + @Test + public void testNormalizeKeyAlgorithm_sshRsa_unchanged() { + assertEquals(RSASHA1Verify.ID_SSH_RSA, + ChannelManager.normalizeKeyAlgorithm(RSASHA1Verify.ID_SSH_RSA)); + } + + @Test + public void testNormalizeKeyAlgorithm_nonRsa_unchanged() { + assertEquals("ssh-ed25519", + ChannelManager.normalizeKeyAlgorithm("ssh-ed25519")); + assertEquals("ssh-dss", + ChannelManager.normalizeKeyAlgorithm("ssh-dss")); + assertEquals("ecdsa-sha2-nistp256", + ChannelManager.normalizeKeyAlgorithm("ecdsa-sha2-nistp256")); + } + + // ---- msgGlobalRequest hostkeys reply tests ---- + + @Test + public void testMsgGlobalRequest_hostkeys_withReply_sendsSuccess() throws Exception { + ExtendedServerHostKeyVerifier mockVerifier = mock(ExtendedServerHostKeyVerifier.class); + when(mockVerifier.getKnownKeyAlgorithmsForHost(anyString(), anyInt())).thenReturn(null); + when(mockTransportConnection.getServerHostKeyVerifier()).thenReturn(mockVerifier); + when(mockTransportConnection.getHostname()).thenReturn("example.com"); + when(mockTransportConnection.getPort()).thenReturn(22); + + byte[] rsaKeyBlob = buildKeyBlob(RSASHA1Verify.ID_SSH_RSA); + byte[] msg = buildHostkeysGlobalRequest( + "hostkeys-00@openssh.com", true, rsaKeyBlob); + + channelManager.handleMessage(msg, msg.length); + + // Should send REQUEST_SUCCESS (not FAILURE) for handled hostkeys request + ArgumentCaptor replyCaptor = ArgumentCaptor.forClass(byte[].class); + verify(mockTransportConnection).sendAsynchronousMessage(replyCaptor.capture()); + assertEquals(Packets.SSH_MSG_REQUEST_SUCCESS, replyCaptor.getValue()[0], + "Hostkeys global request should get SUCCESS reply, not FAILURE"); + } } From 1850f69ea021b1c1b7ae181ece80aaf618520566 Mon Sep 17 00:00:00 2001 From: nindanaoto Date: Thu, 9 Apr 2026 08:20:30 +0000 Subject: [PATCH 3/3] Trigger CI re-run (DropbearCompatibilityTest flaky on Java 21) The canConnectWithRsa Dropbear test failed with "bad signature" on the server side, which is unrelated to the ChannelManager changes in this branch. Only ChannelManager.java and ChannelManagerTest.java are modified and authentication happens outside ChannelManager (message types 50-60 vs ChannelManager's 80-100). Co-Authored-By: Claude Opus 4.6 (1M context)