test: give the wire-compat sample set a non-canonical sender identity

pg-compat-js/src/failures.mjs

@@ -105,16 +105,28 @@ function hex(byte) {
 }
 
 /**
- * A policy as a comparable string: fixed key order, and a missing attribute
- * value distinguished from an empty one.
+ * A policy as a comparable string: attributes in a fixed order, and a missing
+ * attribute value distinguished from an empty one.
+ *
+ * `con` is sorted because the order a reader hands the conjunction back in is
+ * not part of the wire contract, and a reordering must not read as a break.
+ * `describe_policy` in `pg-compat/src/lib.rs` sorts it on the same grounds, so
+ * both halves of the gate rule the same recovered policy a match.
  *
  * @param {{ts: number, con: Array<{t: string, v?: string}>}} policy
  * @returns {string}
  */
 export function describePolicy(policy) {
   if (policy === null || typeof policy !== 'object') return JSON.stringify(policy ?? null);
 
-  const con = (policy.con ?? []).map(({ t, v }) => ({ t, v: v ?? null }));
+  const con = (policy.con ?? [])
+    .map(({ t, v }) => ({ t, v: v ?? null }))
+    .sort((a, b) => {
+      // Not `localeCompare`: the order has to be the same everywhere the gate
+      // runs, and code-unit order is what the Rust half's byte-wise sort does.
+      const [x, y] = [JSON.stringify(a), JSON.stringify(b)];
+      return x < y ? -1 : x > y ? 1 : 0;
+    });
   return JSON.stringify({ ts: policy.ts, con });
 }
 

pg-compat-js/test/failures.test.mjs

@@ -86,6 +86,18 @@ test('an attribute with no value is not the same as one with an empty value', ()
   assert.ok(describePolicyMismatch('public', withValue, withoutValue));
 });
 
+test('a conjunction handed back in another order is not a mismatch', () => {
+  // The order a reader returns the conjunction in is not part of the wire
+  // contract. `sender.private` carries two attributes, so this is reachable.
+  const con = [
+    { t: 'pbdf.gemeente.personalData.fullname', v: 'Sample Sender' },
+    { t: 'pbdf.sidn-pbdf.mobilenumber.mobilenumber', v: '+31612345678' },
+  ];
+  const got = { ts: 1704067200, con: [con[1], con[0]] };
+  const want = { ts: 1704067200, con };
+  assert.equal(describePolicyMismatch('private', got, want), null);
+});
+
 test('a policy that matches reports nothing', () => {
   const policy = { ts: 1704067200, con: [{ t: 'pbdf.sidn-pbdf.email.email', v: 'a@b.test' }] };
   assert.equal(describePolicyMismatch('public', policy, structuredClone(policy)), null);

pg-compat-js/test/gate-teeth.test.mjs

@@ -100,3 +100,78 @@ test('a bumped wire version is reported once, before any ciphertext is opened',
     },
   );
 });
+
+test('a manifest promising the raw sender value is reported, not opened cleanly', async () => {
+  // `sample_set.rs` hands the sealer this value and the manifest promises
+  // `canonicalize` of it, so writing it back into the manifest is what a sealer
+  // that stopped canonicalizing on its way to the wire would have produced.
+  const RAW_SENDER = ' Sender@Sample.TEST ';
+
+  await withDamagedSet(
+    async (dir) => {
+      const path = join(dir, 'manifest.json');
+      const manifest = JSON.parse(await readFile(path, 'utf8'));
+      assert.equal(typeof manifest.sender.public.con[0].v, 'string');
+      manifest.sender.public.con[0].v = RAW_SENDER;
+      await writeFile(path, JSON.stringify(manifest));
+    },
+    (dir) => {
+      const failures = runCase(dir, WASM_READER, 'mem');
+      assert.ok(failures.length > 0, 'a mismatched sender policy was reported as opening cleanly');
+      assert.ok(
+        failures.every((f) => f.includes('public signing policy is')),
+        JSON.stringify(failures),
+      );
+      // One per recipient, each naming which one it was.
+      assert.ok(
+        failures.some((f) => f.includes('mem/alice')),
+        JSON.stringify(failures),
+      );
+      assert.ok(
+        failures.some((f) => f.includes('mem/bob')),
+        JSON.stringify(failures),
+      );
+    },
+  );
+});
+
+test('a manifest promising the wrong private sender value is reported', async () => {
+  // The sealer canonicalizes the two sender policies in separate statements
+  // (`canonical_signing_key` and `with_priv_signing_key`), so a tooth on the
+  // public one leaves the private one as unguarded as no tooth at all. This is
+  // also the only path that reaches the `privateSignatureVisible` branch in
+  // `verify.mjs`, which the wasm reader is alone in taking.
+  const RAW_MOBILE = '+31 (0)6 1234 5678';
+  const MOBILE_TYPE = 'pbdf.sidn-pbdf.mobilenumber.mobilenumber';
+
+  await withDamagedSet(
+    async (dir) => {
+      const path = join(dir, 'manifest.json');
+      const manifest = JSON.parse(await readFile(path, 'utf8'));
+      const attribute = manifest.sender.private.con.find((a) => a.t === MOBILE_TYPE);
+      assert.ok(attribute, 'sender.private carries no mobile number to rewrite');
+      attribute.v = RAW_MOBILE;
+      await writeFile(path, JSON.stringify(manifest));
+    },
+    (dir) => {
+      const failures = runCase(dir, WASM_READER, 'mem-privsig');
+      assert.ok(
+        failures.length > 0,
+        'a mismatched private sender policy was reported as opening cleanly',
+      );
+      assert.ok(
+        failures.every((f) => f.includes('private signing policy is')),
+        JSON.stringify(failures),
+      );
+      // One per recipient, each naming which one it was.
+      assert.ok(
+        failures.some((f) => f.includes('mem-privsig/alice')),
+        JSON.stringify(failures),
+      );
+      assert.ok(
+        failures.some((f) => f.includes('mem-privsig/bob')),
+        JSON.stringify(failures),
+      );
+    },
+  );
+});

pg-compat/README.md

@@ -122,13 +122,19 @@ stream-multi-segment.bin/.plain
   would drift.
 - `wireVersion`: the container version the bytes claim (`VERSION_V3`, `2`).
 - `sender.public`: the policy the sender signed the *header* with, visible to
-  anyone who has the bytes. This is what a reader checks the header signature
-  against, so a JS reader needs it as much as a Rust one.
+  anyone who has the bytes. Recorded in its **canonical** form, while the sealer
+  is handed a deliberately non-canonical value (`sample_set.rs`'s `SENDER`): that
+  disagreement is what makes the field a test rather than a copy of the input, so
+  a canonicalization that stops reaching the wire goes red here. Both halves of
+  the gate compare the sender policy a reader recovered against this.
 - `sender.private`: the policy the sender signed the *payload* with in the
   `*-privsig` cases. Despite the name it is not a secret key; it is the claims a
-  reader may only see after decrypting. It is present in the manifest for every
-  set, but only the cases with `privateSigning: true` were sealed with it, so
-  check it against `privateSigning` rather than against the case list.
+  reader may only see after decrypting. Canonical for the same reason as
+  `sender.public`, and non-canonical in the sealer for a different attribute
+  type, because the sealer canonicalizes the two policies in two separate
+  statements. It is present in the manifest for every set, but only the cases
+  with `privateSigning: true` were sealed with it, so check it against
+  `privateSigning` rather than against the case list.
 - `mode`: `"memory"` for `Sealer<_, SealerMemoryConfig>::seal` (what pg-wasm's
   `seal()` produces), `"stream"` for the segmented container (what cryptify
   stores). pg-js is stream mode in both directions — `toBytes()` seals with

pg-compat/src/lib.rs

@@ -21,6 +21,7 @@ use std::path::{Path, PathBuf};
 use std::process::{Command, ExitStatus, Output};
 
 use serde::Deserialize;
+use serde_json::Value;
 
 pub mod support_window;
 
@@ -42,10 +43,30 @@ pub struct Manifest {
     pub wire_version: u16,
     /// File holding the PKG parameters with the verifying key.
     pub verifying_key: String,
+    /// The policies every container in the set was signed under.
+    pub sender: Sender,
     /// The sealed containers.
     pub cases: Vec<Case>,
 }
 
+/// The sender policies the manifest promises, in the canonical form the sealer
+/// wrote them in.
+///
+/// Held as raw JSON rather than as a `Policy`: every pinned `pg-core` is a
+/// distinct crate with a distinct `Policy` type, so naming one of them in the
+/// shared manifest would tie it to a single reader. The comparison is
+/// structural, through [`describe_policy`].
+#[derive(Debug, Deserialize)]
+pub struct Sender {
+    /// The policy the *header* signature was made under, visible to anyone who
+    /// has the bytes.
+    pub public: Value,
+    /// The policy the *payload* signature was made under in the `*-privsig`
+    /// cases. Present for every set; only checked for a case whose
+    /// `privateSigning` is true.
+    pub private: Option<Value>,
+}
+
 /// One sealed container plus everything needed to open and check it.
 #[derive(Debug, Deserialize)]
 #[serde(rename_all = "camelCase")]
@@ -156,6 +177,59 @@ pub fn describe_plaintext_mismatch(got: &[u8], want: &[u8]) -> String {
     }
 }
 
+/// A policy as a comparable string: attributes in a fixed order, and a missing
+/// value distinguished from an empty one.
+///
+/// `con` is sorted because the order a reader hands the conjunction back in is
+/// not part of the wire contract, and a reordering must not read as a break.
+/// `describePolicy` in `pg-compat-js/src/failures.mjs` sorts it on the same
+/// grounds, so both halves of the gate rule the same recovered policy a match
+/// and name the same fields when they don't. The two texts are not identical:
+/// `serde_json` here has no `preserve_order`, so object keys come out
+/// alphabetically, while the JS half emits `ts` before `con`.
+pub fn describe_policy(policy: &Value) -> String {
+    let mut con: Vec<Value> = policy
+        .get("con")
+        .and_then(Value::as_array)
+        .map(|attributes| {
+            attributes
+                .iter()
+                .map(|a| {
+                    serde_json::json!({
+                        "t": a.get("t").cloned().unwrap_or(Value::Null),
+                        "v": a.get("v").cloned().unwrap_or(Value::Null),
+                    })
+                })
+                .collect()
+        })
+        .unwrap_or_default();
+    con.sort_by_cached_key(Value::to_string);
+
+    serde_json::json!({
+        "ts": policy.get("ts").cloned().unwrap_or(Value::Null),
+        "con": con,
+    })
+    .to_string()
+}
+
+/// Compare a signing policy a reader recovered against the one the manifest
+/// says was signed.
+///
+/// Recovering the right plaintext is not enough on its own: `SignatureExt.pol`
+/// travels in full and a published reader derives the signer's identity from it,
+/// so this is where a canonicalization that stops reaching the wire shows up.
+pub fn describe_policy_mismatch(what: &str, got: &Value, want: &Value) -> Option<String> {
+    let got = describe_policy(got);
+    let want = describe_policy(want);
+    if got == want {
+        return None;
+    }
+
+    Some(format!(
+        "{what} signing policy is {got}, manifest says {want}"
+    ))
+}
+
 /// Open one case in a child process and turn its outcome into failure
 /// messages.
 ///
@@ -268,7 +342,9 @@ macro_rules! reader {
 
             use serde::Deserialize;
 
-            use crate::{describe_plaintext_mismatch, read_file, Case, Manifest};
+            use crate::{
+                describe_plaintext_mismatch, describe_policy_mismatch, read_file, Case, Manifest,
+            };
 
             /// The crates.io version this module reads with.
             pub const VERSION: &str = $version;
@@ -339,6 +415,53 @@ macro_rules! reader {
                                     case.private_signing,
                                 ));
                             }
+                            failures.extend(check_sender(&label, &verified, manifest, case));
+                        }
+                    }
+                }
+
+                failures
+            }
+
+            /// Hold the recovered sender identity to what the manifest
+            /// promises. The policies are serialized back to JSON because this
+            /// module's `Policy` is one pinned crate's type and the manifest is
+            /// shared by all of them.
+            fn check_sender(
+                label: &str,
+                verified: &VerificationResult,
+                manifest: &Manifest,
+                case: &Case,
+            ) -> Vec<String> {
+                let mut failures = Vec::new();
+
+                match serde_json::to_value(&verified.public) {
+                    Ok(got) => failures.extend(
+                        describe_policy_mismatch("public", &got, &manifest.sender.public)
+                            .map(|m| format!("{label}: {m}")),
+                    ),
+                    Err(e) => {
+                        failures.push(format!("{label}: serialize the public signing policy: {e}"))
+                    }
+                }
+
+                // A `private` the reader did not surface is already reported as
+                // a presence mismatch by the caller, so only the both-present
+                // case is left to compare.
+                if case.private_signing {
+                    if let Some(private) = &verified.private {
+                        match (serde_json::to_value(private), &manifest.sender.private) {
+                            (Ok(got), Some(want)) => failures.extend(
+                                describe_policy_mismatch("private", &got, want)
+                                    .map(|m| format!("{label}: {m}")),
+                            ),
+                            (Ok(_), None) => failures.push(format!(
+                                "{label}: recovered a private signing policy, the manifest names \
+                                 none",
+                            )),
+                            (Err(e), _) => failures.push(format!(
+                                "{label}: serialize the private signing policy: {e}",
+                            )),
                         }
                     }
                 }
@@ -472,6 +595,46 @@ mod tests {
         assert!(message.contains("got 0x58, expected 0x64"), "{message}");
     }
 
+    /// The order a reader hands the conjunction back in is not part of the wire
+    /// contract, so a reordering must not read as a break.
+    #[test]
+    fn a_reordered_conjunction_is_not_a_mismatch() {
+        let one = serde_json::json!({"ts": 1, "con": [{"t": "a", "v": "1"}, {"t": "b"}]});
+        let other =
+            serde_json::json!({"ts": 1, "con": [{"t": "b", "v": null}, {"t": "a", "v": "1"}]});
+
+        assert_eq!(describe_policy(&one), describe_policy(&other));
+        assert_eq!(describe_policy_mismatch("public", &one, &other), None);
+    }
+
+    /// The non-canonical sender fixture (#355) turns on exactly this: the raw
+    /// value and the canonical one must not compare equal.
+    #[test]
+    fn a_non_canonical_value_is_reported_with_both_forms() {
+        let raw = serde_json::json!({"ts": 1, "con": [{"t": "e", "v": " Sender@Sample.TEST "}]});
+        let canonical =
+            serde_json::json!({"ts": 1, "con": [{"t": "e", "v": "sender@sample.test"}]});
+
+        let message = describe_policy_mismatch("public", &canonical, &raw)
+            .expect("a raw value must not match its canonical form");
+        assert!(
+            message.starts_with("public signing policy is "),
+            "{message}"
+        );
+        assert!(message.contains("sender@sample.test"), "{message}");
+        assert!(message.contains(" Sender@Sample.TEST "), "{message}");
+    }
+
+    /// A missing value is not an empty one: `Attribute.value` is an `Option`,
+    /// and `to_hidden` blanks a value to `""` rather than dropping it.
+    #[test]
+    fn an_absent_value_differs_from_an_empty_one() {
+        let absent = serde_json::json!({"ts": 1, "con": [{"t": "e"}]});
+        let empty = serde_json::json!({"ts": 1, "con": [{"t": "e", "v": ""}]});
+
+        assert!(describe_policy_mismatch("public", &absent, &empty).is_some());
+    }
+
     #[test]
     fn a_truncated_plaintext_says_so() {
         let message = describe_plaintext_mismatch(b"abc", b"abcde");