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

Update selective disclosure functions and their use in ecdsa-sd sign, derive, verify. #27

Merged
merged 21 commits into from
Aug 22, 2023

Conversation

dlongley
Copy link
Contributor

@dlongley dlongley commented Aug 15, 2023

This PR updates the underlying selective disclosure functions to remove the need to use the JSON-LD framing primitive and to remove the special treatment of credentialSubject for VCs. It accomplishes this by using JSON pointers for selection (filtering and grouping) and skolemization of expanded (and subsequently compacted) JSON-LD documents instead of direct skolemization of N-Quads. Grouping and filter functions have been removed and replaced with a single canonicalizeAndGroup function, and the previous jsonPointersToFrame function has been adjusted to selectJsonLd as it now performs selection directly on skolemized, compact JSON-LD documents instead of producing a frame as an intermediate step.

The formats / expression of proof data in ecdsa-sd is unchanged, only the internals used to implement it are updated.

A few other corrections were made around label factory function production.

Some editorial clean up still needs to be done around the use of <var>, <em> and quoting variables using backticks, but this is not a new issue.


Preview | Diff

Copy link
Collaborator

@Wind4Greg Wind4Greg left a comment

Choose a reason for hiding this comment

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

This is a nice improvement to the internal selective disclosure processing (both mandatory reveal, and selective reveal) to remove the dependency on JSON-LD framing. Key functionality is encapsulated in the canonicalizeAndGroup function.

It should be noted that besides the ECDSA algorithms from FIPS-186-5 (2023) that the EdDSA algorithms (Ed25519, Ed448) can be used. In addition the SD primitives particularly canonicalizeAndGroup maybe applicable for applying BBS to VCs.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
@dlongley
Copy link
Contributor Author

dlongley commented Aug 15, 2023

@TallTed,

Thanks heaps! So I applied all of your suggestions except for the (5) ones that changed the use of "and" when talking about passing parameters. Those suggest using "with" instead and I'm not sure that "with" is better?

Without that suggestion we have examples like this:

Use algorithm X, passing foo as A and B.
Use algorithm X, passing foo as A, bar as B, and C.

With that suggestion it would read like this:

Use algorithm X, passing foo as A with B.
Use algorithm X, passing foo as A, bar as B, with C.

I'm not sure what the consensus is, but "and" reads better to me. I can how the use of "with" reads alright when C is for example "any custom options":

Use algorithm X, passing foo as A, bar as B, and any custom options.
vs.
Use algorithm X, passing foo as A, bar as B, with any custom options.

But "and" still reads better to me personally and is consistent. Thoughts?

@dlongley dlongley requested a review from TallTed August 15, 2023 21:27
@TallTed
Copy link
Member

TallTed commented Aug 15, 2023

Maybe...

Use algorithm X, passing A, and foo as B.
Use algorithm X, passing A, foo as B, and bar as C.
Use algorithm X, passing A, foo as B, and bar as C, along with any custom options.

@dlongley
Copy link
Contributor Author

dlongley commented Aug 15, 2023

@TallTed,

Maybe...

Use algorithm X, passing A, and foo as B.
Use algorithm X, passing A, foo as B, and bar as C.
Use algorithm X, passing A, foo as B, and bar as C, along with any custom options.

I'd be fine with that, but I'm also fine to just leave it as always using "and". If you want to adjust your suggestions accordingly, I'll add them in -- or you can approve as-is. Just let me know.

Copy link
Member

@TallTed TallTed left a comment

Choose a reason for hiding this comment

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

Revised the passing ... and vs passing ... with

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
@dlongley
Copy link
Contributor Author

@TallTed,

Thanks again! I've applied all your suggestions. Two of them don't actually change the text anymore so the UI doesn't allow me to apply them, but ... they don't change the text so there's nothing to do anyway.

@dlongley dlongley requested a review from TallTed August 16, 2023 14:59
@Wind4Greg
Copy link
Collaborator

Hi Dave working on high level test vectors. Noticed redundant step to canonize proofOptions in section 3.5.3 Base Proof Hashing (ecdsa-sd-2023), this is already done in at the end of section 3.5.4 Base Proof Configuration (ecdsa-sd-2023) -- step 9.

@dlongley
Copy link
Contributor Author

@Wind4Greg,

Since there's considerable reworking done in this PR and still some editorial clean up to be done later, I'd like us to get this merged and then work off it as a baseline. That should include any simplifications like you mentioned.

@msporny
Copy link
Member

msporny commented Aug 22, 2023

Normative, multiple reviews, changes requested and made, no objections, merging.

@msporny msporny merged commit b6ecad4 into w3c:main Aug 22, 2023
2 checks passed
@dlongley dlongley deleted the update-sd-functions branch August 23, 2023 01:49
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