Merge pull request #960 from CosmWasm/move-limits

Move ED25519 msg limits from `crypto` to `vm`

Author: webmaster128

CHANGELOG.md

@@ -16,6 +16,9 @@ and this project adheres to
 - cosmwasm-std: Remove `Context` - deprecated in previous release
 - cosmwasm-std: Remove `HandleResponse`, `InitResponse`, and `MigrateResponse` -
   deprecated in previous release
+- cosmwasm-crypto: Remove `ed25519::MESSAGE_MAX_LEN`, `ed25519::BATCH_MAX_LEN`
+  and message length verification as this should not be a concern of
+  `cosmwasm-crypto`.
 
 [#905]: https://github.com/CosmWasm/cosmwasm/issues/905
 
@@ -33,6 +36,8 @@ and this project adheres to
 - cosmwasm-vm: More accurate error messages for op codes related to bulk memory
   operations, reference types, SIMD and the Threads extension.
 - cosmwasm-vm: Update `wasmer` to `2.0.0-rc2`
+- cosmwasm-vm: ED25519 message length and batch length limits are now hardcoded
+  in `cosmwasm-vm` itself instead of being imported from `cosmwasm-crypto`.
 
 ### Fixed
 

packages/crypto/src/ed25519.rs

@@ -5,16 +5,6 @@ use std::convert::TryInto;
 
 use crate::errors::{CryptoError, CryptoResult};
 
-/// Max length of a message for ed25519 verification in bytes.
-/// This is an arbitrary value, for performance / memory contraints. If you need to verify larger
-/// messages, let us know.
-pub const MESSAGE_MAX_LEN: usize = 128 * 1024;
-
-/// Max number of batch messages / signatures / public_keys.
-/// This is an arbitrary value, for performance / memory contraints. If you need to batch-verify a
-/// larger number of signatures, let us know.
-pub const BATCH_MAX_LEN: usize = 256;
-
 /// Length of a serialized public key
 pub const EDDSA_PUBKEY_LEN: usize = 32;
 
@@ -30,7 +20,6 @@ pub const EDDSA_PUBKEY_LEN: usize = 32;
 /// - public key: raw ED25519 public key (32 bytes).
 pub fn ed25519_verify(message: &[u8], signature: &[u8], public_key: &[u8]) -> CryptoResult<bool> {
     // Validation
-    check_message_length(message)?;
     let signature = read_signature(signature)?;
     let pubkey = read_pubkey(public_key)?;
 
@@ -108,7 +97,6 @@ pub fn ed25519_batch_verify(
         .zip(public_keys.iter())
     {
         // Validation
-        check_message_length(message)?;
         let signature = read_signature(signature)?;
         let pubkey = read_pubkey(public_key)?;
 
@@ -149,28 +137,6 @@ fn read_pubkey(data: &[u8]) -> Result<[u8; 32], InvalidEd25519PubkeyFormat> {
     data.try_into().map_err(|_| InvalidEd25519PubkeyFormat)
 }
 
-struct MessageTooLong {
-    limit: usize,
-    actual: usize,
-}
-
-impl From<MessageTooLong> for CryptoError {
-    fn from(original: MessageTooLong) -> Self {
-        CryptoError::message_too_long(original.limit, original.actual)
-    }
-}
-
-fn check_message_length(message: &[u8]) -> Result<(), MessageTooLong> {
-    if message.len() > MESSAGE_MAX_LEN {
-        Err(MessageTooLong {
-            limit: MESSAGE_MAX_LEN,
-            actual: message.len(),
-        })
-    } else {
-        Ok(())
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;

packages/crypto/src/errors.rs

@@ -34,13 +34,6 @@ pub enum CryptoError {
         #[cfg(feature = "backtraces")]
         backtrace: Backtrace,
     },
-    #[error("Message is longer than supported by this implementation (Limit: {limit}, actual length: {actual})")]
-    MessageTooLong {
-        limit: usize,
-        actual: usize,
-        #[cfg(feature = "backtraces")]
-        backtrace: Backtrace,
-    },
     #[error("Invalid recovery parameter. Supported values: 0 and 1.")]
     InvalidRecoveryParam {
         #[cfg(feature = "backtraces")]
@@ -86,15 +79,6 @@ impl CryptoError {
         }
     }
 
-    pub fn message_too_long(limit: usize, actual: usize) -> Self {
-        CryptoError::MessageTooLong {
-            limit,
-            actual,
-            #[cfg(feature = "backtraces")]
-            backtrace: Backtrace::capture(),
-        }
-    }
-
     pub fn invalid_recovery_param() -> Self {
         CryptoError::InvalidRecoveryParam {
             #[cfg(feature = "backtraces")]
@@ -106,7 +90,6 @@ impl CryptoError {
     /// contract VM boundary.
     pub fn code(&self) -> u32 {
         match self {
-            CryptoError::MessageTooLong { .. } => 2,
             CryptoError::InvalidHashFormat { .. } => 3,
             CryptoError::InvalidSignatureFormat { .. } => 4,
             CryptoError::InvalidPubkeyFormat { .. } => 5,
@@ -162,18 +145,6 @@ mod tests {
         }
     }
 
-    #[test]
-    fn message_too_long_works() {
-        let error = CryptoError::message_too_long(5, 7);
-        match error {
-            CryptoError::MessageTooLong { limit, actual, .. } => {
-                assert_eq!(limit, 5);
-                assert_eq!(actual, 7);
-            }
-            _ => panic!("wrong error type!"),
-        }
-    }
-
     #[test]
     fn invalid_pubkey_format_works() {
         let error = CryptoError::invalid_pubkey_format();

packages/crypto/src/lib.rs

@@ -10,9 +10,9 @@ mod identity_digest;
 mod secp256k1;
 
 #[doc(hidden)]
-pub use crate::ed25519::{ed25519_batch_verify, ed25519_verify};
+pub use crate::ed25519::EDDSA_PUBKEY_LEN;
 #[doc(hidden)]
-pub use crate::ed25519::{BATCH_MAX_LEN, EDDSA_PUBKEY_LEN, MESSAGE_MAX_LEN};
+pub use crate::ed25519::{ed25519_batch_verify, ed25519_verify};
 #[doc(hidden)]
 pub use crate::errors::{CryptoError, CryptoResult};
 #[doc(hidden)]

packages/std/src/errors/recover_pubkey_error.rs

@@ -62,7 +62,6 @@ impl PartialEq<RecoverPubkeyError> for RecoverPubkeyError {
 impl From<CryptoError> for RecoverPubkeyError {
     fn from(original: CryptoError) -> Self {
         match original {
-            CryptoError::MessageTooLong { .. } => panic!("Conversion not supported"),
             CryptoError::InvalidHashFormat { .. } => RecoverPubkeyError::InvalidHashFormat,
             CryptoError::InvalidPubkeyFormat { .. } => panic!("Conversion not supported"),
             CryptoError::InvalidSignatureFormat { .. } => {

packages/std/src/errors/verification_error.rs

@@ -12,8 +12,6 @@ pub enum VerificationError {
     BatchErr,
     #[error("Generic error")]
     GenericErr,
-    #[error("Message is longer than supported")]
-    MessageTooLong,
     #[error("Invalid hash format")]
     InvalidHashFormat,
     #[error("Invalid signature format")]
@@ -45,7 +43,6 @@ impl PartialEq<VerificationError> for VerificationError {
         match self {
             VerificationError::BatchErr => matches!(rhs, VerificationError::BatchErr),
             VerificationError::GenericErr => matches!(rhs, VerificationError::GenericErr),
-            VerificationError::MessageTooLong => matches!(rhs, VerificationError::MessageTooLong),
             VerificationError::InvalidHashFormat => {
                 matches!(rhs, VerificationError::InvalidHashFormat)
             }
@@ -77,7 +74,6 @@ impl PartialEq<VerificationError> for VerificationError {
 impl From<CryptoError> for VerificationError {
     fn from(original: CryptoError) -> Self {
         match original {
-            CryptoError::MessageTooLong { .. } => VerificationError::MessageTooLong,
             CryptoError::InvalidHashFormat { .. } => VerificationError::InvalidHashFormat,
             CryptoError::InvalidPubkeyFormat { .. } => VerificationError::InvalidPubkeyFormat,
             CryptoError::InvalidSignatureFormat { .. } => VerificationError::InvalidSignatureFormat,

packages/std/src/imports.rs

@@ -279,7 +279,7 @@ impl Api for ExternalApi {
         match result {
             0 => Ok(true),
             1 => Ok(false),
-            2 => Err(VerificationError::MessageTooLong),
+            2 => panic!("Error code 2 unused since CosmWasm 0.15. This is a bug in the VM."),
             3 => panic!("InvalidHashFormat must not happen. This is a bug in the VM."),
             4 => Err(VerificationError::InvalidSignatureFormat),
             5 => Err(VerificationError::InvalidPubkeyFormat),
@@ -311,7 +311,7 @@ impl Api for ExternalApi {
         match result {
             0 => Ok(true),
             1 => Ok(false),
-            2 => Err(VerificationError::MessageTooLong),
+            2 => panic!("Error code 2 unused since CosmWasm 0.15. This is a bug in the VM."),
             3 => panic!("InvalidHashFormat must not happen. This is a bug in the VM."),
             4 => Err(VerificationError::InvalidSignatureFormat),
             5 => Err(VerificationError::InvalidPubkeyFormat),

packages/vm/src/imports.rs

@@ -7,8 +7,7 @@ use cosmwasm_crypto::{
     ed25519_batch_verify, ed25519_verify, secp256k1_recover_pubkey, secp256k1_verify, CryptoError,
 };
 use cosmwasm_crypto::{
-    BATCH_MAX_LEN, ECDSA_PUBKEY_MAX_LEN, ECDSA_SIGNATURE_LEN, EDDSA_PUBKEY_LEN,
-    MESSAGE_HASH_MAX_LEN, MESSAGE_MAX_LEN,
+    ECDSA_PUBKEY_MAX_LEN, ECDSA_SIGNATURE_LEN, EDDSA_PUBKEY_LEN, MESSAGE_HASH_MAX_LEN,
 };
 
 #[cfg(feature = "iterator")]
@@ -42,6 +41,14 @@ const MAX_LENGTH_HUMAN_ADDRESS: usize = 90;
 const MAX_LENGTH_QUERY_CHAIN_REQUEST: usize = 64 * KI;
 /// Length of a serialized Ed25519  signature
 const MAX_LENGTH_ED25519_SIGNATURE: usize = 64;
+/// Max length of a Ed25519 message in bytes.
+/// This is an arbitrary value, for performance / memory contraints. If you need to verify larger
+/// messages, let us know.
+const MAX_LENGTH_ED25519_MESSAGE: usize = 128 * 1024;
+/// Max number of batch Ed25519 messages / signatures / public_keys.
+/// This is an arbitrary value, for performance / memory contraints. If you need to batch-verify a
+/// larger number of signatures, let us know.
+const MAX_COUNT_ED25519_BATCH: usize = 256;
 
 /// Max length for a debug message
 const MAX_LENGTH_DEBUG: usize = 2 * MI;
@@ -326,9 +333,9 @@ fn do_secp256k1_verify<A: BackendApi, S: Storage, Q: Querier>(
             | CryptoError::InvalidPubkeyFormat { .. }
             | CryptoError::InvalidSignatureFormat { .. }
             | CryptoError::GenericErr { .. } => err.code(),
-            CryptoError::BatchErr { .. }
-            | CryptoError::InvalidRecoveryParam { .. }
-            | CryptoError::MessageTooLong { .. } => panic!("Error must not happen for this call"),
+            CryptoError::BatchErr { .. } | CryptoError::InvalidRecoveryParam { .. } => {
+                panic!("Error must not happen for this call")
+            }
         },
         |valid| if valid { 0 } else { 1 },
     ))
@@ -360,9 +367,9 @@ fn do_secp256k1_recover_pubkey<A: BackendApi, S: Storage, Q: Querier>(
             | CryptoError::InvalidSignatureFormat { .. }
             | CryptoError::InvalidRecoveryParam { .. }
             | CryptoError::GenericErr { .. } => Ok(to_high_half(err.code())),
-            CryptoError::BatchErr { .. }
-            | CryptoError::InvalidPubkeyFormat { .. }
-            | CryptoError::MessageTooLong { .. } => panic!("Error must not happen for this call"),
+            CryptoError::BatchErr { .. } | CryptoError::InvalidPubkeyFormat { .. } => {
+                panic!("Error must not happen for this call")
+            }
         },
     }
 }
@@ -373,7 +380,7 @@ fn do_ed25519_verify<A: BackendApi, S: Storage, Q: Querier>(
     signature_ptr: u32,
     pubkey_ptr: u32,
 ) -> VmResult<u32> {
-    let message = read_region(&env.memory(), message_ptr, MESSAGE_MAX_LEN)?;
+    let message = read_region(&env.memory(), message_ptr, MAX_LENGTH_ED25519_MESSAGE)?;
     let signature = read_region(&env.memory(), signature_ptr, MAX_LENGTH_ED25519_SIGNATURE)?;
     let pubkey = read_region(&env.memory(), pubkey_ptr, EDDSA_PUBKEY_LEN)?;
 
@@ -382,8 +389,7 @@ fn do_ed25519_verify<A: BackendApi, S: Storage, Q: Querier>(
     process_gas_info::<A, S, Q>(env, gas_info)?;
     Ok(result.map_or_else(
         |err| match err {
-            CryptoError::MessageTooLong { .. }
-            | CryptoError::InvalidPubkeyFormat { .. }
+            CryptoError::InvalidPubkeyFormat { .. }
             | CryptoError::InvalidSignatureFormat { .. }
             | CryptoError::GenericErr { .. } => err.code(),
             CryptoError::BatchErr { .. }
@@ -405,17 +411,17 @@ fn do_ed25519_batch_verify<A: BackendApi, S: Storage, Q: Querier>(
     let messages = read_region(
         &env.memory(),
         messages_ptr,
-        (MESSAGE_MAX_LEN + 4) * BATCH_MAX_LEN,
+        (MAX_LENGTH_ED25519_MESSAGE + 4) * MAX_COUNT_ED25519_BATCH,
     )?;
     let signatures = read_region(
         &env.memory(),
         signatures_ptr,
-        (MAX_LENGTH_ED25519_SIGNATURE + 4) * BATCH_MAX_LEN,
+        (MAX_LENGTH_ED25519_SIGNATURE + 4) * MAX_COUNT_ED25519_BATCH,
     )?;
     let public_keys = read_region(
         &env.memory(),
         public_keys_ptr,
-        (EDDSA_PUBKEY_LEN + 4) * BATCH_MAX_LEN,
+        (EDDSA_PUBKEY_LEN + 4) * MAX_COUNT_ED25519_BATCH,
     )?;
 
     let messages = decode_sections(&messages);
@@ -433,7 +439,6 @@ fn do_ed25519_batch_verify<A: BackendApi, S: Storage, Q: Querier>(
     Ok(result.map_or_else(
         |err| match err {
             CryptoError::BatchErr { .. }
-            | CryptoError::MessageTooLong { .. }
             | CryptoError::InvalidPubkeyFormat { .. }
             | CryptoError::InvalidSignatureFormat { .. }
             | CryptoError::GenericErr { .. } => err.code(),
@@ -1561,7 +1566,7 @@ mod tests {
 
         let mut msg = hex::decode(EDDSA_MSG_HEX).unwrap();
         // extend / break msg
-        msg.extend_from_slice(&[0x00; MESSAGE_MAX_LEN + 1]);
+        msg.extend_from_slice(&[0x00; MAX_LENGTH_ED25519_MESSAGE + 1]);
         let msg_ptr = write_data(&env, &msg);
         let sig = hex::decode(EDDSA_SIG_HEX).unwrap();
         let sig_ptr = write_data(&env, &sig);