Non-matching vc_paths included in the matches array returned by selectFrom for pick requirements

[DJHunn39]

• I'm submitting a ...

  • bug report
  • feature request
  • question about the decisions made in the repository
  • question about how to use this project

• Summary

When credentials are selected with PEX.selectFrom, with a presentationDefinition with one pick submission requirement where each input_descriptor in the from group is satisfied by a different credential, selectFrom returns an object with a matches array containing only 1 element with the name of the last input_descriptor, and the vc_path of every credential that matches each input_descriptor (not just the last one), so the name of srm ends up not corresponding to every credential under each vc_path in the srm.

This issue means that, if the first "matching" verifiableCredential is used to submit a presentation, the credential used for submission does not match the requirements of the input_descriptor, so a Could not get evaluation results from presentationResult error is thrown after PEX.evaluatePresentation fails to produce an object with a value property.

• Other information

I'd expect the matches array returned by selectFrom to contain 1 element per input_descriptor with a match, with each element having the name of the input_descriptor it's describing a match for, and only the vc_path of the credential(s) that match that input_descriptor.

Here's a more thorough explanation of the issue from our perspective:

• A presentation request is made to our digital wallet (which is essentially a Credo holder agent, holding exclusively SD-JWT VCs, where presentation is performed using OID4VP). The presentation definition for this request is as follows:

{
  id: `OverAgeCheck`,
  purpose: 'Age check',
  submission_requirements: [
    {
      name: 'Proof of age and photo',
      rule: 'pick',
      count: 1,
      from: 'validAgeCheckInputDescriptor'
    }
  ],
  input_descriptors: [
    {
      name: 'ID date of birth and photo',
      id: 'IDDoB',
      group: ['validAgeCheckInputDescriptor'],
      constraints: {
        limit_disclosure: 'required',
        fields: [
          {
            path: ['$.vc.type.*', '$.vct', '$.type'],
            filter: {
              type: 'string',
              const: 'ID'
            }
          },
          {
            path: ['$.Photo']
          },
          {
            path: ['$.BirthDate']
          }
        ]
      }
    },
    {
      name: 'Driving licence date of birth and photo',
      id: 'DrivingLicenceDoB',
      group: ['validAgeCheckInputDescriptor'],
      constraints: {
        limit_disclosure: 'required',
        fields: [
          {
            path: ['$.vc.type.*', '$.vct', '$.type'],
            filter: {
              type: 'string',
              const: 'DRIVING_LICENCE'
            }
          },
          {
            path: ['$.Driver_Photo']
          },
          {
            path: ['$.Driver_Dob']
          }
        ]
      }
    }
  ]
}

Basically, we're trying to verify the age of the wallet holder, and we'll accept relevant data from either an ID credential or a Driving Licence

• The wallet holds an ID, so it can meet the requirements of the IDDoB input_descriptor, and it also holds a Driving Licence, so it can also meet the requirements of the DrivingLicenceDoB input_descriptor
• However, when the presentation request is resolved, we see that resolvedPresentationRequest.presentationExchange.credentialsForRequest.requirements[0].submissionEntry array contains one object, with an inputDescriptorId of DrivingLicenceDoB, but a verifiableCredentials array that contains both the ID credential as well as the Driving Licence, which doesn't align with the more specific inputDescriptorID.
• The wallet automatically selects the first entry in the verifiableCredentials array to use for the submission. If the first entry is the ID credential, and we attempt to call the Credo holder agent's acceptSiopAuthorizationRequest method with it, we get the Could not get evaluation results from presentationResult error from PEX, so the request cannot be accepted, because an ID can't be used to satisfy a DrivingLicenceDoB input.

In the above scenario, submissionEntry.verifiableCredentials is built by:

• Finding the correct match from the matches array returned by PEX.selectFrom for a given input_descriptor based on said input_descriptor's name, or id if it had no name.
• Finding each matching verifiableCredential in the verifiableCredential array returned by PEX.selectFrom based on the vc_paths in the match for this input_descriptor

Possibly relevant links to code:

• Credo -> PEX.selectFrom during presentation request resolution
• mapMatchingDescriptors, which appears to be where we go from multiple input_descriptors with a matching credential for each to one srm with the name of 1 input_descriptor but including the vc_paths of all matching credentials for all input_descriptors, so I think this is where a fix could be applied (but I'm not familiar enough with the codebase to know with any certainty)
• Credo's getSubmissionForInputDescriptor, where submissionEntry.verifiableCredentials is generated using the results of PEX.selectFrom

Happy to try and provide more info/apply any changes.

For now, we have a workaround where the wallet always selects the last element in submissionEntry.verifiableCredentials for use in a submission, instead of the first, but this isn't a true solution given the presence of the underlying problem.

@TimoGlastra tagging you after our chat about this this morning

[DJHunn39]

Quick update - the simple workaround turned out to be flawed. I've now patched the package, updating EvaluationClientWrapper.mapMatchingDescriptors like so:

/*
   * Rough fix for #161
   * - If the submission requirement requires credentials to be picked from one/many input descriptors
   * - Build an array of srms, one per input descriptor with a matching marked credential.
   * - If multiple marked credentials match the same input descriptor, each of their vc_paths are added to the srm.
   * - Return the first srm in the array - this works for my use case (pick 1 input descriptor in a group), but seems like a patchy/hacky fix, and likely won't work in more complex scenarios.
   */
  private mapMatchingDescriptors(
    pd: IInternalPresentationDefinition,
    sr: SubmissionRequirement,
    marked: HandlerCheckResult[],
  ): SubmissionRequirementMatch {
    const srm: Partial<SubmissionRequirementMatch> = { rule: sr.rule, vc_path: [] };
    if (sr?.from) {
      // Set up an array of srms for all matches - 1 srm per input descriptor with matching credentials
      const srms: Partial<SubmissionRequirementMatch>[] = [];

      for (const m of marked) {
        // Get the full input descriptor that this marked cred is a valid input for
        const inDesc: InputDescriptorV2 = jp.query(pd, m.input_descriptor_path)[0];

        // Check that this input descriptor's group matches the group that the submission requirement requires
        if (inDesc.group && inDesc.group.indexOf(sr.from) === -1) {
          continue;
        }

        // If we already have an SRM for this inputDesc, add this match to it
        const srmNames = srms.map((srm) => srm.name);
        if (srmNames.indexOf(inDesc.name) === -1 && srmNames.indexOf(inDesc.id) === -1) {
          srms.push({ from: sr.from, name: inDesc.name || inDesc.id, rule: sr.rule, vc_path: [] });
        }

        const matchingSrm = srms.find((srm) => srm.name === inDesc.name || srm.name === inDesc.id);
        if (!matchingSrm) {
          continue;
        }

        if (m.payload.group.includes(sr.from)) {
          if (matchingSrm.vc_path?.indexOf(m.verifiable_credential_path) === -1) {
            matchingSrm.vc_path!.push(m.verifiable_credential_path);
          }
        }
      }
      return srms[0] as SubmissionRequirementMatch;
    }
    return srm as SubmissionRequirementMatch;
  }

As mentioned in the comment, this seems to work for me (and all unit tests still pass if I apply these changes in the actual lib), but the end user loses the ability to pick the submission themselves as only one match is returned. It also wouldn't work for submissions that require a user to pick creds matching multiple entries from a group, so I'm not going to open a PR for this change as is.

Keen to get thoughts on those with more familiarity with this codebase.

[TimoGlastra]

I can spend time on a fix for it, but my understanding of these lower level components is a bit limited. Maybe if @nklomp can chime in on the findings to confirm whether it is correct that all matches will have the same input descriptor, and what the desired outcome should be?

[TimoGlastra]

I've done some more research into this, and I think PEX actually returns the correct results here. There are two problems however:

• 1. Credo uses the name property to match the select result to the input descriptor. However in the case of submission requirement, we should look at the from property and extract the group name of the submission requirement. So it does return both credentials, but that means because it is returning a match for the submission requirement (which is pick one credential from this group ,and both input descriptors match this group).
• 2. PEX returns a name of one of the input_descriptors. But in the case of a match for a submission requirement, i think it should return the name of the submission_requirement instead. In addition, and this is something we should fix in PEX, the match only returns the name property. But I think it should also return a type (SubmissionRequirement or InputDescriptor) and an id (the input descriptor id, or the submission requirement index). This way it will be possible to deterministicly match the results back to the presentation definition. I have opened an issue for this a while ago Difficult/impossible to match matches against the input_descriptors that satisfied them  #116)

So we can solve this on the Credo layer, but to properly fix it in Credo, we need to address #116 first. I'll take a look at that now, as if we already have the name it should be quite straightforwrad to also add the id.