fix SignatureShare encoding

ZcashFoundation · Jun 30, 2023 · 2bdbcb0 · 2bdbcb0
1 parent aa193e3
commit 2bdbcb0
Show file tree

Hide file tree

Showing 6 changed files with 74 additions and 38 deletions.
diff --git a/frost-core/src/frost/round2.rs b/frost-core/src/frost/round2.rs
@@ -11,25 +11,48 @@ use crate::{
 #[cfg(feature = "serde")]
 use crate::ScalarSerialization;
 
+// Used to help encoding a SignatureShare. Since it has a Scalar<C> it can't
+// be directly encoded with serde, so we use this struct to wrap the scalar.
+#[cfg(feature = "serde")]
+#[derive(Clone)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
+#[cfg_attr(feature = "serde", serde(try_from = "ScalarSerialization<C>"))]
+#[cfg_attr(feature = "serde", serde(into = "ScalarSerialization<C>"))]
+struct SignatureShareHelper<C: Ciphersuite>(Scalar<C>);
+
+#[cfg(feature = "serde")]
+impl<C> TryFrom<ScalarSerialization<C>> for SignatureShareHelper<C>
+where
+    C: Ciphersuite,
+{
+    type Error = Error<C>;
+
+    fn try_from(value: ScalarSerialization<C>) -> Result<Self, Self::Error> {
+        <<C::Group as Group>::Field>::deserialize(&value.0)
+            .map(|scalar| Self(scalar))
+            .map_err(|e| e.into())
+    }
+}
+
+#[cfg(feature = "serde")]
+impl<C> From<SignatureShareHelper<C>> for ScalarSerialization<C>
+where
+    C: Ciphersuite,
+{
+    fn from(value: SignatureShareHelper<C>) -> Self {
+        Self(<<C::Group as Group>::Field>::serialize(&value.0))
+    }
+}
+
 /// shares into the joint signature.
 #[derive(Clone, Copy, Eq, PartialEq, Getters)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 #[cfg_attr(feature = "serde", serde(deny_unknown_fields))]
-#[cfg_attr(feature = "serde", serde(try_from = "ScalarSerialization<C>"))]
-#[cfg_attr(feature = "serde", serde(into = "ScalarSerialization<C>"))]
+#[cfg_attr(feature = "serde", serde(try_from = "SignatureShareSerialization<C>"))]
+#[cfg_attr(feature = "serde", serde(into = "SignatureShareSerialization<C>"))]
 pub struct SignatureShare<C: Ciphersuite> {
     /// This participant's signature over the message.
     pub(crate) share: Scalar<C>,
-    /// Ciphersuite ID for serialization
-    #[cfg_attr(
-        feature = "serde",
-        serde(serialize_with = "crate::ciphersuite_serialize::<_, C>")
-    )]
-    #[cfg_attr(
-        feature = "serde",
-        serde(deserialize_with = "crate::ciphersuite_deserialize::<_, C>")
-    )]
-    ciphersuite: (),
 }
 
 impl<C> SignatureShare<C>
@@ -41,10 +64,7 @@ where
         bytes: <<C::Group as Group>::Field as Field>::Serialization,
     ) -> Result<Self, Error<C>> {
         <<C::Group as Group>::Field>::deserialize(&bytes)
-            .map(|scalar| Self {
-                share: scalar,
-                ciphersuite: (),
-            })
+            .map(|scalar| Self { share: scalar })
             .map_err(|e| e.into())
     }
 
@@ -60,7 +80,7 @@ where
     ///
     /// [`verify_signature_share`]: https://www.ietf.org/archive/id/draft-irtf-cfrg-frost-11.html#name-signature-share-verificatio
     #[cfg_attr(feature = "internals", visibility::make(pub))]
-    fn verify(
+    pub(crate) fn verify(
         &self,
         identifier: Identifier<C>,
         group_commitment_share: &round1::GroupCommitmentShare<C>,
@@ -80,25 +100,44 @@ where
     }
 }
 
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
+#[cfg_attr(feature = "serde", serde(deny_unknown_fields))]
+struct SignatureShareSerialization<C: Ciphersuite> {
+    share: SignatureShareHelper<C>,
+    /// Ciphersuite ID for serialization
+    #[cfg_attr(
+        feature = "serde",
+        serde(serialize_with = "crate::ciphersuite_serialize::<_, C>")
+    )]
+    #[cfg_attr(
+        feature = "serde",
+        serde(deserialize_with = "crate::ciphersuite_deserialize::<_, C>")
+    )]
+    ciphersuite: (),
+}
+
 #[cfg(feature = "serde")]
-impl<C> TryFrom<ScalarSerialization<C>> for SignatureShare<C>
+impl<C> From<SignatureShareSerialization<C>> for SignatureShare<C>
 where
     C: Ciphersuite,
 {
-    type Error = Error<C>;
-
-    fn try_from(value: ScalarSerialization<C>) -> Result<Self, Self::Error> {
-        Self::from_bytes(value.0)
+    fn from(value: SignatureShareSerialization<C>) -> Self {
+        Self {
+            share: value.share.0,
+        }
     }
 }
 
 #[cfg(feature = "serde")]
-impl<C> From<SignatureShare<C>> for ScalarSerialization<C>
+impl<C> From<SignatureShare<C>> for SignatureShareSerialization<C>
 where
     C: Ciphersuite,
 {
     fn from(value: SignatureShare<C>) -> Self {
-        Self(value.to_bytes())
+        Self {
+            share: SignatureShareHelper(value.share),
+            ciphersuite: (),
+        }
     }
 }
 
@@ -126,10 +165,7 @@ fn compute_signature_share<C: Ciphersuite>(
         + (signer_nonces.binding.0 * binding_factor.0)
         + (lambda_i * key_package.secret_share.0 * challenge.0);
 
-    SignatureShare::<C> {
-        share: z_share,
-        ciphersuite: (),
-    }
+    SignatureShare::<C> { share: z_share }
 }
 
 // // Zeroizes `SignatureShare` to be the `Default` value on drop (when it goes out

diff --git a/frost-ed25519/tests/serde_tests.rs b/frost-ed25519/tests/serde_tests.rs
@@ -169,7 +169,7 @@ fn check_signature_share_serialization() {
     assert!(signature_share == decoded_signature_share);
 
     let json = r#"{
-      "signature": "498d4e9311420c903913a56c94a694b8aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa0a",
+      "share": "498d4e9311420c903913a56c94a694b8aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa0a",
       "ciphersuite": "FROST(Ed25519, SHA-512)"
     }"#;
     let decoded_commitments: SignatureShare = serde_json::from_str(json).unwrap();
@@ -193,7 +193,7 @@ fn check_signature_share_serialization() {
 
     // Extra field
     let invalid_json = r#"{
-        "signature": "498d4e9311420c903913a56c94a694b8aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa0a",
+        "share": "498d4e9311420c903913a56c94a694b8aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa0a",
         "extra": 1,
         "ciphersuite": "FROST(Ed25519, SHA-512)"
       }"#;

diff --git a/frost-ed448/tests/serde_tests.rs b/frost-ed448/tests/serde_tests.rs
@@ -169,7 +169,7 @@ fn check_signature_share_serialization() {
     assert!(signature_share == decoded_signature_share);
 
     let json = r#"{
-      "signature": "4d83e51cb78150c2380ad9b3a18148166024e4c9db3cdf82466d3153aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa2a00",
+      "share": "4d83e51cb78150c2380ad9b3a18148166024e4c9db3cdf82466d3153aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa2a00",
       "ciphersuite": "FROST(Ed448, SHAKE256)"
     }"#;
     let decoded_commitments: SignatureShare = serde_json::from_str(json).unwrap();
@@ -193,7 +193,7 @@ fn check_signature_share_serialization() {
 
     // Extra field
     let invalid_json = r#"{
-        "signature": "4d83e51cb78150c2380ad9b3a18148166024e4c9db3cdf82466d3153aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa2a00",
+        "share": "4d83e51cb78150c2380ad9b3a18148166024e4c9db3cdf82466d3153aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa2a00",
         "extra": 1,
         "ciphersuite": "FROST(Ed448, SHAKE256)"
       }"#;

diff --git a/frost-p256/tests/serde_tests.rs b/frost-p256/tests/serde_tests.rs
@@ -169,7 +169,7 @@ fn check_signature_share_serialization() {
     assert!(signature_share == decoded_signature_share);
 
     let json = r#"{
-      "signature": "aaaaaaaa00000000aaaaaaaaaaaaaaaa7def51c91a0fbf034d26872ca84218e1",
+      "share": "aaaaaaaa00000000aaaaaaaaaaaaaaaa7def51c91a0fbf034d26872ca84218e1",
       "ciphersuite": "FROST(P-256, SHA-256)"
     }"#;
     let decoded_commitments: SignatureShare = serde_json::from_str(json).unwrap();
@@ -193,7 +193,7 @@ fn check_signature_share_serialization() {
 
     // Extra field
     let invalid_json = r#"{
-        "signature": "aaaaaaaa00000000aaaaaaaaaaaaaaaa7def51c91a0fbf034d26872ca84218e1",
+        "share": "aaaaaaaa00000000aaaaaaaaaaaaaaaa7def51c91a0fbf034d26872ca84218e1",
         "extra": 1,
         "ciphersuite": "FROST(P-256, SHA-256)"
       }"#;

diff --git a/frost-ristretto255/tests/serde_tests.rs b/frost-ristretto255/tests/serde_tests.rs
@@ -169,7 +169,7 @@ fn check_signature_share_serialization() {
     assert!(signature_share == decoded_signature_share);
 
     let json = r#"{
-      "signature": "498d4e9311420c903913a56c94a694b8aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa0a",
+      "share": "498d4e9311420c903913a56c94a694b8aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa0a",
       "ciphersuite": "FROST(ristretto255, SHA-512)"
     }"#;
     let decoded_commitments: SignatureShare = serde_json::from_str(json).unwrap();
@@ -193,7 +193,7 @@ fn check_signature_share_serialization() {
 
     // Extra field
     let invalid_json = r#"{
-        "signature": "498d4e9311420c903913a56c94a694b8aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa0a",
+        "share": "498d4e9311420c903913a56c94a694b8aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa0a",
         "extra": 1,
         "ciphersuite": "FROST(ristretto255, SHA-512)"
       }"#;

diff --git a/frost-secp256k1/tests/serde_tests.rs b/frost-secp256k1/tests/serde_tests.rs
@@ -169,7 +169,7 @@ fn check_signature_share_serialization() {
     assert!(signature_share == decoded_signature_share);
 
     let json = r#"{
-      "signature": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa9d1c9e899ca306ad27fe1945de0242b81",
+      "share": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa9d1c9e899ca306ad27fe1945de0242b81",
       "ciphersuite": "FROST(secp256k1, SHA-256)"
     }"#;
     let decoded_commitments: SignatureShare = serde_json::from_str(json).unwrap();
@@ -193,7 +193,7 @@ fn check_signature_share_serialization() {
 
     // Extra field
     let invalid_json = r#"{
-        "signature": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa9d1c9e899ca306ad27fe1945de0242b81",
+        "share": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa9d1c9e899ca306ad27fe1945de0242b81",
         "extra": 1,
         "ciphersuite": "FROST(secp256k1, SHA-256)"
       }"#;