refactor(crypto): CRP-2443 CRP-2235 Use ic-crypto-ed25519 for all Ed25519 key serialization and deserialization

Author: randombit

Cargo.lock

@@ -4,11 +4,9 @@ package(default_visibility = ["//visibility:public"])
 
 DEPENDENCIES = [
     "//rs/crypto/ecdsa_secp256k1",
-    "//rs/crypto/internal/crypto_lib/types",
-    "//rs/crypto/utils/basic_sig",
+    "//rs/crypto/ed25519",
     "//rs/types/base_types",
     "//rs/types/types",
-    "@crate_index//:ed25519-consensus",
     "@crate_index//:rand",
     "@crate_index//:rand_chacha",
 ]

rs/canister_client/sender/BUILD.bazel

@@ -9,9 +9,7 @@ documentation.workspace = true
 [dependencies]
 ic-base-types = { path = "../../types/base_types" }
 ic-crypto-ecdsa-secp256k1 = { path = "../../crypto/ecdsa_secp256k1" }
-ic-crypto-internal-types = { path = "../../crypto/internal/crypto_lib/types" }
-ic-crypto-utils-basic-sig = { path = "../../crypto/utils/basic_sig" }
+ic-crypto-ed25519 = { path = "../../crypto/ed25519" }
 ic-types = { path = "../../types/types" }
-ed25519-consensus = "2.0.1"
 rand = "0.8"
 rand_chacha = "0.3"

rs/canister_client/sender/Cargo.toml

@@ -3,11 +3,6 @@ mod secp256k1_conversions;
 mod tests;
 
 use ic_base_types::PrincipalId;
-use ic_crypto_internal_types::sign::eddsa::ed25519::{
-    PublicKey as Ed25519PublicKey, SecretKey as Ed25519SecretKey,
-};
-use ic_crypto_utils_basic_sig::conversions::Ed25519PemParseError;
-use ic_crypto_utils_basic_sig::conversions::Ed25519SecretKeyConversions;
 use ic_types::crypto::DOMAIN_IC_REQUEST;
 use ic_types::messages::MessageId;
 use rand::{CryptoRng, Rng, SeedableRng};
@@ -37,30 +32,31 @@ pub struct Ed25519KeyPair {
 
 impl Ed25519KeyPair {
     pub fn generate<R: Rng + CryptoRng>(rng: &mut R) -> Self {
-        let rng = ChaCha20Rng::from_seed(rng.gen());
-        let signing_key = ed25519_consensus::SigningKey::new(rng);
+        let mut rng = ChaCha20Rng::from_seed(rng.gen());
+        let key = ic_crypto_ed25519::PrivateKey::generate_using_rng(&mut rng);
         Self {
-            secret_key: signing_key.to_bytes(),
-            public_key: signing_key.verification_key().to_bytes(),
+            secret_key: key.serialize_raw(),
+            public_key: key.public_key().serialize_raw(),
         }
     }
 
     /// Parses an Ed25519KeyPair from a PEM string.
-    pub fn from_pem(pem: &str) -> Result<Self, Ed25519PemParseError> {
-        let (secret_key, public_key) = Ed25519SecretKey::from_pem(pem)?;
+    pub fn from_pem(pem: &str) -> Result<Self, ic_crypto_ed25519::PrivateKeyDecodingError> {
+        let key = ic_crypto_ed25519::PrivateKey::deserialize_pkcs8_pem(pem)?;
         Ok(Ed25519KeyPair {
-            secret_key: secret_key.0,
-            public_key: public_key.0,
+            secret_key: key.serialize_raw(),
+            public_key: key.public_key().serialize_raw(),
         })
     }
 
     pub fn to_pem(&self) -> String {
-        Ed25519SecretKey(self.secret_key).to_pem(&Ed25519PublicKey(self.public_key))
+        let key = ic_crypto_ed25519::PrivateKey::deserialize_raw_32(&self.secret_key);
+        key.serialize_pkcs8_pem(ic_crypto_ed25519::PrivateKeyFormat::Pkcs8v2WithRingBug)
     }
 
     pub fn sign(&self, msg: &[u8]) -> [u8; 64] {
-        let signing_key = ed25519_consensus::SigningKey::from(self.secret_key);
-        signing_key.sign(msg).to_bytes()
+        let key = ic_crypto_ed25519::PrivateKey::deserialize_raw_32(&self.secret_key);
+        key.sign_message(msg)
     }
 }
 

rs/canister_client/sender/src/lib.rs

@@ -104,8 +104,8 @@ impl PrivateKey {
     /// Sign a message and return a signature
     ///
     /// This is the non-prehashed variant of Ed25519
-    pub fn sign_message(&self, msg: &[u8]) -> Vec<u8> {
-        self.sk.sign(msg).to_vec()
+    pub fn sign_message(&self, msg: &[u8]) -> [u8; 64] {
+        self.sk.sign(msg).into()
     }
 
     /// Return the public key associated with this secret key
@@ -127,7 +127,7 @@ impl PrivateKey {
     /// This is just the plain 32 byte random seed from which the
     /// internal key material is derived
     ///
-    /// This corresponds with the format used by PrivateKey::serialize
+    /// This corresponds with the format used by PrivateKey::serialize_raw
     pub fn deserialize_raw(bytes: &[u8]) -> Result<Self, PrivateKeyDecodingError> {
         let bytes = <[u8; Self::BYTES]>::try_from(bytes).map_err(|_| {
             PrivateKeyDecodingError::InvalidKeyEncoding(format!(
@@ -137,8 +137,18 @@ impl PrivateKey {
             ))
         })?;
 
-        let sk = SigningKey::from_bytes(&bytes);
-        Ok(Self { sk })
+        Ok(Self::deserialize_raw_32(&bytes))
+    }
+
+    /// Deserialize an Ed25519 private key from raw format
+    ///
+    /// This is just the plain 32 byte random seed from which the
+    /// internal key material is derived
+    ///
+    /// This corresponds with the format used by PrivateKey::serialize_raw
+    pub fn deserialize_raw_32(bytes: &[u8; 32]) -> Self {
+        let sk = SigningKey::from_bytes(bytes);
+        Self { sk }
     }
 
     /// Serialize the Ed25519 secret key in PKCS8 format
@@ -241,6 +251,10 @@ impl PrivateKey {
 /// An invalid key was encountered
 #[derive(Clone, Debug)]
 pub enum PublicKeyDecodingError {
+    /// The outer PEM encoding is invalid
+    InvalidPemEncoding(String),
+    /// The PEM label was not the expected value
+    UnexpectedPemLabel(String),
     /// The encoding of the public key is invalid, the string contains details
     InvalidKeyEncoding(String),
     /// The public key had a valid encoding, but contains elements of the torsion
@@ -330,6 +344,17 @@ impl PublicKey {
             .to_vec()
     }
 
+    /// Serialize this public key as a PEM encoded structure
+    ///
+    /// See RFC 8410 for details on the format
+    pub fn serialize_rfc8410_pem(&self) -> Vec<u8> {
+        pem::encode(&pem::Pem {
+            tag: "PUBLIC KEY".to_string(),
+            contents: self.serialize_rfc8410_der(),
+        })
+        .into()
+    }
+
     /// Deserialize the DER encoded public key
     ///
     /// See RFC 8410 for details on the format. This cooresponds to
@@ -340,6 +365,20 @@ impl PublicKey {
         Self::new(pk)
     }
 
+    /// Deserialize the PEM encoded public key
+    ///
+    /// See RFC 8410 for details on the format. This cooresponds to
+    /// Self::serialize_rfc8410_pem
+    pub fn deserialize_rfc8410_pem(pem: &str) -> Result<Self, PublicKeyDecodingError> {
+        let der = pem::parse(pem)
+            .map_err(|e| PublicKeyDecodingError::InvalidPemEncoding(format!("{:?}", e)))?;
+        if der.tag != "PUBLIC KEY" {
+            return Err(PublicKeyDecodingError::UnexpectedPemLabel(der.tag));
+        }
+
+        Self::deserialize_rfc8410_der(&der.contents)
+    }
+
     /// Verify a Ed25519 signature
     ///
     /// Returns Ok if the signature is valid, or Err otherwise

rs/crypto/ed25519/src/lib.rs

@@ -80,7 +80,7 @@ fn public_key_deserialization_rejects_keys_of_incorrect_length() {
 
 #[test]
 fn batch_verification_works() {
-    fn batch_verifies(msg: &[[u8; 32]], sigs: &[Vec<u8>], keys: &[PublicKey]) -> bool {
+    fn batch_verifies(msg: &[[u8; 32]], sigs: &[[u8; 64]], keys: &[PublicKey]) -> bool {
         let msg_ref = msg.iter().map(|m| m.as_ref()).collect::<Vec<&[u8]>>();
         let sig_ref = sigs.iter().map(|s| s.as_ref()).collect::<Vec<&[u8]>>();
         PublicKey::batch_verify(&msg_ref, &sig_ref, keys).is_ok()
@@ -215,6 +215,65 @@ fn can_parse_pkcs8_v2_der_secret_key() {
     );
 }
 
+#[test]
+fn can_parse_dfx_created_private_key() {
+    let dfx_key: &str = "-----BEGIN PRIVATE KEY-----\nMFMCAQEwBQYDK2VwBCIEIPXo8WUQM26wS/cT6mmHO1ClYHixF46uhRoQlLmPfsQl\noSMDIQAY6M8L0Ocji3w8k2EBMTwhVJT0G6HI1ZZmWrPOzv8L1Q==\n-----END PRIVATE KEY-----\n";
+
+    match PrivateKey::deserialize_pkcs8_pem(dfx_key) {
+        Ok(_sk) => { /* success */ }
+        Err(e) => panic!("Unexpected error serializing DFX generated key {:?}", e),
+    }
+}
+
+#[test]
+fn can_pass_old_basic_sig_utils_parsing_tests() {
+    struct PublicKeyParsingTest {
+        raw: [u8; 32],
+        der: [u8; 44],
+        pem: String,
+    }
+
+    impl PublicKeyParsingTest {
+        fn new(raw: [u8; 32], der: [u8; 44], pem: &'static str) -> Self {
+            Self {
+                raw,
+                der,
+                pem: pem.to_string(),
+            }
+        }
+    }
+
+    let tests = [
+        PublicKeyParsingTest::new(hex!("B3997656BA51FF6DA37B61D8D549EC80717266ECF48FB5DA52B654412634844C"),
+                                  hex!("302A300506032B6570032100B3997656BA51FF6DA37B61D8D549EC80717266ECF48FB5DA52B654412634844C"),
+                                  "-----BEGIN PUBLIC KEY-----\nMCowBQYDK2VwAyEAs5l2VrpR/22je2HY1UnsgHFyZuz0j7XaUrZUQSY0hEw=\n-----END PUBLIC KEY-----\n"
+        ),
+        PublicKeyParsingTest::new(hex!("A5AFB5FEB6DFB6DDF5DD6563856FFF5484F5FE304391D9ED06697861F220C610"),
+                                  hex!("302A300506032B6570032100A5AFB5FEB6DFB6DDF5DD6563856FFF5484F5FE304391D9ED06697861F220C610"),
+                                  "-----BEGIN PUBLIC KEY-----\nMCowBQYDK2VwAyEApa+1/rbftt313WVjhW//VIT1/jBDkdntBml4YfIgxhA=\n-----END PUBLIC KEY-----\n"
+        ),
+        PublicKeyParsingTest::new(hex!("C8413108F121CB794A10804D15F613E40ECC7C78A4EC567040DDF78467C71DFF"),
+                                  hex!("302A300506032B6570032100C8413108F121CB794A10804D15F613E40ECC7C78A4EC567040DDF78467C71DFF"),
+                                  "-----BEGIN PUBLIC KEY-----\nMCowBQYDK2VwAyEAyEExCPEhy3lKEIBNFfYT5A7MfHik7FZwQN33hGfHHf8=\n-----END PUBLIC KEY-----\n"
+        ),
+    ];
+
+    for test in &tests {
+        let from_raw = PublicKey::deserialize_raw(&test.raw).expect("Invalid public key (raw)");
+        let from_der =
+            PublicKey::deserialize_rfc8410_der(&test.der).expect("Invalid public key (DER)");
+        let from_pem =
+            PublicKey::deserialize_rfc8410_pem(&test.pem).expect("Invalid public key (PEM)");
+
+        assert_eq!(from_raw, from_der);
+        assert_eq!(from_raw, from_pem);
+
+        assert_eq!(from_raw.serialize_raw(), test.raw);
+        assert_eq!(from_der.serialize_raw(), test.raw);
+        assert_eq!(from_pem.serialize_raw(), test.raw);
+    }
+}
+
 #[test]
 fn should_pass_wycheproof_test_vectors() {
     let test_set = wycheproof::eddsa::TestSet::load(wycheproof::eddsa::TestName::Ed25519)

rs/crypto/ed25519/tests/tests.rs

@@ -14,14 +14,11 @@ rust_library(
         "//rs/types/types",
         "@crate_index//:hex",
         "@crate_index//:simple_asn1",
-        "@crate_index//:zeroize",
     ],
 )
 
 rust_test(
     name = "der_utils_test",
     crate = ":der_utils",
-    deps = [
-        "//rs/crypto/internal/test_vectors",
-    ],
+    deps = [],
 )

rs/crypto/internal/crypto_lib/basic_sig/der_utils/BUILD.bazel

@@ -9,9 +9,4 @@ documentation.workspace = true
 [dependencies]
 hex = "0.4.2"
 simple_asn1 = { workspace = true }
-zeroize = { version = "1.4.3", features = ["zeroize_derive"] }
 ic-types = { path = "../../../../../types/types" }
-
-[dev-dependencies]
-ic-crypto-internal-test-vectors = { path = "../../../test_vectors" }
-