fix(dpp): refuse encryptedFor paths inside a transient object (PV14)

book/src/data-model/documents.md

@@ -514,7 +514,7 @@ A byte array property may hold ciphertext that only one identity can read. Befor
 
 All four keys are required. `recipient` is the dotted path of an identifier property of the same document type whose value is the recipient identity's id, or `$ownerId` for a message the writer encrypts to themself. `recipientKey` and `senderKey` are dotted paths of integer properties of the same document type carrying the recipient's and the sender's identity key ids; each must declare `minimum` at least 0 and `maximum` at most 4294967295, read from the schema itself, so the rule holds whatever `sizedIntegerTypes` the contract sets. `scheme` is a closed set with one member today.
 
-The parser (generation 3, meta-schema v3) admits the keyword on byte array properties only, never on an identifier (`contentMediaType` set) or any other type, and checks at contract registration that the three named properties exist with those types, that none of them is `transient` (a transient property is stripped before storage, which would leave the stored ciphertext without its recipe), and that the byte array's own `maxItems` can hold the scheme's shortest ciphertext. A contract update that adds, removes or changes an `encryptedFor` declaration is an incompatible schema change (`IncompatibleDocumentTypeSchemaError`, 10246): documents already written under the old recipe could not be read under the new one. Contracts parsed before protocol version 14 ignore the keyword entirely.
+The parser (generation 3, meta-schema v3) admits the keyword on byte array properties only, never on an identifier (`contentMediaType` set) or any other type, and checks at contract registration that the three named properties exist with those types, that none of them is `transient` or sits inside a transient object (a transient value is stripped before storage, which would leave the stored ciphertext without its recipe), and that the byte array's own `maxItems` can hold the scheme's shortest ciphertext. A contract update that adds, removes or changes an `encryptedFor` declaration is an incompatible schema change (`IncompatibleDocumentTypeSchemaError`, 10246): documents already written under the old recipe could not be read under the new one. Contracts parsed before protocol version 14 ignore the keyword entirely.
 
 ### The `ecdh-secp256k1-aes256-cbc` layout
 

packages/rs-dpp/schema/meta_schemas/document/v3/document-meta.json

@@ -114,7 +114,7 @@
           "$ref": "#/$defs/documentArrayItem"
         },
         "encryptedFor": {
-          "description": "byte array properties that are not identifiers only: how the property's ciphertext was produced, so wallets and SDKs read the recipe from the contract. recipient names an identifier property of the same document type whose value is the recipient identity's id, or is \"$ownerId\" for a message the writer encrypts to themself; recipientKey and senderKey name integer properties of the same document type (minimum at least 0, maximum at most 4294967295) carrying the recipient's and the sender's identity key ids; scheme names how the bytes were made. Under ecdh-secp256k1-aes256-cbc, the scheme of the dashpay contact request, the shared key is the libsecp256k1 ECDH of the sender's private key and the recipient's public key (SHA256 of the product point's parity byte and x coordinate) and the value is a random 16-byte IV followed by the plaintext under AES-256-CBC with PKCS7 padding and that IV, so at least 32 bytes and a multiple of 16. All four keys are required and the three named properties must exist with those types. Consensus checks only that shape on every create and replace (InvalidEncryptedPropertyShapeError, 10420): who can decrypt the bytes, and whether they decrypt at all, is not verifiable on chain",
+          "description": "byte array properties that are not identifiers only: how the property's ciphertext was produced, so wallets and SDKs read the recipe from the contract. recipient names an identifier property of the same document type whose value is the recipient identity's id, or is \"$ownerId\" for a message the writer encrypts to themself; recipientKey and senderKey name integer properties of the same document type (minimum at least 0, maximum at most 4294967295) carrying the recipient's and the sender's identity key ids; scheme names how the bytes were made. Under ecdh-secp256k1-aes256-cbc, the scheme of the dashpay contact request, the shared key is the libsecp256k1 ECDH of the sender's private key and the recipient's public key (SHA256 of the product point's parity byte and x coordinate) and the value is a random 16-byte IV followed by the plaintext under AES-256-CBC with PKCS7 padding and that IV, so at least 32 bytes and a multiple of 16. All four keys are required; the three named properties must exist with those types, and none of them may be transient or inside a transient object (a transient value is never stored, so the stored document would lose its recipe). Consensus checks only that shape on every create and replace (InvalidEncryptedPropertyShapeError, 10420): who can decrypt the bytes, and whether they decrypt at all, is not verifiable on chain",
           "type": "object",
           "properties": {
             "recipient": {

packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/mod.rs

@@ -9,7 +9,7 @@ use crate::data_contract::document_type::v0::DocumentTypeV0;
 use crate::data_contract::document_type::v1::DocumentTypeV1;
 use crate::data_contract::document_type::v2::DocumentTypeV2;
 use crate::data_contract::document_type::{
-    is_referenced_system_agreement_property, is_referring_system_agreement_property,
+    is_referenced_system_agreement_property, is_referring_system_agreement_property, is_transient,
     property_names, ContractReferenceModeration, ContractReferenceOwner,
     ContractReferenceRequirements, DistinctFrom, DocumentProperty, DocumentPropertyReferenceTarget,
     DocumentPropertyType, DocumentPropertyTypeParsingOptions, DocumentReferenceLookup,
@@ -1385,11 +1385,12 @@ fn apply_encrypted_for_v0(
 /// identifier property (or `$ownerId`), the two key properties integers whose
 /// schema declares `minimum` at least 0 and `maximum` at most 4294967295 (read
 /// from the schema itself, so the rule holds whatever `sizedIntegerTypes` the
-/// contract sets), none of them may be `transient` (a transient property is
-/// stripped before storage, which would leave the stored ciphertext without
-/// its recipe), and the byte array's own `maxItems` must hold the scheme's
-/// shortest ciphertext. Paths are looked up among the flattened properties,
-/// so a nested property is named by its dotted path.
+/// contract sets), none of them may be `transient` or sit inside a transient
+/// object (a transient value is stripped before storage, which would leave
+/// the stored ciphertext without its recipe), and the byte array's own
+/// `maxItems` must hold the scheme's shortest ciphertext. Paths are looked up
+/// among the flattened properties, so a nested property is named by its
+/// dotted path.
 ///
 /// Owned by parser generation 3: the only generation that admits the keyword.
 pub(super) fn validate_encrypted_for_declarations(
@@ -1439,10 +1440,11 @@ pub(super) fn validate_encrypted_for_declarations(
                     )));
                 }
             }
-            if document_type.transient_fields.contains(recipient_path) {
+            if is_transient(DocumentTypeRef::V2(document_type), recipient_path) {
                 return Err(structure_error(format!(
-                    "recipient \"{recipient_path}\" is transient: a transient property is never \
-                     stored, so a reader could not tell whom the bytes are for"
+                    "recipient \"{recipient_path}\" is transient or inside a transient object: \
+                     a transient value is never stored, so a reader could not tell whom the \
+                     bytes are for"
                 )));
             }
         }
@@ -1463,10 +1465,11 @@ pub(super) fn validate_encrypted_for_declarations(
                     u32::MAX
                 )));
             }
-            if document_type.transient_fields.contains(key_path) {
+            if is_transient(DocumentTypeRef::V2(document_type), key_path) {
                 return Err(structure_error(format!(
-                    "{key} \"{key_path}\" is transient: a transient property is never stored, \
-                     so a reader could not tell which key decrypts the bytes"
+                    "{key} \"{key_path}\" is transient or inside a transient object: a \
+                     transient value is never stored, so a reader could not tell which key \
+                     decrypts the bytes"
                 )));
             }
         }
@@ -4095,6 +4098,63 @@ mod tests {
         }
     }
 
+    /// `transient` lists the object, not its leaves, and the whole object is
+    /// stripped before storage: a recipient or key id inside it is gone from
+    /// the stored document however required it is.
+    #[test]
+    fn should_reject_encrypted_for_naming_a_recipient_or_key_inside_a_transient_object() {
+        for (key, path) in [
+            ("recipient", "meta.authorId"),
+            ("recipientKey", "meta.keyId"),
+            ("senderKey", "meta.keyId"),
+        ] {
+            let mut declaration = encrypted_for_declaration();
+            declaration[key] = json!(path);
+            let mut schema = encrypted_schema(declaration);
+            schema["properties"]["meta"]["properties"]["keyId"] = json!({
+                "type": "integer",
+                "minimum": 0,
+                "maximum": 4294967295_u64,
+                "position": 1
+            });
+            schema["properties"]["meta"]["required"] = json!(["authorId", "keyId"]);
+            schema["required"] = json!(["meta"]);
+
+            // Stored with the document, the nested path is accepted
+            try_document_type_from_schema_full_validation(schema.clone())
+                .unwrap_or_else(|err| panic!("{key}={path} should parse: {err}"));
+
+            schema["transient"] = json!(["meta"]);
+            let fragment = format!("{key} \"{path}\" is transient or inside a transient object");
+            for err in [
+                try_document_type_from_schema(schema.clone()).expect_err("should be refused"),
+                try_document_type_from_schema_full_validation(schema.clone())
+                    .expect_err("should be refused under full validation"),
+            ] {
+                assert!(
+                    err.to_string().contains(&fragment),
+                    "{key}={path}: expected {fragment:?}, got {err}"
+                );
+            }
+        }
+    }
+
+    #[test]
+    fn should_find_a_path_transient_through_itself_or_an_enclosing_object_only() {
+        let mut schema = encrypted_schema(encrypted_for_declaration());
+        schema["transient"] = json!(["meta", "note"]);
+        let document_type = try_document_type_from_schema(schema).expect("should parse");
+        let document_type = document_type.as_ref();
+
+        assert!(is_transient(document_type, "note"));
+        assert!(is_transient(document_type, "meta"));
+        assert!(is_transient(document_type, "meta.authorId"));
+        assert!(is_transient(document_type, "meta.inner.leaf"));
+        // A prefix counts only up to a dot: "metadata" is not inside "meta"
+        assert!(!is_transient(document_type, "metadata.authorId"));
+        assert!(!is_transient(document_type, "recipientId"));
+    }
+
     #[test]
     fn should_reject_encrypted_for_on_a_byte_array_too_short_for_the_scheme() {
         let mut schema = encrypted_schema(encrypted_for_declaration());

packages/rs-dpp/src/data_contract/document_type/property/mod.rs

@@ -17,7 +17,8 @@ use crate::data_contract::config::v0::DataContractConfigGettersV0;
 use crate::data_contract::config::v1::DataContractConfigGettersV1;
 use crate::data_contract::config::v2::DataContractConfigGettersV2;
 use crate::data_contract::config::DataContractConfig;
-use crate::data_contract::document_type::property_names;
+use crate::data_contract::document_type::accessors::DocumentTypeV0Getters;
+use crate::data_contract::document_type::{property_names, DocumentTypeRef};
 use crate::data_contract::DataContract;
 use crate::document::property_names::{CREATOR_ID, OWNER_ID};
 use crate::identity::identity_public_key::accessors::v0::IdentityPublicKeyGettersV0;
@@ -1117,6 +1118,18 @@ pub fn is_referring_system_agreement_property(name: &str) -> bool {
     REFERRING_SYSTEM_AGREEMENT_PROPERTIES.contains(&name)
 }
 
+/// Whether the property at the dotted `path` of `document_type`, or an object
+/// around it, is transient: either way its value is never stored.
+/// `transient_fields()` holds the paths as declared, so a leaf of a transient
+/// object is found only through the object's path, a prefix of its own.
+pub(crate) fn is_transient(document_type: DocumentTypeRef, path: &str) -> bool {
+    let transient_fields = document_type.transient_fields();
+    path.match_indices('.')
+        .map(|(end, _)| &path[..end])
+        .chain(std::iter::once(path))
+        .any(|prefix| transient_fields.contains(prefix))
+}
+
 impl std::fmt::Display for DocumentPropertyReferenceTarget {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {