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

Refactor KeyType #38

Open
amika-sq opened this issue Dec 5, 2023 · 8 comments
Open

Refactor KeyType #38

amika-sq opened this issue Dec 5, 2023 · 8 comments
Assignees

Comments

@amika-sq
Copy link
Contributor

amika-sq commented Dec 5, 2023

KeyType is confusing. Use what Kotlin does and separate out Algorithm/Curve.

@decentralgabe
Copy link
Member

decentralgabe commented Dec 5, 2023

Alg/Curve isn't the right abstraction either, unfortunately, since it doesn't cover non-curve key types like RSA or Dilithium.

For example, an RSA JWK would use the kty value of RSA with no crv property. A Dilithium JWK would use thekty value LWE with no crv property, but necessitate the alg of CRYDI5.

I would recommend we either stick to key type or create unique mappings of [alg, crv?, kty] which we can identify like JOSE does.

Of course this raises the question of whether we're using JOSE for everything. If we are - no problem. If not then it doesn't make sense to use their mappings.

@amika-sq
Copy link
Contributor Author

amika-sq commented Dec 5, 2023

@decentralgabe I was under the impression that Kotlin was doing that unique mappings of alg/kty here:

Then the consumer could get at the alg/kty like so: https://github.com/TBD54566975/web5-kt/blob/main/crypto/src/main/kotlin/web5/sdk/crypto/Crypto.kt#L41-L46

Did you have something else in mind here?

@decentralgabe
Copy link
Member

decentralgabe commented Dec 5, 2023

@amika-sq that abstraction isn't exactly right, since Ed25519 is a specific usage of the EdDSA signature algorithm. The underlying curve, Curve25519 can be used with Ed25519 (EdDSA) or X25519.

Similarly Secp256k1 is bound to a JWA (ES256K), so it does seem like it's chosen a JOSE approach. Secp256k1 can be used for ECDH as well, so keeping it as a 'signer' seems like the wrong abstraction here.

I think something like this might make sense:

  • Key Type (Whether curve, octet, lattice, isogency, etc.) (e.g. RSA, Ed25519, Secp256k1)
  • Operation and Algorithm pairs e.g. (sign/verify = Ed25519, ECDH = X25519)

Tink has solved this but in a confusing way. Since they refer to key/algorithm pairs as key types. When using JOSE they differentiate key types.

So to provide a concrete recommendation, I'd just mimic the abstraction JOSE has.

@amika-sq
Copy link
Contributor Author

amika-sq commented Dec 5, 2023

So to provide a concrete recommendation, I'd just mimic the abstraction JOSE has.

Could you point me to where that is? Do you mean the Tink abstraction?

@decentralgabe
Copy link
Member

@KendallWeihe
Copy link
Contributor

We implement this as Curve

pub enum Curve {

@decentralgabe
Copy link
Member

^^ fine for now until we need to support keys that don't use ECC 😄 (no reqs yet but will be eventually with PQC)

@KendallWeihe
Copy link
Contributor

Alright I'm going to re-open this actually because Curve feels slightly constricting, so leaving it open if only to hope that one day somebody has the cycles to come to a comprehensive solution here (meaning, how the above commentary maps onto the existing code base and what the downstream implications are)

@KendallWeihe KendallWeihe reopened this May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants