urbit / urbit

An operating function
https://urbit.org
MIT License
3.43k stars 358 forks source link

zuse: jet mismatch for X25519 Diffie-Hellman key exchange #7072

Open newxiphiness opened 1 month ago

newxiphiness commented 1 month ago

shar:ed has a jet mismatch.

> =my-shar =,  ed:crypto  |=  [pub=@ sek=@]
        ^-  @ux
        =+  exp=(shal (rsh [0 3] b) (suck sek))
        =.  exp  (dis exp (can 0 ~[[3 0] [251 (fil 0 251 1)]]))
        =.  exp  (con exp (lsh [3 31] 0b100.0000))
        =+  prv=(end 8 exp)
        =+  crv=(fra.fq (sum.fq 1 pub) (dif.fq 1 pub))
        (curt prv crv)
> (my-shar (shax 'foo') (shax 'bar'))
0x4fb0.74a6.7aba.56cb.f1be.1e2a.98a0.d5e5.fb8a.009e.1cf8.26f4.d640.b6a4.948c.a7bd
> (shar:ed:crypto (shax 'foo') (shax 'bar'))
0x7d79.c915.9a14.5af7.70fd.7ea9.4352.34fb.f30c.f65b.43b0.3e01.60fd.f8ab.5c39.1476

The issue is with the calculation of crv. Calculating the montgomery point's X value is "montgomeryX = (edwardsY + 1)*inverse(1 - edwardsY) mod p" as per https://github.com/orlp/ed25519/blob/master/src/key_exchange.c#L30C29-L30C86). So, we should calculate montgomeryX only with the y value of the public key point. Here's a fixed version

> =my-shar =,  ed:crypto  |=  [pub=@ sek=@]
        ^-  @ux
        =+  exp=(shal (rsh [0 3] b) (suck sek))
        =.  exp  (dis exp (can 0 ~[[3 0] [251 (fil 0 251 1)]]))
        =.  exp  (con exp (lsh [3 31] 0b100.0000))
        =+  prv=(end 8 exp)
        =.  pub  +:(need (deco pub))
        =+  crv=(fra.fq (sum.fq 1 pub) (dif.fq 1 pub))
        (curt prv crv)
> (my-shar (shax 'foo') (shax 'bar'))
0x7d79.c915.9a14.5af7.70fd.7ea9.4352.34fb.f30c.f65b.43b0.3e01.60fd.f8ab.5c39.1476

I'm not sure if the jet would crash if (deco pub) would fail, it should be noted. Moreover, there's some general things we don't check for, for example, one should try to generate a new key if the first 256 bits of the public key (after the bit shifts and bit setting) is divisible by l. Preserving deterministic key generation is still possible here, for example, by just incrementing the seed until a valid keypair is formed. I believe there's a few other checks that may be done to ensure a secure key, but this would be a larger change and would only impact newly keyed azimuth points, so can be delayed for now.

newxiphiness commented 1 month ago

also noting that our hoon implementation for private key computation of ed25519, here, a

      =+  h=(shal (rsh [0 3] b) sk)
      =+  ^=  a
          %+  add
            (bex (sub b 2))
          (lsh [0 3] (cut 0 [3 (sub b 5)] h))

differs from the code used here, where the private key is prv

        =+  exp=(shal (rsh [0 3] b) (suck sek))
        =.  exp  (dis exp (can 0 ~[[3 0] [251 (fil 0 251 1)]]))
        =.  exp  (con exp (lsh [3 31] 0b100.0000))
        =+  prv=(end 8 exp)

For good measure should use the same code, which in C, as per https://github.com/orlp/ed25519/blob/master/src/keypair.c, is

    sha512(seed, 32, private_key);
    private_key[0] &= 248;
    private_key[31] &= 63;
    private_key[31] |= 64;