From 56de4d4d3515d3dbcd05242e4ffbf9c27ac9a7ff Mon Sep 17 00:00:00 2001 From: mpet Date: Mon, 19 Feb 2024 12:17:26 +0100 Subject: [PATCH] Trilead ssh2 fix big integer removes leading zero (#178) * Update TimeoutService.java interrupt hanging timeoutservice threads * Update TimeoutService.java * Update TimeoutService.java * Update Connection.java * Update Connection.java * Update TimeoutService.java * Update TimeoutService.java * Update Connection.java * fix BigInteger --------- Co-authored-by: mikael petterson --- .gitignore | 9 +++ .../ssh2/packets/PacketKexDHReply.java | 10 +-- .../trilead/ssh2/transport/KexManager.java | 2 +- .../crypto/dh/Curve25519ExchangeTest.java | 66 +++++++++++++++++++ 4 files changed, 81 insertions(+), 6 deletions(-) diff --git a/.gitignore b/.gitignore index ffc86925..bfb2730a 100644 --- a/.gitignore +++ b/.gitignore @@ -14,3 +14,12 @@ target/ .metadata .loadpath bin/ + +#Netbeans files +nbproject/private/ +build/ +nbbuild/ +dist/ +nbdist/ +nbactions.xml +nb-configuration.xml \ No newline at end of file diff --git a/src/com/trilead/ssh2/packets/PacketKexDHReply.java b/src/com/trilead/ssh2/packets/PacketKexDHReply.java index 27f9f914..bc820e7f 100644 --- a/src/com/trilead/ssh2/packets/PacketKexDHReply.java +++ b/src/com/trilead/ssh2/packets/PacketKexDHReply.java @@ -2,7 +2,6 @@ import java.io.IOException; -import java.math.BigInteger; /** * PacketKexDHReply. @@ -15,7 +14,7 @@ public class PacketKexDHReply byte[] payload; byte[] hostKey; - BigInteger f; + byte [] publicKey; byte[] signature; public PacketKexDHReply(byte payload[], int off, int len) throws IOException @@ -32,15 +31,16 @@ public PacketKexDHReply(byte payload[], int off, int len) throws IOException + packet_type + ")"); hostKey = tr.readByteString(); - f = tr.readMPINT(); + publicKey = tr.readByteString(); signature = tr.readByteString(); if (tr.remain() != 0) throw new IOException("PADDING IN SSH_MSG_KEXDH_REPLY!"); } - public BigInteger getF() + public byte[] getF() { - return f; + return publicKey; + } public byte[] getHostKey() diff --git a/src/com/trilead/ssh2/transport/KexManager.java b/src/com/trilead/ssh2/transport/KexManager.java index f9ed387b..c2ec2b0f 100644 --- a/src/com/trilead/ssh2/transport/KexManager.java +++ b/src/com/trilead/ssh2/transport/KexManager.java @@ -658,7 +658,7 @@ public synchronized void handleMessage(byte[] msg, int msglen) throws IOExceptio throw new IOException("The server hostkey was not accepted by the verifier callback"); } - kxs.dhx.setF(dhr.getF().toByteArray()); + kxs.dhx.setF(dhr.getF()); try { diff --git a/test/com/trilead/ssh2/crypto/dh/Curve25519ExchangeTest.java b/test/com/trilead/ssh2/crypto/dh/Curve25519ExchangeTest.java index 701fbaf9..a5bcb888 100644 --- a/test/com/trilead/ssh2/crypto/dh/Curve25519ExchangeTest.java +++ b/test/com/trilead/ssh2/crypto/dh/Curve25519ExchangeTest.java @@ -1,14 +1,19 @@ package com.trilead.ssh2.crypto.dh; import com.google.crypto.tink.subtle.X25519; +import com.trilead.ssh2.packets.PacketKexDHReply; +import java.io.IOException; import org.junit.Test; import java.math.BigInteger; +import java.security.InvalidKeyException; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.fail; /** * Created by Kenny Root on 1/23/16. @@ -76,4 +81,65 @@ public void knownValues_Bob() throws Exception { ex.setF(ALICE_PUBLIC); assertEquals(KNOWN_SHARED_SECRET_BI, ex.sharedSecret); } + + @Test + public void testBigIntegerRemovesLeadingZero() throws Exception { + + BigInteger bigIntegerWithoutLeadingZero = new BigInteger(1, toByteArray("4a5d9d5ba4ce2de1728e3bf480350f25e07e21c947d19e3376f09b3c1e161742")); + + BigInteger bigIntegerWithLeadingZero = new BigInteger(1, toByteArray("015d9d5ba4ce2de1728e3bf480350f25e07e21c947d19e3376f09b3c1e161742")); + //The key with same length does not become the same when using BigInteger when a key has leading zeros. This is because BigInteger has stripLeadingZeroBytes method. + assertNotEquals(Integer.valueOf(bigIntegerWithoutLeadingZero.bitLength()),Integer.valueOf(bigIntegerWithLeadingZero.bitLength())); + + + + } + + + @Test + public void testKeyWithLeadingZeros() throws InvalidKeyException{ + + //When the message contains leading 0 then the BigInteger class + //will remove the leading zero since it has a function to do so internally. + Curve25519Exchange curve25519Exchange = new Curve25519Exchange(ALICE_PRIVATE); + + //public Diffie-Hellman key and other parameters in message. This one has + //leading zero.0, 0, 0, 2, 2, 2, 2, 2 + byte[] msg = new byte[]{ + 31, + + 0, 0, 0, 32, + 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, + + 0, 0, 0, 32, + 0, 0, 0, 2, 2, 2, 2, 2, + 2, 2, 2, 2, 2, 2, 2, 2, + 2, 2, 2, 2, 2, 2, 2, 2, + 2, 2, 2, 2, 2, 2, 2, 2, + + 0, 0, 0, 32, + 3, 3, 3, 3, 3, 3, 3, 3, + 3, 3, 3, 3, 3, 3, 3, 3, + 3, 3, 3, 3, 3, 3, 3, 3, + 3, 3, 3, 3, 3, 3, 3, 3 + }; + PacketKexDHReply dhr = null; + + try { + dhr = new PacketKexDHReply(msg, 0, msg.length); + } catch (IOException ioex) { + fail("We should not get exception when creating the PacketKexDHReply "+ioex.getMessage()); + } + try { + curve25519Exchange.setF(dhr.getF()); + } catch (IOException ioex) { + fail("We should not get exception while setting curve key "+ioex.getMessage()); + + } + } + } +