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

Pseudonyms and Anonymous Holder Binding optional features #145

Closed
wants to merge 17 commits into from

Conversation

Wind4Greg
Copy link
Collaborator

@Wind4Greg Wind4Greg commented Mar 7, 2024

This PR addresses issue #127 by showing how procedures can be modified to support the anonymous holder binding, pseudonyms with issuer known pid, and pseudonyms with hidden pid optional features based on emerging IETF/CFRG work. An outline of a section describing these features is provided.

Feedback is requested earlier rather than later to help improve this PR. Thanks ahead of time Greg.


Preview | Diff

msporny

This comment was marked as outdated.

msporny

This comment was marked as outdated.

index.html Outdated Show resolved Hide resolved
Copy link
Member

@msporny msporny left a comment

Choose a reason for hiding this comment

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

Approved as long as we add an at risk marker for the optional BBS features that depend on IETF RFCs.

Copy link
Contributor

@dlongley dlongley left a comment

Choose a reason for hiding this comment

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

Still working my way through this, just have one change suggestion so far. I think we're mostly in good shape here -- but trying to test against an implementation to catch anything else.

index.html Outdated
Comment on lines 1288 to 1294
If |commitment_with_proof| is empty and |use_pseudonyms| is true. Generate a
cryptographically random 32 byte |pid| value. Compute the
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should organize this such that you either pass in pid or not -- not that we require implementations to generate a random value for it. We can make a suggestion that that's one way to do it, but it should be done externally, IMO, and passed in. So it may be that we either have commitment_with_proof or pid passed here (it's an error to pass both) -- and then just one of them is used internally. This prevents us from being rigidly opinionated on how pid is generated.

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.

grammar, punctuation, clarified text

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
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.

More punctuation, typos, grammar, etc.

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
@TallTed
Copy link
Member

TallTed commented Mar 8, 2024

It seems that this is less about "Pseudonyms" in general, and more about "Pseudonymous Holders"...

If that's correct, some further exploration/clarification in the text seems warranted.

@Wind4Greg
Copy link
Collaborator Author

Yes, @TallTed the pseudonyms are for the holder but they are tied to a verifier. I'm doing some technical updates so will try to add a bit more explanation.

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.

small, for clarity

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
Wind4Greg and others added 2 commits March 8, 2024 10:46
…unctuation, and formatting improvements.

Co-authored-by: Ted Thibodeau Jr <[email protected]>
index.html Outdated Show resolved Hide resolved
index.html Outdated
Comment on lines 1298 to 1304
the `bbsSignature` using the `Sign` procedure of [[CFRG-Blind-BBS-Signature]]
and [[CFRG-Pseudonym-BBS-Signature]], with appropriate key material, `bbsHeader`
for the `header`, `bbsMessages` for the `messages`, and |commitment_with_proof|
for the `commitment_with_proof`. If the signing procedure uses the optional
|signer_blind| parameter retain this value for use when calling
<a href="#serializebaseproofvalue"></a> below.
This provides for the "pseudonym with hidden pid" feature.
Copy link
Member

Choose a reason for hiding this comment

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

This must be answered before the suggestion can be complete: Is it using the `Sign` procedure of both [[CFRG-Blind-BBS-Signature]] and [[CFRG-Pseudonym-BBS-Signature]]? Or is it using the `Sign` procedure of whichever is relevant, Blind or Pseudonym [[CFRG-Blind-BBS-Signature]] or [[CFRG-Pseudonym-BBS-Signature]]?

Suggested change
the `bbsSignature` using the `Sign` procedure of [[CFRG-Blind-BBS-Signature]]
and [[CFRG-Pseudonym-BBS-Signature]], with appropriate key material, `bbsHeader`
for the `header`, `bbsMessages` for the `messages`, and |commitment_with_proof|
for the `commitment_with_proof`. If the signing procedure uses the optional
|signer_blind| parameter retain this value for use when calling
<a href="#serializebaseproofvalue"></a> below.
This provides for the "pseudonym with hidden pid" feature.
the `bbsSignature` using the `Sign` procedure of [[CFRG-Blind-BBS-Signature]]
and [[CFRG-Pseudonym-BBS-Signature]], with appropriate key material, `bbsHeader`
for `header`, `bbsMessages` for `messages`, and |commitment_with_proof|
for `commitment_with_proof`. If the signing procedure uses the optional
|signer_blind| parameter, retain this value for use when calling
<a href="#serializebaseproofvalue"></a> (below).
This provides for the "pseudonym with hidden pid" feature.

Copy link
Collaborator Author

@Wind4Greg Wind4Greg Mar 8, 2024

Choose a reason for hiding this comment

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

For now I'll put with the Pseudonym draft. If the Pseudonym draft gets absorbed into the Blind Signatures draft we can update.

Copy link
Contributor

@dlongley dlongley Mar 8, 2024

Choose a reason for hiding this comment

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

+1 to just go with what's there today (use "easiest" from above).

index.html Outdated Show resolved Hide resolved
…unctuation, and formatting improvements.

Co-authored-by: Ted Thibodeau Jr <[email protected]>
index.html Outdated
@@ -596,7 +608,8 @@ <h4>parseBaseProofValue</h4>
disclosure base proof value. The required input is a proof value
(<var>proofValue</var>). A single object, <em>parsed base proof</em>, containing
five elements, using the names "bbsSignature", "bbsHeader", "publicKey",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
five elements, using the names "bbsSignature", "bbsHeader", "publicKey",
three to five elements, using the names "bbsSignature", "bbsHeader", "publicKey",

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be five (no pseudonym feature) or seven (pid and signer_blind present to signal pseudonym use, but possibly empty).

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
Wind4Greg and others added 4 commits March 8, 2024 12:34
…provements for optional parameters

Co-authored-by: Dave Longley <[email protected]>
…provements for optional parameters

Co-authored-by: Ted Thibodeau Jr <[email protected]>
…provements for optional parameters

Co-authored-by: Dave Longley <[email protected]>
Copy link
Contributor

@dlongley dlongley left a comment

Choose a reason for hiding this comment

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

Minor tweak here, then ready to go, IMO. Thanks!

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
Co-authored-by: Dave Longley <[email protected]>
Co-authored-by: Ted Thibodeau Jr <[email protected]>
@w3c w3c deleted a comment Mar 14, 2024
@w3c w3c deleted a comment Mar 14, 2024
@w3c w3c deleted a comment Mar 14, 2024
@w3c w3c deleted a comment Mar 14, 2024
@w3c w3c deleted a comment Mar 14, 2024
@w3c w3c deleted a comment Mar 14, 2024
@w3c w3c deleted a comment Mar 14, 2024
@w3c w3c deleted a comment Mar 14, 2024
@msporny
Copy link
Member

msporny commented Mar 14, 2024

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

@msporny
Copy link
Member

msporny commented Mar 14, 2024

I did end up merging this (even though it said this PR was closed and not merged), but had to do it manually because of a series of merge conflicts due to a non-rebased merge in this PRs history. @Wind4Greg we'll have to discuss how to avoid those sorts of things during our next sync call.

@Wind4Greg Wind4Greg deleted the pseudonyms-binding branch April 9, 2024 17:07
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