From 3614df792a5fa1f4ffcf3b6212ad1da0e0595be7 Mon Sep 17 00:00:00 2001 From: bwty Date: Thu, 9 Feb 2023 23:36:50 +0800 Subject: [PATCH] feat: #41 and #42 Signed-off-by: bwty --- src/data_types/pres_request.rs | 59 ++++++++- src/ffi/presentation.rs | 5 +- src/services/helpers.rs | 51 +------- src/services/verifier.rs | 231 +++++++++++++-------------------- tests/anoncreds_demos.rs | 3 + tests/multiple-credentials.rs | 16 +-- tests/utils/mock.rs | 1 + 7 files changed, 161 insertions(+), 205 deletions(-) diff --git a/src/data_types/pres_request.rs b/src/data_types/pres_request.rs index 58de223f..ef9ac15c 100644 --- a/src/data_types/pres_request.rs +++ b/src/data_types/pres_request.rs @@ -122,12 +122,59 @@ impl Serialize for PresentationRequest { #[allow(unused)] pub type PresentationRequestExtraQuery = HashMap; -#[derive(Clone, Debug, PartialEq, Eq, Hash, Deserialize, Serialize)] +#[derive(Clone, Default, Debug, PartialEq, Eq, Hash, Deserialize, Serialize)] pub struct NonRevocedInterval { pub from: Option, pub to: Option, } +impl NonRevocedInterval { + // Returns the most stringent interval, + // i.e. the latest from and the earliest to + pub fn compare_and_set(&mut self, to_compare: &NonRevocedInterval) { + // Update if + // - the new `from` value is later, smaller interval + // - the new `from` value is Some if previouly was None + match (self.from, to_compare.from) { + (Some(old_from), Some(new_from)) => { + if old_from.lt(&new_from) { + self.from = to_compare.from + } + } + (None, Some(_)) => self.from = to_compare.from, + _ => (), + } + // Update if + // - the new `to` value is earlier, smaller interval + // - the new `to` value is Some if previouly was None + match (self.to, to_compare.to) { + (Some(old_to), Some(new_to)) => { + if new_to.lt(&old_to) { + self.to = to_compare.to + } + } + (None, Some(_)) => self.to = to_compare.to, + _ => (), + } + } + + pub fn update_with_override(&mut self, override_map: &HashMap) { + self.from.map(|from| { + override_map + .get(&from) + .map(|&override_timestamp| self.from = Some(override_timestamp)) + }); + } + + pub fn is_valid(&self, timestamp: u64) -> Result<(), ValidationError> { + if timestamp.lt(&self.from.unwrap_or(0)) || timestamp.gt(&self.to.unwrap_or(u64::MAX)) { + Err(invalid!("Invalid timestamp")) + } else { + Ok(()) + } + } +} + #[derive(Clone, Debug, Eq, PartialEq, Deserialize, Serialize)] pub struct AttributeInfo { #[serde(skip_serializing_if = "Option::is_none")] @@ -330,4 +377,14 @@ mod tests { serde_json::from_str::(&req_json).unwrap_err(); } } + + #[test] + fn override_works() { + let mut interval = NonRevocedInterval::default(); + let override_map = HashMap::from([(10u64, 5u64)]); + + interval.from = Some(10); + interval.update_with_override(&override_map); + assert_eq!(interval.from.unwrap(), 5u64); + } } diff --git a/src/ffi/presentation.rs b/src/ffi/presentation.rs index bfb542d0..c56c1d86 100644 --- a/src/ffi/presentation.rs +++ b/src/ffi/presentation.rs @@ -202,10 +202,13 @@ pub extern "C" fn anoncreds_create_presentation( /// E.g. if the ledger has Revocation Status List at timestamps [0, 100, 200], /// let's call them List0, List100, List200. Then: /// +/// ```txt +/// /// List0 is valid List100 is valid /// ______|_______ _______|_______ /// | | | /// List 0 ----------- 100 ----------- 200 +/// ``` /// /// A `nonrevoked_interval = {from: 50, to: 150}` should accept both List0 and /// List100. @@ -310,7 +313,7 @@ pub extern "C" fn anoncreds_verify_presentation( let override_entries = { let override_ffi_entries = nonrevoked_interval_override.as_slice(); - override_ffi_entries.into_iter().try_fold( + override_ffi_entries.iter().try_fold( Vec::with_capacity(override_ffi_entries.len()), |mut v, entry| -> Result> { v.push(entry.load()?); diff --git a/src/services/helpers.rs b/src/services/helpers.rs index ba8633e2..2f14ed78 100644 --- a/src/services/helpers.rs +++ b/src/services/helpers.rs @@ -3,7 +3,7 @@ use std::collections::{HashMap, HashSet}; use crate::data_types::{ credential::AttributeValues, nonce::Nonce, - pres_request::{AttributeInfo, NonRevocedInterval, PredicateInfo}, + pres_request::{AttributeInfo, PredicateInfo}, }; use crate::utils::hash::SHA256; @@ -134,55 +134,6 @@ pub fn build_sub_proof_request( Ok(res) } -pub fn get_non_revoc_interval( - global_interval: &Option, - local_interval: &Option, -) -> Option { - trace!( - "get_non_revoc_interval >>> global_interval: {:?}, local_interval: {:?}", - global_interval, - local_interval - ); - - let interval = local_interval - .clone() - .or_else(|| global_interval.clone().or(None)); - - trace!("get_non_revoc_interval <<< interval: {:?}", interval); - - interval -} - pub fn new_nonce() -> Result { Nonce::new().map_err(err_map!(Unexpected)) } - -#[cfg(test)] -mod tests { - use super::*; - - fn _interval() -> NonRevocedInterval { - NonRevocedInterval { - from: None, - to: Some(123), - } - } - - #[test] - fn get_non_revoc_interval_for_global() { - let res = get_non_revoc_interval(&Some(_interval()), &None).unwrap(); - assert_eq!(_interval(), res); - } - - #[test] - fn get_non_revoc_interval_for_local() { - let res = get_non_revoc_interval(&None, &Some(_interval())).unwrap(); - assert_eq!(_interval(), res); - } - - #[test] - fn get_non_revoc_interval_for_none() { - let res = get_non_revoc_interval(&None, &None); - assert_eq!(None, res); - } -} diff --git a/src/services/verifier.rs b/src/services/verifier.rs index d3b92730..85787c3d 100644 --- a/src/services/verifier.rs +++ b/src/services/verifier.rs @@ -44,6 +44,7 @@ pub fn verify_presentation( cred_defs: &HashMap<&CredentialDefinitionId, &CredentialDefinition>, rev_reg_defs: Option<&HashMap<&RevocationRegistryDefinitionId, &RevocationRegistryDefinition>>, rev_status_lists: Option>, + // Override Map: HashMap nonrevoke_interval_override: Option< &HashMap<&RevocationRegistryDefinitionId, HashMap>, >, @@ -51,7 +52,7 @@ pub fn verify_presentation( trace!("verify >>> presentation: {:?}, pres_req: {:?}, schemas: {:?}, cred_defs: {:?}, rev_reg_defs: {:?} rev_status_lists: {:?}", presentation, pres_req, schemas, cred_defs, rev_reg_defs, rev_status_lists); - let pres_req = pres_req.value(); + // These values are from the prover and cannot be trusted let received_revealed_attrs: HashMap = received_revealed_attrs(presentation)?; let received_unrevealed_attrs: HashMap = @@ -59,6 +60,9 @@ pub fn verify_presentation( let received_predicates: HashMap = received_predicates(presentation)?; let received_self_attested_attrs: HashSet = received_self_attested_attrs(presentation); + let pres_req = pres_req.value(); + + // Ensures that all attributes in the request is also in the presentation compare_attr_from_proof_and_request( pres_req, &received_revealed_attrs, @@ -67,8 +71,10 @@ pub fn verify_presentation( &received_predicates, )?; + // This ensures the encoded values are same as request verify_revealed_attribute_values(pres_req, presentation)?; + // This does not verify non-revoked requirements verify_requested_restrictions( pres_req, schemas, @@ -80,16 +86,6 @@ pub fn verify_presentation( &received_self_attested_attrs, )?; - // makes sure the for revocable request or attribute, - // there is a timestamp in the `Identifier` - compare_timestamps_from_proof_and_request( - pres_req, - &received_revealed_attrs, - &received_unrevealed_attrs, - &received_self_attested_attrs, - &received_predicates, - )?; - let mut proof_verifier = CryptoVerifier::new_proof_verifier()?; let non_credential_schema = build_non_credential_schema()?; @@ -127,26 +123,49 @@ pub fn verify_presentation( Into::>::into(*list) .ok_or_else(|| err_msg!(Unexpected, "RevStatusList missing Accum"))?; - if map.get(&id).and_then(|t| t.get(×tamp)).is_some() { - return Err(err_msg!( - Unexpected, - "Duplicated timestamp for Revocation Status List" - )); - } else if map.get(&id).is_none() { - map.insert(id, HashMap::from([(timestamp, rev_reg)])); - } else { - map.get_mut(&id).unwrap().insert(timestamp, rev_reg); - } + map.entry(id) + .or_insert_with(HashMap::new) + .insert(timestamp, rev_reg); } Some(map) } else { None }; - let (rev_reg_def, rev_reg) = if let Some(timestamp) = identifier.timestamp { - let rev_reg_id = identifier.rev_reg_id.clone().ok_or_else(|| { - err_msg!("Timestamp provided but Revocation Registry Id not found") - })?; + let (attrs_for_credential, attrs_nonrevoked_interval) = + get_revealed_attributes_for_credential( + sub_proof_index, + &presentation.requested_proof, + pres_req, + )?; + let (predicates_for_credential, pred_nonrevoked_interval) = get_predicates_for_credential( + sub_proof_index, + &presentation.requested_proof, + pres_req, + )?; + + // Collaspe to the most stringent interval, + // we can do this because there is only 1 revocation status list for this credential + // if it satsifies the most stringent interval, it will satisfy all intervals + let mut cred_nonrevoked_interval = attrs_nonrevoked_interval; + cred_nonrevoked_interval.compare_and_set(&pred_nonrevoked_interval); + if let Some(interval) = pres_req.non_revoked.as_ref() { + cred_nonrevoked_interval.compare_and_set(interval); + }; + + // Revocation checks is required iff both conditions are met: + // - Credential is revokable (cred_defs is input by the verifier, trustable) + // - PresentationReq has asked for NRP* (input from verifier, trustable) + // + // * This is done by setting a NonRevokedInterval either for attr / predicate / global + let (rev_reg_def, rev_reg) = if let (Some(_), true) = ( + cred_def.value.revocation.as_ref(), + cred_nonrevoked_interval.from.is_some() || cred_nonrevoked_interval.to.is_some(), + ) { + let timestamp = identifier + .timestamp + .ok_or_else(|| err_msg!("Identifier timestamp not found for revocation check"))?; + if rev_reg_defs.is_none() { return Err(err_msg!( "Timestamp provided but no Revocation Registry Definitions found" @@ -158,8 +177,22 @@ pub fn verify_presentation( )); } + let rev_reg_id = identifier + .rev_reg_id + .clone() + .ok_or_else(|| err_msg!("Revocation Registry Id not found for revocation check"))?; + // Revocation registry definition id is the same as the rev reg id let rev_reg_def_id = RevocationRegistryDefinitionId::new(rev_reg_id.clone())?; + + // Override Interval if an earlier `from` value is accepted by the verifier + nonrevoke_interval_override.map(|maps| { + maps.get(&rev_reg_def_id) + .map(|map| cred_nonrevoked_interval.update_with_override(map)) + }); + + cred_nonrevoked_interval.is_valid(timestamp)?; + let rev_reg_def = Some( rev_reg_defs .as_ref() @@ -193,17 +226,6 @@ pub fn verify_presentation( (None, None) }; - let attrs_for_credential = get_revealed_attributes_for_credential( - sub_proof_index, - &presentation.requested_proof, - pres_req, - )?; - let predicates_for_credential = get_predicates_for_credential( - sub_proof_index, - &presentation.requested_proof, - pres_req, - )?; - let credential_schema = build_credential_schema(&schema.attr_names.0)?; let sub_pres_request = build_sub_proof_request(&attrs_for_credential, &predicates_for_credential)?; @@ -240,10 +262,11 @@ fn get_revealed_attributes_for_credential( sub_proof_index: usize, requested_proof: &RequestedProof, pres_req: &PresentationRequestPayload, -) -> Result> { +) -> Result<(Vec, NonRevocedInterval)> { trace!("_get_revealed_attributes_for_credential >>> sub_proof_index: {:?}, requested_credentials: {:?}, pres_req: {:?}", sub_proof_index, requested_proof, pres_req); + let mut nonrevoked_interval = NonRevocedInterval::default(); let mut revealed_attrs_for_credential = requested_proof .revealed_attrs .iter() @@ -251,7 +274,13 @@ fn get_revealed_attributes_for_credential( sub_proof_index == revealed_attr_info.sub_proof_index as usize && pres_req.requested_attributes.contains_key(attr_referent) }) - .map(|(attr_referent, _)| pres_req.requested_attributes[attr_referent].clone()) + .map(|(attr_referent, _)| { + let info = pres_req.requested_attributes[attr_referent].clone(); + if info.non_revoked.is_some() { + nonrevoked_interval.compare_and_set(info.non_revoked.as_ref().unwrap()); + } + info + }) .collect::>(); revealed_attrs_for_credential.append( @@ -262,7 +291,13 @@ fn get_revealed_attributes_for_credential( sub_proof_index == revealed_attr_info.sub_proof_index as usize && pres_req.requested_attributes.contains_key(attr_referent) }) - .map(|(attr_referent, _)| pres_req.requested_attributes[attr_referent].clone()) + .map(|(attr_referent, _)| { + let info = pres_req.requested_attributes[attr_referent].clone(); + if info.non_revoked.is_some() { + nonrevoked_interval.compare_and_set(info.non_revoked.as_ref().unwrap()); + } + info + }) .collect::>(), ); @@ -271,17 +306,18 @@ fn get_revealed_attributes_for_credential( revealed_attrs_for_credential ); - Ok(revealed_attrs_for_credential) + Ok((revealed_attrs_for_credential, nonrevoked_interval)) } fn get_predicates_for_credential( sub_proof_index: usize, requested_proof: &RequestedProof, pres_req: &PresentationRequestPayload, -) -> Result> { +) -> Result<(Vec, NonRevocedInterval)> { trace!("_get_predicates_for_credential >>> sub_proof_index: {:?}, requested_credentials: {:?}, pres_req: {:?}", sub_proof_index, requested_proof, pres_req); + let mut nonrevoked_interval = NonRevocedInterval::default(); let predicates_for_credential = requested_proof .predicates .iter() @@ -291,7 +327,13 @@ fn get_predicates_for_credential( .requested_predicates .contains_key(predicate_referent) }) - .map(|(predicate_referent, _)| pres_req.requested_predicates[predicate_referent].clone()) + .map(|(predicate_referent, _)| { + let info = pres_req.requested_predicates[predicate_referent].clone(); + if info.non_revoked.is_some() { + nonrevoked_interval.compare_and_set(info.non_revoked.as_ref().unwrap()); + } + info + }) .collect::>(); trace!( @@ -299,7 +341,7 @@ fn get_predicates_for_credential( predicates_for_credential ); - Ok(predicates_for_credential) + Ok((predicates_for_credential, nonrevoked_interval)) } fn compare_attr_from_proof_and_request( @@ -343,92 +385,6 @@ fn compare_attr_from_proof_and_request( Ok(()) } -// This does not actually compare the non_revoke interval -// see `validate_timestamp` function comments -fn compare_timestamps_from_proof_and_request( - pres_req: &PresentationRequestPayload, - received_revealed_attrs: &HashMap, - received_unrevealed_attrs: &HashMap, - received_self_attested_attrs: &HashSet, - received_predicates: &HashMap, -) -> Result<()> { - pres_req - .requested_attributes - .iter() - .map(|(referent, info)| { - validate_timestamp( - received_revealed_attrs, - referent, - &pres_req.non_revoked, - &info.non_revoked, - ) - .or_else(|_| { - validate_timestamp( - received_unrevealed_attrs, - referent, - &pres_req.non_revoked, - &info.non_revoked, - ) - }) - .or_else(|_| { - received_self_attested_attrs - .get(referent) - .map(|_| ()) - .ok_or_else(|| err_msg!("Missing referent: {}", referent)) - }) - }) - .collect::>>()?; - - pres_req - .requested_predicates - .iter() - .map(|(referent, info)| { - validate_timestamp( - received_predicates, - referent, - &pres_req.non_revoked, - &info.non_revoked, - ) - }) - .collect::>>()?; - - Ok(()) -} - -// This validates that a timestamp is given if either: -// - the `global_interval` rev requirement -// - the `local_interval` rev requirement -// from the PresentationRequest are satisfied. -// -// If either the attribute nor the request has a revocation internal -// i.e. they are non-revocable, then `OK` is returned directly. -// -// Otherwise the Identifier for the referent (attribute) has to have a timestamp, -// which was added by the prover when creating `PresentCredentials`, -// an arg for `create_presentation`. -// -// TODO: this timestamp should be compared with the provided interval -fn validate_timestamp( - received_: &HashMap, - referent: &str, - global_interval: &Option, - local_interval: &Option, -) -> Result<()> { - if get_non_revoc_interval(global_interval, local_interval).is_none() { - return Ok(()); - } - - if !received_ - .get(referent) - .map(|attr| attr.timestamp.is_some()) - .unwrap_or(false) - { - return Err(err_msg!("Missing timestamp")); - } - - Ok(()) -} - fn received_revealed_attrs(proof: &Presentation) -> Result> { let mut revealed_identifiers: HashMap = HashMap::new(); for (referent, info) in proof.requested_proof.revealed_attrs.iter() { @@ -600,6 +556,7 @@ fn verify_revealed_attribute_value( Ok(()) } +#[allow(clippy::too_many_arguments)] fn verify_requested_restrictions( pres_req: &PresentationRequestPayload, schemas: &HashMap<&SchemaId, &Schema>, @@ -894,7 +851,7 @@ fn process_filter( } tag_ @ "schema_name" => precess_filed(tag_, &filter.schema_name, tag_value), tag_ @ "schema_version" => precess_filed(tag_, &filter.schema_version, tag_value), - tag_ @ "cred_def_id" => precess_filed(tag_, &filter.cred_def_id.to_string(), tag_value), + tag_ @ "cred_def_id" => precess_filed(tag_, filter.cred_def_id.to_string(), tag_value), tag_ @ "issuer_did" => precess_filed(tag_, filter.issuer_id.to_owned(), tag_value), tag_ @ "issuer_id" => precess_filed(tag_, filter.issuer_id.to_owned(), tag_value), x if is_attr_internal_tag(x, attr_value_map) => { @@ -1313,18 +1270,4 @@ mod tests { to: Some(1234), } } - - #[test] - fn validate_timestamp_works() { - validate_timestamp(&_received(), "referent_1", &None, &None).unwrap(); - validate_timestamp(&_received(), "referent_1", &Some(_interval()), &None).unwrap(); - validate_timestamp(&_received(), "referent_1", &None, &Some(_interval())).unwrap(); - } - - #[test] - fn validate_timestamp_not_work() { - validate_timestamp(&_received(), "referent_2", &Some(_interval()), &None).unwrap_err(); - validate_timestamp(&_received(), "referent_2", &None, &Some(_interval())).unwrap_err(); - validate_timestamp(&_received(), "referent_3", &None, &Some(_interval())).unwrap_err(); - } } diff --git a/tests/anoncreds_demos.rs b/tests/anoncreds_demos.rs index 45efa978..1a430f24 100644 --- a/tests/anoncreds_demos.rs +++ b/tests/anoncreds_demos.rs @@ -220,6 +220,7 @@ fn anoncreds_works_for_single_issuer_single_prover() { &cred_defs, None, None, + None, ) .expect("Error verifying presentation"); assert!(valid); @@ -432,6 +433,7 @@ fn anoncreds_with_revocation_works_for_single_issuer_single_prover() { &cred_defs, Some(&rev_reg_def_map), Some(rev_status_list.clone()), + None, ) .expect("Error verifying presentation"); assert!(valid); @@ -477,6 +479,7 @@ fn anoncreds_with_revocation_works_for_single_issuer_single_prover() { &cred_defs, Some(&rev_reg_def_map), Some(rev_status_list), + None, ) .expect("Error verifying presentation"); assert!(!valid); diff --git a/tests/multiple-credentials.rs b/tests/multiple-credentials.rs index a72aa5d7..6e0ecb8f 100644 --- a/tests/multiple-credentials.rs +++ b/tests/multiple-credentials.rs @@ -28,9 +28,6 @@ static CRED_DEF_ID_2: &'static str = "mock:uri:2"; static REV_REG_ID_1: &'static str = "mock:uri:revregid1"; static REV_REG_ID_2: &'static str = "mock:uri:revregid2"; -// Credential 1 is revocable -// Credential 2 is non-revocable -// There are 2 definitions, issued by 2 issuers fn test_2_different_revoke_reqs() -> Vec { let nonce_1 = verifier::generate_nonce().expect("Error generating presentation request nonce"); let nonce_2 = verifier::generate_nonce().expect("Error generating presentation request nonce"); @@ -91,6 +88,9 @@ fn anoncreds_with_multiple_credentials_per_request() { let mut mock = utils::Mock::new(&[ISSUER_ID], &[PROVER_ID], TF_PATH, MAX_CRED_NUM); // These are what the issuer knows + // Credential 1 is revocable + // Credential 2 is non-revocable + // There are 2 definitions, issued by 1 issuer let issuer1_creds: utils::IsserValues = HashMap::from([ ( CRED_DEF_ID_1, @@ -116,9 +116,7 @@ fn anoncreds_with_multiple_credentials_per_request() { ("house", "Hufflepuff"), ("year", "1990"), ]), - // Issue 42 addresses if it is not revocable, - // revocation should still pass - true, + false, REV_REG_ID_2, REV_IDX_2, ), @@ -138,9 +136,9 @@ fn anoncreds_with_multiple_credentials_per_request() { mock.ledger.schemas = schemas; - // This is out of range of revoke interval, should get warning - let time_initial_rev_reg = 123u64; - let time_after_credential = 124u64; + // These are within interval + let time_initial_rev_reg = 8u64; + let time_after_credential = 10u64; let issuance_by_default = true; // To test: diff --git a/tests/utils/mock.rs b/tests/utils/mock.rs index d06ed66a..acf8573d 100644 --- a/tests/utils/mock.rs +++ b/tests/utils/mock.rs @@ -93,6 +93,7 @@ impl<'a> Mock<'a> { &cred_defs, Some(&rev_reg_def_map), Some(rev_status_lists.clone()), + None, ) .expect("Error verifying presentation"); results.push(valid);