windows: re-verify NCRYPT_UI_POLICY before every sign and decrypt (#62)

* windows: re-verify NCRYPT_UI_POLICY before every sign and decrypt

The UI policy (NCRYPT_UI_PROTECT_KEY_FLAG) is set on CNG keys once,
at key-creation time, via set_ui_policy. If an attacker with write
access to the user's CNG key store pre-plants a key under the same
name but without the UI_PROTECT_KEY_FLAG, the app would open and
sign with that key — and because Windows Hello only fires when the
flag is set, the prompt never comes up and the intended
user-presence gate is bypassed. The flag is the only CNG-side
defense; nothing else makes the hardware require biometrics.

Add ui_policy::verify_ui_policy_matches that reads the key's actual
NCRYPT_UI_POLICY via NCryptGetProperty and compares
UI_PROTECT_KEY_FLAG against the metadata's AccessPolicy. Called from
both TpmSigner::sign and TpmEncryptor::decrypt right after
open_key. A policy mismatch returns Error::KeyOperation and the
sign/decrypt is aborted before any TPM operation happens.

Threat-model section on NCRYPT UI policy rewritten to describe the
new runtime check.

* Fix Windows types in verify_ui_policy_matches

Three type-mismatch errors caught by Windows CI:
- NCRYPT_UI_POLICY.dwFlags is u32, not NCRYPT_FLAGS (set_ui_policy
  was already using u32 correctly; only the new function got it
  wrong).
- NCryptGetProperty's dwflags parameter is OBJECT_SECURITY_INFORMATION,
  not NCRYPT_FLAGS. Pass OBJECT_SECURITY_INFORMATION(0) for no-flags.
- NCRYPT_UI_PROTECT_KEY_FLAG is a bare u32 constant, not a
  newtype-wrapped flag. Bitwise-AND it directly; no .0 accessor.

Add Win32_Security to the windows crate feature list because
OBJECT_SECURITY_INFORMATION lives in Win32::Security, not in the
already-enabled Win32_Security_Cryptography.

* Rewrite UI-policy check as bool comparison to silence match_same_arms

---------

Co-authored-by: Jay Gowdy <jay@gowdy.me>

Author: jgowdy-godaddy

Cargo.toml

@@ -63,7 +63,7 @@ aes-gcm = "0.10"
 elliptic-curve = { version = "0.13", features = ["sec1"] }
 
 # Windows
-windows = { version = "0.58", features = ["Win32_Security_Cryptography", "Win32_Foundation", "Security_Credentials_UI", "Foundation"] }
+windows = { version = "0.58", features = ["Win32_Security", "Win32_Security_Cryptography", "Win32_Foundation", "Security_Credentials_UI", "Foundation"] }
 
 # Linux TPM
 tss-esapi = "7"

THREAT_MODEL.md

@@ -79,7 +79,7 @@ Unsafe FFI surfaces are trusted by design but fragile.
 
 - **Swift ↔ Rust bridge** (`crates/enclaveapp-apple/src/ffi.rs` + `crates/enclaveapp-apple/swift/bridge.swift`). Out-buffer convention relies on the caller sizing buffers correctly; `SE_ERR_BUFFER_TOO_SMALL` return codes can mask unrelated errors if the Rust side assumes buffer sizing is the only failure mode. Any change to Swift bridge signatures requires a coordinated Rust FFI update.
 - **Windows CNG raw-pointer casts** (`crates/enclaveapp-windows/src/ui_policy.rs`). `NCRYPT_UI_POLICY` is passed to `NCryptSetProperty` via `&ui_policy as *const _ as *const u8` with a computed `size_of::<NCRYPT_UI_POLICY>()`. This is correct today but fragile to future struct layout changes.
-- **NCRYPT UI policy is set at key creation only.** The library does not re-read `NCRYPT_UI_PROTECT_KEY_FLAG` before signing. An attacker-planted TPM key with the expected name would sign without triggering Windows Hello. Integration testing on real Windows TPM hardware is a tracked follow-up.
+- **NCRYPT UI policy is re-verified before every sign/decrypt.** `ui_policy::verify_ui_policy_matches` reads the CNG key's actual `NCRYPT_UI_POLICY` via `NCryptGetProperty` and rejects the operation if the `UI_PROTECT_KEY_FLAG` does not match the metadata's `AccessPolicy`. Closes the attacker-planted-TPM-key bypass: an attacker who writes a TPM key with the expected CNG name but no UI protect flag no longer gets signatures without Windows Hello. Integration testing on real Windows TPM hardware is still a tracked follow-up.
 
 ### Keychain and key-backend-specific risks
 

crates/enclaveapp-windows/src/encrypt.rs

@@ -202,6 +202,18 @@ impl EnclaveEncryptor for TpmEncryptor {
         let provider = provider::open_provider()?;
         let key_handle = key::open_key(&provider, &self.app_name, label)?;
 
+        // Re-verify the key's NCRYPT_UI_POLICY matches metadata's
+        // AccessPolicy before decrypt. See the same check in
+        // TpmSigner::sign for the rationale — closes the
+        // pre-planted-key bypass on the encryption path too.
+        let dir = self.keys_dir();
+        let expected_policy = match metadata::load_meta(&dir, label) {
+            Ok(meta) => meta.access_policy,
+            Err(Error::KeyNotFound { .. }) => AccessPolicy::None,
+            Err(err) => return Err(err),
+        };
+        crate::ui_policy::verify_ui_policy_matches(&key_handle, expected_policy)?;
+
         unsafe { ecies_decrypt(&key_handle, ephemeral_pub, nonce, ct, tag) }
     }
 }

crates/enclaveapp-windows/src/sign.rs

@@ -124,6 +124,21 @@ impl EnclaveSigner for TpmSigner {
         let provider = provider::open_provider()?;
         let key_handle = key::open_key(&provider, &self.app_name, label)?;
 
+        // Re-verify the key's NCRYPT_UI_POLICY matches the metadata's
+        // AccessPolicy before signing. Without this check, a same-user
+        // attacker who pre-planted a TPM key with the expected CNG
+        // name (but without UI_PROTECT_KEY_FLAG) could get sshenc to
+        // sign without triggering Windows Hello — the hardware enforces
+        // the policy that's actually set on the key, not what the app
+        // thinks was set.
+        let dir = self.keys_dir();
+        let expected_policy = match metadata::load_meta(&dir, label) {
+            Ok(meta) => meta.access_policy,
+            Err(Error::KeyNotFound { .. }) => AccessPolicy::None,
+            Err(err) => return Err(err),
+        };
+        crate::ui_policy::verify_ui_policy_matches(&key_handle, expected_policy)?;
+
         // Pre-hash with SHA-256.
         let digest = Sha256::digest(data);
 

crates/enclaveapp-windows/src/ui_policy.rs

@@ -10,6 +10,7 @@ use crate::provider::NcryptHandle;
 use enclaveapp_core::AccessPolicy;
 use windows::core::PCWSTR;
 use windows::Win32::Security::Cryptography::*;
+use windows::Win32::Security::OBJECT_SECURITY_INFORMATION;
 
 /// Set a Windows Hello UI policy on a key handle.
 ///
@@ -54,3 +55,86 @@ pub fn set_ui_policy(
         detail: format!("NCryptSetProperty(UI Policy): {e}"),
     })
 }
+
+/// Read back the `NCRYPT_UI_POLICY` actually set on a CNG key and
+/// assert that it matches the requested `AccessPolicy`.
+///
+/// Defends against a same-user attacker who pre-plants a TPM key with
+/// the expected CNG name but without `NCRYPT_UI_PROTECT_KEY_FLAG`. If
+/// the app were to open and sign with such a key, the Windows Hello
+/// prompt would not fire and the intended presence check would be
+/// bypassed. Re-verifying the flag before signing closes that gap.
+///
+/// Returns `Ok(())` if the key carries the correct `NCRYPT_UI_PROTECT_KEY_FLAG`
+/// for the requested policy (or no flag when `expected == None`). Returns
+/// an `Error::KeyOperation` if the policy does not match or the query
+/// itself fails.
+pub fn verify_ui_policy_matches(
+    key_handle: &NcryptHandle,
+    expected: AccessPolicy,
+) -> enclaveapp_core::Result<()> {
+    let prop_name: Vec<u16> = "UI Policy"
+        .encode_utf16()
+        .chain(std::iter::once(0))
+        .collect();
+
+    let mut actual = NCRYPT_UI_POLICY {
+        dwVersion: 0,
+        dwFlags: 0,
+        pszCreationTitle: PCWSTR::null(),
+        pszFriendlyName: PCWSTR::null(),
+        pszDescription: PCWSTR::null(),
+    };
+    let mut actual_size: u32 = 0;
+    let buf = unsafe {
+        std::slice::from_raw_parts_mut(
+            &mut actual as *mut _ as *mut u8,
+            std::mem::size_of::<NCRYPT_UI_POLICY>(),
+        )
+    };
+
+    let result = unsafe {
+        NCryptGetProperty(
+            NCRYPT_HANDLE(key_handle.as_key().0),
+            PCWSTR(prop_name.as_ptr()),
+            Some(buf),
+            &mut actual_size,
+            OBJECT_SECURITY_INFORMATION(0),
+        )
+    };
+
+    let actual_flags: u32 = match result {
+        Ok(()) => actual.dwFlags,
+        Err(e) => {
+            // SPC_E_NO_POLICY / NTE_NOT_FOUND both surface as a missing
+            // policy — translate to "no flag set" so the comparison
+            // below treats it as AccessPolicy::None.
+            if expected == AccessPolicy::None {
+                return Ok(());
+            }
+            return Err(enclaveapp_core::Error::KeyOperation {
+                operation: "verify_ui_policy".into(),
+                detail: format!(
+                    "NCryptGetProperty(UI Policy) for key with expected policy {expected:?}: {e}",
+                ),
+            });
+        }
+    };
+
+    let has_protect_flag =
+        (actual_flags & NCRYPT_UI_PROTECT_KEY_FLAG) == NCRYPT_UI_PROTECT_KEY_FLAG;
+    let expected_protect_flag = expected != AccessPolicy::None;
+
+    if has_protect_flag == expected_protect_flag {
+        return Ok(());
+    }
+    let detail = if expected_protect_flag {
+        format!("key is missing NCRYPT_UI_PROTECT_KEY_FLAG but metadata expects {expected:?}")
+    } else {
+        "key has NCRYPT_UI_PROTECT_KEY_FLAG set but metadata expects AccessPolicy::None".into()
+    };
+    Err(enclaveapp_core::Error::KeyOperation {
+        operation: "verify_ui_policy".into(),
+        detail,
+    })
+}