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

ecdsa-jcs-2019 Signature encoding / verification #1

Closed
stevenvegt opened this issue Aug 15, 2024 · 7 comments
Closed

ecdsa-jcs-2019 Signature encoding / verification #1

stevenvegt opened this issue Aug 15, 2024 · 7 comments

Comments

@stevenvegt
Copy link

Hi 👋

I'm implementing the Trust DID Web specification in Go and using the output of this library to verify my implementation. I'm unable to verify signatures of the ecdsa-jcs-2019 cryptosuite. Since all the other data (hashed data and public key) are equal between our libs, I suspect the perhaps encoding of the signature.

This is how the VC Proof specification of the crypto suite specifies the proofBytes:

Let proofBytes be the result of applying the Elliptic Curve Digital Signature Algorithm (ECDSA) [FIPS-186-5], with hashData as the data to be signed using the private key specified by privateKeyBytes. proofBytes will be exactly 64 bytes in size for a P-256 key, and 96 bytes in size for a P-384 key.

And indeed, after the multibase decoding of the signature, the resultingproofBytes, has a length of 96. But if I check the FIPS 186 specification on section 6.4.1, I see the signature is a coordinate on the curve, consisting of 2 components,r and s. How are these encoded into the 96 byte array?

The Go library has two Verify methods, one accepting an ASN.1 encoded structure, the other accepting the r and s coordinates.
The proofBytes is not ASN.1 encoded (it would be longer than 96 bytes), so I somehow need to 'decode' it into the correct parts.

The Rust lib used by this library suggest that they are just concatenated, however splitting it equally like this does not seem to work.

// split the signature in half to get the s and r values
r := big.NewInt(0).SetBytes(signature[:len(signature)/2])
s := big.NewInt(0).SetBytes(signature[len(signature)/2:])

if !ecdsa.Verify(pubKey.(*ecdsa.PublicKey), input, r, s) {
  return fmt.Errorf("failed to verify proof")
}

Do you perhaps have any pointers?

Thanks!

@andrewwhitehead
Copy link
Contributor

I'm not seeing any obvious issues with the signature decoding, it looks like SetBytes expects big-endian format and that is the expected format. Maybe it's an issue with the public key encoding. In any case, P-384 isn't actually supported in the current specification (although this implementation still allows it) so you might just want to focus on Ed25519 support.

@swcurran
Copy link

And to be clear — the DIDDoc that is published using did:tdw can support any key types needed by the DID Controller. It’s the signature in the DI Proof on the DID Log Entry that is currently only supporting the ecdsa-jcs-2019 cryptosuite and signatures.

@andrewwhitehead — I don’t think there is anything in the spec that says anything about limiting the public key types to Ed25519. It just says that ecdsa-jcs-2019 is the only cryptosuite used. Do we need to be more precise in the specification about what to use, if we aren’t supporting everything supported by ecdsa-jcs-2019? Or am I not understanding your comment?

@andrewwhitehead
Copy link
Contributor

andrewwhitehead commented Aug 15, 2024

Hm, I thought we had resolved to just use Ed25519 for now. Currently the cryptosuite parameter definition says that only eddsa-jcs-2022 is allowed, which only supports Ed25519 keys (I'm a little surprised Ed448 isn't an option there). All the example proofs in the spec currently use ecdsa-jcs-2019 which seems incorrect.

@swcurran
Copy link

Hmm…I got the sample proofs from the Python implementation — copy paste. Checked — the spec says to use eddsa-jcs-2022, but the examples use ecdsa-jcs-2019 — in the spec. and in the trustdidweb-py repo. I also looked up in the eddsa-jcs-2022 section of the vc-di-eddsa spec and confirmed that only Ed25519 keys are supported.

@andrewwhitehead — can you update the Python implementation and examples, and I’ll update the spec?

@stevenvegt — does that answer your question?

@stevenvegt
Copy link
Author

does that answer your question?

Yes, mostly. My eddsa-jcs-2022 implementation works fine with logs generated by both mine and the Python implementation, so I guess that confirms at least some part of my implementation is correct (or we both made the same mistake) 😅 .

I think bcgov/trustdidweb#87 can help with creating an universal test and validation suite, so we don't rely on one implementation.

It still bothers me a little bit that somewhere something is broken (either my lib or the python one), but for now that has become irrelevant.

Thank you both for the quick responses 👍 I will publish and open source the lib soon. If it is up to your standards feel free to add it to the list with implementations.

@swcurran
Copy link

Great stuff. Do you have feedback to provide on the spec? Suggestions of changes, clarifications in the spec, etc.? We will have a working group starting very soon to maintain the spec, but for now we have a Signal group of collaborators if you want to join.

@andrewwhitehead
Copy link
Contributor

The latest sample document does have the correct cryptosuite in the DI proofs, so I think this can be closed.

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

3 participants