Skip to content

Commit

Permalink
[refactor]: forbid construction of queries with invalid signature
Browse files Browse the repository at this point in the history
Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
  • Loading branch information
mversic committed Oct 17, 2023
1 parent 4fec24e commit 5a6c592
Show file tree
Hide file tree
Showing 5 changed files with 119 additions and 103 deletions.
84 changes: 7 additions & 77 deletions cli/src/torii/routing.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand All @@ -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::{
Expand All @@ -26,15 +24,12 @@ use iroha_data_model::{
},
VersionedCommittedBlock,
},
predicate::PredicateBox,
prelude::*,
query,
query::error::QueryExecutionFail,
};
#[cfg(feature = "telemetry")]
use iroha_telemetry::metrics::Status;
use pagination::{paginate, Paginate};
use parity_scale_codec::{Decode, Encode};
use tokio::task;

use super::*;
Expand All @@ -45,65 +40,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>,
Expand Down Expand Up @@ -135,19 +71,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 {
Expand All @@ -163,20 +95,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() {
Expand Down Expand Up @@ -206,7 +136,7 @@ fn apply_sorting_and_pagination(
.paginate(pagination)
.collect()
} else {
vec_of_val.into_iter().paginate(pagination).collect()
vec_of_val.paginate(pagination).collect()
}
}

Expand Down
4 changes: 0 additions & 4 deletions client/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,8 +247,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`]
Expand Down Expand Up @@ -281,7 +279,6 @@ where
pagination,
sorting,
total,
filter,
}: PaginatedQueryResult,
) -> Result<Self> {
let output = R::Output::try_from(result.into())
Expand All @@ -293,7 +290,6 @@ where
pagination,
sorting,
total,
filter,
})
}
}
Expand Down
22 changes: 13 additions & 9 deletions core/src/smartcontracts/isi/asset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -445,15 +445,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())
}
}

Expand Down
39 changes: 29 additions & 10 deletions core/src/smartcontracts/isi/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?))
}
}

Expand Down
73 changes: 70 additions & 3 deletions data_model/src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1280,7 +1280,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
Expand All @@ -1302,15 +1302,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
Expand Down

0 comments on commit 5a6c592

Please sign in to comment.