fix(stack-encrypt)!: report a ciphertext that opens as the wrong type as OpenedType, not Aead

languages/golang/encrypt/guest/src/status.rs

@@ -137,6 +137,10 @@ pub fn status_for_error(error: &stack_encrypt::Error) -> u32 {
         // statement about the caller's input, raised by the engine the
         // record exports run their plans through.
         stack_encrypt::Error::Plan(_) => STATUS_ENCODING,
+        // A ciphertext that authenticated but holds another type than the
+        // one it was opened as: sealed as that type, not tampered with, so
+        // the caller's input and not STATUS_AUTH.
+        stack_encrypt::Error::OpenedType { .. } => STATUS_ENCODING,
         _ => STATUS_INTERNAL,
     }
 }
@@ -282,6 +286,10 @@ mod tests {
     #[test]
     fn aead_and_composition_errors_map_to_the_vitaminc_codes() {
         assert_eq!(status_for_error(&stack_encrypt::Error::Aead), STATUS_AUTH);
+        assert_eq!(
+            status_for_error(&stack_encrypt::Error::OpenedType { expected: "i32" }),
+            STATUS_ENCODING
+        );
         assert_eq!(
             status_for_error(&stack_encrypt::Error::DescriptorTooLong { len: 513 }),
             STATUS_ENCODING

packages/stack-encrypt/CHANGELOG.md

@@ -179,6 +179,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Breaking
 
+- **A ciphertext that opens as the wrong type is `Error::OpenedType`, not
+  `Error::Aead`** (#1118). A typed decrypt of a leaf that authenticates but
+  holds another type (an integer opened as `String`, a value too wide for
+  the target) was reported as `Aead`, the error for a wrong key or a
+  tampered ciphertext, and the Go guest reported it as `STATUS_AUTH`. The new
+  variant names only the type the target reads (`expected`). The Go guest
+  maps it to `STATUS_ENCODING`, as it does a typed record field that opens to
+  another kind. A wrong key, a wrong context and a tampered ciphertext are
+  still `Aead`. A container whose shape the target refuses (a map missing a
+  field) is `OpenedType` too: a container's membership and order are not
+  authenticated, by design, since stored values map to rows and columns a
+  database returns in any order and a query selects in part.
 - **An 8- or 16-bit integer in a dynamic value keeps its width** (`dynamic`
   feature; #1144). The value model used to widen it to 32 bits before it
   was sealed; now it is sealed as itself, so a field declared `int16` seals

packages/stack-encrypt/src/cipher.rs

@@ -105,8 +105,8 @@ use stack_kms::{FallbackKeyProvider, KeyProvider, KeyProviderError, ProfileStore
 use uuid::Uuid;
 use vitaminc_aead::seal::seal_all;
 use vitaminc_aead::{
-    Cipher, CipherText, Context, Decrypt, Encrypt, IntoAad, IntoContext, LocalCipherText, Node,
-    Pending, Unspecified,
+    Cipher, CipherText, Context, DecodeError, Decrypt, DecryptError, Encrypt, IntoAad, IntoContext,
+    LocalCipherText, Node, Pending, Unspecified,
 };
 use vitaminc_encrypt::{Aes256Cipher, Key as AesKey};
 use vitaminc_protected::Protected;
@@ -141,15 +141,38 @@ pub enum Error {
     #[error(transparent)]
     #[diagnostic(transparent)]
     Kms(#[from] stack_kms::Error),
-    /// AEAD sealing/opening failed, or the ciphertext shape did not match the
-    /// requested type. On decrypt this is the expected outcome for a wrong key,
-    /// wrong AAD, or tampered ciphertext.
+    /// AEAD sealing/opening failed, or the ciphertext tree is malformed. On
+    /// decrypt this is the expected outcome for a wrong key, wrong AAD, or
+    /// tampered ciphertext. A ciphertext that opens but holds a value of
+    /// another type is [`OpenedType`](Self::OpenedType), not this.
     #[error("AEAD operation failed (wrong key, AAD mismatch, or malformed ciphertext)")]
     #[diagnostic(
         code(stack_encrypt::aead),
         help("Open the value under the context it was sealed with. If the context is right, the ciphertext is damaged or was altered.")
     )]
     Aead,
+    /// A ciphertext opened, so every leaf in it is authentic and unaltered,
+    /// but it holds a value of another type or shape than the one it was
+    /// opened as: text opened as an integer, an integer too wide for the
+    /// target, a map without a field the target needs. The value was sealed
+    /// as another type or shape, or arrived as part of one, and is not
+    /// tampered with.
+    ///
+    /// A container's membership and order are not authenticated, by design:
+    /// a stored sequence is a set of rows and a stored map a row's columns,
+    /// and a database returns rows in any order and a query selects some
+    /// columns, not all. So a map missing a field, or a sequence of another
+    /// length, is a shape the target refuses, not a failed authentication.
+    ///
+    /// `expected` names what the target reads (a vitaminc kind such as
+    /// `i32`, or the Rust type). The message never names what the value
+    /// held, and never a map key.
+    #[error("the ciphertext opened, but it does not hold a value of the type {expected}")]
+    #[diagnostic(
+        code(stack_encrypt::opened_type),
+        help("Open the value as the type it was sealed as. The ciphertext is authentic: it was sealed as another type, not altered.")
+    )]
+    OpenedType { expected: &'static str },
     /// ZeroKMS returned a different number of keys than were requested.
     #[error("expected {expected} data keys from ZeroKMS but received {received}")]
     #[diagnostic(code(stack_encrypt::key_count_mismatch))]
@@ -320,6 +343,7 @@ impl crate::ErrorPayload for Error {
                 ("expected", (*expected).into()),
                 ("received", (*received).into()),
             ]),
+            Self::OpenedType { expected } => payload([("expected", (*expected).into())]),
             Self::DescriptorTooLong { len } => payload([
                 ("len", (*len).into()),
                 ("limit", Descriptor::MAX_LEN.into()),
@@ -1509,21 +1533,98 @@ impl Cipher for BoundKeys {
     }
 }
 
-/// Open a key-bound tree into `T` under `aad`. Every failure is
-/// [`Error::Aead`]: a wrong key or AAD, a tampered leaf or marker, a
-/// malformed tree, and a value that is not a `T` all read the same to the
-/// caller, as they did before vitaminc split them.
+/// Open a key-bound tree into `T` under `aad`. A wrong key or AAD, a
+/// tampered leaf or marker, and a malformed tree are [`Error::Aead`]. A tree
+/// that opens but holds a value that is not a `T` is [`Error::OpenedType`]:
+/// every leaf authenticated, so it was sealed as another type, not altered.
+///
+/// Each leaf authenticates on its own, under its context and its place's
+/// AAD (a map entry's key). Which entries a map holds and the order of a
+/// sequence are deliberately not authenticated: stored values map to rows
+/// and columns that a database returns in any order and a query selects in
+/// part. A refusal of a container's shape is therefore `OpenedType` like any
+/// other, not `Aead`. Authenticating membership may suit other uses later;
+/// it is not what this crate does.
 pub(crate) fn open_keyed<'a, T: Decrypt>(
     keyed: KeyedCipherText,
     aad: impl IntoAad<'a>,
 ) -> Result<T, Error> {
-    vitaminc_aead::decrypt(&BoundKeys, keyed, aad).map_err(|_| Error::Aead)
+    vitaminc_aead::decrypt(&BoundKeys, keyed, aad).map_err(opened_error::<T>)
+}
+
+/// What a failed open is to the caller. Only the sink's refusal is told
+/// apart, and only by the type it wanted: the `DecodeError` itself can name
+/// a map key, which is not repeated.
+fn opened_error<T>(error: DecryptError<DecodeError, Unspecified>) -> Error {
+    match error {
+        DecryptError::Sink(DecodeError::Expected(expected)) => Error::OpenedType { expected },
+        DecryptError::Sink(_) => Error::OpenedType {
+            expected: std::any::type_name::<T>(),
+        },
+        _ => Error::Aead,
+    }
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
 
+    /// A failed open is `Aead` unless the leaves authenticated and only the
+    /// sink refused: then it is `OpenedType`, naming the kind the sink
+    /// wanted, or the target type when the refusal names no kind (and never
+    /// the map key a refusal can carry).
+    #[test]
+    fn only_a_sink_refusal_is_told_apart_from_aead() {
+        let opened = |error| opened_error::<u32>(error);
+        assert!(matches!(
+            opened(DecryptError::Sink(DecodeError::Expected("i32"))),
+            Error::OpenedType { expected: "i32" }
+        ));
+        for refusal in [
+            DecodeError::MissingField("age"),
+            DecodeError::unknown_key("ssn"),
+            DecodeError::duplicate_key("ssn"),
+        ] {
+            assert!(matches!(
+                opened(DecryptError::Sink(refusal)),
+                Error::OpenedType { expected: "u32" }
+            ));
+        }
+        for failure in [
+            DecryptError::Cipher(Unspecified),
+            DecryptError::Auth,
+            DecryptError::Malformed,
+        ] {
+            assert!(matches!(opened(failure), Error::Aead));
+        }
+    }
+
+    /// Through a real open: a leaf of another kind, and a container whose
+    /// shape the target refuses, are both `OpenedType`. Membership and order
+    /// are not authenticated, by design (see `open_keyed`), so a container
+    /// the target cannot read is a shape mismatch, not tampering.
+    #[tokio::test]
+    async fn a_leaf_or_a_container_of_another_shape_is_opened_type() {
+        let cipher = StackCipher::builder()
+            .kms(stack_kms::FakeDataKeySource::new())
+            .init()
+            .await
+            .unwrap();
+        let keyset = cipher.default_keyset();
+        let leaf = keyset.encrypt(7u32, "users/age").await.unwrap();
+        let opened: Result<String, _> = cipher.decrypt(leaf, "users/age").await;
+        assert!(
+            matches!(opened, Err(Error::OpenedType { .. })),
+            "{opened:?}"
+        );
+        let seq = keyset.encrypt(vec![7u32, 8], "users/age").await.unwrap();
+        let opened: Result<u32, _> = cipher.decrypt(seq, "users/age").await;
+        assert!(
+            matches!(opened, Err(Error::OpenedType { .. })),
+            "{opened:?}"
+        );
+    }
+
     /// A cipher's `Debug` — and a keyset handle's — names the keyset and the
     /// source's type and stops there: the cache behind a cipher holds every
     /// loaded keyset's index key, a handle carries its keyset's, and the

packages/stack-encrypt/src/codes.rs

@@ -48,6 +48,7 @@ mod tests {
         errors.extend(boxed(variants![
             Error::Kms(_) => Error::Kms(crate::kms::Error::Unexpected("kms".into())),
             Error::Aead => Error::Aead,
+            Error::OpenedType { .. } => Error::OpenedType { expected: "i32" },
             Error::KeyCountMismatch { .. } => Error::KeyCountMismatch {
                 expected: 2,
                 received: 1,
@@ -377,6 +378,10 @@ mod tests {
                 json!({ "left": a.to_string(), "right": b.to_string() }),
             ),
             (Box::new(Error::Aead), json!({})),
+            (
+                Box::new(Error::OpenedType { expected: "i32" }),
+                json!({ "expected": "i32" }),
+            ),
             (
                 Box::new(LeafBytesError::UnknownVersion(9)),
                 json!({ "version": 9 }),

packages/stack-encrypt/src/dynamic/record.rs

@@ -6231,8 +6231,13 @@ mod tests {
                 let mut age = map(node(&mut fields, "age"));
                 let as_string: Result<String, _> =
                     cipher.decrypt(node(&mut age, "c"), age_label.clone()).await;
+                // The leaf authenticates, so a reader of another kind is
+                // told the type, not that the leaf was tampered with.
                 assert!(
-                    matches!(as_string, Err(crate::Error::Aead)),
+                    matches!(
+                        as_string,
+                        Err(crate::Error::OpenedType { expected: "string" })
+                    ),
                     "a uint32 leaf is not text, type {ty:?}: {as_string:?}"
                 );
             }