-
Bug
-
Resolution: Unresolved
-
Major
The following testcase will fail with the current code base:
@Test public void testKeyWithLeadingZeros() { //When the message contains leading 0. Then the BigInteger class //will remove the leading zero since it has a function to do so. Curve25519Exchange curve25519Exchange = new Curve25519Exchange(); //public Diffie-Hellman key and other parameters in message. 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()); } }
If we make a change to use byte [] for BigInteger in PacketKexDHReply the above will not fail.
The RFC https://datatracker.ietf.org/doc/draft-ietf-curdle-ssh-curves/05/
Mentions this:
Public keys are defined as
strings of 32 bytes for Curve25519 and 56 bytes for Curve448.
Clients and servers MUST fail the key exchange if the length of the
received public keys are not the expected lengths, or if the derived
shared secret only consists of zero bits.
If we use the BigInteger it has a method to remove leading zeros.
public BigInteger(int signum, byte[] magnitude, int off, int len) { if (signum < -1 || signum > 1) { throw(new NumberFormatException("Invalid signum value")); } Objects.checkFromIndexSize(off, len, magnitude.length); // stripLeadingZeroBytes() returns a zero length array if len == 0 this.mag = stripLeadingZeroBytes(magnitude, off, len);
Then with this case the check for same length will fail but if we use byte [] it is ok.
