decentralized-identity / did-jwt

Create and verify DID verifiable JWT's in Javascript
Apache License 2.0
331 stars 70 forks source link

Implementing JWE for P256 #295

Closed bshambaugh closed 2 months ago

bshambaugh commented 11 months ago

I am starting to work through #225 and the first file that I changed was ECDH.ts. Please forgive my deletion of yarn.lock. If this is an issue, perhaps I can address it later. This does not have tests yet, but neither does the prexisting x25519 function. I do need to make sure that when I get a compressed P256 public key as input to getSharedSecret I get the correct results.

bshambaugh commented 11 months ago

I did git checkout upstream/master -- yarn.lock , git add yarn.lock , git commit -m "add removed yarn.lock file" , and git push -u origin master to get to the present state on my fork. I thought that diff for yarn.lock would show no changes, but it still does.

mirceanis commented 11 months ago

I did git checkout upstream/master -- yarn.lock , git add yarn.lock , git commit -m "add removed yarn.lock file" , and git push -u origin master to get to the present state on my fork. I thought that diff for yarn.lock would show no changes, but it still does.

Perhaps try to git fetch --all before the checkout

bshambaugh commented 11 months ago

I cloned this repository in a new directory and compared it to the fork and did what you said with git fetch and then did a local diff on yarn.lock between my fork and the clone. I even browsed through the file on github. The file yarn.lock doesn't appear to be different, yet it is showing differences in files changed for this PR.

bshambaugh commented 11 months ago

But, the last change in git log that is not by me is on sept 8th. On this upstream remote, the last change is sept 27th,

bshambaugh commented 11 months ago

git merge origin/master upstream/master then git push did the trick.

bshambaugh commented 10 months ago

Awesome. I was gone for 2 weeks in the 1st half of October for and IIW et al. trip, but I have started to look at JWE again. This means I have been combing through the code base and searching the web and books and making notes in the past few weeks.

tl;dr: good timing. more code may be coming soon.

bshambaugh commented 9 months ago

As I was looking through some repositories on didcomm I realized that there was some replication: https://github.com/decentralized-identity/veramo/tree/next/packages/did-comm/src/encryption

This is not bad because I realized I was doing similar things Screenshot_2023-11-22_14-04-20

Screenshot_2023-11-22_14-04-34

mirceanis commented 9 months ago

As I was looking through some repositories on didcomm I realized that there was some replication

The code duplication from the did-comm implementation you linked to is not from my proudest moments..

This is not bad because I realized I was doing similar things

There are definitely improvements to be made, so if you do manage to avoid duplication, that would be even better.


also maybe related: https://github.com/paulmillr/noble-ciphers/discussions/18

bshambaugh commented 9 months ago

Status update:

I'm trying to refactor code, but I am getting stuck. I apologize for the screenshots, but it seems to allow me to make this concise.

In https://codesandbox.io/p/github/bshambaugh/did-jwt/master?file=%2Fsrc%2Fencryption%2[…]1%2C1-84%2C1&workspaceId=2a49a562-e345-4cca-bc1f-efecc0ea40b6

Start also looks like the form in createEncrypterStart.ts

Finish looks like the form in createEncrytperFinal.ts

start finsh

createEncrypterFinal.ts will be renamed to createEncrypter.ts

crv is a property in JsonWebKey in type KekCreator

export type KekCreator = {
  createKek(
    recipientPublicKey: Uint8Array,
    senderSecret: Uint8Array | ECDH | undefined,
    // key agreement + key wrapping algorithm. e.g. ECDH-ES+A256KW
    alg: string,
    apu: string | undefined,
    apv: string | undefined,
    ephemeralKeyPair: EphemeralKeyPair | undefined
  ): Promise<{ epk: JsonWebKey; kek: Uint8Array }>

  // key agreement algorithm
  alg: 'ECDH-ES' | 'ECDH-1PU' | string
}
export type JsonWebKey = {
  crv: string
  kty: string
  x?: string
  y?: string
  // eslint-disable-next-line @typescript-eslint/no-explicit-any
  [key: string]: any
}

crv is also a property in EphemeralPublicKey

export interface EphemeralPublicKey { kty?: string //ECC crv?: string x?: string y?: string //RSA n?: string e?: string }

EphemeralPublicKey is in EphemeralKeyPair:

export interface EphemeralKeyPair {
  publicKeyJWK: EphemeralPublicKey
  secretKey: Uint8Array
}
bshambaugh commented 9 months ago

Okay, https://github.com/bshambaugh/did-jwt/blob/X25519-ECDH-1PU-ES/src/encryption/createEncrypter.ts seems okay, but needs to be tested.

There are multiple feature branches going on. X25519-ECDH-1PU-ES just keeps the same alg and enc parameters as for crv x25519 ( const alg = 'ECDH-ES+XC20PKW' const enc = 'XC20P') but changes the crv to P-256. as256kwExperiments tries to introduce ECDH-ES-A256KW as the alg and A256GCM as the enc and changes to crv to P-256.

For X25519-ECDH-1PU-ES this still needs to be fixed: https://github.com/bshambaugh/did-jwt/blob/X25519-ECDH-1PU-ES/src/encryption/xc20pEncryption.ts#L145-L191

For as256kwExperiments a lot still needs to be done. You can see the present noble and stablelib having to be there to provide GCM and KW in https://github.com/bshambaugh/did-jwt/blob/as256kwExperiments/src/encryption/a256kwgcm.ts . createEncrypter.ts in the as256kwExperiments branch will need to look like createEncrypter.ts in the X25519-ECDH-1PU-ES branch...

& tests for both.

bshambaugh commented 9 months ago

https://github.com/bshambaugh/did-jwt/blob/X25519-ECDH-1PU-ES/src/encryption/createEncrypter.ts#L28 was added

ephemeralKeyPair?: EphemeralKeyPair

to get https://github.com/bshambaugh/did-jwt/blob/X25519-ECDH-1PU-ES/src/encryption/createEncrypter.ts#L80-L84

if(ephemeralKeyPair?.publicKeyJWK.crv != null) { //uses added ephemeralKeyPair, the one in encryptCek not in scope
    return { alg: keyWrapper.alg, enc: contentEncrypter.enc, encrypt, encryptCek, genEpk: prefixToDriverMap[ephemeralKeyPair!.publicKeyJWK.crv!] } 
  } else {
    return { alg: keyWrapper.alg, enc: contentEncrypter.enc, encrypt, encryptCek, genEpk: genX25519EphemeralKeyPair }
  }

to work. Except it doesn't yet[?]. I need a better way to construct a conditional which depends on the cryptographic curve. https://gist.github.com/bshambaugh/9953749a2d1d985b8eea5d6bb2405b43

I added resolveP256Encrypters which is like resolveX25519Encrypters

https://github.com/bshambaugh/did-jwt/blob/X25519-ECDH-1PU-ES/src/encryption/xc20pEncryption.ts#L449-L504 https://github.com/bshambaugh/did-jwt/blob/X25519-ECDH-1PU-ES/src/encryption/xc20pEncryption.ts#L145-L191

which is what @oed exported. https://github.com/oed/did-jwt/tree/fix/export-x25519-resolver .

bshambaugh commented 9 months ago

I fixed some stuff and All tests on my X25519-ECDH-1PU-ES branch appear to pass now.

stale[bot] commented 4 months ago

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.

bshambaugh commented 4 months ago

not stale. still working on.

stale[bot] commented 2 months ago

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.