[DONE] How can we improve our VC & VP validation API?

[olivereanderson]

Introduction

Validation is use case dependent by definition (see Verification and validation and/or Software verification and validation). In the context of verifiable credentials (VCs) an example would be a customer presenting a drivers licence as a VC. Then a car rental service and a concert hall would quite possibly validate this credential in different ways. The former is interested in the fact that the VC's subject can legally drive for the entire duration of the rental, while the latter may only be interested in knowing that the subject is old enough to attend a concert.

Our current VC validation process does not do much to accommodate for application dependent requirements during validation. It merely produces a CredentialValidation that has a verified field that informs us whether the issuer's signature was verified and that all the VC's subjects have active DID documents. Issue #312 requests errors to be thrown during validation, but what is to be considered an error is highly use case dependent. The question we want to answer is therefore: How can we make credential validation flexible enough to accommodate for different modes of failure?

Some ideas

Let us explore some possible approaches.

Approach 1: Composition of validation units

We expose several single concern validator functions (validation units) such as: valid_issuer_signature, expires_after, issuance_date_after, trusted_issuer etc. and then each application can use these (and possibly their own functions) to compose their own validator. In order to avoid the need for every single function to resolve DID documents we would need to separate the resolution process and validation process. Hence we would have something like:

struct ResolvedCredential {
    pub credential: Credential, 
    pub issuer: ResolvedIotaDocument, 
    pub subjects: BTreeMap<String, ResolvedIotaDocument>, 
}

and the Resolver would have a corresponding method:
async fn resolve_credential(&self) -> Result<ResolvedCredential>

then the validation units would look something like this:

fn valid_issuer_signature(resolved_credential: &ResolvedCredential, verifier_options: VerifierOptions) -> bool;

// checks that the supplied IotaDID's have been resolved and have active documents 
fn active_subject_documents(&[IotaDID], &ResolvedCredential) -> bool;

// checks that the issuer of the resolved credential is contained in trusted_issuers 
fn trusted_issuer(trusted_issuers: &[IotaDID], &ResolvedCredential) -> bool;

// etc 

Then each application bakes their own validator by composing these functions as they wish, and if they need any validation units that we haven't thought of they can easily create their own within their own application .

Pros:

• Flexible: Applications decide themselves what and how to compose. Anything that is missing can be created at the application level.
• Simple: Simple data structures and functions which is usually a good thing in terms of FFI. This approach also avoids the need for error codes as applications are expected to create their own use case tailored errors.
• Separates IO bound code (resolving DID documents) from CPU bound code (verification/validation): This usually makes unit testing easier (as mocking can be avoided) and can have benefits for the async/await executor (although signature verification is relatively cheap and thus is unlikely to block the executor for too long).

Cons:

• Pay for what you use only applies to the happy path: All DID's get resolved before one can for instance check that the issuer's signature is correct.
• The fact that validation has taken place is not encoded in the type system: We do not introduce any types that prove that the corresponding credential has been validated.
• Perhaps too unopinionated: The vast majority of applications will want to check the issuer's signature, but this is not emphasised in our code.

Alternative 2: Validation with a simple "allowed deficiencies" config.

We introduce an enum

enum CredentialDeficiency {
    Expired, 
    Dormant, 
    DeactivatedSubjectDocument 
}

and change the signature of CredentialValidator's validate_credential method to be

fn async validate_credential<T>(&self, credential: Credential<T>, verifier_options: VerifierOptions, allowed_deficiencies: &[CredentialDeficiency]) -> Result<CredentialValidation>

then if signature verification fails or any deficiency (such as the current timestamp is later than the credential's expiry date) that is not in allowed_deficiencies leads to a failure. Since this is not a very expressive API (it is for instance not possible to state that an expiry is allowed exactly when it happened less than two weeks ago) we need to change the returned CredentialValidation to accommodate for application dependent post validation checks. This could look something like this:

#[derive(Clone, Debug, PartialEq)]
pub struct CredentialValidation<T = Object> {
  pub credential: Credential<T>,
  pub issuer: DocumentValidation<Active>,
  pub active_subject_documents: Option<BTreeMap<String, DocumentValidation<Active>>>,
  pub deactivated_subject_documents: Option<BTreeMap<String, DocumentValidation<Deactivated>>>,
  pub encountered_allowed_deficiencies: Vec<CredentialDeficiency>,
}

and in the case where encountered_allowed_deficiencies is not empty additional checks may be needed for this struct at the application level.

Pros:

• More efficient unhappy path than in Alternative 1: Stops resolving documents if the issuer's signature fails to be verified, or if any unacceptable deficiency is encountered.
• Flexible: Applications may introduce additional checks of their own after calling validate_credential.
• Increased type safety: The CredentialValidation type can only be constructed by calling validate_credential. This does still not necessarily prove that validation was successful however.

Cons:

• The CredentialValidation struct adds more complexity.
• Mixes IO & CPU bound code

Alternative 3: Validation with an advanced config

We introduce a CredentialValidationOptions structure with builder methods:
with_earliest_expiration_date, allow_deactivated_subject_documents_for, with_latest_issuance_date, with_trusted_issuers and so on. Then this is passed as an additional argument to validate_credential (in place of allowed_deficiencies in Alternative 2).

Pros:

• More efficient unhappy path than both alternative 1 & 2: Exits as soon as anything considered an error is encountered.
• More type safe than alternative 2: Now the existence of CredentialValidation means that the corresponding credential satisfies the needs of the application.
• The return value CredentialValidation in the happy case of validate_credential is simpler than in alternative 2: Now the error reports any validation failure instead.

Cons:

• Less flexible than Alternative 1 & 2: It is harder for applications to add their own validation logic.
• Unclear how detailed the error returned in the unhappy case should be.
• Unclear how validate_presentation should be altered to accommodate for CredentialValidationOptions.
• The builder pattern in CredentialValidationOptions adds more complexity to this library.

Any thoughts on the proposed ideas or any other suggestions would be greatly appreciated.

[eike-hass]

Did we rule out making application specific validation an application level concern? e.g. :
Alternative 4: Ensure technical validity in library, let implementers validate application-specific requirements

[olivereanderson]

Good question,

After thinking about this I am not sure what "technical validity" actually means. The spec mentions many possible fields including: credentialSchema, refreshService, termsOfUse, evidence and so on. In the strictest interpretation of ensuring technical validity we then need to parse/interpret every field and verify it according to some definition/standard that we set. In a much less strict interpretation of technical validity one only needs to check whether it is possible to deserialize the credential into a predefined data structure such as Credential without actually checking the truth of any claims.

[cycraig]

Yes, much of the technical validation required by the specification is enforced by the type system thankfully. There are a wide range of requirements detailed in the implementation report: https://w3c.github.io/vc-test-suite/implementations/

So in terms of being a conformant implementation of verifiable credentials we must validate at least those. You can take a look at some of the other implementers on that page to get an idea of what and how they validate credentials.

We should still endeavour to provide as many convenience functions for validation of common fields as per https://www.w3.org/TR/vc-data-model/#validation (aside from fitness-for-purpose, which is definitely application-specific but we can still assist that, for example by fetching and validating the credential against its credentialSchema or an expected JSON-schema).

[eike-hass]

From the Alternative 1-3 I vastly prefer Alternative 1. Fully agree with the Pros and Cons although I think we need to make sure we have a default configuration for minimal technical verification (signature valid, not expired) that can be overridden and extended.

[olivereanderson]

I think we need to make sure we have a default configuration for minimal technical verification (signature valid, not expired) that can be overridden and extended.

I would also prefer a default minimal technical verification, but I am not sure it is possible to decide on what this should be. For many the credentialStatus field will be used instead of or in addition to the expirationDate, but verifying the former can be hard to do at the library level. If we are to set a default I am somewhat against using something like the following formula : "how important it is"/"implementation difficulty" to determine what gets validated.

From the Alternative 1-3 I vastly prefer Alternative 1.

I am also leaning towards Alternative 1.

[eike-hass]

I think we need to make sure we have a default configuration for minimal technical verification (signature valid, not expired) that can be overridden and extended.

I would also prefer a default minimal technical verification, but I am not sure it is possible to decide on what this should be. For many the credentialStatus field will be used instead of or in addition to the expirationDate, but verifying the former can be hard to do at the library level. If we are to set a default I am somewhat against using something like the following formula : "how important it is"/"implementation difficulty" to determine what gets validated.

From the Alternative 1-3 I vastly prefer Alternative 1.

I am also leaning towards Alternative 1.

Not saying it is trivial but I think we should be able to come up with a sensible default based on what the library supports. This also feeds into the response to my other comment about application level concerns, if we support one or more credentialStatus methods we should consider it for the "technical validity". It gets interesting when there is an unsupported method in a field that we would usually validate. Thinking about that case: Should we throw in then or have a warning level or something similar?

[cycraig]

I agree Approach 1 is more user-friendly and informative than the current approach.

However, I would still prefer to return errors for individual validation functions. "Why" something failed validation is often just as important as the failure, particularly when considering user interfaces where feedback is expected. Without validation errors, developers would need to check cases individually to find out what went wrong (e.g. "was the signature expired?", "was the challenge wrong?", etc.) effectively duplicating a large part of the validation we do internally.

I too think we need to have "sensible defaults" in addition to fine-grained validation checks, since we want to ensure as many developers validate credentials properly and completely to avoid perceived security flaws. That may mean something similar to Alternative 3 or the approach taken with VerifierOptions, where we pass e.g. CredentialValidationOptions to a single validate_credential function or even to Resolver::resolve_credential.

Q1: Are the validation functions on the ResolvedCredential struct or contained in some other struct or module? Does this eliminate the need for CredentialValidator?

Q2: How does this affect validating Presentations? The example works for an individual Credential but the usual scenario is exchanging signed Presentations. Would it be similar to the current PresentationValidation?
E.g.

pub struct ResolvedPresentation<T = Object, U = Object> {
  pub presentation: Presentation<T, U>,
  pub holder: ResolvedIotaDocument,
  pub credentials: Vec<ResolvedCredential<U>>,
}

with similar associated functions?

[PhilippGackstatter]

Agree with Craig. I like the third alternative, in order to provide defaults but let users overwrite certain parts of it. It's nicer over alternative 2 because one does not need pre- and post steps in the validation. True, applications cannot add their own logic as an option on CredentialValiadationOptions, but they can still validate everything that can be done with those options, and then add their own validation after getting a successful result back, right? That seems very acceptable to me usability-wise.

Alternative 1 is fine, it just lacks a really good way to provide sensible defaults. You can provide a function that does our defaults, but if someone wants to deviate from those just a little, then they already need to fully copy our implementation and modify a small part of it. Alternative 3 is more fine-grained in that regard. And if testability is a priority over efficiency, you can still architect alternative 3 using alternative 1 internally.

[cycraig]

Hence I am leaning towards having the validation functions in another module. This does indeed eliminate the need for the current CredentialValidator.

Even if the functions are in another module I don't see a good way for applications to extend them with their own validation functions. I.e. whether they're defined as e.g. ResolvedCredential::validate_signature() or credential_validation::validate_signature(), whatever functions the application defines would be separate. I also don't think there's a particular need to dictate how applications would define their own validation functions (since our interface wouldn't consume/call them) so I'm more in favour of the functions being in the ResolvedCredential struct.

Here is a small toy example illustrating what I had in mind: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=7d408f35d65fd956531d9de7aa179b7e
alternatively the validation functions could also return Result<(), ValidationUnitDependentError>.

Thanks for the example. In my opinion something like ValidationError::SignatureError still does not give enough information about why the signature validation failed, it should still bubble up the internal error since there are at least nine different ways that a signature might fail validation. I'm not saying a developer would need conditional code based on each individual case, but rather that that information is useful and should not be elided in the logs or user error messages.

If we can come to an agreement on what the sensible defaults should be I would be more than happy having a validate_credential method on the resolver that uses these. Any ideas here would be highly appreciated.

As mentioned in a previous comment, basically everything suggested by the specification as a starting point: https://www.w3.org/TR/vc-data-model/#validation

That is, the default validation might include:

• proof (with VerifierOptions)
• issuanceDate
• expirationDate
• credentialStatus
• supported issuers

Edit: in addition to the above, the current CredentialValidator also validates:

• subject DID documents (conformant and not deactivated)
• holder DID document for presentations

[JelleMillenaar]

Not much to add, but really want to highlight the importance for sensible defaults to me. Its kind of like a low and high level API again, which IMO is a good move :)

[olivereanderson]

Even if the functions are in another module I don't see a good way for applications to extend them with their own validation functions. I.e. whether they're defined as e.g. ResolvedCredential::validate_signature() or credential_validation::validate_signature(), whatever functions the application defines would be separate. I also don't think there's a particular need to dictate how applications would define their own validation functions (since our interface wouldn't consume/call them) so I'm more in favour of the functions being in the ResolvedCredential struct.

I am fine with having the validation functions as methods on ResolvedCredential, it does indeed better convey what can be done with a ResolvedCredential.

In my opinion something like ValidationError::SignatureError still does not give enough information about why the signature validation failed, it should still bubble up the internal error since there are at least nine different ways that a signature might fail validation.

Good point, in the case of Alternative 1, the validation units should return Result<(), Something> and not just a bool then.

That is, the default validation might include: ...

Thanks for the suggestions! I have a couple of follow up questions to this.

Q1: Regarding validating supported_issuers: This is most likely highly context dependent. One might trust a certified driving instructor to issue a drivers licence, but not necessarily also a diving licence. Should this then be coupled with for instance the type field? Note that this issue becomes even more important in the context of validating VPs that may very well contain credentials issued from different authorities.

Q2: Regarding validating subject DID documents (conformant and not deactivated): Should the validation options specify whether deactivated subject documents are allowed in a binary fashion? I think there are several use cases where one is only interested in validating a subset of the subject documents.

[PhilippGackstatter]

Should this then be coupled with for instance the type field?

This is quite difficult, and I don't have a strong opinion. To provide full security, we should indeed let users specify the trusted issuers for each type separately. That would be quite cumbersome though, and rarely needed, I think. Even if a university I trust issues a driver's license instead of a Bachelor's degree, wouldn't that fail at the application-specific check (e.g. the schema check, I probably expected a Bachelor's degree, not a Driver's license from a university?) that probably always exists in addition to our "technical validation"? Trusting issuers fully is definitely a caveat we would have to communicate well, though. In cases where an issuer issues a wide range of types, and an application does not trust them for everything, they can add that check on top of ours.

Should the validation options specify whether deactivated subject documents are allowed in a binary fashion?

I think we should keep it simple and make it a boolean for all subject documents. If someone needs more fine-grained validation, they can write it themselves.

I guess the recurring theme is that I'm leaning towards minimizing complexity on our side and push most of it to applications.

[olivereanderson]

Thank you for all your input so far,

Let us briefly summarize where we are at the moment before I ask some follow up questions.

1. There seems to be a consensus that it would be preferable to have some kind of default validation provided by the library.
2. Approach 1 could be provided as a low level API, but there should also be a higher level version.

Now for the follow up questions:
With regards to point 1, what is the exact purpose of our default validation? Is it to provide something that is almost impossible to use insecurely, or is it more like a filter that helps applications reduce the number of validations they need to perform on their own?

In the case of the former in the context of Alternative 3, I think we would need a method of the form trusted_types_per_issuer(issuer_to_types: HashMap<IotaDID, Vec<Vec<String>>>) -> Self as part of the builder, which is indeed a bit cumbersome. It is also unclear to which extent we can achieve providing a validation that both supports everything mentioned in the specification and does this in a way that is almost impossible to get wrong from a security perspective.

In the case of the latter I am thinking of doing something along the following lines:
The Resolver gets methods

resolve_credential(credential: Credential, options: &FilterOptions<Credential>) -> Result<ResolvedCredential, Something>

and

resolve_presentation(presentation: Presentation, options: &FilterOptions<Presentation>) -> Result<ResolvedPresentation, Something>

where FilterOptions (can also be called something else) is like CredentialValidationOptions in Alternative 3, and has methods like acceptable_issuers(self, issuers: Vec<IotaDID> ) -> Self , acceptable_types(self, types: Vec<String>) -> Self, etc.

For those that want to validate everything on their own we provide methods requiring a feature flag resolve_credential_unfiltered, resolve_presentation_unfilteredand we have low level single purpose validation methods on ResolvedCredential(and ResolvedPresentation) as introduced in Alternative 1 (also requiring a feature flag).

What do you think of these ideas?

[PhilippGackstatter]

Is it to provide something that is almost impossible to use insecurely, or is it more like a filter that helps applications reduce the number of validations they need to perform on their own?

I'm thinking along the lines of the second suggestion, because the first one is pretty hard to do from our perspective anyway. We're lacking application-specific context to make things really secure. But we can help users get there by helping with common validations.

I want to mention that I haven't written a ton of credential validation code myself, so take my input with a grain of salt. Maybe we could involve users who have interacted more with credentials what they think of the question you posed.

[cycraig]

I think your suggestion to have the resolve_* functions perform validation (with resolve_*_unchecked/unfiltered to bypass it completely) makes sense since most users should want all basic validations performed.

With regards to the issuer validation: I think it should just be a basic list of trusted issuers like your latter suggestion, e.g. trusted_issuers(self, issuers: Vec<IotaDID>). This list is either applied to all credentials in the presentation for resolve_presentation or to an individual credential in resolve_credential. We can't really dictate exactly how trusted issuers may be determined for credentials (it could be by type or schema or even the subject) so I'm fine leaving fine-grained checking up to developers.

The function signatures could be something like:

• resolve_credential(self, credential: Credential, options: &CredentialValidationOptions) -> Result<ResolvedCredential, Error>
• resolve_presentation(self, presentation: Presentation, presentation_options: &PresentationValidationOptions, credential_options: &CredentialValidationOptions) -> Result<ResolvedPresentation, Error>

This is looking good to me.

Edit:

and we have low level single purpose validation methods on ResolvedCredential(and ResolvedPresentation) as introduced in Alternative 1 (also requiring a feature flag).

Not sure if we should feature-gate the unchecked and individual validation functions, they seem useful (e.g. checking trusted issuers per credential) and will always be present for Wasm?

[JelleMillenaar]

There seems to be a consensus that it would be preferable to have some kind of default validation provided by the library.

A small addition I would like to see, regardless of which approach is chosen. Is too have a simple Boolean parameter in the default validation function for fail_fast, and the validation should then return an array of errors (OneOrMany? NoneOneOrMany?). This way you can also just use the default validator and determine how you deal with the errors. On the otherhand, some might be strict around the validation and will not accept any errors, so they can fail fast.

I think we would need a method of the form trusted_types_per_issuer(issuer_to_types: HashMap<IotaDID, Vec<Vec<String>>>) -> Self as part of the builder

So technically speaking, both schema validation and trusted issuers as part of the validation process where, from my point of view, out of scope for now. I think this requires a more elegant solution and is part of the "Managing Trust" item on the long-term roadmap. Since schema validation is probably easy, I would not mind that we have at least a validation unit for it available. But perhaps let the Verifier application developers deal with the "which Issuer do I trust" issue. Keep in mind, they have access to the VC/VP data, so they can do custom validation regardless of what approach we choose.

[olivereanderson]

A small addition I would like to see, regardless of which approach is chosen. Is too have a simple Boolean parameter in the default validation function for fail_fast, and the validation should then return an array of errors (OneOrMany? NoneOneOrMany?). This way you can also just use the default validator and determine how you deal with the errors. On the otherhand, some might be strict around the validation and will not accept any errors, so they can fail fast.

What is suggested here is really (at least in my understanding) a coarser version of Alternative 2. Instead of allowed_deficiencies, it is either allow all deficiencies or none (encoded by the fail_fast parameter). If we include this as an option we have to make it extremely clear exactly what is considered a failure and that additional application specific checks are most likely required. I think the name CredentialValidatior is somewhat misleading in this regard.

So technically speaking, both schema validation and trusted issuers as part of the validation process where, from my point of view, out of scope for now. I think this requires a more elegant solution and is part of the "Managing Trust" item on the long-term roadmap.

I agree that it is hard to find an elegant solution when it comes to providing an API for filtering on trusted issuers at this stage in the development cycle, but we also have to keep in mind that the more things we keep out of the higher level default validation the less useful/reliable it becomes.

[olivereanderson]

Not sure if we should feature-gate the unchecked and individual validation functions, they seem useful (e.g. checking trusted issuers per credential) and will always be present for Wasm?

Good point, perhaps especially about Wasm,

I am a little concerned about making it look like we are promoting the use of this low level API (as opposed to building ones own entirely, or maybe even using an external service) by having them as easily accessible methods on ResolvedCredential. I think it would be clearer that this is a low level API one can use by either placing the validation units as free functions in another module and/or requiring a feature flag. I do however see other benefits to having them as methods instead of free functions (they can then return Result<Self, ValidationError> and thus become more composable). Hence if we can come up with a good way to convey that these methods are lower level validators one may use then I am all for that.

[olivereanderson]

Here is a new list of possible ways forward (taking into account the comments made in this discussion so far).

1. Approach 1 (should really have been called Alternative 1) from above.
2. A coarser variant of Alternative 2 from above, where allowed_deficiencies is replaced by a boolean flag fail_fast.
3. Alternative 3 from above, but on the resolver instead of having a CredentialValidator as we do now. The exact details of what the CredentialValidationOptions should include can be figured out at the PR review level.
4. Both 1 & 3 (low and high level API)
5. We take the stance that we only verify proofs/signatures and any other kind of validation is left to application developers. Note that this is kind of what we are currently doing in the CredentialValidator. This means that we can close [Task] Let VCs and VPs throw errors during validation, include expiration functionalities. #312 right away since it will then be defined to be an application concern.

[cycraig]

I think Option 4 is the closest to what was discussed/partially agreed upon in the previous comment chain: a combination of low-level validator methods which are both used by a "sensible defaults" resolve_credential/presentation() function (with options to configure/disable certain validations) and exposed if developers wish to bypass that default validation entirely and call the low-level functions manually after resolve_credential/presentation_unchecked(). This direction would be my personal preference.

I believe we have discarded Option 5 in favour of a "sensible defaults" validation that the majority of developers will wish to use out-of-the-box.

The fail_fast discussion is kind of orthogonal to the current decision in my mind since it can be incorporated either into resolve_credential or added as a separate function, so it can be achieved either way.

Edit: to elaborate on what Option 4 might look like (in my mind) based on previous comments:

let resolver: Resolver = ...
let presentation: Presentation = ...
let resolved_presentation: ResolvedPresentation = resolver.resolve_presentation(
    presentation,
    PresentationValidationOptions::default(),
    VerifierOptions::new().challenge("expected-challenge-abcdefg123456789"), // <- these options could be rolled into PresentationValidationOptions
).await?;
// then run custom validation code per credential, e.g.
for resolved_credential in &resolved_presentation.credentials {
    resolved_credential.validate_issuer(vec![IotaDID::parse("did:iota:123...")?])?;
    resolved_credential.validate_schema().await?;
    // ...
}

Splitting the resolution and validation into two steps might also be fine, e.g.

let resolver_presentation: ResolvedPresentation = resolver.resolve_presentation(presentation).await?;
resolver_presentation.validate(
    PresentationValidationOptions::default(), 
    VerifierOptions::new().challenge("expected-challenge-abcdefg123456789"),
)?;

I think it's worth considering where to place these functions (maybe retaining CredentialValidator or a Validator equivalent), since I recall being able to pass a custom resolver/client map to the validator being a consideration.