Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[proposal] SD-jwt integration #1276

Open
cre8 opened this issue Oct 18, 2023 · 16 comments
Open

[proposal] SD-jwt integration #1276

cre8 opened this issue Oct 18, 2023 · 16 comments
Labels
enhancement New feature or request pinned don't close this just for being stale planned-feature don't close this just for being stale work-in-progress

Comments

@cre8
Copy link
Contributor

cre8 commented Oct 18, 2023

Is your feature request related to a problem? Please describe.
The SD-JWT is used in the ARF and gets a lot of attention by EU projects. Main key points are the capability of selective disclosure AND being built on the widely adopted format JWT.

Describe the solution you'd like
We should be able to use some of the existing libraries to create sd-jwts and create a wrapper to use them in Veramo.

Describe alternatives you've considered
Right now I see no other approach using selective disclosure with Veramo (maybe with micro credentials where each credential only includes one value)

Additional context
We need to challenge how the existing selective disclosure approach can be used or even becomes a problem when integrating sd-jwts.

@cre8
Copy link
Contributor Author

cre8 commented Oct 18, 2023

I think using this library as a base dependency should be fine: https://github.com/transmute-industries/vc-jwt-sd or https://github.com/meeco/sd-jwt

I would not suggest to implement the logic again in an own package.

@nklomp
Copy link
Member

nklomp commented Dec 14, 2023

Did I read correctly you are working on this @cre8 using the library created by Berend? https://github.com/berendsliedrecht/sd-jwt-ts

Reason I am asking is that work supporting sd-jwt is also landing in our lower level libraries. Which is based upon some future seperation of libraries from the above library. Meaning we do not have to depend on the full library. However for us at Sphereon the missing component would obviously be the integration into Veramo.

So if you are working on that happy to help or have a look at it, given we for sure will need it soon as well.

@cre8
Copy link
Contributor Author

cre8 commented Dec 14, 2023

Hey @nklomp ,
I only had some free time for a quick PoC like:

import { KeyLike, createHash, getRandomValues, verify } from 'node:crypto';
import {
  SaltGenerator,
  HasherAlgorithm,
  HasherAndAlgorithm,
  SdJwt,
  SignatureAndEncryptionAlgorithm,
  DisclosureFrame,
  Verifier,
} from '@sd-jwt/core';
import { TAgent } from '@veramo/core';
import { AgentType } from './agent';
import { getDefaultDID, getDefaultKid } from '@ssi/agent';
import { base64url, importJWK } from 'jose';
import { bytesToBase64url, base58ToBytes } from '@veramo/utils';
import { VerificationMethod } from 'did-resolver';

/**
 * Hasher and algorithm that will be used to hash the disclosures.
 */
export const hasherAndAlgorithm: HasherAndAlgorithm = {
  hasher: (input: string) => createHash('sha256').update(input).digest(),
  algorithm: HasherAlgorithm.Sha256,
};

/**
 * Creates a salt that can be used for the creation process.
 * @returns
 */
const saltGenerator: SaltGenerator = () =>
  getRandomValues(Buffer.alloc(16)).toString('base64url');

/**
 * Header of the JWT
 */
type JWTHeader = {
  alg?: SignatureAndEncryptionAlgorithm.EdDSA;
  typ?: 'sd-jwt';
};

/**
 * The JWT that is used to create the presentation.
 */
export class SdJwtVerifier {
  /**
   * Creates a new instance of the SdJwtVerifier.
   * @param agent
   */
  constructor(private agent: TAgent<AgentType>) {}

  /**
   * Verifies the presentation.
   * @param presentation
   * @param claims
   * @returns
   */
  async verify<Payload>(
    presentation: string,
    //using | string will ignore the auto completion. But when we remove the string option, we need to add the Payload attribute, but will get not suggestions.
    claims: Array<keyof Payload | string>
  ) {
    const sdJwtVerifier =
      SdJwt.fromCompact(presentation).withHasher(hasherAndAlgorithm);

    const verifyCb: Verifier = async ({ header, message, signature }) => {
      //get the public key from the issuer
      const didDocument = await this.agent
        .resolveDid({
          didUrl: sdJwtVerifier.payload.iss as string,
        })
        .then((res) => res.didDocument!);
      const key = didDocument.verificationMethod!.find(
        (vm) => vm.id === sdJwtVerifier.payload.iss
      ) as VerificationMethod;

      const jwk: KeyLike = (await importJWK({
        kty: 'OKP',
        crv: 'Ed25519',
        x: bytesToBase64url(base58ToBytes(key.publicKeyBase58!)),
      })) as KeyLike;

      if (header.alg !== SignatureAndEncryptionAlgorithm.EdDSA) {
        throw new Error('only EdDSA is supported');
      }
      return verify(null, Buffer.from(message), jwk, signature);
    };

    // list the values from the presentation
    return sdJwtVerifier
      .verify(verifyCb, claims as string[])
      .then((res) => res.isValid);
  }
}

/**
 * Creates a JWT that can be used to create a presentation.
 */
export class SdJWTCreator {
  keyId: string;

  /**
   * Creates a new instance of the SdJWTCreator.
   * @param agent
   * @param agentConfig
   * @returns
   */
  static async create(agent: TAgent<AgentType>): Promise<SdJWTCreator> {
    const did = await getDefaultDID(agent);
    if (!did) throw Error('No default DID found');
    const keyRef = await getDefaultKid(agent, { did });
    if (!keyRef) throw Error('No default key found');
    return new SdJWTCreator(agent, keyRef, did);
  }

  /**
   * Creates a new instance of the SdJWTCreator.
   * @param agent
   * @param keyRef
   * @param did
   */
  constructor(
    private agent: TAgent<AgentType>,
    private keyRef: string,
    private did: string
  ) {
    this.keyId = this.keyRef;
  }

  /**
   * Gets the signer that will be used to sign the JWT.
   * @param input
   * @param header
   * @returns
   */
  private async getSigner(input: string, header: JWTHeader): Promise<Buffer> {
    if (header.alg !== SignatureAndEncryptionAlgorithm.EdDSA) {
      throw new Error('only EdDSA is supported');
    }
    const signature = await this.agent.keyManagerSign({
      keyRef: this.keyRef,
      data: input,
    });
    return Buffer.from(base64url.decode(signature));
  }

  /**
   * Creates a new JWT.
   * @param payload
   * @param disclosureFrame
   * @returns
   */
  create<Payload extends Record<string, unknown>>(
    payload: Payload,
    sub: string,
    disclosureFrame: DisclosureFrame<Payload>
  ): SdJwt<JWTHeader, Payload> {
    return new SdJwt<JWTHeader, Payload>(
      {
        header: {
          alg: SignatureAndEncryptionAlgorithm.EdDSA,
          typ: 'sd-jwt',
        },
        payload: {
          ...payload,
          iss: `${this.did}#${this.keyId}`,
          sub,
          iat: new Date().getTime(),
        },
      },
      {
        saltGenerator,
        signer: this.getSigner.bind(this),
        hasherAndAlgorithm,
        disclosureFrame,
      }
    );
  }
}

I don't think I will have time in the near future to implement a version that is worth to be published in case of quality and that integrates fully in veramo in parallel to the w3c stuff on my own ;) But I am happy to support any work sphereon is on!

@nklomp
Copy link
Member

nklomp commented Dec 14, 2023

Thanks. That does look like a start. Basically removing the dep on node crypto/jose and then moving them to one or two agent plugins would provide a nice start.

Will have a look how to go about it next week, given we do need to start implementing it.

@nickreynolds @mirceanis any insights, objections?

@mirceanis
Copy link
Member

No objections from me.
I'm glad the work is starting with a discussion so that we can avoid duplication and arrive at a good implementation faster.

The draft implementation is a great start. I agree with @nklomp that we shouldn't depend on node:crypto, and for similar reasons avoid Buffer

I'm happy to support this work in any way that is needed, even help publish it under @veramo-community if you think it's too early for core @veramo

Also, maybe we can finally replace the uPort-era selective-disclosure package with a proper sd-jwt plugin 🚀

@nklomp
Copy link
Member

nklomp commented Dec 14, 2023

Yeah, I think it makes sense to think about it a bit. We are also working on an Mdoc/MDL implementation. Whether that should be supported in the future of course needs to be discussed, but the selective disclosure in Mdocs is very similar to SD-JWT, so it might make sense to abstract some concepts a bit more. Probably it makes sense to start out in @veramo-community before we can stabelize the API a bit

@mirceanis
Copy link
Member

it might make sense to abstract some concepts a bit more.

I agree, and I'll be looking for your input into a good enough abstraction for these.

Probably it makes sense to start out in @veramo-community before we can stabelize the API a bit

I forgot to mention that we also have the option to use an @alpha specifier on the new plugin, to flag it.
I know we've been overusing the @beta tag but then treating it as @public when counting breaking changes anyway.
As this framework keeps growing, I'd like us to agree on a stability/maturity specifier for various features.

@cre8
Copy link
Contributor Author

cre8 commented Dec 15, 2023

@mirceanis when adding sd-jwt and maybe also mdoc, I would love to remove the current selective disclosure approach. It is not aligned with the current technologies used out there and could mislead developers to use it. To not break any current systems, I suggest to add a deprecated warning to the package and the functions to inform the users that there are better ways to go now.

Also we should keep in mind for sd-jwt, that the usage of a did is the issuer is not required. The HAIP protocol used by Germany is using a X509 chain or a reference to a public key under .well-known. For the first version I would be fine with did only, but we should keep that in mind for the future.

@nklomp in case you published a package under the sphereon repo just give me a ping and I can review it :)

@mirceanis
Copy link
Member

@cre8 agreed. I'll add a deprecated warning for the next release.

Also we should keep in mind for sd-jwt, that the usage of a did is the issuer is not required.

This is true for credentials as well and I'd love to discuss how to handle cases like this in general.
On the issuer side it's not very difficult as it's mostly just about adding an issuer string somewhere, but on the verifier side it gets more complicated.

Let's discuss this in a different location, though

@Eengineer1
Copy link
Contributor

Eengineer1 commented Jan 3, 2024

Glad to see progress with this initiative.

Here's a Node.js (browser will follow) plugin implementation based on the underlying SD-JWT library I wrote as well:

https://github.com/Eengineer1/sd-jwt-veramo

It's tied 1-1 to the base frame implementation of credential-w3c, in an easy way to ultimately natively support within.

Regarding dependencies, there's no direct import of Node.js crypto, being the sole engine specific direct dependency, given an extensible underlying crypto provider written with already existing Veramo crypto primitives.

That is, the package will be able to technically run in the browser, regardless, in the next release.

@lukasjhan
Copy link

Hi guys, I want to contribute sd-jwt integration for Veramo.

I'm currently developing the SD JWT project at the Open Wallet Foundation. Here is the link. (https://github.com/openwallet-foundation-labs/sd-jwt-js)
Welcome any feedback and evaluation for the contribution.

Also, reading from the thread, I’ve realized that you prefer a SD JWT project that doesn't have node crypto, jose, or broswer dependencies.
So I'm considering to implement sd-jwt-js in that direction.

Will it be better to contribute with these conditions are included in our project?

Copy link

stale bot commented May 4, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label May 4, 2024
@mirceanis mirceanis added enhancement New feature or request work-in-progress planned-feature don't close this just for being stale pinned don't close this just for being stale and removed wontfix This will not be worked on labels May 4, 2024
@cre8 cre8 removed their assignment May 6, 2024
@cre8
Copy link
Contributor Author

cre8 commented May 29, 2024

Hey all, I do not have the time right now to continue the work of the veramo project. I am fine when it get migrated from https://github.com/cre8/sd-jwt-veramo the to official repo.
By doing this, I think it's also important to update the selective disclosure documentation since this approach is not aligned with it.

@lukasjhan
Copy link

lukasjhan commented May 29, 2024

Thank you @cre8.
In fact, I'm migrating your project in #1358. I'm currently try to add sd-jwt features in Veramo react playground as @nickreynolds requested.

If you suggest a different approach, please let me know. I'll proceed the way you say :)

@cre8
Copy link
Contributor Author

cre8 commented May 29, 2024

@lukasjhan sounds good to me. Reach out when you need any help :)

@lukasjhan
Copy link

Thank you :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request pinned don't close this just for being stale planned-feature don't close this just for being stale work-in-progress
Projects
None yet
Development

No branches or pull requests

5 participants