Skip to content

Conversation

@PatStLouis
Copy link
Contributor

As discussed in our last meeting, here is the ecdsa-jcs-2019 algorithm file.

@PatStLouis PatStLouis marked this pull request as ready for review November 17, 2024 18:20
@PatStLouis PatStLouis requested review from BigBlueHat, aljones15 and gannan08 and removed request for BigBlueHat November 17, 2024 18:20
@PatStLouis PatStLouis changed the title Ecdsa jcs 2019 algorithms ecdsa-jcs-2019 algorithms Nov 17, 2024
Copy link
Member

@BigBlueHat BigBlueHat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @PatStLouis

@BigBlueHat BigBlueHat removed the request for review from aljones15 November 18, 2024 16:11
Copy link
Collaborator

@gannan08 gannan08 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1. I don't see where we test this Create Proof

If unsecuredDocument.@context is present, set proof.@context to unsecuredDocument.@context.

During create proof, a compliant cryptosuite should be adding a context to the proof.

2. Nor this Verify Proof

If proofOptions.@context exists:
Check that the securedDocument.@context starts with all values contained in the proofOptions.@context in the same order. Otherwise, set verified to false and skip to the last step.
Set unsecuredDocument.@context equal to proofOptions.@context.

During verify proof, a compliant cryptosuite should be matching the context on the secured document with the context on the proof if proofOptions.@context exists.

@PatStLouis
Copy link
Contributor Author

@gannan08 thanks for the review, I can add two tests for the highlighted sections, however I want to point out that they don't contain normative statements.

In the Data Model for proof representation:

The type property MUST be DataIntegrityProof.

The cryptosuite property MUST be ecdsa-rdfc-2019, ecdsa-jcs-2019, or ecdsa-sd-2023.

The value of the proofValue property is produced according to the cryptosuite type and is specified in either Section 3.2.1 Create Proof (ecdsa-rdfc-2019), or Section 3.3.1 Create Proof (ecdsa-jcs-2019), or Section 3.6.1 Create Base Proof (ecdsa-sd-2023), or Section 3.6.6 Add Derived Proof (ecdsa-sd-2023).

The later doesn't have any normative statements. Would you suggest to change the text to:

The value of the proofValue property MUST be produced according to ...

I'm trying to be pragmatic about limiting test cases for normative statements only. Happy to make an exception if there's a specific statement we need tested which isn't normative.

@PatStLouis
Copy link
Contributor Author

I also want to point out that the vectors for jcs doesn't have context in the proof:

Implementations I've seen also omit this. I will raise an issue on the spec about this.

@PatStLouis
Copy link
Contributor Author

Discussed this further, I will add missing tests.

@PatStLouis PatStLouis force-pushed the ecdsa-jcs-2019-algorithms branch from b952576 to 60ed0b6 Compare November 20, 2024 16:16
@PatStLouis PatStLouis merged commit bcef380 into main Nov 20, 2024
2 checks passed
@PatStLouis PatStLouis deleted the ecdsa-jcs-2019-algorithms branch November 20, 2024 16:17
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

Successfully merging this pull request may close these issues.

4 participants