advisory: Double Public Key Signing Function Oracle Attack on Ed25519

[blackheaven]

---

Advisory

• It's not duplicated
• All fields are filled
• It is validated by hsec-tools

Discussed here: awakesecurity/gen-ed25-keypair#8 (comment)

[TristanCacqueray]

Thinking about it some more, I wonder if the affected libraries can/will fix this issue. Perhaps this should only concern users who don't validate that the public key is valid, like the gen-ed25-keypair tool. I guess it may be acceptable for crypton* to expose dangerous functions when the caller is responsible of using them correctly.

[blackheaven]

Thinking about it some more, I wonder if the affected libraries can/will fix this issue. Perhaps this should only concern users who don't validate that the public key is valid, like the gen-ed25-keypair tool. I guess it may be acceptable for crypton* to expose dangerous functions when the caller is responsible of using them correctly.

I'm not sure we can actually prevent it, but, at least, it should be part of the documentation of exposed functions.

[frasertweedale]

If we issue the advisory, please also include this article in the references:

• https://github.com/MystenLabs/ed25519-unsafe-libs

I am unsure whether issuing an advisory against the cryptographic implementations themselves is the correct approach.
The functions are not vulnerable per se. Instead, the API is susceptible to misuse / error by consuming libraries. It is certainly the case that documentation of these functions should be improved. Here are my thoughts / observations:

• Any identified libraries / programs that use the affected API(s) in an insecure way should have advisories (we can group them all under HSEC-2025-0002).
• The Ed448 implementation also seems to have been vulnerable up until crypton-1.0.1, where the cbits got an update and they now unconditionally rederive the public key. See commit kazu-yamamoto/crypton@b6bac16 . We could include this info in the advisory too, or issue a separate historical advisory for Ed448.
• Before merging the advisory we should liaise with crypton maintainers to see what they want to do. Given Ed448 implementation rederives the public key, perhaps they can apply the same approach for Ed25519. Alternatively, they could extend the API with safer variant(s) (suggestions below), and/or improve the documentation.

-- | Derives public key from secret key
signSafe :: ByteArrayAccess ba => SecretKey -> ba -> Signature

-- | Checks that public key corresponds to secret key
signChecked :: ByteArrayAccess ba => SecretKey -> PublicKey -> ba -> CryptoFailable Signature

Ping @kazu-yamamoto - what do you think?

[frasertweedale]

I think we should drop the historical tag here, because the issue is in current releases.

[blackheaven]

Thanks, I've dropped it

[kazu-yamamoto]

Thank you for informing this issue. Either approach (1) and approach (2) is fine with me.

1. Keep the backward compatibility: keep sign as is but add the deprecated pragma, add new functions and release a minor version.
2. Make the libary completely safe: change the definition of safe and release a major version.

I don't have a strong opinion on this but it seems to me that approach (2) seems better in the long run.

[kazu-yamamoto]

Or what about changing the currentsign to ignore the parameter of public key?

[frasertweedale]

Or what about changing the currentsign to ignore the parameter of public key?

@kazu-yamamoto yes, if you ignore the argument and re-derive the public key internally, that is the safest way. The downside is that it causes a constant overhead for every signing operation (this is why the "optimisation" exists in the first place). I think it's acceptable to change sign to ignore the public key argument, and it is a backwards-compatible change.

It might be worth considering exposing the existing behaviour as a functional named, say, signUnsafe, with appropriate warnings, for performance-critical use cases.

[kazu-yamamoto]

Please give a look at: kazu-yamamoto/crypton#47

If OK, I will change Ed448, too.

[frasertweedale]

@kazu-yamamoto thanks! I left some comments in the PR.

[blackheaven]

Added the fixed release for crypton

[frasertweedale]

I added a couple additional references, and updated the fixed field to v1.0.3 (1.0.4 just fixed a minor performance regression).

Thanks for preparing the advisory. Merging now.