decentralized-identity / did-key.rs

Rust implementation of the did:key method
Apache License 2.0
47 stars 24 forks source link

Valid secp256k1 did:key values cause panic #28

Closed vdods closed 2 years ago

vdods commented 2 years ago
fn main() {
    did_key::resolve("did:key:zQ3shokFTS3brHcDQrn82RUDfCZESWL1ZdCEJwekUDPQiYBme").unwrap();
}

Panics with not implemented: unsupported key type, but this is a test vector from the did:key method spec: https://w3c-ccg.github.io/did-method-key/#secp256k1 The other two test vectors in the spec also cause the same panic.

The fix is

diff --git a/src/lib.rs b/src/lib.rs
index d641679..5bc3414 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -177,7 +177,7 @@ impl TryFrom<&str> for KeyPair {
             [0xec, 0x1] => KeyPair::X25519(X25519KeyPair::from_public_key(&pub_key[2..])),
             [0xee, 0x1] => KeyPair::Bls12381G1G2(Bls12381KeyPairs::from_public_key(&pub_key[2..])),
             [0x80, 0x24] => KeyPair::P256(P256KeyPair::from_public_key(&pub_key[2..])),
-            [0xe7, 0x0] => KeyPair::Secp256k1(Secp256k1KeyPair::from_public_key(&pub_key[2..])),
+            [0xe7, 0x1] => KeyPair::Secp256k1(Secp256k1KeyPair::from_public_key(&pub_key[2..])),
             _ => unimplemented!("unsupported key type"),
         });
     }

There was no test exercising the code path for Secp256k1, so the bug was not exposed.

tmarkovski commented 2 years ago

Fixed with https://github.com/decentralized-identity/did-key.rs/pull/29

@vdods thanks for reporting this