Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve error message when using ED25519 with hashedrekord #851

Closed
haydentherapper opened this issue May 31, 2022 · 3 comments · Fixed by #862
Closed

Improve error message when using ED25519 with hashedrekord #851

haydentherapper opened this issue May 31, 2022 · 3 comments · Fixed by #862
Labels
enhancement New feature or request

Comments

@haydentherapper
Copy link
Contributor

Description

ED25519 signatures are not supported with the hashedrekord type, though they are supported with rekord. The reason is that ED25519 computes the digest as part of its algorithm, so the original artifact is needed to verify a signature.

The hashedrekord type passes no message contents to Verify (code) and subsequently to VerifySignature. For ED25519 verification, the digest from options.WithDigest is ignored (code) intentionally, because ED25519 handles the calculation of the digest. When ComputeDigestForVerifying is called, no digest is present nor is a message present, so it'll hit this error condition, message cannot be nil, getting passed back up to the caller.

This is an unclear error message. We should proactively check when an ED25519 key or key in certificate is provided with a hashedrekord type, and return an actionable error.

@haydentherapper haydentherapper added the enhancement New feature or request label May 31, 2022
@znewman01
Copy link

There are variants of EdDSA that support prehashing ("HashEdDSA" vs. "PureEdDSA", see sec. 4 of 8032) should we consider supporting ed25519ph here?

@haydentherapper
Copy link
Contributor Author

ED25519's internal hash is SHA512, right now Rekor only supports SHA256 so we'd have to make changes around that.

There's active (very active, as in a week ago) conversation on supporting ed25519ph in golang - golang/go#31804

I also don't know if we open ourselves up to any risks with using a prehashed algorithm.

@znewman01
Copy link

znewman01 commented Jun 1, 2022

I also don't know if we open ourselves up to any risks with using a prehashed algorithm.

The main one is:

[Pure] EdDSA is secure even if it is feasible to compute
collisions for the hash function

whereas HashEdDSA is not. I'm not terribly convinced this is something to worry about (and for our current signature algorithms, this is already be an issue).


We could also just sign the hash itself instead of the artifact. This is a little gross because we'd need to special-case this codepath, but the only possible security issue is if there'd ever be ambiguity between ed25519(sha256(m)) and ed25519(m') where m' = sha256(m). I can't imagine such a case.

Edit: top answer on this crypto.stackexchange post is a good explanation.


That said, "good error message on ed25519" is way better than the status quo so let's definitely start there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants