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

Add Verification Method types to v2 context #1260

Closed
wants to merge 1 commit into from

Conversation

OR13
Copy link
Contributor

@OR13 OR13 commented Aug 26, 2023

No description provided.

"@id": "https://w3id.org/security#controller",
"@type": "@id"
},
"revoked": {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just noticed that Multikey supports "expires" and "revoked" but "JsonWebKey" does not.

That seems like it will cause bias / interop issues... should we add these properties to "JsonWebKey" ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm fine with "JsonWebKey" adopting "expires" and "revoked". Or not -- depending on what people think makes the most sense for users of the two approaches.

That being said, I do not understand how you have been using the word "bias" in a number of places. I can only make sense of your use of it by thinking you mean that if two things are not exactly the same then one may favor one over the other for a variety of reasons. But I don't find that definition of "bias" to identify something that ought to be (or even practically can be) avoided. Nor is that definition what I think most people think of when they use the word. It's not what I mean when I say "bias". I think most people think of "bias" as "undue influence" or "lack of fairness".

A securing mechanism that doesn't change the format of the input has advantages. Does this mean that all securing mechanisms that do change the format don't have their own advantages and should be changed?

A spec that allows you to choose between two different key formats has the advantage of choice; another spec that recommends a particular one has the advantage of simplicity. These are not the same; should it be impossible to have both of these at the same time?

I choose these examples because I don't believe you've raised them as potentially causing "bias issues", but it feels like I'm applying the same principle you seem to apply here and elsewhere. I don't agree with that principle. I think the principle we should be applying is related to fairness, not difference.

While a difference is required in order for something to be unfair, clearly something just being different doesn't mean there is a lack of fairness or undue influence. Otherwise everything in every spec we produce would have to be exactly the same to eradicate this unfairness. Taken to its logical end, to remove that kind of "bias" we would have to eliminate all specs since defining anything creates a distinction between what matches the definition and what does not. So I presume this can't be what you mean by "bias" and the desire to eliminate it, but then I haven't been able to think of any consistent definition for what you may mean.

Perhaps you think there's something unfair about one key format being able to express X and another not being able to. But I don't understand that either. I think there are simply trade offs -- and it's those trade offs that should inform whether it makes sense (or not) to adopt certain features in any format. If you think "JsonWebKey" should do X, or Y, or Z, please just say so and see if others agree or disagree. Raising the specter of "bias" seems unnecessary and confusing.

Copy link
Member

@msporny msporny Aug 26, 2023

Choose a reason for hiding this comment

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

I'm fine w/ JsonWebKey supporting the expires and revoked properties. Suggest adding it to align the two verification method classes. I will do a similar PR in the Data Integrity spec.

Copy link
Member

Choose a reason for hiding this comment

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

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.

What is the use case for a VC expressing secret key material, or keys at all? Is this a use case that we expect to see broad adoption? The only one I can think of is confidenceMethod, and that doesn't seem like it's going to achieve the implementation requirements by the time we hit PR in the WG.

Concrete change requests:

  • What issue on the VCDM is this related to, please raise and discuss issues before raising PRs.
  • Remove the expression of secret key material in VCs; there is no broadly supported use case for doing this.
  • Add revoked to JsonWebKey to bring the properties into alignment between both verification method classes.

Comment on lines +85 to +88
"secretKeyJwk": {
"@id": "https://w3id.org/security#secretKeyJwk",
"@type": "@json"
}
Copy link
Member

@msporny msporny Aug 26, 2023

Choose a reason for hiding this comment

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

A VC shouldn't express private key material.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

^ this is not in the scope of the the "VerifiableCredential" RDF class.

... a VC can express RDF, if we want a VC's concept of "Multikey" or "JsonWebKey" to be the same, they need to have the same defintion.

Copy link
Contributor

Choose a reason for hiding this comment

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

The definitions need to be consistent between what's in the broken out contexts and this one to ensure their compatibility.

Comment on lines +113 to +116
"secretKeyMultibase": {
"@id": "https://w3id.org/security#secretKeyMultibase",
"@type": "https://w3id.org/security#multibase"
}
Copy link
Member

@msporny msporny Aug 26, 2023

Choose a reason for hiding this comment

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

A VC shouldn't express private key material.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

@OR13
Copy link
Contributor Author

OR13 commented Aug 28, 2023

(Out of order replies)

  • What issue on the VCDM is this related to, please raise and discuss issues before raising PRs.

This PR was follow up from this issue: w3c/vc-data-integrity#164 (comment)

Its also related to the ongoing work and discussions in calls related to referencing controller documents.

  • Remove the expression of secret key material in VCs; there is no broadly supported use case for doing this.

I'm happy to do this, if its removed from the other contexts... but I don't think thats a good idea.

The idea that "not defining something in a JSON-LD context" is some kind of securing "feature" is wrong, and needs to be challenged.

It contradicts the idea that the core context is supposed to make life easy for developers.

Because the v2 context already ships a default vocabulary, there will be 0 difference except for the terms and shape of the private key in RDF (security theater).

There is a threat that some RDF processor might not be able to find secretKeyMultibase without its fully expanded IRI... but thats not a problem for any of the processors of this spec, and if it were a problem, defining secretKeyMultibase would be better than allowing it to redefined by callers.

  • Add revoked to JsonWebKey to bring the properties into alignment between both verification method classes.

I filed #1262 I will ensure the RDF Class definitions are the same.

What is the use case for a VC expressing secret key material, or keys at all? Is this a use case that we expect to see broad adoption?

There are a lot of use cases related to private key material, some involve law enforcement or military use cases.

Here are 2 examples:

  1. classifying stolen property (although in this case, you probably want to preserve the bytes, not the abstract representation).
  2. Impersonating an account (as undercover work, or as part of some other operation type).

I don't expect VC's signing private keys to be used often, but I don't see why if they are used, the RDF should not match up with public key systems... So that RDF aware processors can leverage graph queries and experience from the existing data model.

The only one I can think of is confidenceMethod, and that doesn't seem like it's going to achieve the implementation requirements by the time we hit PR in the WG.

I would would not assume anything about the Domain and Range of that property, it's possible there will be some intermediate RDF type between them, or that confidenceMethod is actually just authentication in disguise.

I don't think we need to assume use cases to know that a consistent data model will be easier to query and comprehend by controllers (which includes data subjects who are controllers).

@iherman
Copy link
Member

iherman commented Sep 3, 2023

The issue was discussed in a meeting on 2023-08-29

  • no resolutions were taken
View the transcript

1.7. Add Verification Method types to v2 context (pr vc-data-model#1260)

See github pull request vc-data-model#1260.

Manu Sporny: adds key types to base context. Don't know if we have general use cases for expressing keys in a verifiable credential. Don't know if it is a good idea until we have use cases warranting this.

Kristina Yasuda: will bring this up in tomorrow's working group call.

Phillip Long: Why would anyone want to publish their private key in a VC???

Manu Sporny: exactly :).

David Waite: (zk selective disclosure).

Manu Sporny: add support for DIDs issue is providing a controller document, danger here is that definitions will deviate due to copies in DID core, DI and vc-jose-cose.

Michael Jones: a reason we can't just cite did-core?

Manu Sporny: we need to update the text without a mandate to update did-core.
… the point in which the group should get concerned is if these copy-pasted sections start giving opposite advice.

@iherman
Copy link
Member

iherman commented Sep 3, 2023

The issue was discussed in a meeting on 2023-08-30

  • no resolutions were taken
View the transcript

2. Add Verification Method types to v2 context (pr vc-data-model#1260)

See github pull request vc-data-model#1260.

Orie Steele: this PR was raised after a thread in DI repo about how many contexts are needed.

Joe Andrieu: .,, vc-di has the vc-di context, but only with limited terms.

Orie Steele: [more details].
… in DI, we have three contexts. one for proof and DI proof. one for multi-key and one for webkey.
… for StatusList2021 you can use without one of the contexts, but that context is bundled into v2 context.
… you can use multikey and json multikey but these are not bundled.
… so this adds those terms to that bundle, so you can get the same URI in all situations.
… the contentious part is the private key representations.
… manu said he doesn't know a good use case for private keys.
… my preference is that the term definitions be consistent, as its harder to prevent them from ...
… people can do whatever they want. my preference would be to make sure the graph looks right.
… also, the keys part of a JSON web key set.
… similiar to JWK in v2.

Manu Sporny: I think that's a fair introduction to the issue.
… to hone in on the parts that are contentious.
… One is, do we have a use case where people are going to express private key material in a VC and signing it?
… If so, they can always do that.
… but its a corner case, so it's not clear we want to promote that by default.
… I don't think that as a working group we should be promoting that.
… Signing over private key material doesn't seem like something we should support.
… We don't currently have an example of representing this.
… if we had confidenceMethod is included, that might make sense.
… but that doesn't look to be on a trajectory to make it into the spec, so I think we should hold off.
… If we get a super valid use case, we can always put that into the context later.
… We have text in the spec that the context file might change during CR and we can deal with it then.

Phillip Long: +1 to NOT supporting signing over private credentials - not something we should encourage. As we're in an open world model we can't prevent this.

Manu Sporny: I don't think its a good idea to merge this PR at this time. We need a good use case.

Phillip Long: private credentials should have read private keys.

Sebastian Crane: Orie, I know where you're coming from. I asked Manu something similar last week.
… given the VC flexibility, you can use them to cross-sign keys.
… any one of those use cases could be addressed with cross-signing.

Orie Steele: You can sign whatever you want... this working group does not control what issuers sign... and using RDF to try and enforce that is... not a good idea.

Manu Sporny: To be clear - we're not blocking any of the use cases Orie is talking about... we're just saying: "That's not a mainstream use case, but if you want to do it, there is a mechanism there to do it (include the other context).".

Sebastian Crane: at no point do you actually need a private key to move.

Orie Steele: The confidenceMethod is reserved. Not clear if we settled domain & range.
… There is a question if we should define those for reserved properties.
… You could say the range is "verification methods".
… then you could expect to see those defined.
… For those who want confidence method, one of the use cases is to bind to a DID.
… the other is to bind to some key material.
… the only contentious issue I see here is the secret key material in them.
… if its better to do that not in the V2 context, but can be in a different one.
… I was surprised to see secret keys in the context definition.
… I want to move away from the multi-base approach but support issuer-provided means.

Brent Zundel: this is good, about ready to move on.

Manu Sporny: two contentious part here. one confidenceMethod doesn't exist (because of no implementations).
… we've agreed that if you don't have an implementation, don't add it to the context.
… I agree. In theory you could use confidence method, but that will need another context (because it isn't in V2).
… So we shouldn't put that into the spec.

Orie Steele: confidenceMethod the "predicate" is reserved, and exists.

Orie Steele: ConfidenceMethod the RDF class does not exist, but "VerificationMethod" does... afaik.

Orie Steele: disagree with basically everything Manu is saying... but happy to keep talking about it on issues.

Manu Sporny: Two, yes. We've been arguing for a long time. Orie, you said you would object if it isn't in there. So that was a compromise made.
… That is shifting into core context where it doesn't look like it belongs.
… There is no use case that's broad enough for secret keys to be in the base context.

Orie Steele: I will object if core context defines confidenceMethod, but not JsonWebKey and Multikey.

Manu Sporny: if its not there, it can be an issuer-specific context.

@iherman
Copy link
Member

iherman commented Sep 5, 2023

The issue was discussed in a meeting on 2023-09-05

  • no resolutions were taken
View the transcript

3.4. Add Verification Method types to v2 context (pr vc-data-model#1260)

See github pull request vc-data-model#1260.

Orie Steele: do you need multiple @contexts? Ans was no, they should be bundled into one.
… any time you see json web key you might not get the right @context and therefor not get the right definition (i.e. issuer defined vs. W3C defined).
… there are no valid use cases for signing a secret key.

@Sakurann
Copy link
Contributor

1 request for changes, no approvals, open for 3 weeks, no objections to close at TPAC VC WG day 1.

@Sakurann Sakurann closed this Sep 14, 2023
@iherman
Copy link
Member

iherman commented Sep 15, 2023

The issue was discussed in a meeting on 2023-09-14

  • no resolutions were taken
View the transcript

1.2. Add Verification Method types to v2 context (pr vc-data-model#1260)

See github pull request vc-data-model#1260.

Brent Zundel: Next one is 1260.
� Raised by Orie. Open for some time. Currently 1 pending change request from manu.

Manu Sporny: It's a general objection to the PR. We have a use case where expressing public and private keys in the context. It would change if we had something like confidence method to the base context. -1 until that's the case.

Brent Zundel: Since no approvals, there is no consensus and one objection. We plan on closing.

Ivan Herman: the secret is already in the vocabulary. Just noting that.

Manu Sporny: We might want to look at that, it might be a mistake.

Brent Zundel: We need an open issue to track a possible change to the vocab. We intend to close this PR due to lack of consensus.

@msporny msporny deleted the feat/add-verification-method-types-to-v2-context branch November 11, 2023 16:01
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.

5 participants