Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-72715

Using a byte [] containing leading zeros in PacketKexDHReply will remove leading zeros that make crypto to fail

XMLWordPrintable

      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.

            Unassigned Unassigned
            eraonel eraonel
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Created:
              Updated:
              Resolved: