Lock x25519-dalek to 1.1

rubdos commented 2 years ago

Curve25519 is currently replaced with the lizard2 branch, based on curve25519-dalek 3.0.0, but x25519-dalek 1.2 requires a higher version.

Specify <1.2 such that cargo update can be freely run.

FWIW, the error I get with 1.2:

error[E0599]: the method `is_identity` exists for struct `MontgomeryPoint`, but its trait bounds were not satisfied
   --> /home/rsmet/.cargo/registry/src/github.com-1ecc6299db9ec823/x25519-dalek-1.2.0/src/x25519.rs:265:17
    |
265 |         !self.0.is_identity()
    |                 ^^^^^^^^^^^ method cannot be called on `MontgomeryPoint` due to unsatisfied trait bounds
    |
   ::: /home/rsmet/.cargo/git/checkouts/curve25519-dalek-72abe9b51f92c496/b3329ed/src/montgomery.rs:70:1
    |
70  | pub struct MontgomeryPoint(pub [u8; 32]);
    | -----------------------------------------
    | |
    | doesn't satisfy `MontgomeryPoint: Identity`
    | doesn't satisfy `MontgomeryPoint: IsIdentity`
    |
    = note: the following trait bounds were not satisfied:
            `MontgomeryPoint: Identity`
            which is required by `MontgomeryPoint: IsIdentity`

This is merely a workaround; it would be beneficial to merge upstream curve25519-dalek into the lizard branch.

jrose-signal commented 2 years ago

Ooh, that's annoying. I would have liked to specify this requirement in the crate that actually depends on the fork, which is zkgroup. But zkgroup doesn't depend on x25519-dalek, and the other two crates don't depend on the fork.

I don't think cargo update is enough of a reason to add this, since it's an artificial limitation for the other crates. You're right about the real answer, too; we mostly haven't done this because we aren't using anything past curve25519-dalek 3.0.0 (and there haven't been any security issues there or in x25519-dalek), but we should just update.

Is there an outside-workspace use case for this? Someone depending on the zkgroup and libsignal-protocol packages together, maybe?

rubdos commented 2 years ago

Someone depending on the zkgroup and libsignal-protocol packages together, maybe?

Yes, that's right. In Whisperfish, we depend on zkgroup and libsignal-protocol, so cargo update in our crate messes up the compilation over here.

Should I attempt to update the lizard2 branch, maybe?

jrose-signal commented 2 years ago

That'd be appreciated as an alternative, sure.

rubdos commented 2 years ago

See https://github.com/signalapp/curve25519-dalek/pull/3 for a first attempt at that merger.

rubdos commented 2 years ago

Superseded by https://github.com/signalapp/curve25519-dalek/pull/4

signalapp / libsignal

Lock x25519-dalek to 1.1 #460