feat(rust): improve PurposeKey and Credential verification

implementations/rust/ockam/ockam_command/src/util/output.rs

@@ -679,7 +679,7 @@ impl fmt::Display for IdentityDisplay {
             writeln!(
                 f,
                 "    revoke_all_purpose_keys: {}",
-                change.revoke_all_purpose_keys()
+                change.data().revoke_all_purpose_keys
             )?;
         }
 

implementations/rust/ockam/ockam_identity/src/credentials/credentials.rs

@@ -10,7 +10,7 @@ use core::time::Duration;
 use ockam_core::compat::sync::Arc;
 use ockam_core::compat::vec::Vec;
 use ockam_core::Result;
-use ockam_vault::{SecretType, Signature, SigningVault, VerifyingVault};
+use ockam_vault::{Signature, SigningVault, VerifyingVault};
 
 /// Structure with both [`CredentialData`] and [`PurposeKeyAttestationData`] that we get
 /// after parsing and verifying corresponding [`Credential`] and [`super::super::models::PurposeKeyAttestation`]
@@ -67,7 +67,10 @@ impl Credentials {
     ) -> Result<CredentialAndPurposeKeyData> {
         let purpose_key_data = self
             .purpose_keys
-            .verify_purpose_key_attestation(&credential_and_purpose_key.purpose_key_attestation)
+            .verify_purpose_key_attestation(
+                None,
+                &credential_and_purpose_key.purpose_key_attestation,
+            )
             .await?;
 
         if !authorities.contains(&purpose_key_data.subject) {
@@ -93,8 +96,8 @@ impl Credentials {
             CredentialSignature::Ed25519Signature(signature) => {
                 Signature::new(signature.0.to_vec())
             }
-            CredentialSignature::P256ECDSASignature(_) => {
-                return Err(IdentityError::InvalidKeyType.into())
+            CredentialSignature::P256ECDSASignature(signature) => {
+                Signature::new(signature.0.to_vec())
             }
         };
 
@@ -113,36 +116,60 @@ impl Credentials {
 
         let credential_data = CredentialData::get_data(&versioned_data)?;
 
+        if credential_data.subject.is_none() {
+            // Currently unsupported
+            return Err(IdentityError::CredentialVerificationFailed.into());
+        }
+
         if credential_data.subject.is_none() && credential_data.subject_latest_change_hash.is_none()
         {
+            // At least one should be always present, otherwise it's unclear who this credential belongs to
             return Err(IdentityError::CredentialVerificationFailed.into());
         }
 
         if expected_subject.is_some() && credential_data.subject.as_ref() != expected_subject {
+            // We expected credential that belongs to someone else
             return Err(IdentityError::CredentialVerificationFailed.into());
         }
 
         if credential_data.created_at < purpose_key_data.created_at {
+            // Credential validity time range should be inside the purpose key validity time range
             return Err(IdentityError::CredentialVerificationFailed.into());
         }
 
         if credential_data.expires_at > purpose_key_data.expires_at {
+            // Credential validity time range should be inside the purpose key validity time range
             return Err(IdentityError::CredentialVerificationFailed.into());
         }
 
         let now = now()?;
 
         if credential_data.created_at > now {
+            // Credential can't be created in the future
             return Err(IdentityError::CredentialVerificationFailed.into());
         }
 
         if credential_data.expires_at < now {
+            // Credential expired
             return Err(IdentityError::CredentialVerificationFailed.into());
         }
 
-        // FIXME: credential_data.subject_latest_change_hash
-        // FIXME: Verify if given authority is allowed to issue credentials with given Schema
-        // FIXME: Verify if Schema aligns with Attributes
+        if let Some(_subject_latest_change_hash) = &credential_data.subject_latest_change_hash {
+            // TODO: Check how that aligns with the ChangeHistory of the subject that we have in the storage
+            //     For example, if we just established a secure channel with that subject,
+            //     latest_change_hash MUST be equal to the one in present ChangeHistory.
+            //     If credential_data.subject_latest_change_hash equals to some older value from the
+            //     subject's ChangeHistory, that means that subject hasn't updated its Credentials
+            //     after the Identity Key rotation, which is suspicious, such Credential should be rejected
+            //     If credential_data.subject_latest_change_hash equals to some future value that we haven't yet
+            //     observed, than subject should had presented its newer Changes as well. We should
+            //     reject such Credential, unless we have cases where subject may not had an opportunity
+            //     to present its newer Changes (e.g., if we receive its Credential from someone else).
+            //     In such cases some limited tolerance may be introduced.
+        }
+
+        // FIXME: Verify if given authority is allowed to issue credentials with given Schema <-- Should be handled somewhere in the TrustContext
+        // FIXME: Verify if Schema aligns with Attributes <-- Should be handled somewhere in the TrustContext
 
         Ok(CredentialAndPurposeKeyData {
             credential_data,
@@ -192,20 +219,12 @@ impl Credentials {
 
         let versioned_data_hash = self.verifying_vault.sha256(&versioned_data).await?;
 
-        if issuer_purpose_key.purpose() != Purpose::Credentials {
-            return Err(IdentityError::InvalidKeyType.into());
-        }
-
-        if issuer_purpose_key.stype() != SecretType::Ed25519 {
-            return Err(IdentityError::InvalidKeyType.into());
-        }
-
         let signature = self
             .signing_vault
             .sign(issuer_purpose_key.key_id(), &versioned_data_hash)
             .await?;
         let signature: Vec<u8> = signature.into();
-        let signature = Ed25519Signature(signature.try_into().unwrap());
+        let signature = Ed25519Signature(signature.try_into().unwrap()); // FIXME
         let signature = CredentialSignature::Ed25519Signature(signature);
 
         let credential = Credential {

implementations/rust/ockam/ockam_identity/src/identities/identities_creation.rs

@@ -45,7 +45,7 @@ impl IdentitiesCreation {
         let identity =
             Identity::import(expected_identifier, data, self.verifying_vault.clone()).await?;
 
-        self.store_imported_identity(&identity).await?;
+        self.update_identity(&identity).await?;
 
         Ok(identity)
     }
@@ -64,7 +64,7 @@ impl IdentitiesCreation {
         )
         .await?;
 
-        self.store_imported_identity(&identity).await?;
+        self.update_identity(&identity).await?;
 
         Ok(identity)
     }
@@ -109,7 +109,7 @@ impl IdentitiesCreation {
         key_id: &KeyId,
     ) -> Result<Identity> {
         let identity = self.import(None, identity_change_history).await?;
-        if identity.get_public_key()? != self.signing_vault.get_public_key(key_id).await? {
+        if identity.get_latest_public_key()? != self.signing_vault.get_public_key(key_id).await? {
             return Err(IdentityError::WrongSecretKey.into());
         }
 
@@ -121,8 +121,12 @@ impl IdentitiesCreation {
 }
 
 impl IdentitiesCreation {
-    async fn store_imported_identity(&self, identity: &Identity) -> Result<()> {
-        // TODO: Duplicate
+    /// Compare Identity that was received by any side-channel (e.g., Secure Channel) to the
+    /// version we have observed and stored before.
+    ///   - Do nothing if they're equal
+    ///   - Throw an error if the received version has conflict or is older that previously observed
+    ///   - Update stored Identity if the received version is newer
+    pub async fn update_identity(&self, identity: &Identity) -> Result<()> {
         if let Some(known_identity) = self
             .repository
             .retrieve_identity(identity.identifier())

implementations/rust/ockam/ockam_identity/src/identities/identity_keys.rs

@@ -209,7 +209,7 @@ mod test {
         .await?;
 
         let secret1 = identity_keys.get_secret_key(&identity).await?;
-        let public1 = identity.get_public_key()?;
+        let public1 = identity.get_latest_public_key()?;
 
         let identity = identity_keys.rotate_key(identity).await?;
 
@@ -222,7 +222,7 @@ mod test {
         .await?;
 
         let secret2 = identity_keys.get_secret_key(&identity).await?;
-        let public2 = identity.get_public_key()?;
+        let public2 = identity.get_latest_public_key()?;
 
         if secret1 == secret2 {
             return test_error("secret did not change after rotate_key");

implementations/rust/ockam/ockam_identity/src/identity/identity.rs

@@ -11,7 +11,7 @@ use ockam_core::compat::vec::Vec;
 use ockam_core::Result;
 use ockam_vault::{PublicKey, VerifyingVault};
 
-/// Identity implementation
+/// Verified Identity
 #[derive(Clone, Debug)]
 pub struct Identity {
     identifier: Identifier,
@@ -31,6 +31,8 @@ impl PartialEq for Identity {
 
 impl Identity {
     /// Create a new identity
+    /// NOTE: This is intentionally private, so that the only way to create such struct is by
+    /// going through the verification process
     fn new(
         identifier: Identifier,
         changes: Vec<VerifiedChange>,
@@ -115,13 +117,23 @@ impl Identity {
 
 impl Identity {
     /// Get latest public key
-    pub fn get_public_key(&self) -> Result<PublicKey> {
+    pub fn get_latest_public_key(&self) -> Result<PublicKey> {
         if let Some(last_change) = self.changes().last() {
             Ok(last_change.primary_public_key().clone())
         } else {
             Err(IdentityError::EmptyIdentity.into())
         }
     }
+
+    /// Get latest [`VerifiedChange`]
+    pub fn get_latest_change(&self) -> Result<VerifiedChange> {
+        if let Some(last_change) = self.changes().last() {
+            Ok(last_change.clone())
+        } else {
+            Err(IdentityError::EmptyIdentity.into())
+        }
+    }
+
     /// Add a new key change to the change history
     pub async fn add_change(
         self,

implementations/rust/ockam/ockam_identity/src/identity/identity_verification.rs

@@ -100,9 +100,9 @@ impl Identity {
             }
 
             to_be_verified_changes.push(VerifiedChange::new(
+                change_details.change_data.clone(),
                 change_details.change_hash.clone(),
                 change_details.change_data.primary_public_key.clone().into(),
-                change_details.change_data.revoke_all_purpose_keys,
             ));
 
             previous_change_details = Some(change_details);

implementations/rust/ockam/ockam_identity/src/identity/verified_change.rs

@@ -1,39 +1,39 @@
-use super::super::models::ChangeHash;
+use super::super::models::{ChangeData, ChangeHash};
 use ockam_vault::PublicKey;
 
 /// Verified Changes of an [`Identity`]
 #[derive(Clone, Debug)]
 pub struct VerifiedChange {
+    data: ChangeData,
     change_hash: ChangeHash,
     primary_public_key: PublicKey,
-    revoke_all_purpose_keys: bool,
 }
 
 impl VerifiedChange {
-    /// [`ChangeHash`]
-    pub fn change_hash(&self) -> &ChangeHash {
-        &self.change_hash
-    }
-
-    /// [`PrimaryPublicKey`]
-    pub fn primary_public_key(&self) -> &PublicKey {
-        &self.primary_public_key
-    }
-
-    /// Are purpose keys revoked with this rotation
-    pub fn revoke_all_purpose_keys(&self) -> bool {
-        self.revoke_all_purpose_keys
-    }
-
     pub(crate) fn new(
+        data: ChangeData,
         change_hash: ChangeHash,
         primary_public_key: PublicKey,
-        revoke_all_purpose_keys: bool,
     ) -> Self {
         Self {
+            data,
             change_hash,
             primary_public_key,
-            revoke_all_purpose_keys,
         }
     }
+
+    /// [`ChangeData`]
+    pub fn data(&self) -> &ChangeData {
+        &self.data
+    }
+
+    /// [`ChangeHash`]
+    pub fn change_hash(&self) -> &ChangeHash {
+        &self.change_hash
+    }
+
+    /// [`PrimaryPublicKey`]
+    pub fn primary_public_key(&self) -> &PublicKey {
+        &self.primary_public_key
+    }
 }