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

Consolidate public key hash proof types #152

Open
clehner opened this issue Mar 30, 2021 · 4 comments
Open

Consolidate public key hash proof types #152

clehner opened this issue Mar 30, 2021 · 4 comments
Labels
spec-alignment Changes needed to align with specification updates

Comments

@clehner
Copy link
Contributor

clehner commented Mar 30, 2021

These verification method types and proof types should probably be renamed, since they no longer depend specifically on BLAKE2B, etc.:

Proof type Verification method type
Ed25519BLAKE2BDigestSize20Base58CheckEncodedSignature2021 Ed25519PublicKeyBLAKE2BDigestSize20Base58CheckEncoded2021
P256BLAKE2BDigestSize20Base58CheckEncodedSignature2021 P256PublicKeyBLAKE2BDigestSize20Base58CheckEncoded2021

These signature suites were originally added for did:tz, and relied on the verification method DID URL in the proof to verify against the hash of the public key. A Tezos address is based on its public key, hashed using BLAKE2B, digest size 20, base58-check-encoded, as implemented in ssi::blakesig::hash_public_key. But now we may also use these proof types with did:pkh, where the DID is not necessarily based on a Tezos address. These proof are verified using the blockchainAccountId property of the verification method in the resolved DID document, to make sure that the public key in the proof is valid for that blockchain account id:
https://github.com/spruceid/ssi/blob/b3bc3dc/src/ldp.rs#L780-L790
similarly to in EcdsaSecp256k1RecoverySignature2020:
https://github.com/spruceid/ssi/blob/b3bc3dc/src/ldp.rs#L691-L699

I'm not sure what should be in the name instead of BLAKE2BDigestSize20Base58CheckEncoded. Maybe something to express that the public key should be in the proof, and/or that the blockchainAccountId property is used to verify the public key. Ideas:

  • Ed25519PublicKeyInProofSignature2021
  • Ed25519FauxRecoverySignature2021
  • Ed25519BlockchainAccountIdSignature2021

Maybe the Ed25519 and P-256 ones could be unified into one signature suite? This could cover the EcdsaSecp256k1Recovery suite as well:

  • BlockchainAccountSignature2021

Edit: BlockchainSignature2021 and BlockchainVerificationMethod2021 exist: https://or13.github.io/lds-blockchain2021/ - They would just have to be updated probably to allow including the publicKey in the proof object.

@wyc
Copy link
Contributor

wyc commented Mar 30, 2021

Thanks, will fully digest this soon. One stray thought that came up was that we should anticipate the use of P-256 in signature recovery situations and key provided situations. Ideally we support both across secp256k1 and P-256.

This thread has useful context about an implementation approach.
AntonKueltz/fastecdsa#15 (comment)

@wyc
Copy link
Contributor

wyc commented Apr 1, 2021

I think for now BlockchainAccountSignature2021 may be the best option, and we can further define account verification steps per blockchain type within the suite. We could even leverage existing verification methods such as EcdsaSecp256k1RecoverySignature2020 by defining their invocation as dependent on the blockchain and account types, such as tz2 mapping to EcdsaSecp256k1RecoverySignature2020 after verifying the BLAKE2B hash. Does that sound reasonable?

@clehner
Copy link
Contributor Author

clehner commented Apr 1, 2021

@wyc yes, that make sense.

Also that is cool that public key recovery can be done with P-256.

@clehner
Copy link
Contributor Author

clehner commented Jul 2, 2021

BlockchainSignature2021 and BlockchainVerificationMethod2021 are here: https://or13.github.io/lds-blockchain2021/

@clehner clehner changed the title Rename tz1 and tz3 proof types Consolidate public key hash proof types Jul 2, 2021
@clehner clehner added the spec-alignment Changes needed to align with specification updates label Apr 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec-alignment Changes needed to align with specification updates
Projects
None yet
Development

No branches or pull requests

2 participants