Allow issuance of expired and 'to-be-valid' credentials

[lleifermann]

While using this library to issue credentials i noticed that the _checkCredential method has some opinionated approach to check both issuanceDate and expirationDate before issuing or verifying a credential.

The VC-Data Model Standard states (https://www.w3.org/TR/vc-data-model/#issuance-date)

The value of the issuanceDate property MUST be a string value of an [XMLSCHEMA11-2] combined date-time string representing the date and time the credential becomes valid, which could be a date and time in the future.

Issuing a credential with an issuanceDate in the future the _checkCredential method will now error. See: https://github.com/digitalcredentials/vc/blob/main/lib/index.js#L624-L628

The same behavior appears for expirationDates which already happened. See: https://github.com/digitalcredentials/vc/blob/main/lib/index.js#L673-L678

Again the VC-Data Model Standard just states that his has to be some time it ceases to become valid. No constraint which time this can be. https://www.w3.org/TR/vc-data-model/#expiration

I believe both of these checks should be removed, or have the ability to be disabled. What are your thoughts on this?

[nklomp]

Not a maintainer, but defaulting to throw an error for a future date makes sense. The credentials are not valid yet according to the issuer, that is what the whole date is about. Let's say I am a new hire and are getting an employment credential, that allows me to prove I am an employee and also allows me to access certain systems for instance. You certainly don't want that to happen before the issuanceDate.

Would be nice to add a flag to allow for it. In some situations you simply want to allow for expired or maybe even revoked and not yet valid VCs. Just plainly removing these checks first of all will change the behaviour of the library. Secondly it will not protect implementers anymore for the vast majority of use cases with VCs.

[lleifermann]

@nklomp But shouldn't this dependency closely reflect what the standard dictates? This is non-standard behavior IMHO. I understand that this check makes sense in verification scenarios, but if i want to issue a credential why check this at all? I just gave the dependency some values to form a credential with. Throwing an error there feels off.

[mirceanis]

I believe most of the checks in _checkCredentials make sense to be run both during issuance and during verification, except for the timestamp checks, which should be skipped during issuance for the same reasons listed by @lleifermann and @nklomp

These should definitely not be skipped by default during verification, but it does make sense to allow them to be skipped with a flag, for questions like "was this credential ever valid"?

---

That being said, should this issue be raised upstream in @digitalbazaar/vc, where this repository was forked from?

[fermentfan]

I don't like the checks in the VC creation if they're based on an opinionated interpretation of the data I supply. The checks should only be done if they're supported by the spec. Of course it makes sense to most persons that an expiration date should be in the past, but why shouldn't I be allowed to create those credentials as an implementer? Test cases for example might require creating such kind of credentials. The failed verification status doesn't mean the credential isn't conforming to the VC spec.

If you are in favour of these checks, then why aren't we doing this for credential status? Following this argumentation there should be credential status stuff happening here else we're just doing half the work.

[lleifermann]

@mirceanis i already opened an issue there as well (https://github.com/digitalbazaar/vc-js/issues/127) but thought that this fork is the more up-to-date implementation of things.

On another note: The _checkCredential is also executed BEFORE checking the signature of the presentation.

See: https://github.com/digitalcredentials/vc/blob/main/lib/index.js#L295-L320

Shouldn't the first action always be checking the signature and then running sanity checks on the contents? If someone now sends me a tampered presentation this library will exit and i will never know that the data i received was actually faulty and will not produce a correct signature.

[mirceanis]

If you are in favour of these checks, then why aren't we doing this for credential status? Following this argumentation there should be credential status stuff happening here else we're just doing half the work.

To be honest I didn't review all the checks but at a brief glance nothing popped out that shouldn't be there.

i already opened an issue there as well (https://github.com/digitalbazaar/vc-js/issues/127)

I did not know that. Good call!

On another note: The _checkCredential is also executed BEFORE checking the signature of the presentation.

This is likely just premature optimization, as it's easier to fail fast during these checks than it is to do all the relatively complicated JSON-LD signature verification. I don't have an opinion about the order of these checks performed during verification.

---

In the meantime, there is a workaround for issuing or verifying credentials or presentations for a different timeline; the now parameter. It doesn't solve this issue completely, but it should be usable for most situations.

[nklomp]

Wait sorry. I missed that it throws an error with an issuanceDate in the future during actual issuance. Upon rereading your ticket, you do say that clearly.

You definitely should be able to issue credentials in the future yes. Next to maintaining current behaviour it would be nice to have an additional argument for checks and issuance to allow for future credentials.

[lleifermann]

This is likely just premature optimization, as it's easier to fail fast during these checks than it is to do all the relatively complicated JSON-LD signature verification. I don't have an opinion about the order of these checks performed during verification.

Just imagine your browser telling you it received malformed HTML and cannot display the webpage before doing any TLS checks :)

The more i think about this, the more i get the feeling that a library like this should give the caller the biggest flexibility for deciding what to accept and what not. Maybe there are even usecases for people who want to issue completely broken and non standard credentials because they have to test for this in their products.

Another analogy why issuing credentials that are expired is also important: Lets imagine my school i went to now adopts credentials and determines to issue every student a 'membership' credential. Every enlisted student would get a normal credential with some expiration-date in the future. But i want to get one too for my time at that school. They would now issue an expired credential to me just for good measure and maybe i can use this credential even it's expired state for some things down the road. For example presenting that once was a valid student. Its up to the verifier to determine what he takes out of the credential being 'expired'.

[mirceanis]

I was already convinced that more flexibility is needed :)

[nklomp]

Yeah. Specs like presentation exchange even allow for explicitly stating you are okay with revoked VCs or VCs having a certain status constraint. https://identity.foundation/presentation-exchange/#credential-status-constraint-feature

It helps if libs allow for this type of flexibility as well.

[dmitrizagidulin]

@lleifermann great point. This is a good opportunity to revisit the timestamps anyway (since VC 2.0 group is hoping to deprecate the previous timestamp fields and rename them)

[lleifermann]

@lleifermann great point. This is a good opportunity to revisit the timestamps anyway (since VC 2.0 group is hoping to deprecate the previous timestamp fields and rename them)

If you don't mind I can pick this up and propose a PR the coming days. WDYT?

[dmitrizagidulin]

@lleifermann sure! that would be great!

[lleifermann]

As this was merged i will close this one. Thanks for the contribution everyone :smiley: