w3c / vc-di-ecdsa-test-suite

Interoperability Test Suite for Data Integrity Ecdsa Signatures.
https://w3c.github.io/vc-di-ecdsa-test-suite/
BSD 3-Clause "New" or "Revised" License
5 stars 6 forks source link

Incorrect hashing used in tests for `ecdsa-rdfc-2019` `P-384` #86

Open sbihel opened 1 month ago

sbihel commented 1 month ago

Hiya, the following tests are failing for P-384 with our implementation:

After trying some random things, I noticed that if we used SHA-256 for the hash data instead of SHA-384 for P-384 keys, then all the tests would pass. So I'm assuming that the implementation used by the tests is incorrect, but maybe our interpretation of the specs is wrong?

aljones15 commented 1 month ago

Your interpretation is probably correct:

This specification uses either the RDF Dataset Canonicalization Algorithm [RDF-CANON] or the JSON Canonicalization Scheme [RFC8785] to transform the input document into its canonical form. It uses one of two mechanisms to digest and sign: SHA-256 [RFC6234] as the message digest algorithm and ECDSA with Curve P-256 as the signature algorithm, or SHA-384 [RFC6234] as the message digest algorithm and ECDSA with Curve P-384 as the signature algorithm.

I'll look into this when I get a chance, but I think you're correct here and the problem is either that our verifier is incorrect or the implementation is incorrect. the implementation was use locally is DB's: https://github.com/digitalbazaar/ecdsa-rdfc-2019-cryptosuite/tree/main/lib

Thanks for the bug report.

aljones15 commented 1 month ago

@sbihel if you can supply a VC signed with your implementation's p-384 key, I can run it through our stuff locally and debug a bit faster.

sbihel commented 1 month ago

@sbihel if you can supply a VC signed with your implementation's p-384 key, I can run it through our stuff locally and debug a bit faster.

Oh yes of course, I forgot. Here is one:

{
  "@context": [
    "https://www.w3.org/ns/credentials/v2"
  ],
  "id": "urn:uuid:7a6cafb9-11c3-41a8-98d8-8b5a45c2548f",
  "type": [
    "VerifiableCredential"
  ],
  "credentialSubject": {
    "id": "did:key:z6MkhTNL7i2etLerDK8Acz5t528giE5KA4p75T6ka1E1D74r"
  },
  "issuer": "did:key:z82LkvutaARmY8poLhUnMCAhFbts88q4yDBmkqwRFYbxpFvmE1nbGUGLKf9fD66LGUbXDce",
  "proof": {
    "type": "DataIntegrityProof",
    "cryptosuite": "ecdsa-rdfc-2019",
    "created": "2024-07-04T13:52:02.458Z",
    "verificationMethod": "did:key:z82LkvutaARmY8poLhUnMCAhFbts88q4yDBmkqwRFYbxpFvmE1nbGUGLKf9fD66LGUbXDce#z82LkvutaARmY8poLhUnMCAhFbts88q4yDBmkqwRFYbxpFvmE1nbGUGLKf9fD66LGUbXDce",
    "proofPurpose": "assertionMethod",
    "proofValue": "z2PiWogU4CmVnUDLNKtUrT2cygxw8JWC4CFA7EzuSG1m5fa3xHsJtUNKQSDWXfdtwB2At3n3ew8RjXEgCNPt58F4jH5EveJ2YmM8sbq1ND8qX66Zcw4i7HdetygcSSi3mmmn2"
  }
}
aljones15 commented 1 month ago

Just so this known: here is the ecdsa-rdfc-2019 verifier code: https://github.com/digitalbazaar/ecdsa-multikey/blob/41c5a490a7acbf0fc11d37789768c27cdc1b73f3/lib/factory.js#L29-L50

And when it sees a P-384 it should be using the correct hash, so the issue might be that our implementation's publicKey algorithm is finding the wrong curve.

Interestingly the test project for ecdsa multikey does not actually have a test to ensure that createVerifier works with P-384 so I'll start with adding that: https://github.com/digitalbazaar/ecdsa-multikey/blob/41c5a490a7acbf0fc11d37789768c27cdc1b73f3/test/EcdsaMultikey.spec.js#L29

WRT to your key:

> bs58.decode('82LkvutaARmY8poLhUnMCAhFbts88q4yDBmkqwRFYbxpFvmE1nbGUGLKf9fD66LGUbXDce')
Uint8Array(51) [
  129,  36,   3,  27,  79,  78,  10, 193, 231, 161,
  233, 251,  33, 105, 192, 244,  68, 196,  42,  94,
  205, 152, 252,  26, 146, 145, 232, 244, 163, 135,
  229, 181,   1, 218, 114, 115, 202, 126,  89, 114,
  143, 170, 168,  23,  33, 169,  72, 181, 250, 230,
  179
]
// p384 public key header
> new Uint8Array([0x81, 0x24]);
Uint8Array(2) [ 129, 36 ]

so the verificaitonMethod you gave me does seem to match the P-384 header which should in turn fetch the correct hash.

aljones15 commented 1 month ago

@sbihel just so you know a PR was merged to improve the test suite of ecdsa-multikey on our side. That pr does not show any regressions such as incorrect hashing, so that probably means the bug is in the test suite or your implementation. Taken that the prefix of your did key shows the correct header for P-384 I'm not sure on where the bug is coming from. the primary reason for this message is just to let you know that all of the current test developers are in a sprint that won't be over for a few weeks and no one has time to look at this bug right now so if you have a moment to track it down, please do.

You can check the test project here: https://github.com/digitalbazaar/ecdsa-multikey Which in turn will depend on: https://github.com/digitalbazaar/ecdsa-rdfc-2019-cryptosuite which in turn is used by: https://github.com/digitalbazaar/vc/tree/update-vc-2.0 which is then used by: https://github.com/w3c/vc-di-ecdsa-test-suite/blob/main/tests/vc-verifier/index.js

aljones15 commented 4 days ago

@sbihel a patch release was made to digital bazaar's ecdsa-rdfc-2019 libraries WRT to the hash used for the P-384 keys: https://github.com/digitalbazaar/ecdsa-rdfc-2019-cryptosuite/blob/main/CHANGELOG.md

This might be relevant to your issue.