w3c / vc-di-ecdsa

Data Integrity specification for ECDSA using NIST-compliant curves
https://w3c.github.io/vc-di-ecdsa/
Other
8 stars 9 forks source link

Test vectors for ECDSA #13

Closed Wind4Greg closed 1 year ago

Wind4Greg commented 1 year ago

Produced test vectors for:

See notes below on ReSpec and test vectors.

Including Test Vectors in W3C Specifications

This specification includes procedures for cryptographic processing of content. To aid developers in implementing specifications of this type. We provide test vectors illustrating this processing. These test vectors also demonstrate intermediate processing steps as well as initial inputs and outputs. This specification is written in HTML along with the ReSpec library. The raw test vector data is text based, e.g., JSON, N-quads, etc... and generally requires significant "HTML escaping" if embedded directly into the document.

ReSpec has a data-include directive that can be used to include separate files into a specification when it is viewed. Such files must be kept along with the index.html document for the specification. We have currently chosen to put the test vectors in the directory TestVectors. As an example, to show the unsigned document we use the following HTML/ReSpec:

<pre class="example" title="Credential without Proof" data-include="TestVectors/unsigned.json" 
data-include-format="text"></pre>

ReSpec will pull in the content from the referenced file when the index.html document is viewed. Note that when viewing the index.html locally you will need to run a local webserver, e.g., serve, since the ReSpec library needs a server to deliver the referenced files. Hence you cannot simply "double click" the index.html file to preview the document.

Note that the test vector files are generated by programs outside this repository that themselves depend on other libraries and programming languages, hence we probably don't want to include those programs in this repo and hence should just revision control the test vector files.

What's Important to Review

Everything of course but some things stem from others... In out case the two main inputs to test vector generation are TestVectors/unsigned.json and TestVectors/keyPair.json. The explanatory text is all in updates to index.html and a informative section towards the end of the document. Note we have three different processing cases which leads to three similar sets of test vectors and their descriptions. Hence consider if you want changes to index.html concerning the test vector to apply to all cases.


Preview | Diff

OR13 commented 1 year ago

seems like the naming convention is wrong here...

Prefer to see:

urdna2015-ecdsa-2019 with curve P-256
urdna2015-ecdsa-2019 with curve P-384
jcs-ecdsa-2019 with curve P-256
jcs-ecdsa-2019 with curve P-384

technically jcs is already a dependency of urdna2015, so the jcs suites have strictly speaking less dependencies, not more... but the naming convention confuses this issue.

Wind4Greg commented 1 year ago

@OR13 Are you talking about changing the section titles for the test vectors or the cipher suite names?

Changing a test vector section title can be done but changing a ciphersuite name is outside the scope of this PR. This is just creating test vectors to go with the existing ciphersuites and the existing names.

Changing the ciphersuite name requires regenerating all "down stream" test vectors. Please reach consensus with other authors/reviewers before asking me to regenerate the test vectors.

OR13 commented 1 year ago

The names for suites are inconsistent, if you prefer to file an issue marker, and address later that seems fine.

The problem is the current names don't communicate accurately what the suites are doing.

One suite uses jcs, the other uses urdna2015 (and jcs for cases where @type is @json) ...

If we are going to name one suite jcs why are we not naming the other urdna2015?...

Developers will be confused, it also appears to bias towards urdna2015 but in reality that suite uses jcs under the hood and is more complicated and less safe.

It is important to communicate clearly about Canonicalization since the approaches are very different, jcs canonicalizes the same media type ( JSON )... Urdna2015 converts JSON-LD to application/n-quads... It does not canonicalize JSON-LD, it transforms it to another media type....

It's misleading to use jcs in one name and not urdna2015 in the other.

If you add an issue marker, I'm happy to approve and follow up on an issue, or we could do a special topic call.

Wind4Greg commented 1 year ago

How do we add an issue marker? That sounds best. We get some test vectors to use then can get consensus on the names before I regenerate the test vectors. Consistent naming is very nice for developers and users.

OR13 commented 1 year ago

You can add a p tag with class issue and a data-number that points to GitHub issue nunber associated with the current repo. If you search for respec you can find the exact syntax.

I would put the marker near the place where the crypto suite name is defined.

Wind4Greg commented 1 year ago

@OR13 Are you going to open an issue on "cryptosuite naming" for ECDSA? There isn't one for me to reference yet.

OR13 commented 1 year ago

Can you file the issue, when you open the marker? You can reference this comment thread.

Wind4Greg commented 1 year ago

The problem is I'm fine with the current cryptosuite names and just trying to furnish test vectors. If you have an issue with the cryptosuite names you should file an issue or make a PR. Right? I'm more used to IETF procedures...

msporny commented 1 year ago

Can you file the issue, when you open the marker? You can reference this comment thread.

This issue is being tracked here: https://github.com/w3c/vc-data-integrity/issues/38 and an issue marker was added in https://github.com/w3c/vc-di-ecdsa/pull/13/commits/42896d71b007eb9f4d52eebe6ea39179d8b8111f

msporny commented 1 year ago

Editorial, multiple reviews, changes requested and made (issue markers added to track ongoing concerns), no objections, merging.