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

Remove issue VC option to change verification method. #263

Merged
merged 2 commits into from
Mar 15, 2022

Conversation

dlongley
Copy link
Contributor

I do not think it should be possible to change the verification method when issuing a VC.

I think the chosen VC issuer endpoint should have a valid configuration that is known to the caller of the API. If a different cryptosuite is a requirement for the caller, a different URL should be hit that has a different configuration, for example:

Configuration 1: <myserver>/issuers/123/credentials/issue
Configuration 2: <myserver>/issuers/456/credentials/issue

A client should only care that the VC that is issued has an expected issuer field and it uses an acceptable cryptosuite and credential status mechanism. Any further details of how these things are implemented are not the client's responsibility -- and I believe that making them so constitutes a layering violation and leads to trouble. The parties that setup issuer endpoints should have to think deliberately about which configurations they are offering, as well. For example, there are important privacy considerations.

To further support this argument:

  1. Consider if the party that setup the issuer needs to rotate keys -- it would need to update or notify all clients that use that endpoint to make changes. Additionally, there is now more information a client may need to know and configure in order to use the issuer service vs. just setting a URL (and any authz credentials).
  2. Consider if the client chooses a "sub configuration" that leads to incompatibilities. For example, a client chooses to issue a VC using a verification method or cryptosuite X -- but the credential status method results in a status list credential that is signed with verification or cryptosuite Y. Now consumers that were expecting to be able to verify using X cannot check status without using Y (or there is some business policy violation). This could be made even worse if the issuer service uses the same VM to issue both the requested VC and the status list VC -- changing up the verification method used on the static list VC based on whatever the latest issued VC was. To remedy this, the issuer services may need to validate all possible different "sub configuration" combinations resulting in unnecessary complexity to meet use cases.
  3. Allowing any possible parameter to be configured by the client absolves the party that configures the issuer instances to avoid thinking about privacy considerations: Offering many different possible cryptosuites can result in harming the herd privacy afforded by credential status lists VCs.

I'm generally in favor of reducing the options that a client can make when hitting an issuer URL -- leaving most, if not all, options up to a configuration that is pinned to the URL. This PR just starts with "verification method" as I see it as the most immediately problematic.

@peacekeeper
Copy link
Member

Hmm I don't have a strong opinion, but here are some reactions.

Isn't it a feature that an issuer DID can have multiple keys, and that the client can choose which one to use depending on the situation?

  • For example, country A may only consider verification method type X to create legally binding proofs, whereas country B may only allow verification method type Y. If an issuer wants to issue VCs to users from both countries, why would they have to set up more than one endpoint for that (considering that it's still only one issuer).
  • For some high value credentials I may want to use a stronger proof type, and for other less important credentials a weaker proof type.

Well I guess that can also be achieved with your proposal to simply have multiple endpoints.

Consider if the party that setup the issuer needs to rotate keys -- it would need to update or notify all clients

Only if the DID URL changes after key rotation. This depends on the DID method, some use fragments such as #verkey that don't change. Changing the DID URL after key rotation (and therefore having to notify all clients) could be a problem or a feature, depending on the use case, no?

Consider if the client chooses a "sub configuration" that leads to incompatibilities. For example, a client chooses to issue a VC using a verification method or cryptosuite X -- but the credential status method results in a status list credential that is signed with verification or cryptosuite Y

Couldn't there be situations where you actually want to use a different cryptosuite for the VC and for the status list credential?

Anyway, I do feel there's some security/privacy value in what you suggest, i.e. moving more options away from the client into the API implementation. So I wouldn't be opposed to making the change. I think however I would prefer to make this optional rather than remove it, and some implementations may have good reasons to decide to simply not support it.

Copy link
Contributor

@OR13 OR13 left a comment

Choose a reason for hiding this comment

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

A client should be able to ask for a credential from ANY of the keys listed in the assertionMethod block of the issuer's did.

Accepting this PR would make the verificationMethod associated with the issuer ambiguous.

I can't imagine a version of this PR that I would accept.

@dlongley
Copy link
Contributor Author

@peacekeeper,

Isn't it a feature that an issuer DID can have multiple keys, and that the client can choose which one to use depending on the situation?

Yes, an issuer can have multiple keys. My argument here is that the client of the issuer VC API should not be choosing a different key as a standalone parameter to the VC API.

Rather, the entity that sets up issuer instances should create N many acceptable configurations for the client to choose from (by selecting a URL). As mentioned above, selecting a different key as a single parameter change (leaving the rest of the configuration the same) may create problems (and almost definitely will). I mentioned some of these problems above. Ultimately, the client shouldn't even care which key is being used, but rather which cryptosuite.

Configuration parameters are interdependent. One cannot simply change the verification method for the VC being issued and not also change the verification method being used to sign the VC that provides the status list. And the problems only continue from here (see above).

So, no, the client should not, in a vacuum, choose the verification method to use depending on the situation. The client should choose an issuer URL that maps a fully internally compatible configuration that is acceptable for the situation.

  • For example, country A may only consider verification method type X to create legally binding proofs, whereas country B may only allow verification method type Y. If an issuer wants to issue VCs to users from both countries, why would they have to set up more than one endpoint for that (considering that it's still only one issuer).
  • For some high value credentials I may want to use a stronger proof type, and for other less important credentials a weaker proof type.

Well I guess that can also be achieved with your proposal to simply have multiple endpoints.

Yes, that's right. The approach I offered meets the use cases you mentioned whilst avoiding the problems I highlighted with the other approach. As for your "why would they have to set up more than one endpoint" -- I believe I gave the reasons for this in my original post.

Consider if the party that setup the issuer needs to rotate keys -- it would need to update or notify all clients...

Only if the DID URL changes after key rotation. This depends on the DID method, some use fragments such as #verkey that don't change. Changing the DID URL after key rotation (and therefore having to notify all clients) could be a problem or a feature, depending on the use case, no?

Yes, and this discrepancy is yet another reason to support the approach I'm advocating. Pushing the complexity of understanding these variants into more layers is another strike against the current approach. By the way, just think of trouble that could arise if the key is changed to a different cryptosuite but the ID is kept the same. It's better to code against a particular issuer configuration that can ensure the configured cryptosuite is appropriate for the configured key.

Couldn't there be situations where you actually want to use a different cryptosuite for the VC and for the status list credential?

If so, such an issuer configuration can be made available per the approach I'm advocating. But this decision cannot be made in a vacuum by a single client -- it would cause trouble for every other client using the same issuer URL. Instead, create different acceptable configurations and use different URLs to access them. This establishes a separation of concerns and avoids the trouble I've mentioned -- whilst still addressing the use cases.

Anyway, I do feel there's some security/privacy value in what you suggest, i.e. moving more options away from the client into the API implementation. So I wouldn't be opposed to making the change. I think however I would prefer to make this optional rather than remove it, and some implementations may have good reasons to decide to simply not support it.

Understood. But if the approach I advocate works for the use cases, I'd rather us have a single way of doing it. It's not a hill I'd die on, but having more than one way to do things will harm interoperability with some issuers rejecting requests that include individualized parameters.

@dlongley
Copy link
Contributor Author

@OR13,

A client should be able to ask for a credential from ANY of the keys listed in the assertionMethod block of the issuer's did.

And to accomplish this with the safer (read: non-broken) method I'm suggesting is to expose issuer URLs that map to configurations for each one. That way appropriate choices can be made and bound to how those verification methods will work in conjunction with the other configured parameters and the security/privacy outcomes, e.g., status list VCs.

Accepting this PR would make the verificationMethod associated with the issuer ambiguous.

First of all, this seems to be an orthogonal issue. It also seems limited to knowledge prior to signing. It is certainly known post signing. An endpoint that exposes the configuration information could also indicate which verification method would be used by the issuer.

Second, whether or not this is important information to the client (as opposed to the client just knowing the cryptosuite) requires a use case. What is it? I think it's better for the client to not have this responsibility and to allow them to depend on issuers to practice good key hygiene without clients having to intervene.

Third, I've highlighted worse ambiguities with the current approach.

@OR13
Copy link
Contributor

OR13 commented Feb 22, 2022

@dlongley your comments on this PR are really substantial, I feel like it would be much better split into several issues, each of which should be discussed on a call.

@peacekeeper
Copy link
Member

Related to #67 which asks about a "default" verification method.

@mkhraisha
Copy link
Collaborator

To be clear @dlongley are you proposing:

  1. binding a single DID to an endpoint AND
  2. binding a single crypto-suite to an endpoint?

I believe a much simpler solution would be to change the configuration object from this:

options: {
      "type": "Ed25519Signature2018",
      "verificationMethod": "did:example:123#z6MksHh7qHWvybLg5QTPPdG2DgEjjduBDArV9EF9mRiRzMBN",
      "proofPurpose": "assertionMethod",
      "credentialStatus": {
        "type": "RevocationList2020Status"
      }
}

to

options: {
      "type": "Ed25519Signature2018",
      "DID": "did:example:123",
      "proofPurpose": "assertionMethod",
      "credentialStatus": {
        "type": "RevocationList2020Status"
      }
}

Allowing the issuer to decide which key to issue from. This allows rotation of keys without the need to notify the clients, and allows the issuer to choose which key to issue the revocation list with.

@OR13
Copy link
Contributor

OR13 commented Mar 1, 2022

I opened #267 to try and make more sense of where @dlongley is wishing for these changes to head.

@dlongley
Copy link
Contributor Author

dlongley commented Mar 1, 2022

@mkhraisha,

See #267 for a more complete proposal. But, in short, I don't yet know what the significant advantage would be to allowing multiple suites per issuer configuration/instance. It seems like the same could be achieved by minting as many configurations as are desired (and should be allowed given various considerations such as privacy, etc.) whilst allowing for the implementation to apply cleaner constraints / boundaries / responsibilities per instance.

@@ -20,9 +20,6 @@ components:
type:
type: string
description: The type of the proof. Default is an appropriate proof type corresponding to the verification method.
verificationMethod:
type: string
description: The URI of the verificationMethod used for the proof. If omitted, a default verification method will be used.
proofPurpose:
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this as well.

@@ -20,9 +20,6 @@ components:
type:
Copy link
Contributor

Choose a reason for hiding this comment

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

suggest we remove this and have the server choose.

@@ -45,7 +42,6 @@ components:
example:
{
"type": "Ed25519Signature2018",
"verificationMethod": "did:example:123#z6MksHh7qHWvybLg5QTPPdG2DgEjjduBDArV9EF9mRiRzMBN",
"proofPurpose": "assertionMethod",
"created": "2020-04-02T18:48:36Z",
"domain": "revocation.example",
Copy link
Contributor

Choose a reason for hiding this comment

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

domain should not be present in this example

@@ -45,7 +42,6 @@ components:
example:
{
"type": "Ed25519Signature2018",
"verificationMethod": "did:example:123#z6MksHh7qHWvybLg5QTPPdG2DgEjjduBDArV9EF9mRiRzMBN",
"proofPurpose": "assertionMethod",
Copy link
Contributor

Choose a reason for hiding this comment

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

proofPurpose should be removed

@@ -45,7 +42,6 @@ components:
example:
{
"type": "Ed25519Signature2018",
Copy link
Contributor

Choose a reason for hiding this comment

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

type should be removed and set by the server.

@dlongley
Copy link
Contributor Author

dlongley commented Mar 1, 2022

@OR13, applied your suggestions.

@dlongley dlongley requested a review from OR13 March 1, 2022 22:11
@msporny msporny merged commit 8a419de into main Mar 15, 2022
@msporny msporny deleted the dlongley-remove-verification-method branch March 15, 2022 20:19
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