Made to_bytes return an option

chain-signatures/contract/src/lib.rs

@@ -4,6 +4,7 @@ use crypto_shared::{
     derive_epsilon, derive_key, kdf::check_ec_signature, near_public_key_to_affine_point,
     types::SignatureResponse, ScalarExt as _, SerializableScalar,
 };
+use k256::Scalar;
 use near_sdk::borsh::{self, BorshDeserialize, BorshSerialize};
 use near_sdk::collections::LookupMap;
 use near_sdk::serde::{Deserialize, Serialize};
@@ -98,13 +99,16 @@ pub struct YieldIndex {
 #[borsh(crate = "near_sdk::borsh")]
 pub struct SignatureRequest {
     pub epsilon: SerializableScalar,
-    pub payload_hash: [u8; 32],
+    pub payload_hash: SerializableScalar,
 }
 
 impl SignatureRequest {
-    pub fn new(payload_hash: [u8; 32], predecessor_id: &AccountId, path: &str) -> Self {
-        let scalar = derive_epsilon(predecessor_id, path);
-        let epsilon = SerializableScalar { scalar };
+    pub fn new(payload_hash: Scalar, predecessor_id: &AccountId, path: &str) -> Self {
+        let epsilon = derive_epsilon(predecessor_id, path);
+        let epsilon = SerializableScalar { scalar: epsilon };
+        let payload_hash = SerializableScalar {
+            scalar: payload_hash,
+        };
         SignatureRequest {
             epsilon,
             payload_hash,
@@ -167,6 +171,9 @@ impl VersionedMpcContract {
             key_version,
         } = request;
         let latest_key_version: u32 = self.latest_key_version();
+        // It's important we fail here because the MPC nodes will fail in an identical way.
+        // This allows users to get the error message
+        let payload = Scalar::from_bytes(payload).expect("Payload hash is bad");
         assert!(
             key_version <= latest_key_version,
             "This version of the signer contract doesn't support versions greater than {}",
@@ -280,6 +287,7 @@ impl VersionedMpcContract {
 
     pub fn respond(&mut self, request: SignatureRequest, response: SignatureResponse) {
         let protocol_state = self.mutable_state();
+
         if let ProtocolContractState::Running(_) = protocol_state {
             let signer = env::signer_account_id();
             log!(
@@ -301,7 +309,7 @@ impl VersionedMpcContract {
                 &expected_public_key,
                 &response.big_r.affine_point,
                 &response.s.scalar,
-                k256::Scalar::from_bytes(&request.payload_hash[..]),
+                request.payload_hash.scalar,
                 response.recovery_id,
             )
             .is_err()

chain-signatures/crypto-shared/Cargo.toml

@@ -18,6 +18,7 @@ near-account-id = "1"
 serde_json = "1"
 near-sdk = { version = "5.2.1", features = ["unstable"] }
 sha3 = "0.10.8"
+subtle = "2.6.1"
 
 [target.'cfg(target_arch = "wasm32")'.dependencies]
 getrandom = { version = "0.2.12", features = ["custom"] }

chain-signatures/crypto-shared/src/kdf.rs

@@ -22,7 +22,8 @@ pub fn derive_epsilon(predecessor_id: &AccountId, path: &str) -> Scalar {
     let derivation_path = format!("{EPSILON_DERIVATION_PREFIX}{},{}", predecessor_id, path);
     let mut hasher = Sha3_256::new();
     hasher.update(derivation_path);
-    Scalar::from_bytes(&hasher.finalize())
+    let hash: [u8; 32] = hasher.finalize().into();
+    Scalar::from_non_biased(hash)
 }
 
 pub fn derive_key(public_key: PublicKey, epsilon: Scalar) -> PublicKey {

chain-signatures/crypto-shared/src/types.rs

@@ -7,23 +7,51 @@ use serde::{Deserialize, Serialize};
 
 pub type PublicKey = <Secp256k1 as CurveArithmetic>::AffinePoint;
 
-pub trait ScalarExt {
-    fn from_bytes(bytes: &[u8]) -> Self;
+pub trait ScalarExt: Sized {
+    fn from_bytes(bytes: [u8; 32]) -> Option<Self>;
+    fn from_non_biased(bytes: [u8; 32]) -> Self;
 }
 
 impl ScalarExt for Scalar {
-    fn from_bytes(bytes: &[u8]) -> Self {
-        let bytes = U256::from_be_slice(bytes);
-        Scalar::from_repr(bytes.to_be_byte_array()).expect("Byte conversion to scalar failed")
+    /// Returns nothing if the bytes are greater than the field size of Secp256k1.
+    /// This will be very rare with random bytes as the field size is 2^256 - 2^32 - 2^9 - 2^8 - 2^7 - 2^6 - 2^4 - 1
+    fn from_bytes(bytes: [u8; 32]) -> Option<Self> {
+        let bytes = U256::from_be_slice(bytes.as_slice());
+        Scalar::from_repr(bytes.to_be_byte_array()).into_option()
+    }
+
+    /// When the user can't directly select the value, this will always work
+    /// Use cases are things that we know have been hashed
+    fn from_non_biased(hash: [u8; 32]) -> Self {
+        // This should never happen.
+        // The space of inputs is 2^256, the space of the field is ~2^256 - 2^32.
+        // This mean that you'd have to run 2^224 hashes to find a value that causes this to fail.
+        Scalar::from_bytes(hash).expect("Derived epsilon value falls outside of the field")
     }
 }
 
+#[test]
+fn scalar_fails_as_expected() {
+    let too_high = [0xFF; 32];
+    assert!(Scalar::from_bytes(too_high).is_none());
+
+    let mut not_too_high = [0xFF; 32];
+    not_too_high[27] = 0xFD;
+    assert!(Scalar::from_bytes(not_too_high).is_some());
+}
+
 // Is there a better way to force a borsh serialization?
 #[derive(Debug, PartialEq, Eq, Serialize, Deserialize, Clone, Copy)]
 pub struct SerializableScalar {
     pub scalar: Scalar,
 }
 
+impl From<Scalar> for SerializableScalar {
+    fn from(scalar: Scalar) -> Self {
+        SerializableScalar { scalar }
+    }
+}
+
 impl BorshSerialize for SerializableScalar {
     fn serialize<W: std::io::prelude::Write>(&self, writer: &mut W) -> std::io::Result<()> {
         let to_ser: [u8; 32] = self.scalar.to_bytes().into();
@@ -34,7 +62,10 @@ impl BorshSerialize for SerializableScalar {
 impl BorshDeserialize for SerializableScalar {
     fn deserialize_reader<R: std::io::prelude::Read>(reader: &mut R) -> std::io::Result<Self> {
         let from_ser: [u8; 32] = BorshDeserialize::deserialize_reader(reader)?;
-        let scalar = Scalar::from_bytes(&from_ser[..]);
+        let scalar = Scalar::from_bytes(from_ser).ok_or(std::io::Error::new(
+            std::io::ErrorKind::InvalidData,
+            "Scalar bytes are not in the k256 field",
+        ))?;
         Ok(SerializableScalar { scalar })
     }
 }
@@ -67,7 +98,7 @@ fn serializeable_scalar_roundtrip() {
         Scalar::ZERO,
         Scalar::ONE,
         Scalar::from_u128(u128::MAX),
-        Scalar::from_bytes(&[3; 32]),
+        Scalar::from_bytes([3; 32]).unwrap(),
     ];
 
     for scalar in test_vec.into_iter() {

chain-signatures/node/src/indexer.rs

@@ -2,7 +2,9 @@ use crate::gcp::GcpService;
 use crate::kdf;
 use crate::protocol::{SignQueue, SignRequest};
 use crate::types::LatestBlockHeight;
-use crypto_shared::derive_epsilon;
+use anyhow::Context as _;
+use crypto_shared::{derive_epsilon, ScalarExt};
+use k256::Scalar;
 use near_account_id::AccountId;
 use near_lake_framework::{LakeBuilder, LakeContext};
 use near_lake_primitives::actions::ActionMetaDataExt;
@@ -67,17 +69,26 @@ impl Options {
 }
 
 #[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq)]
-pub struct SignArguments {
-    pub request: ContractSignRequest,
+struct SignArguments {
+    request: UnvalidatedContractSignRequest,
 }
 
+/// What is recieved when sign is called
 #[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq)]
-pub struct ContractSignRequest {
+struct UnvalidatedContractSignRequest {
     pub payload: [u8; 32],
     pub path: String,
     pub key_version: u32,
 }
 
+/// A validated version of the sign request
+#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq)]
+pub struct ContractSignRequest {
+    pub payload: Scalar,
+    pub path: String,
+    pub key_version: u32,
+}
+
 #[derive(LakeContext)]
 struct Context {
     mpc_contract_id: AccountId,
@@ -113,6 +124,8 @@ async fn handle_block(
                             tracing::warn!("`sign` did not produce entropy");
                             continue;
                         }
+                        let payload = Scalar::from_bytes(arguments.request.payload)
+                            .context("Payload cannot be converted to scalar, not in k256 field")?;
                         let Ok(entropy) = serde_json::from_str::<'_, [u8; 32]>(&receipt.logs()[1])
                         else {
                             tracing::warn!(
@@ -133,9 +146,14 @@ async fn handle_block(
                             entropy = hex::encode(entropy),
                             "indexed new `sign` function call"
                         );
+                        let request = ContractSignRequest {
+                            payload,
+                            path: arguments.request.path,
+                            key_version: arguments.request.key_version,
+                        };
                         pending_requests.push(SignRequest {
                             receipt_id,
-                            request: arguments.request,
+                            request,
                             epsilon,
                             delta,
                             entropy,

chain-signatures/node/src/kdf.rs

@@ -13,7 +13,7 @@ pub fn derive_delta(receipt_id: CryptoHash, entropy: [u8; 32]) -> Scalar {
     let info = format!("{DELTA_DERIVATION_PREFIX}:{}", receipt_id);
     let mut okm = [0u8; 32];
     hk.expand(info.as_bytes(), &mut okm).unwrap();
-    Scalar::from_bytes(&okm)
+    Scalar::from_non_biased(okm)
 }
 
 // Constant prefix that ensures delta derivation values are used specifically for

chain-signatures/node/src/protocol/signature.rs

@@ -9,8 +9,8 @@ use crate::util::AffinePointExt;
 use cait_sith::protocol::{Action, InitializationError, Participant, ProtocolError};
 use cait_sith::{FullSignature, PresignOutput};
 use chrono::Utc;
+use crypto_shared::SerializableScalar;
 use crypto_shared::{derive_key, PublicKey};
-use crypto_shared::{ScalarExt, SerializableScalar};
 use k256::{Scalar, Secp256k1};
 use mpc_contract::SignatureRequest;
 use rand::rngs::StdRng;
@@ -58,7 +58,7 @@ impl SignQueue {
     pub fn add(&mut self, request: SignRequest) {
         tracing::info!(
             receipt_id = %request.receipt_id,
-            payload = hex::encode(request.request.payload),
+            payload = hex::encode(request.request.payload.to_bytes()),
             entropy = hex::encode(request.entropy),
             "new sign request"
         );
@@ -263,7 +263,7 @@ impl SignatureManager {
             me,
             derive_key(public_key, epsilon),
             output,
-            Scalar::from_bytes(&request.payload),
+            request.payload,
         )?);
         Ok(SignatureGenerator::new(
             protocol,
@@ -470,7 +470,7 @@ impl SignatureManager {
                         self.completed.insert(generator.presignature_id, Instant::now());
                         let request = SignatureRequest {
                             epsilon: SerializableScalar {scalar: generator.epsilon},
-                            payload_hash: generator.request.payload,
+                            payload_hash: generator.request.payload.into(),
                         };
                         if generator.proposer == self.me {
                             self.signatures
@@ -585,7 +585,7 @@ impl SignatureManager {
                 &expected_public_key,
                 &signature.big_r,
                 &signature.s,
-                Scalar::from_bytes(&request.payload_hash),
+                request.payload_hash.scalar,
             ) else {
                 tracing::error!(%receipt_id, "Failed to generate a recovery ID");
                 continue;

integration-tests/chain-signatures/tests/actions/mod.rs

@@ -88,14 +88,14 @@ pub async fn request_sign(
 pub async fn assert_signature(
     account_id: &near_workspaces::AccountId,
     mpc_pk_bytes: &[u8],
-    payload: &[u8; 32],
+    payload: [u8; 32],
     signature: &FullSignature<Secp256k1>,
 ) {
     let mpc_point = EncodedPoint::from_bytes(mpc_pk_bytes).unwrap();
     let mpc_pk = AffinePoint::from_encoded_point(&mpc_point).unwrap();
     let epsilon = derive_epsilon(account_id, "test");
     let user_pk = derive_key(mpc_pk, epsilon);
-    assert!(signature.verify(&user_pk, &Scalar::from_bytes(payload),));
+    assert!(signature.verify(&user_pk, &Scalar::from_bytes(payload).unwrap(),));
 }
 
 // A normal signature, but we try to insert a bad response which fails and the signature is generated
@@ -129,7 +129,7 @@ pub async fn single_signature_rogue_responder(
     //     hex::encode(&payload_hash),
     //     account.id(),
     // );
-    assert_signature(account.id(), &mpc_pk_bytes, &payload_hash, &signature).await;
+    assert_signature(account.id(), &mpc_pk_bytes, payload_hash, &signature).await;
 
     Ok(())
 }
@@ -143,7 +143,7 @@ pub async fn single_signature_production(
 
     let mut mpc_pk_bytes = vec![0x04];
     mpc_pk_bytes.extend_from_slice(&state.public_key.as_bytes()[1..]);
-    assert_signature(account.id(), &mpc_pk_bytes, &payload_hash, &signature).await;
+    assert_signature(account.id(), &mpc_pk_bytes, payload_hash, &signature).await;
 
     Ok(())
 }
@@ -169,7 +169,7 @@ pub async fn rogue_respond(
     let epsilon = derive_epsilon(predecessor, path);
 
     let request = SignatureRequest {
-        payload_hash,
+        payload_hash: Scalar::from_bytes(payload_hash).unwrap().into(),
         epsilon: SerializableScalar { scalar: epsilon },
     };
 
@@ -294,7 +294,7 @@ pub async fn single_payload_signature_production(
     assert_signature(
         account.clone().id(),
         &mpc_pk_bytes,
-        &payload_hash,
+        payload_hash,
         &signature,
     )
     .await;
@@ -370,7 +370,7 @@ async fn signatures_havent_changed() {
             .try_into()
             .unwrap();
 
-    let payload_hash_scalar = Scalar::from_bytes(&payload_hash);
+    let payload_hash_scalar = Scalar::from_bytes(payload_hash).unwrap();
 
     // Derive and convert user pk
     let mpc_pk = hex::decode(mpc_key).unwrap();
@@ -398,8 +398,8 @@ async fn signatures_havent_changed() {
     let big_r_y_parity = big_r.y_is_odd().unwrap_u8() as i32;
     assert!(big_r_y_parity == 0 || big_r_y_parity == 1);
 
-    let s = hex::decode(s).unwrap();
-    let s = k256::Scalar::from_bytes(s.as_slice());
+    let s = hex::decode(s).unwrap().try_into().unwrap();
+    let s = k256::Scalar::from_bytes(s).unwrap();
     let r = x_coordinate::<k256::Secp256k1>(&big_r);
 
     let signature = cait_sith::FullSignature::<Secp256k1> { big_r, s };

integration-tests/chain-signatures/tests/cases/mod.rs

@@ -124,7 +124,7 @@ async fn test_key_derivation() -> anyhow::Result<()> {
                     &user_pk,
                     &sig.big_r,
                     &sig.s,
-                    k256::Scalar::from_bytes(&payload_hashed),
+                    k256::Scalar::from_bytes(payload_hashed).unwrap(),
                 )
                 .unwrap();