berendsliedrecht / sd-jwt-ts Goto Github PK

JWT libraries I've used in the past (such as jsonwebtoken allow you to pass a nonce (as well as other values such as aud) to the verification method and it will be checked against the value in the JWT.

Is that something we can add to this library as well?

I wanted to add it from the SD-JWT KB level up (as SD-JWT KB requires nonce and aud to be present), but I was running into a problem that the methods all extend the verify from the class they extend and thus we can't really change the parameters for the higher level classes

This seems like a limitation to me that isn't really beneficial as it makes sense that you want to provide more parameters to the SD-JWT VC verify method than you do to the JWT verify method. I think this is mainly if the number of arguments change and it would be fine if we make it a single options object that can be extended with multiple layers.

Something like this: https://www.typescriptlang.org/play?ssl=23&ssc=19&pln=23&pc=37#code/JYOwLgpgTgZghgYwgAgFIHcwDVrBgTwHkAHMYAexAGdkBvAKGWWCqoH4AuZKsKUAc0bI4AVwAmnbrwFCQlJJJ58QggL716CADZxWaTHSEA3XAQAU5UhWpcM2U0SuUqASkNMmQ9evqhIsRBQAZTE7HD4CEjJnZAgAD0gQMRowhyjrGgYmAGsIfAAhUDEBSSyPORAFLiUZD2FxRWkVL3ofbV0aELtYhIgklIMy7IAjboBeZBAIdH0wMxcNJhMI-AsnG2QuzHC8R2jqNzKmKhFiaAA6Zd21-ddFjzAACxZzkbtLhzMjj1ExLktbq88oUkiVzr8ADRCOoVJD-dZUIEFIpg2EQaHIVQLJjeIA

Not fully happy with it, as it means all properties exposed on JwtVerifyOptions (such as aud and nonce are now also exposed in SdJwt verify method (but you don't want to check these in the SD-JWT credential, but rather in the KB-JWT)

I moved the create function into a class because I need to define the signer dynamic at runtime like

  async create<Payload extends Record<string, unknown>>(
    payload: Payload,
    disclosureFrame: DisclosureFrame<Payload>
  ): Promise<SdJwt> {
    const sdJwt = new SdJwt<JWTHeader, Payload>(
      {
        header: {
          alg: SignatureAndEncryptionAlgorithm.EdDSA,
          typ: 'sd-jwt',
        },
        payload: payload,
      },
      {
        saltGenerator,
        signer: this.getSigner.bind(this),
        hasherAndAlgorithm,
        disclosureFrame,
      }
    );
    return sdJwt;
  }

But when I call it I get different errors:

export interface Foo {
  name: string;
  [key: string]: any;
}

const sdJwt = await SdJWTCreator.create(agent);
sdJwt.create<Foo>({ name: 'Mirko' }, { name: true, __decoyCount: 2 });

This will throw an error for name: Type 'string' is not assignable to type 'boolean | undefined'.ts(2322)

When I remove the [key: string]: any definition from my interface, it throws: Type 'Foo' does not satisfy the constraint 'Record<string, unknown>'. Index signature for type 'string' is missing in type 'Foo'.

I think I am defining the types wrong when transforming the examples to a class based approach. An example of it in the code folder would help a lot! :)

When you use the default type Record<string, unknown> this causes issues for nested frames.

So let's say I dezerialize an SD-JWT ey123.ey123.ey123~123~456~ into a sd-jwt the types for Header and Payload will be Record<string, unknown>.

If I now want to do create a presentation frame (or disclosure frame, they're based on the same base frame) it will result in errors If I use a nested object:

{
   // errors as 'nested' key is expected to have boolean | undefined value
   nested: {
    claim: true
  }
}

I think when we don't know the value of something (= unknown) we should not say the value should be boolean, but rather it can be an array with booleans an objects with new key values or a boolean, as we don't know the structure of the object

The spec describes that if no _sd_alg property is present at the top-level, a default value of sha-256 MUST be used.

So I think we need to tweak the checkHasher method to use this as value to compare against the algorithm.

If the _sd_alg claim is not present at the top level, a default value of sha-256 MUST be used.

https://datatracker.ietf.org/doc/html/draft-ietf-oauth-selective-disclosure-jwt-06#name-hash-function-claim

With the split into multiple packages there seems to be quite some duplication between the core and new packages. Some methods are just straight up available in both (e.g. swapClaims) and others have different methods but the logic is duplicated across packages.

Just opening an issue so we can clean this up, as it will probably lead to some inconsistencies if we don't update all code in core to depend on the code in the subpackages.

I get a TypeScript error with the following disclosure frame:

.withDisclosureFrame({
            __decoyCount: 2,
            users: [{ name: true }, true, false],
            user: {
                __decoyCount: 3,
                dateOfBirth: true,
                name: true,
                lastName: false
            },
            license: false
        })

It seems to expect only boolean values for the users array, but it is possible according to the spec to selectively disclose the name property within an array as well.

I think this might be related to #2

The SdJwt.present method currently takes an array of indices. As the doc for the method already says, this is not the most convenient API.

What if we re-use the disclosure frame approach for the present method? This would allow for a much simpler interface, and we could then derive the disclosures that should be exposed based on the payload we want to disclose.

The method could take one option:

• Whether you it to automatically disclose everything that is required based on the disclosure frame, or only disclose exactly what is in the disclosure frame, and throw an error if that is not possible based on the disclosures.

Let's say I have the following SD-JWT payload (in pretty format):

{
  "iss": "something",
  "user": {
     "firstName": "Timo",
     "lastName": "Glastra"
  },
  "license": {
     "number": 10,
     "state": "CA"
  }
}

I could make a present call like this:

sdJWt.present({
  user: {
    firstName: true
  }
}, {
  includeRequiredForDisclosureFrame: true
})

This will return the following object, as the license and iss fields MUST be disclosed.

{
  "iss": "something",
   "user": {
      "firstName": "Timo",
    },
   "license": {
     "number": 10,
     "state": "CA"
    }
}

If user was not separately selectively disclosable, with the includeRequiredForDisclosureFrame, it could return this:

{
  "iss": "something",
   "user": {
      "firstName": "Timo",
      "lastName": "Glastra"
    },
   "license": {
     "number": 10,
     "state": "CA"
    }
}

with the includeRequiredForDisclosureFrame set to false it would throw an error.

For the PEX implementation we could then just include only the properties that are requested in the presentation definition, and based on that we will get the minimally required disclosed values.

Now when installing @sd-jwt/core it will prompt you which version of the other @sd-jwt you want to install as it can't find a match for workspace:*

Currently when doing verification and passing the verifier according to the Verifier interface, the publicKeyJwk is optionally undefined.

I see in AFJ this is resolved by already passing the signerKey when creating the verifier (so you extract the key that the issuer should have used to sign beforehand).

But I think this can be quite confusing and may lead to people not verifying the link between the iss and the key used to sign the credential.

For supporting dids for the cnf claim I'll need to add a way to verify the link between cnf and the KB-JWT signature also, and because we can use dids we can't just extract the jwk from the cnf and the jwt header.

So I was thinking of implementing a similar interface as we have in the AFJ JWS service to add a jwkResolver. Based on some input fields, you need to return a JWK.

The input will be what is present in a JWT header + iss, or what is present in the cnf claim.

E.g. it could look something like this:

sdJwt.withJwkResolver(({ kid, iss, jwk, /* in future e.g. x5c could also be supported */ }) => {
  if (kid) {
    if (kid.startsWith('did:') {
      // resolve did to JWK

		return jwk
    }
  }

})

// or pass directly to `verify` method:
sdJwt.verify({
  jwkResolver
})
``


Thoughts on this? that way the verify method would ALWAYS get a `publicKeyJwk`, and you MUST verify that the signature is signed with a private key that matches that publicKeyJwk. I think this makes it a bit more explicit that you should verify the relation between these two

The payload property serves two purposes: One is the unsigned/pretty payload that is used before the SD-JWT is signed, and then it is overwritten once the payload is signed with the correct _sd values etc..

This makes the value very vague, and incosistent, and I think it should be split up into two different properties, or at least one of the two purposes should not use the payload property if this value is exposed publicly.

The SD-JWT spec describes the possibility of recursive disclosures. Where you can have a selectively discloseable value, that then contains selectively discloseable values within the selectively discloseable value: https://www.ietf.org/archive/id/draft-ietf-oauth-selective-disclosure-jwt-06.html#name-example-sd-jwt-with-recursi

Doesn't seem like a really important feature to support right away, but opening this issue to keep track of the status.

We would probably have to add another field that can be added to the disclosureFrame to indicate the property itself should be recursively dislcosed. Currently when you provide the following dislcosure frame:

{
            credentialSubject: true,
            __decoyCount: 2,
            credential: {
                __decoyCount: 3,
                dateOfBirth: true,
                name: true,
                lastName: true
            }
        }

You get the following output (kinda)

{
    iat: 1700555090380,
    iss: 'did:key:some-random-did-key',
    nbf: 1700555090480,
    credential: { _sd: ['credential-disclosure-digest-1', 'credential-disclosure-digest-2'] },
    _sd_alg: 'sha-256',
    _sd: [
      'txiegb4QZDKwnzkeYSzDCqdbR9uHUSU7RW2uPIFO2_k',
      'uNPp8xv9pJwWUkMDY17jEiUuPjIcZOJwS75GXJ8SIW8',
      'xZapa_MT9GR3NpyhrgEyM12sUNOknfRYTSahiQISUlM'
    ]
  }

Following the structure of __decoyCount, I think we can do something like this for the disclosure frame:

{
            credentialSubject: true,
            __decoyCount: 2,
            credential: {
               __recursiveDisclosure: true
                __decoyCount: 3,
                dateOfBirth: true,
                name: true,
                lastName: true
            }
        }

To receive this output:

{
    iat: 1700555090380,
    iss: 'did:key:some-random-did-key',
    nbf: 1700555090480,
    _sd_alg: 'sha-256',
    _sd: [
      'txiegb4QZDKwnzkeYSzDCqdbR9uHUSU7RW2uPIFO2_k',
      'uNPp8xv9pJwWUkMDY17jEiUuPjIcZOJwS75GXJ8SIW8',
      'xZapa_MT9GR3NpyhrgEyM12sUNOknfRYTSahiQISUlM',
      'this-is-the-digest-for-credential-the-disclosure-contains-the-nested-disclosures'
    ]
  }

Not fully happy with the __recursiveDisclosure name yet, but I hope you get the idea.

One of the quirks of SD-JWT is that the disclosures themselves can be encoded however the issuer deems right. There is no necessity to define rules like always using minified JSON or certain characters etc.

This means that as a holder, you have to keep the original base64 disclosure to compute hashes and disclose them. From my understanding you are parsing the disclosures and only working on the parsed disclosures - losing the initial encoding. As long as the issuer uses the same kind of encoding (e.g., JSON minified with utf-8) everything is fine, but if for example the JSON is not minified by the issuer the result will be different hashes breaking everything.

I created a small test based on your examples that uses an example of the sd-jwt-vc spec (with the python implementation from Daniel that does not minify JSON) that loses the disclosures in the process: https://gist.github.com/c2bo/06bb2fc028c31e01a5ba9108e2f738df

Output is

{
  iss: 'https://example.com/issuer',
  iat: 1683000000,
  exp: 1883000000,
  vct: 'https://credentials.example.com/identity_credential',
  cnf: {
    jwk: {
      kty: 'EC',
      crv: 'P-256',
      x: 'TCAER19Zvu3OHF4j4W4vfSVoHIP1ILilDls7vCeGemc',
      y: 'ZxjiWWbZMQGHVWKVQ4hbSIirsVfuecCE6t4jT9F2HZQ'
    }
  }
}

My understanding would be, that the output of getPrettyClaims() should include the resolved disclosures, for example

"given_name": "John"

which does not happen because the calculated digests differ from the ones in the SD-JWT (because of different encoding). Output of the digests results in:

Disclosures and digests differ:

"WyIyR0xDNDJzS1F2ZUNmR2ZyeU5STjl3IiwiZ2l2ZW5fbmFtZSIsIkpvaG4iXQ" -> ["2GLC42sKQveCfGfryNRN9w","given_name","John"]

whereas the issuer provided

"WyIyR0xDNDJzS1F2ZUNmR2ZyeU5STjl3IiwgImdpdmVuX25hbWUiLCAiSm9obiJd" -> ["2GLC42sKQveCfGfryNRN9w", "given_name", "John"]

E.g. payload for SD-JWT VC MUST include vct and some other properties. So that means:

• the input payload must have the property
• and after validation (that checks if vct is present) the property should also be there

currently almost all interfaces are types as Record<string, unknown> for the header and payload, which makes the library API less nice to work with

See https://datatracker.ietf.org/doc/html/draft-ietf-oauth-selective-disclosure-jwt-06#name-key-binding-jwt