hyperledger / anoncreds-clsignatures-rs

Apache License 2.0
7 stars 12 forks source link

Schema claims are optional, but creating a proof for a credential with optional claims fails #42

Open goncalo-frade-iohk opened 4 months ago

goncalo-frade-iohk commented 4 months ago

Hi there.

I found this issue, and as per my understanding a credential issued CAN have any of the claims specified on the schema.

So basically if I got a schema like:

{
    "name": "Driving licence Anoncred Schema",
    "version": "1.0",
    "attrNames": [
        "emailAddress",
        "familyName",
        "dateOfIssuance",
        "drivingLicenseID",
        "drivingClass"
    ],
    "issuerId": "did:test:mock"
}

If then I issue a credential with only "familyName". The credential is issued successfully, but because of this code, any credential that doest fully have all claims defined of a schema will not be able to create a proof:

let schema_attrs = non_credential_schema
            .attrs
            .union(&cred_schema.attrs)
            .cloned()
            .collect::<BTreeSet<String>>();

        let cred_attrs = BTreeSet::from_iter(cred_values.attrs_values.keys().cloned());

        if schema_attrs != cred_attrs {
            return Err(err_msg!(
                "Credential doesn't correspond to credential schema"
            ));
        }

Here is the location of the code: https://github.com/hyperledger/anoncreds-clsignatures-rs/blob/3ba4f232616ae2504efb31db8b093f18674d6410/src/prover.rs#L1137

If this logic is correct then IMO a credential cannot be issued without all the claims on a schema, and should fail there.

berendsliedrecht commented 4 months ago

I just went over the AnonCreds specification quickly, and I could not find that a subset of the schema may be issued instead of every attribute.

Do you have a link to where it defines that a subset of the claims may be issued in a credential?

This is the only relevant text I could find:

"The Schema is a JSON structure that can be manually constructed, containing the list of attributes (claims) that will be included in each AnonCreds credential of this type."

And to me this reads as only the full set of attributes may be issued as a credential.

goncalo-frade-iohk commented 4 months ago

@berendsliedrecht Indeed I could not find it either, so im going with the current implementations.

That's why we got a inconsistency because right now we can ISSUE a credential that doesn't fulfil all the claims of the schema, but later the Prover cannot create a presentation with the credential.

So IMO if this logic is correct then any credential without all the claims in a schema is invalid, and the issuing should fail.

Would this be an issue more for anoncreds-rs?

swcurran commented 4 months ago

The specification is intended to say that you can’t have a credDef that doesn’t match the schema, and you can’t issue credentials that don’t match the CredDef. Some have requested such a feature be enabled, but the spec editors decided that is not appropriate and should not be supported.

If issuing a subset is possible, that is a bug that should be corrected.

andrewwhitehead commented 4 months ago

This library previously allowed for optional credential attributes, which was behaviour inherited from indy-sdk. There were checks added recently to reject such credentials during issuance and proving, while they were already rejected during verification. For attributes defined by the credential definition, there cannot be 'optional' attributes as they would effectively be signed as the zero integer value and could be presented that way.