From 1bd5343e71b7ebd09aabaebba282227a7786e5ba Mon Sep 17 00:00:00 2001 From: Kenny Root Date: Fri, 25 Dec 2015 20:16:23 -0600 Subject: Add length check with test --- sshlib/build.gradle | 2 + .../com/trilead/ssh2/crypto/SimpleDERReader.java | 8 ++- .../trilead/ssh2/crypto/SimpleDERReaderTest.java | 67 ++++++++++++++++++++++ 3 files changed, 76 insertions(+), 1 deletion(-) create mode 100644 sshlib/src/test/java/com/trilead/ssh2/crypto/SimpleDERReaderTest.java diff --git a/sshlib/build.gradle b/sshlib/build.gradle index e22fe39..d43c56e 100644 --- a/sshlib/build.gradle +++ b/sshlib/build.gradle @@ -15,4 +15,6 @@ dependencies { compile fileTree(dir: 'libs', include: ['*.jar']) compile 'com.jcraft:jzlib:1.1.3' compile 'org.connectbot:simplesocks:1.0.1' + + testCompile 'junit:junit:4.12' } diff --git a/sshlib/src/main/java/com/trilead/ssh2/crypto/SimpleDERReader.java b/sshlib/src/main/java/com/trilead/ssh2/crypto/SimpleDERReader.java index ff8112a..beca5f7 100644 --- a/sshlib/src/main/java/com/trilead/ssh2/crypto/SimpleDERReader.java +++ b/sshlib/src/main/java/com/trilead/ssh2/crypto/SimpleDERReader.java @@ -68,7 +68,8 @@ public class SimpleDERReader return count; } - private int readLength() throws IOException + /* visible for testing */ + int readLength() throws IOException { int len = readByte() & 0xff; @@ -79,6 +80,8 @@ public class SimpleDERReader if (remain == 0) return -1; + else if (remain > 4) + return -1; len = 0; @@ -89,6 +92,9 @@ public class SimpleDERReader remain--; } + if (len < 0) + return -1; + return len; } diff --git a/sshlib/src/test/java/com/trilead/ssh2/crypto/SimpleDERReaderTest.java b/sshlib/src/test/java/com/trilead/ssh2/crypto/SimpleDERReaderTest.java new file mode 100644 index 0000000..3eaec20 --- /dev/null +++ b/sshlib/src/test/java/com/trilead/ssh2/crypto/SimpleDERReaderTest.java @@ -0,0 +1,67 @@ +package com.trilead.ssh2.crypto; + +import org.junit.Test; + +import java.io.IOException; + +import static org.junit.Assert.*; +import static org.hamcrest.CoreMatchers.*; + +/** + * Created by kenny on 12/25/15. + */ +public class SimpleDERReaderTest { + @Test + public void readLength_Extended_OverlyLongLength() throws Exception { + byte[] vector = new byte[] { + (byte) 0x85, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF + }; + SimpleDERReader reader = new SimpleDERReader(vector); + assertEquals(-1, reader.readLength()); + } + + @Test + public void readLength_Extended_TooLongForInt() throws Exception { + byte[] vector = new byte[] { + (byte) 0x84, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF + }; + SimpleDERReader reader = new SimpleDERReader(vector); + assertEquals(-1, reader.readLength()); + } + + @Test + public void readLength_Extended_Zero() throws Exception { + byte[] vector = new byte[] { + (byte) 0x80, (byte) 0x01 + }; + SimpleDERReader reader = new SimpleDERReader(vector); + assertEquals(-1, reader.readLength()); + } + + @Test + public void readLength_Extended_Valid() throws Exception { + byte[] vector = new byte[] { + (byte) 0x82, (byte) 0x05, (byte) 0xFF + }; + SimpleDERReader reader = new SimpleDERReader(vector); + assertEquals(0x5FF, reader.readLength()); + } + + @Test + public void readLength_Short_Zero() throws Exception { + byte[] vector = new byte[] { + (byte) 0x00 + }; + SimpleDERReader reader = new SimpleDERReader(vector); + assertEquals(0, reader.readLength()); + } + + @Test + public void readLength_Short_Regular() throws Exception { + byte[] vector = new byte[] { + (byte) 0x09 + }; + SimpleDERReader reader = new SimpleDERReader(vector); + assertEquals(9, reader.readLength()); + } +} \ No newline at end of file -- cgit v1.2.3 From d60c23560bd0464b2f8862f43711781eb45eac49 Mon Sep 17 00:00:00 2001 From: Kenny Root Date: Fri, 25 Dec 2015 20:41:18 -0600 Subject: Add tests for readInt --- .../trilead/ssh2/crypto/SimpleDERReaderTest.java | 48 ++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/sshlib/src/test/java/com/trilead/ssh2/crypto/SimpleDERReaderTest.java b/sshlib/src/test/java/com/trilead/ssh2/crypto/SimpleDERReaderTest.java index 3eaec20..cabadce 100644 --- a/sshlib/src/test/java/com/trilead/ssh2/crypto/SimpleDERReaderTest.java +++ b/sshlib/src/test/java/com/trilead/ssh2/crypto/SimpleDERReaderTest.java @@ -3,6 +3,7 @@ package com.trilead.ssh2.crypto; import org.junit.Test; import java.io.IOException; +import java.math.BigInteger; import static org.junit.Assert.*; import static org.hamcrest.CoreMatchers.*; @@ -64,4 +65,51 @@ public class SimpleDERReaderTest { SimpleDERReader reader = new SimpleDERReader(vector); assertEquals(9, reader.readLength()); } + + @Test + public void readInt_MaxInt() throws Exception { + byte[] vector = new byte[] { + (byte) 0x02, (byte) 0x04, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, + }; + SimpleDERReader reader = new SimpleDERReader(vector); + assertEquals(BigInteger.valueOf(0xFFFFFFFF), reader.readInt()); + } + + @Test + public void readInt_NotReallyInteger() throws Exception { + byte[] vector = new byte[] { + (byte) 0x01, (byte) 0x04, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, + }; + SimpleDERReader reader = new SimpleDERReader(vector); + try { + reader.readInt(); + } catch (IOException expected) { + assertThat(expected.getMessage(), containsString("Expected DER Integer")); + } + } + + @Test + public void readInt_InvalidLength() throws Exception { + byte[] vector = new byte[] { + (byte) 0x02, (byte) 0x80, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, + }; + SimpleDERReader reader = new SimpleDERReader(vector); + try { + reader.readInt(); + } catch (IOException expected) { + assertThat(expected.getMessage(), containsString("Illegal len")); + } + } + + @Test + public void readInt_ShortArray() throws Exception { + byte[] vector = new byte[] { + (byte) 0x02, (byte) 0x02, (byte) 0xFF + }; + SimpleDERReader reader = new SimpleDERReader(vector); + try { + reader.readInt(); + } catch (IOException expected) { + } + } } \ No newline at end of file -- cgit v1.2.3 From 93e998a377cf6bd4098ec38e99d6c191e274c3a9 Mon Sep 17 00:00:00 2001 From: Kenny Root Date: Fri, 25 Dec 2015 22:58:09 -0600 Subject: Add unit tests for readOid --- .../trilead/ssh2/crypto/SimpleDERReaderTest.java | 72 ++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/sshlib/src/test/java/com/trilead/ssh2/crypto/SimpleDERReaderTest.java b/sshlib/src/test/java/com/trilead/ssh2/crypto/SimpleDERReaderTest.java index cabadce..6a97218 100644 --- a/sshlib/src/test/java/com/trilead/ssh2/crypto/SimpleDERReaderTest.java +++ b/sshlib/src/test/java/com/trilead/ssh2/crypto/SimpleDERReaderTest.java @@ -112,4 +112,76 @@ public class SimpleDERReaderTest { } catch (IOException expected) { } } + + @Test + public void readOid_InvalidLength() throws Exception { + byte[] vector = new byte[]{ + (byte) 0x02, (byte) 0x80, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, + }; + SimpleDERReader reader = new SimpleDERReader(vector); + try { + reader.readOid(); + } catch (IOException expected) { + } + } + + @Test + public void readOid_TooShort() throws Exception { + byte[] vector = new byte[]{ + (byte) 0x02, (byte) 0x00 + }; + SimpleDERReader reader = new SimpleDERReader(vector); + try { + reader.readOid(); + } catch (IOException expected) { + } + } + + @Test + public void readOid_NotOidValue() throws Exception { + byte[] vector = new byte[]{ + (byte) 0x02, (byte) 0x04, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, + }; + SimpleDERReader reader = new SimpleDERReader(vector); + try { + reader.readOid(); + } catch (IOException expected) { + } + } + + @Test + public void readOid_Valid1() throws Exception { + byte[] vector = new byte[]{ + (byte) 0x06, (byte) 0x01, (byte) 0x28 + }; + SimpleDERReader reader = new SimpleDERReader(vector); + assertEquals("1.0", reader.readOid()); + } + + @Test + public void readOid_Valid1Prefix() throws Exception { + byte[] vector = new byte[]{ + (byte) 0x06, (byte) 0x09, (byte) 0x2a, (byte) 0x86, (byte) 0x48, (byte) 0x86, (byte) 0xf7, (byte) 0x0d, (byte) 0x01, (byte) 0x01, (byte) 0x0b + }; + SimpleDERReader reader = new SimpleDERReader(vector); + assertEquals("1.2.840.113549.1.1.11", reader.readOid()); + } + + @Test + public void readOid_Valid0Prefix() throws Exception { + byte[] vector = new byte[]{ + (byte) 0x06, (byte) 0x0A, (byte) 0x09, (byte) 0x92, (byte) 0x26, (byte) 0x89, (byte) 0x93, (byte) 0xF2, (byte) 0x2C, (byte) 0x64, (byte) 0x04, (byte) 0x0D + }; + SimpleDERReader reader = new SimpleDERReader(vector); + assertEquals("0.9.2342.19200300.100.4.13", reader.readOid()); + } + + @Test + public void readOid_Valid2Prefix() throws Exception { + byte[] vector = new byte[]{ + (byte) 0x06, (byte) 0x03, (byte) 0x55, (byte) 0x1D, (byte) 0x0E + }; + SimpleDERReader reader = new SimpleDERReader(vector); + assertEquals("2.5.29.14", reader.readOid()); + } } \ No newline at end of file -- cgit v1.2.3 From 065d92593af8bd43b47f016ebf6b4c1dd24fb769 Mon Sep 17 00:00:00 2001 From: Kenny Root Date: Sat, 26 Dec 2015 00:13:30 -0600 Subject: Add tests for ECDSASHA2Verify Since SimpleDERReader is now more tested and this class duplicates the functionality of that one, we will switch to using SimpleDERReader enstead. --- .../trilead/ssh2/signature/ECDSASHA2Verify.java | 80 ++-------------------- .../ssh2/signature/ECDSASHA2VerifyTest.java | 66 ++++++++++++++++++ 2 files changed, 71 insertions(+), 75 deletions(-) create mode 100644 sshlib/src/test/java/com/trilead/ssh2/signature/ECDSASHA2VerifyTest.java diff --git a/sshlib/src/main/java/com/trilead/ssh2/signature/ECDSASHA2Verify.java b/sshlib/src/main/java/com/trilead/ssh2/signature/ECDSASHA2Verify.java index f628ae7..89cacfb 100644 --- a/sshlib/src/main/java/com/trilead/ssh2/signature/ECDSASHA2Verify.java +++ b/sshlib/src/main/java/com/trilead/ssh2/signature/ECDSASHA2Verify.java @@ -24,6 +24,7 @@ import java.security.spec.KeySpec; import java.util.Map; import java.util.TreeMap; +import com.trilead.ssh2.crypto.SimpleDERReader; import com.trilead.ssh2.log.Logger; import com.trilead.ssh2.packets.TypesReader; import com.trilead.ssh2.packets.TypesWriter; @@ -267,24 +268,6 @@ public class ECDSASHA2Verify { } } - private static final int readLength(byte[] sig, int offset, int numOctets) throws IOException { - if (numOctets > 4 || numOctets <= 0) { - throw new IOException("Cannot decode DER length"); - } - - long length = 0L; - for (int i = 0; i < numOctets; i++) { - length <<= 8; - length |= sig[offset++]; - } - - if (length > 0xFFFFFFL || length < 0L) { - throw new IOException("Invalid DER length"); - } - - return (int) length; - } - public static byte[] encodeSSHECDSASignature(byte[] sig, ECParameterSpec params) throws IOException { TypesWriter tw = new TypesWriter(); @@ -299,64 +282,11 @@ public class ECDSASHA2Verify { * 0x02 */ - if (sig[0] != 0x30) { - throw new IOException("Invalid signature format"); - } - - final int seqHeaderLength; - final int seqLength; - if ((sig[1] & 0x80) != 0) { - int seqHeaderOctets = sig[1] & 0x7F; - seqHeaderLength = seqHeaderOctets + 1; - seqLength = readLength(sig, 2, seqHeaderOctets); - } else { - seqHeaderLength = 1; - seqLength = sig[1]; - } - - if ((seqLength == 0) || (1 + seqHeaderLength + seqLength != sig.length) || (sig[1 + seqHeaderLength] != 0x02)) { - throw new IOException("Invalid signature format"); - } - - final int rHeaderLength; - final int rLength; - if ((sig[1 + seqHeaderLength + 1] & 0x80) != 0) { - int rHeaderOctets = sig[seqHeaderLength + 2] & 0x7F; - rHeaderLength = rHeaderOctets + 1; - rLength = readLength(sig, seqHeaderLength + 3, rHeaderOctets); - } else { - rHeaderLength = 1; - rLength = sig[seqHeaderLength + 2]; - } - - if ((rLength == 0) || (rLength > seqLength - (rHeaderLength + 1 + 1 + 1)) || - sig[1 + seqHeaderLength + 1 + rHeaderLength + rLength] != 0x02) { - throw new IOException("Invalid signature format"); - } - - final int sHeaderLength; - final int sLength; - if ((sig[1 + seqHeaderLength + 1 + rHeaderLength + rLength + 1] & 0x80) != 0) { - int sHeaderOctets = sig[1 + seqHeaderLength + 1 + rHeaderLength + rLength + 1] & 0x7F; - sHeaderLength = sHeaderOctets + 1; - sLength = readLength(sig, 4 + rHeaderLength + rLength, sHeaderOctets); - } else { - sHeaderLength = 1; - sLength = sig[1 + seqHeaderLength + 1 + rHeaderLength + rLength + 1]; - } - - if ((sLength == 0) || 2 + rHeaderLength + rLength + sHeaderLength + sLength > seqLength) { - throw new IOException("Invalid signature format"); - } - - byte[] rArray = new byte[rLength]; - byte[] sArray = new byte[sLength]; - - System.arraycopy(sig, 1 + seqHeaderLength + 1, rArray, 0, rLength); - System.arraycopy(sig, 1 + seqHeaderLength + 1 + rLength + 1 + sHeaderLength, sArray, 0, sLength); + SimpleDERReader reader = new SimpleDERReader(sig); + SimpleDERReader seqReader = new SimpleDERReader(reader.readSequenceAsByteArray()); - BigInteger r = new BigInteger(1, rArray); - BigInteger s = new BigInteger(1, sArray); + BigInteger r = seqReader.readInt(); + BigInteger s = seqReader.readInt(); // Write the to its own types writer. TypesWriter rsWriter = new TypesWriter(); diff --git a/sshlib/src/test/java/com/trilead/ssh2/signature/ECDSASHA2VerifyTest.java b/sshlib/src/test/java/com/trilead/ssh2/signature/ECDSASHA2VerifyTest.java new file mode 100644 index 0000000..d7b9316 --- /dev/null +++ b/sshlib/src/test/java/com/trilead/ssh2/signature/ECDSASHA2VerifyTest.java @@ -0,0 +1,66 @@ +package com.trilead.ssh2.signature; + +import org.junit.Test; + +import static org.junit.Assert.*; + +/** + * Created by kenny on 12/25/15. + */ +public class ECDSASHA2VerifyTest { + private static final byte[] DER_ENCODED_P521_SIG = new byte[]{ + (byte) 0x30, (byte) 0x81, (byte) 0x88, (byte) 0x02, (byte) 0x42, (byte) 0x00, (byte) 0xFB, (byte) 0x41, (byte) 0xFD, (byte) 0xBD, (byte) 0x61, (byte) 0x5D, + (byte) 0xFE, (byte) 0x3F, (byte) 0x0C, (byte) 0xA1, (byte) 0xF0, (byte) 0x73, (byte) 0xF1, (byte) 0x18, (byte) 0xFB, (byte) 0x25, (byte) 0x57, (byte) 0xF4, + (byte) 0xDE, (byte) 0xF5, (byte) 0xC1, (byte) 0xAA, (byte) 0xB2, (byte) 0xA7, (byte) 0x2B, (byte) 0x9F, (byte) 0x81, (byte) 0xD1, (byte) 0x21, (byte) 0x9D, + (byte) 0x48, (byte) 0xC8, (byte) 0xC9, (byte) 0x31, (byte) 0xB9, (byte) 0x9B, (byte) 0xE5, (byte) 0x97, (byte) 0x94, (byte) 0x2F, (byte) 0xD5, (byte) 0x7E, + (byte) 0x0C, (byte) 0x32, (byte) 0x2D, (byte) 0xF9, (byte) 0x76, (byte) 0xC6, (byte) 0x33, (byte) 0x2C, (byte) 0x49, (byte) 0x1D, (byte) 0xDF, (byte) 0x51, + (byte) 0xA2, (byte) 0xD2, (byte) 0xB0, (byte) 0x72, (byte) 0x9B, (byte) 0x26, (byte) 0xC4, (byte) 0xB2, (byte) 0xA0, (byte) 0xF0, (byte) 0x7E, (byte) 0x02, + (byte) 0x42, (byte) 0x01, (byte) 0x56, (byte) 0x94, (byte) 0x9B, (byte) 0xAB, (byte) 0x00, (byte) 0x6D, (byte) 0x3C, (byte) 0x28, (byte) 0x34, (byte) 0x1B, + (byte) 0x00, (byte) 0xF3, (byte) 0xDF, (byte) 0xF7, (byte) 0x42, (byte) 0xAD, (byte) 0x8B, (byte) 0x20, (byte) 0x55, (byte) 0x2E, (byte) 0x80, (byte) 0x4F, + (byte) 0xDE, (byte) 0x0F, (byte) 0xBC, (byte) 0xE7, (byte) 0xE2, (byte) 0x7C, (byte) 0xF3, (byte) 0x3B, (byte) 0xFD, (byte) 0x95, (byte) 0xB0, (byte) 0xF7, + (byte) 0xD4, (byte) 0xE0, (byte) 0x63, (byte) 0xA9, (byte) 0x86, (byte) 0xA6, (byte) 0x49, (byte) 0xF4, (byte) 0x69, (byte) 0x66, (byte) 0x10, (byte) 0xD5, + (byte) 0x3F, (byte) 0xB6, (byte) 0x30, (byte) 0xDC, (byte) 0x01, (byte) 0x0E, (byte) 0xBE, (byte) 0xD1, (byte) 0x62, (byte) 0x86, (byte) 0x2B, (byte) 0xE4, + (byte) 0xF2, (byte) 0xF3, (byte) 0x6D, (byte) 0x4C, (byte) 0xE1, (byte) 0xD0, (byte) 0x5C + }; + + private static final byte[] SSH_ENCODED_P521_SIG = new byte[] { + (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x13, + (byte) 0x65, (byte) 0x63, (byte) 0x64, (byte) 0x73, (byte) 0x61, (byte) 0x2D, (byte) 0x73, (byte) 0x68, + (byte) 0x61, (byte) 0x32, (byte) 0x2D, (byte) 0x6E, (byte) 0x69, (byte) 0x73, (byte) 0x74, (byte) 0x70, + (byte) 0x35, (byte) 0x32, (byte) 0x31, + (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x8C, + (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x42, + (byte) 0x00, (byte) 0xFB, (byte) 0x41, (byte) 0xFD, (byte) 0xBD, (byte) 0x61, (byte) 0x5D, + (byte) 0xFE, (byte) 0x3F, (byte) 0x0C, (byte) 0xA1, (byte) 0xF0, (byte) 0x73, (byte) 0xF1, (byte) 0x18, + (byte) 0xFB, (byte) 0x25, (byte) 0x57, (byte) 0xF4, (byte) 0xDE, (byte) 0xF5, (byte) 0xC1, (byte) 0xAA, + (byte) 0xB2, (byte) 0xA7, (byte) 0x2B, (byte) 0x9F, (byte) 0x81, (byte) 0xD1, (byte) 0x21, (byte) 0x9D, + (byte) 0x48, (byte) 0xC8, (byte) 0xC9, (byte) 0x31, (byte) 0xB9, (byte) 0x9B, (byte) 0xE5, (byte) 0x97, + (byte) 0x94, (byte) 0x2F, (byte) 0xD5, (byte) 0x7E, (byte) 0x0C, (byte) 0x32, (byte) 0x2D, (byte) 0xF9, + (byte) 0x76, (byte) 0xC6, (byte) 0x33, (byte) 0x2C, (byte) 0x49, (byte) 0x1D, (byte) 0xDF, (byte) 0x51, + (byte) 0xA2, (byte) 0xD2, (byte) 0xB0, (byte) 0x72, (byte) 0x9B, (byte) 0x26, (byte) 0xC4, (byte) 0xB2, + (byte) 0xA0, (byte) 0xF0, (byte) 0x7E, + (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x42, + (byte) 0x01, (byte) 0x56, (byte) 0x94, (byte) 0x9B, (byte) 0xAB, (byte) 0x00, (byte) 0x6D, (byte) 0x3C, + (byte) 0x28, (byte) 0x34, (byte) 0x1B, (byte) 0x00, (byte) 0xF3, (byte) 0xDF, (byte) 0xF7, (byte) 0x42, + (byte) 0xAD, (byte) 0x8B, (byte) 0x20, (byte) 0x55, (byte) 0x2E, (byte) 0x80, (byte) 0x4F, (byte) 0xDE, + (byte) 0x0F, (byte) 0xBC, (byte) 0xE7, (byte) 0xE2, (byte) 0x7C, (byte) 0xF3, (byte) 0x3B, (byte) 0xFD, + (byte) 0x95, (byte) 0xB0, (byte) 0xF7, (byte) 0xD4, (byte) 0xE0, (byte) 0x63, (byte) 0xA9, (byte) 0x86, + (byte) 0xA6, (byte) 0x49, (byte) 0xF4, (byte) 0x69, (byte) 0x66, (byte) 0x10, (byte) 0xD5, (byte) 0x3F, + (byte) 0xB6, (byte) 0x30, (byte) 0xDC, (byte) 0x01, (byte) 0x0E, (byte) 0xBE, (byte) 0xD1, (byte) 0x62, + (byte) 0x86, (byte) 0x2B, (byte) 0xE4, (byte) 0xF2, (byte) 0xF3, (byte) 0x6D, (byte) 0x4C, (byte) 0xE1, + (byte) 0xD0, (byte) 0x5C + }; + + @Test + public void encodeSSHECDSASignature() throws Exception { + byte[] encoded = ECDSASHA2Verify.encodeSSHECDSASignature(DER_ENCODED_P521_SIG, + ECDSASHA2Verify.getCurveForSize(521)); + assertArrayEquals(SSH_ENCODED_P521_SIG, encoded); + } + + @Test + public void decodeSSHECDSASignature() throws Exception { + byte[] encoded = ECDSASHA2Verify.decodeSSHECDSASignature(SSH_ENCODED_P521_SIG); + assertArrayEquals(DER_ENCODED_P521_SIG, encoded); + } +} \ No newline at end of file -- cgit v1.2.3