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

Change on what exactly must be secured in a presentation #1515

Closed
wants to merge 2 commits into from

Conversation

iherman
Copy link
Member

@iherman iherman commented Jul 1, 2024

This is to answer to the issue raised in #1512


Preview | Diff

@iherman iherman self-assigned this Jul 1, 2024
@iherman iherman added editorial Purely editorial changes to the specification. CR1 This item was processed during CR1 labels Jul 1, 2024
@@ -4266,9 +4266,10 @@ <h3>Securing Mechanism Specifications</h3>
[=default graph=].
Copy link
Contributor

@dlongley dlongley Jul 1, 2024

Choose a reason for hiding this comment

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

I can't make a suggestion here, but I believe this should not be a statement about a "property" but about a securing mechanism, and it should be simplified to "The securing mechanism MUST secure all graphs in a [=verifiable credential=] not referred to by the property itself."

Similarly, the above "The property MUST define" seems more like it should say "The securing mechanism MUST define". This list should perhaps be about the requirements for the property and the new securing mechanism.

Copy link
Member

Choose a reason for hiding this comment

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

I wonder whether —

Securing mechanism specifications that create new types of [=embedded proofs=] 
MUST specify a [=property=] for securing

— should become the following (which implies further changes to the bullet list, and possibly more) —

Securing mechanism specifications that create new types of [=embedded proofs=]
MUST specify an object for securing both [=verifiable credentials=] and
[=verifiable presentations=].

— (should object be [=object=]?) with appropriate bullet adjustments, such as —

The object MUST provide values for all [=properties] used by the [=proof graph=].

Copy link
Member Author

Choose a reason for hiding this comment

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

I not against making this change, but that is more complex change than what this PR proposes to do. I would prefer to finalize this PR first and, if needed, do that in a separate PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

B.t.w., @TallTed, the (RDF) "object" term is not used in the core VCDM terminology, only "value".

index.html Outdated Show resolved Hide resolved
Copy link
Contributor

@longpd longpd left a comment

Choose a reason for hiding this comment

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

With dlongley's amendments made by TallTed I support PR #1515

Co-authored-by: Dave Longley <dlongley@digitalbazaar.com>
@iherman
Copy link
Member Author

iherman commented Jul 2, 2024

I have merged the proposal of @dlongley but I am not absolutely sure this is 100% correct. Let us take the hypothetical situation as follows.

  • I use a securing mechanism using proof from DI with one of the cryptosuites.
  • I also use, in the same VP (or VC) a securing mechanism that defined by the property signature.

Question: does proof have to also secure the proof graph defined by signature? In my original text, which explicitly lists what has to be secured, the answer is 'no'. In the text proposed by @dlongley the answer is, actually, 'yes'. I believe the intention is 'no', because the goal is to secure the core information in the VC/VP, but that requires an explicit listing of what is secured...

@iherman iherman added the DO NOT MERGE PR contains something that should not be merged. label Jul 3, 2024
@iherman
Copy link
Member Author

iherman commented Jul 3, 2024

We should close this PR without merge, and concentrate on the more comprehensive #1519 instead.

@iherman
Copy link
Member Author

iherman commented Jul 7, 2024

The discussion has clearly moved to #1519, closing this PR without further action, to avoid misunderstandings.

@iherman iherman closed this Jul 7, 2024
@msporny msporny deleted the definition-for-graphs-to-be-proved branch July 21, 2024 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CR1 This item was processed during CR1 DO NOT MERGE PR contains something that should not be merged. editorial Purely editorial changes to the specification.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants