[refactor]: forbid construction of queries with invalid signature

Signed-off-by: Marin Veršić <marin.versic101@gmail.com>

cli/src/torii/routing.rs

@@ -1,7 +1,6 @@
 //! Routing functions for Torii. If you want to add an endpoint to
 //! Iroha you should add it here by creating a `handle_*` function,
-//! and add it to impl Torii. This module also defines the `VerifiedQuery`,
-//! which is the only kind of query that is permitted to execute.
+//! and add it to impl Torii.
 
 // FIXME: This can't be fixed, because one trait in `warp` is private.
 #![allow(opaque_hidden_inferred_bound)]
@@ -17,7 +16,6 @@ use iroha_config::{
     GetConfiguration, PostConfiguration,
 };
 use iroha_core::{smartcontracts::isi::query::ValidQueryRequest, sumeragi::SumeragiHandle};
-use iroha_crypto::SignatureOf;
 use iroha_data_model::{
     block::{
         stream::{
@@ -26,16 +24,13 @@ use iroha_data_model::{
         },
         VersionedCommittedBlock,
     },
-    predicate::PredicateBox,
     prelude::*,
-    query,
     query::error::QueryExecutionFail,
 };
 use iroha_logger::prelude::*;
 #[cfg(feature = "telemetry")]
 use iroha_telemetry::metrics::Status;
 use pagination::{paginate, Paginate};
-use parity_scale_codec::{Decode, Encode};
 use tokio::task;
 
 use super::*;
@@ -46,65 +41,6 @@ pub fn sorting() -> impl warp::Filter<Extract = (Sorting,), Error = warp::Reject
     warp::query()
 }
 
-/// Query Request verified on the Iroha node side.
-#[derive(Debug, Decode, Encode)]
-pub struct VerifiedQuery {
-    /// Payload, containing the time, the query, the authenticating
-    /// user account and a filter
-    payload: query::http::QueryPayload,
-    /// Signature of the authenticating user
-    signature: SignatureOf<query::http::QueryPayload>,
-}
-
-impl VerifiedQuery {
-    /// Validate query.
-    ///
-    /// # Errors
-    /// - Account doesn't exist
-    /// - Account doesn't have the correct public key
-    /// - Account has incorrect permissions
-    pub fn validate(
-        self,
-        wsv: &mut WorldStateView,
-    ) -> Result<(ValidQueryRequest, PredicateBox), ValidationFail> {
-        let account_has_public_key = wsv
-            .map_account(&self.payload.authority, |account| {
-                account.signatories.contains(self.signature.public_key())
-            })
-            .map_err(QueryExecutionFail::from)?;
-        if !account_has_public_key {
-            return Err(QueryExecutionFail::Signature(String::from(
-                "Signature public key doesn't correspond to the account.",
-            ))
-            .into());
-        }
-        wsv.validator_view().clone().validate(
-            wsv,
-            &self.payload.authority,
-            self.payload.query.clone(),
-        )?;
-        Ok((
-            ValidQueryRequest::new(self.payload.query),
-            self.payload.filter,
-        ))
-    }
-}
-
-impl TryFrom<SignedQuery> for VerifiedQuery {
-    type Error = QueryExecutionFail;
-
-    fn try_from(query: SignedQuery) -> Result<Self, Self::Error> {
-        query
-            .signature
-            .verify(&query.payload)
-            .map(|_| Self {
-                payload: query.payload,
-                signature: query.signature,
-            })
-            .map_err(|e| Self::Error::Signature(e.to_string()))
-    }
-}
-
 #[iroha_futures::telemetry_future]
 pub(crate) async fn handle_instructions(
     queue: Arc<Queue>,
@@ -136,19 +72,15 @@ pub(crate) async fn handle_queries(
     sorting: Sorting,
     request: VersionedSignedQuery,
 ) -> Result<Scale<VersionedPaginatedQueryResult>> {
-    let VersionedSignedQuery::V1(request) = request;
-    let request: VerifiedQuery = request.try_into().map_err(ValidationFail::from)?;
-
-    let (result, filter) = {
+    let result = {
         let mut wsv = sumeragi.wsv_clone();
-        let (valid_request, filter) = request.validate(&mut wsv)?;
-        let original_result = valid_request.execute(&wsv).map_err(ValidationFail::from)?;
-        (filter.filter(original_result), filter)
+        let valid_request = ValidQueryRequest::validate(request, &mut wsv)?;
+        valid_request.execute(&wsv).map_err(ValidationFail::from)?
     };
 
     let (total, result) = if let Value::Vec(vec_of_val) = result {
         let len = vec_of_val.len();
-        let vec_of_val = apply_sorting_and_pagination(vec_of_val, &sorting, pagination);
+        let vec_of_val = apply_sorting_and_pagination(vec_of_val.into_iter(), &sorting, pagination);
 
         (len, Value::Vec(vec_of_val))
     } else {
@@ -164,20 +96,18 @@ pub(crate) async fn handle_queries(
         result,
         pagination,
         sorting,
-        filter,
         total,
     };
     Ok(Scale(paginated_result.into()))
 }
 
 fn apply_sorting_and_pagination(
-    vec_of_val: Vec<Value>,
+    vec_of_val: impl Iterator<Item = Value>,
     sorting: &Sorting,
     pagination: Pagination,
 ) -> Vec<Value> {
     if let Some(key) = &sorting.sort_by_metadata_key {
         let mut pairs: Vec<(Option<Value>, Value)> = vec_of_val
-            .into_iter()
             .map(|value| {
                 let key = match &value {
                     Value::Identifiable(IdentifiableBox::Asset(asset)) => match asset.value() {
@@ -207,7 +137,7 @@ fn apply_sorting_and_pagination(
             .paginate(pagination)
             .collect()
     } else {
-        vec_of_val.into_iter().paginate(pagination).collect()
+        vec_of_val.paginate(pagination).collect()
     }
 }
 

client/src/client.rs

@@ -249,8 +249,6 @@ where
 {
     /// Query output
     pub output: R::Output,
-    /// The filter that was applied to the output.
-    pub filter: PredicateBox,
     /// See [`iroha_data_model::prelude::PaginatedQueryResult`]
     pub pagination: Pagination,
     /// See [`iroha_data_model::prelude::PaginatedQueryResult`]
@@ -283,7 +281,6 @@ where
             pagination,
             sorting,
             total,
-            filter,
         }: PaginatedQueryResult,
     ) -> Result<Self> {
         let output = R::Output::try_from(result.into())
@@ -295,7 +292,6 @@ where
             pagination,
             sorting,
             total,
-            filter,
         })
     }
 }

core/src/smartcontracts/isi/asset.rs

@@ -449,15 +449,19 @@ pub mod query {
     impl ValidQuery for FindAllAssets {
         #[metrics(+"find_all_assets")]
         fn execute(&self, wsv: &WorldStateView) -> Result<Self::Output, Error> {
-            let mut vec = Vec::new();
-            for domain in wsv.domains().values() {
-                for account in domain.accounts.values() {
-                    for asset in account.assets.values() {
-                        vec.push(asset.clone())
-                    }
-                }
-            }
-            Ok(vec)
+            Ok(wsv
+                .domains()
+                .values()
+                .map(|domain| {
+                    domain
+                        .accounts
+                        .values()
+                        .map(|account| account.assets.values())
+                        .flatten()
+                })
+                .flatten()
+                .cloned()
+                .collect())
         }
     }
 

core/src/smartcontracts/isi/query.rs

@@ -13,24 +13,43 @@ use crate::{prelude::ValidQuery, WorldStateView};
 
 /// Query Request statefully validated on the Iroha node side.
 #[derive(Debug, Decode, Encode)]
-pub struct ValidQueryRequest {
-    query: QueryBox,
-}
+pub struct ValidQueryRequest(VersionedSignedQuery);
 
 impl ValidQueryRequest {
+    /// Validate query.
+    ///
+    /// # Errors
+    /// - Account doesn't exist
+    /// - Account doesn't have the correct public key
+    /// - Account has incorrect permissions
+    pub fn validate(
+        query: VersionedSignedQuery,
+        wsv: &mut WorldStateView,
+    ) -> Result<Self, ValidationFail> {
+        let account_has_public_key = wsv
+            .map_account(query.authority(), |account| {
+                account.signatories.contains(query.signature().public_key())
+            })
+            .map_err(Error::from)?;
+        if !account_has_public_key {
+            return Err(Error::Signature(String::from(
+                "Signature public key doesn't correspond to the account.",
+            ))
+            .into());
+        }
+        wsv.validator_view()
+            .clone()
+            .validate(wsv, query.authority(), query.query().clone())?;
+        Ok(ValidQueryRequest(query))
+    }
+
     /// Execute contained query on the [`WorldStateView`].
     ///
     /// # Errors
     /// Forwards `self.query.execute` error.
     #[inline]
     pub fn execute(&self, wsv: &WorldStateView) -> Result<Value, Error> {
-        self.query.execute(wsv)
-    }
-
-    /// Construct `ValidQueryRequest` from a validated query
-    #[must_use]
-    pub const fn new(query: QueryBox) -> Self {
-        Self { query }
+        Ok(self.0.filter().filter(self.0.query().execute(wsv)?))
     }
 }
 

data_model/src/query.rs

@@ -1318,7 +1318,7 @@ pub mod http {
         }
 
         /// I/O ready structure to send queries.
-        #[derive(Debug, Clone, Decode, Encode, Deserialize, Serialize, IntoSchema)]
+        #[derive(Debug, Clone, Encode, Serialize, IntoSchema)]
         #[version_with_scale(n = 1, versioned = "VersionedSignedQuery")]
         pub struct SignedQuery {
             /// Payload
@@ -1340,15 +1340,82 @@ pub mod http {
         pub struct QueryResult(pub Value);
     }
 
+    mod candidate {
+        use parity_scale_codec::Input;
+
+        use super::*;
+
+        #[derive(Decode, Deserialize)]
+        struct SignedQueryCandidate {
+            payload: QueryPayload,
+            signature: SignatureOf<QueryPayload>,
+        }
+
+        impl SignedQueryCandidate {
+            fn validate(self) -> Result<SignedQuery, &'static str> {
+                #[cfg(feature = "std")]
+                if self.signature.verify(&self.payload).is_err() {
+                    return Err("Query signature not valid");
+                }
+
+                Ok(SignedQuery {
+                    payload: self.payload,
+                    signature: self.signature,
+                })
+            }
+        }
+        impl Decode for SignedQuery {
+            fn decode<I: Input>(input: &mut I) -> Result<Self, parity_scale_codec::Error> {
+                SignedQueryCandidate::decode(input)?
+                    .validate()
+                    .map_err(Into::into)
+            }
+        }
+        impl<'de> Deserialize<'de> for SignedQuery {
+            fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+            where
+                D: serde::Deserializer<'de>,
+            {
+                use serde::de::Error as _;
+
+                SignedQueryCandidate::deserialize(deserializer)?
+                    .validate()
+                    .map_err(D::Error::custom)
+            }
+        }
+    }
+
+    #[cfg(feature = "transparent_api")]
+    impl VersionedSignedQuery {
+        /// Return query signature
+        pub fn signature(&self) -> &SignatureOf<QueryPayload> {
+            let VersionedSignedQuery::V1(query) = self;
+            &query.signature
+        }
+        /// Return query payload
+        pub fn query(&self) -> &QueryBox {
+            let VersionedSignedQuery::V1(query) = self;
+            &query.payload.query
+        }
+        /// Return query authority
+        pub fn authority(&self) -> &AccountId {
+            let VersionedSignedQuery::V1(query) = self;
+            &query.payload.authority
+        }
+        /// Return query filter
+        pub fn filter(&self) -> &PredicateBox {
+            let VersionedSignedQuery::V1(query) = self;
+            &query.payload.filter
+        }
+    }
+
     /// Paginated Query Result
     // TODO: This is the only structure whose inner fields are exposed. Wrap it in model macro?
     #[derive(Debug, Clone, Decode, Encode, Deserialize, Serialize, IntoSchema)]
     #[version_with_scale(n = 1, versioned = "VersionedPaginatedQueryResult")]
     pub struct PaginatedQueryResult {
         /// The result of the query execution.
         pub result: QueryResult,
-        /// The filter that was applied to the Query result. Returned as a sanity check, but also to ease debugging on the front-end.
-        pub filter: PredicateBox,
         /// pagination
         pub pagination: Pagination,
         /// sorting