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

Sign over the algorithm choice #12

Open
QuinnWilton opened this issue Oct 3, 2023 · 4 comments
Open

Sign over the algorithm choice #12

QuinnWilton opened this issue Oct 3, 2023 · 4 comments

Comments

@QuinnWilton
Copy link

I don't believe the current spec actually signs over the algorithm choice at all, potentially opening signatures up to algorithm confusion attacks, where an attacker modifies the unsigned header to weaken or disable the security offered by the signature.

The solution here is probably some variant of prepending the header to the body prior to signing.

@expede
Copy link
Collaborator

expede commented Oct 3, 2023

[Quinn and I have spoken about this out of band, so none of the below is opposition to the point raised. I'm very much on board with attempting to fix this.]

It's likely super tricky to enforce this system-wide in the Varsig spec directly. Many other specs include some other header but expect no other bytes in the signature, or they expect to be signed with no additional bytes at all and a strictly incrementing nonce. For instance, signing an Ethereum message with EIP-191 prepends the following to the message:

0x19 <0x45 (E)> <thereum Signed Message:\n" + len(message)> <data to sign>

This has a signature type captured in it, but it's not the Varsig identifier. This case uses a single, specific signature algorithm, and is probably okay for this use case (well, excpet that it doens't include the chain ID, which is a whole other domain separation issue).

This is all to say that I'm unsure that we can enforce this at the level of this spec for all cases. However, we can definitely RECOMMEND that this header be put inside the payload, and even suggest some formats for that. Perhaps implementations should have a strict mode that requires this hypothetical format but with a list of exceptions for known cases like EIP-191 above.

@chunningham
Copy link

This is a valid concern, but including sig type info in the signed bytes in a generalised way would preclude representing signatures over arbitrary payloads as varsigs, which seems to be the stated goal of the spec. I think I agree with recommending inclusion of signature type in payloads without requiring it. In many cases, existing payloads will include this information already (e.g. identifying the signer via a DID will involve resolving the relevant key material upon verification, said key material would then identify the relevant signature type (specific example being did:key)).

@expede
Copy link
Collaborator

expede commented Oct 7, 2023

In many cases, existing payloads will include this information already

Right, so then the question becomes: why track this information in Varsig if it's also in the signature body? (In those cases)

@dhuseby
Copy link

dhuseby commented Oct 8, 2023

This seems like an implementation detail. I see varsig as a self-describing detached signature over an arbitrary set of bytes. If you need to have the extra added security of covering the algorithm choice in the signature then your application should do that.

In theory, varsig already has signature-type-dependent behavior so why not define something like a "signature identifier" blog that is defined for each signature type. The spec can say, "always prepend the signature identifier with the bytes to be signed" and then have a definition of what goes into the signature identifier for each signature type. If it's just the algorithm identifier, great. If it's the algorithm identifier and nonce, great. The only limit is that the data in the signature identifier must be in the varsig data so that given the data and the varsig, a verifier can recreate the signed data by pulling the signature identifier data from the varsig, prepending it with the bytes, and then use that to verify the signature.

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

No branches or pull requests

4 participants