crypto: AuthoritySignature commits to epoch id

crates/sui-core/src/authority.rs

@@ -2314,12 +2314,14 @@ impl AuthorityState {
                     })?;
             }
             ConsensusTransactionKind::Checkpoint(fragment) => {
-                fragment.verify().map_err(|err| {
-                    warn!(
-                        "Ignoring malformed fragment (failed to verify) from {}: {:?}",
-                        transaction.consensus_output.certificate.header.author, err
-                    );
-                })?;
+                fragment
+                    .verify(self.committee.load().epoch)
+                    .map_err(|err| {
+                        warn!(
+                            "Ignoring malformed fragment (failed to verify) from {}: {:?}",
+                            transaction.consensus_output.certificate.header.author, err
+                        );
+                    })?;
             }
         }
         Ok(VerifiedSequencedConsensusTransaction(transaction))

crates/sui-core/src/checkpoints/tests/checkpoint_tests.rs

@@ -1428,7 +1428,7 @@ async fn test_fragment_full_flow() {
     let cps0 = &mut test_stores[0].1;
     let mut all_fragments = Vec::new();
     while let Ok(fragment) = rx.try_recv() {
-        fragment.verify().unwrap();
+        fragment.verify(committee.epoch).unwrap();
         all_fragments.push(fragment.clone());
         assert!(cps0
             .handle_internal_fragment(seq.clone(), fragment.message, &committee)

crates/sui-types/src/crypto.rs

@@ -430,11 +430,16 @@ impl FromStr for AuthorityPublicKeyBytes {
 //
 
 pub trait SuiAuthoritySignature {
-    fn new<T>(value: &T, secret: &dyn Signer<Self>) -> Self
+    fn new<T>(value: &T, epoch_id: EpochId, secret: &dyn Signer<Self>) -> Self
     where
         T: Signable<Vec<u8>>;
 
-    fn verify<T>(&self, value: &T, author: AuthorityPublicKeyBytes) -> Result<(), SuiError>
+    fn verify<T>(
+        &self,
+        value: &T,
+        epoch_id: EpochId,
+        author: AuthorityPublicKeyBytes,
+    ) -> Result<(), SuiError>
     where
         T: Signable<Vec<u8>>;
 
@@ -453,16 +458,22 @@ pub trait SuiAuthoritySignature {
 }
 
 impl SuiAuthoritySignature for AuthoritySignature {
-    fn new<T>(value: &T, secret: &dyn Signer<Self>) -> Self
+    fn new<T>(value: &T, epoch_id: EpochId, secret: &dyn Signer<Self>) -> Self
     where
         T: Signable<Vec<u8>>,
     {
         let mut message = Vec::new();
         value.write(&mut message);
+        epoch_id.write(&mut message);
         secret.sign(&message)
     }
 
-    fn verify<T>(&self, value: &T, author: AuthorityPublicKeyBytes) -> Result<(), SuiError>
+    fn verify<T>(
+        &self,
+        value: &T,
+        epoch_id: EpochId,
+        author: AuthorityPublicKeyBytes,
+    ) -> Result<(), SuiError>
     where
         T: Signable<Vec<u8>>,
     {
@@ -475,6 +486,7 @@ impl SuiAuthoritySignature for AuthoritySignature {
         // serialize the message (see BCS serialization for determinism)
         let mut message = Vec::new();
         value.write(&mut message);
+        epoch_id.write(&mut message);
 
         // perform cryptographic signature check
         public_key
@@ -1025,13 +1037,14 @@ pub fn add_to_verification_obligation_and_verify<S, T>(
     sig: &S,
     data: &T,
     committee: &Committee,
+    epoch_id: EpochId,
 ) -> SuiResult
 where
     S: AuthoritySignInfoTrait,
     T: Signable<Vec<u8>>,
 {
     let mut obligation = VerificationObligation::default();
-    let idx = obligation.add_message(data);
+    let idx = obligation.add_message(data, epoch_id);
     sig.add_to_verification_obligation(committee, &mut obligation, idx)?;
     obligation.verify_all()?;
     Ok(())
@@ -1045,7 +1058,7 @@ pub struct AuthoritySignInfo {
 }
 impl AuthoritySignInfoTrait for AuthoritySignInfo {
     fn verify<T: Signable<Vec<u8>>>(&self, data: &T, committee: &Committee) -> SuiResult<()> {
-        add_to_verification_obligation_and_verify(self, data, committee)
+        add_to_verification_obligation_and_verify(self, data, committee, self.epoch)
     }
 
     fn add_to_verification_obligation(
@@ -1087,7 +1100,7 @@ impl AuthoritySignInfo {
         Self {
             epoch,
             authority: name,
-            signature: AuthoritySignature::new(value, secret),
+            signature: AuthoritySignature::new(value, epoch, secret),
         }
     }
 }
@@ -1155,7 +1168,7 @@ impl<const STRONG_THRESHOLD: bool> AuthoritySignInfoTrait
     for AuthorityQuorumSignInfo<STRONG_THRESHOLD>
 {
     fn verify<T: Signable<Vec<u8>>>(&self, data: &T, committee: &Committee) -> SuiResult<()> {
-        add_to_verification_obligation_and_verify(self, data, committee)
+        add_to_verification_obligation_and_verify(self, data, committee, self.epoch)
     }
 
     fn add_to_verification_obligation(
@@ -1372,6 +1385,15 @@ where
     }
 }
 
+impl<W> Signable<W> for EpochId
+where
+    W: std::io::Write,
+{
+    fn write(&self, writer: &mut W) {
+        bcs::serialize_into(writer, &self).expect("Message serialization should not fail");
+    }
+}
+
 impl<T> SignableBytes for T
 where
     T: bcs_signable::BcsSignable,
@@ -1442,12 +1464,13 @@ impl VerificationObligation {
 
     /// Add a new message to the list of messages to be verified.
     /// Returns the index of the message.
-    pub fn add_message<T>(&mut self, message_value: &T) -> usize
+    pub fn add_message<T>(&mut self, message_value: &T, epoch: EpochId) -> usize
     where
         T: Signable<Vec<u8>>,
     {
         let mut message = Vec::new();
         message_value.write(&mut message);
+        epoch.write(&mut message);
 
         self.signatures.push(AggregateAuthoritySignature::default());
         self.public_keys.push(Vec::new());
@@ -1521,7 +1544,7 @@ pub mod bcs_signable_test {
     {
         let mut obligation = VerificationObligation::default();
         // Add the obligation of the authority signature verifications.
-        let idx = obligation.add_message(value);
+        let idx = obligation.add_message(value, 0);
         (obligation, idx)
     }
 }

crates/sui-types/src/messages.rs

@@ -2037,7 +2037,7 @@ impl ConsensusTransaction {
             ConsensusTransactionKind::UserTransaction(certificate) => {
                 certificate.verify_signature(committee)
             }
-            ConsensusTransactionKind::Checkpoint(fragment) => fragment.verify(),
+            ConsensusTransactionKind::Checkpoint(fragment) => fragment.verify(committee.epoch),
         }
     }
 }

crates/sui-types/src/messages_checkpoint.rs

@@ -391,7 +391,7 @@ impl CertifiedCheckpointSummary {
             SuiError::from("Epoch in the summary doesn't match with the committee")
         );
         let mut obligation = VerificationObligation::default();
-        let idx = obligation.add_message(&self.summary);
+        let idx = obligation.add_message(&self.summary, self.auth_signature.epoch);
         self.auth_signature
             .add_to_verification_obligation(committee, &mut obligation, idx)?;
 
@@ -689,7 +689,13 @@ impl CheckpointFragment {
     ) -> Vec<SignedCheckpointFragmentMessage> {
         self.to_message_chunks()
             .into_iter()
-            .map(|message| SignedCheckpointFragmentMessage::new(message, signer))
+            .map(|message| {
+                SignedCheckpointFragmentMessage::new(
+                    message,
+                    self.proposer.auth_signature.epoch,
+                    signer,
+                )
+            })
             .collect()
     }
 
@@ -798,18 +804,19 @@ impl Hash for SignedCheckpointFragmentMessage {
 impl SignedCheckpointFragmentMessage {
     pub fn new(
         message: CheckpointFragmentMessage,
+        epoch: EpochId,
         signer: &dyn signature::Signer<AuthoritySignature>,
     ) -> Self {
-        let signature = AuthoritySignature::new(&message, signer);
+        let signature = AuthoritySignature::new(&message, epoch, signer);
         Self { message, signature }
     }
 
-    pub fn verify(&self) -> SuiResult {
+    pub fn verify(&self, epoch: EpochId) -> SuiResult {
         let proposer = match &self.message {
             CheckpointFragmentMessage::Header(header) => *header.proposer.authority(),
             CheckpointFragmentMessage::Chunk(chunk) => chunk.proposer,
         };
-        self.signature.verify(&self.message, proposer)
+        self.signature.verify(&self.message, epoch, proposer)
     }
 }
 

crates/sui-types/src/unit_tests/base_types_tests.rs

@@ -288,7 +288,7 @@ fn test_transaction_digest_serde_human_readable() {
 #[test]
 fn test_authority_signature_serde_not_human_readable() {
     let (_, key): (_, AuthorityKeyPair) = get_key_pair();
-    let sig = AuthoritySignature::new(&Foo("some data".to_string()), &key);
+    let sig = AuthoritySignature::new(&Foo("some data".to_string()), 0, &key);
     let serialized = bincode::serialize(&sig).unwrap();
     let bcs_serialized = bcs::to_bytes(&sig).unwrap();
 
@@ -300,7 +300,7 @@ fn test_authority_signature_serde_not_human_readable() {
 #[test]
 fn test_authority_signature_serde_human_readable() {
     let (_, key): (_, AuthorityKeyPair) = get_key_pair();
-    let sig = AuthoritySignature::new(&Foo("some data".to_string()), &key);
+    let sig = AuthoritySignature::new(&Foo("some data".to_string()), 0, &key);
     let serialized = serde_json::to_string(&sig).unwrap();
     assert_eq!(
         format!(r#"{{"sig":"{}"}}"#, Base64::encode(sig.as_ref())),

crates/sui-types/src/unit_tests/messages_tests.rs

@@ -238,7 +238,7 @@ fn test_handle_reject_malicious_signature() {
         AuthorityStrongQuorumSignInfo::new_from_auth_sign_infos(signatures, &committee).unwrap();
     {
         let (_, sec): (_, AuthorityKeyPair) = get_key_pair();
-        let sig = AuthoritySignature::new(&message, &sec);
+        let sig = AuthoritySignature::new(&message, committee.epoch, &sec);
         quorum.signature.add_signature(sig).unwrap();
     }
     let (mut obligation, idx) = get_obligation_input(&message);
@@ -248,6 +248,45 @@ fn test_handle_reject_malicious_signature() {
     assert!(obligation.verify_all().is_err());
 }
 
+#[test]
+fn test_auth_sig_commit_to_wrong_epoch_id_fail() {
+    let message: Foo = Foo("some data".to_string());
+    let mut signatures: Vec<AuthoritySignInfo> = Vec::new();
+    let mut authorities: BTreeMap<AuthorityPublicKeyBytes, u64> = BTreeMap::new();
+
+    for _ in 0..5 {
+        let (_, sec): (_, AuthorityKeyPair) = get_key_pair();
+        let name = AuthorityPublicKeyBytes::from(sec.public());
+        authorities.insert(name, 1);
+        signatures.push(AuthoritySignInfo::new(
+            1,
+            &Foo("some data".to_string()),
+            name,
+            &sec,
+        ));
+    }
+    // committee set up with epoch 1
+    let committee = Committee::new(1, authorities.clone()).unwrap();
+    let mut quorum =
+        AuthorityStrongQuorumSignInfo::new_from_auth_sign_infos(signatures, &committee).unwrap();
+    {
+        let (_, sec): (_, AuthorityKeyPair) = get_key_pair();
+        // signature commits to epoch 0
+        let sig = AuthoritySignature::new(&message, 1, &sec);
+        quorum.signature.add_signature(sig).unwrap();
+    }
+    let (mut obligation, idx) = get_obligation_input(&message);
+    assert!(quorum
+        .add_to_verification_obligation(&committee, &mut obligation, idx)
+        .is_ok());
+    assert_eq!(
+        obligation.verify_all(),
+        Err(SuiError::InvalidSignature {
+            error: "General cryptographic error".to_string()
+        })
+    );
+}
+
 #[test]
 fn test_bitmap_out_of_range() {
     let message: Foo = Foo("some data".to_string());