SE050Sim: always enforce symmetric key read policy

SE050Sim/sdk-test/test_se050.c

@@ -58,6 +58,26 @@ static ex_sss_boot_ctx_t g_ctx;
 static sss_se05x_session_t *g_session;
 static sss_se05x_key_store_t g_ks;
 
+/* An HMACKey object created with no policy attached cannot be read back
+ * (the applet requires POLICY_OBJ_ALLOW_READ on symmetric key objects,
+ * on every applet generation), so the ECDH derive targets are created
+ * with a common policy granting read, write and delete. An attached
+ * policy replaces the applet default entirely, so write (the ECDH
+ * result) and delete (cleanup) must be granted explicitly as well. */
+static sss_policy_u g_derive_common_pol = {
+    .type = KPolicy_Common,
+    .auth_obj_id = 0,
+    .policy = { .common = {
+        .can_Read = 1,
+        .can_Write = 1,
+        .can_Delete = 1,
+    }},
+};
+static sss_policy_t g_derive_policy = {
+    .policies = { &g_derive_common_pol },
+    .nPolicies = 1,
+};
+
 /* Object ID base — use high range to avoid conflicts */
 #define OBJ_ID_BASE 0x10000000
 
@@ -371,7 +391,8 @@ static void test_ecdh(const char *name, uint32_t obj_a, uint32_t obj_b,
      * object whose size must equal the secret exactly; create it before
      * the derive */
     status = sss_key_store_set_key(&g_ks, &derived_key, dummy,
-        key_bytes, key_bytes * 8, NULL, 0);
+        key_bytes, key_bytes * 8, &g_derive_policy,
+        sizeof(g_derive_policy));
     ASSERT_OK(status, "derived pre-create");
     status = sss_derive_key_dh(&derive_ctx, &key_b, &derived_key);
     ASSERT_OK(status, "derive_key_dh");
@@ -1434,7 +1455,8 @@ static void test_x25519_ecdh(void)
     /* Applet 7.2+: derive target must be a pre-existing HMACKey object
      * sized exactly to the shared secret */
     status = sss_key_store_set_key(&g_ks, &derived_a, dummy,
-        sizeof(dummy), sizeof(dummy) * 8, NULL, 0);
+        sizeof(dummy), sizeof(dummy) * 8, &g_derive_policy,
+        sizeof(g_derive_policy));
     ASSERT_OK(status, "derived_a pre-create");
 
     status = sss_derive_key_context_init(&derive_ctx, &g_ctx.session,
@@ -1457,7 +1479,8 @@ static void test_x25519_ecdh(void)
     ASSERT_OK(status, "derived_b allocate");
 
     status = sss_key_store_set_key(&g_ks, &derived_b, dummy,
-        sizeof(dummy), sizeof(dummy) * 8, NULL, 0);
+        sizeof(dummy), sizeof(dummy) * 8, &g_derive_policy,
+        sizeof(g_derive_policy));
     ASSERT_OK(status, "derived_b pre-create");
 
     status = sss_derive_key_context_init(&derive_ctx, &g_ctx.session,

SE050Sim/se050-sim/src/dispatch.rs

@@ -120,8 +120,7 @@ pub fn dispatch(apdu: &ParsedApdu, store: &mut ObjectStore) -> ApduResponse {
                 ApduResponse::success_with_tlvs(
                     &[crate::tlv::Tlv::new(crate::tlv::TAG_1, &curve_list)])
             }
-            _ => handlers::object_mgmt::handle_read(
-                apdu, store, handlers::ec::strict_ecdh_from_env()),
+            _ => handlers::object_mgmt::handle_read(apdu, store),
         },
 
         INS_CRYPTO => match (cred_type, apdu.p2) {

SE050Sim/se050-sim/src/handlers/ec.rs

@@ -463,10 +463,10 @@ pub fn handle_verify(apdu: &ParsedApdu, store: &mut ObjectStore) -> ApduResponse
 
 /// Whether the applet 7.2 strict ECDH InObject contract is enforced.
 /// Off by default so hosts that predate the contract keep working; set
-/// SE050_SIM_STRICT_ECDH=1 to enforce it. Strict mode also enforces the
-/// derive-target read policy: ReadObject on an HMACKey object is refused
-/// with SW_COMMAND_NOT_ALLOWED unless the policy attached at creation
-/// grants POLICY_OBJ_ALLOW_READ (see object_mgmt::handle_read).
+/// SE050_SIM_STRICT_ECDH=1 to enforce it. Note the symmetric key read
+/// policy is unrelated to this switch: ReadObject on an HMACKey object
+/// without POLICY_OBJ_ALLOW_READ is always refused, as every real
+/// applet generation does (see object_mgmt::handle_read).
 pub fn strict_ecdh_from_env() -> bool {
     std::env::var("SE050_SIM_STRICT_ECDH").map(|v| v == "1").unwrap_or(false)
 }

SE050Sim/se050-sim/src/handlers/object_mgmt.rs

@@ -34,12 +34,14 @@ pub fn handle_write(apdu: &ParsedApdu, store: &mut ObjectStore) -> ApduResponse
     }
 }
 
-/// Handle READ commands for objects. With `strict` (the applet 7.2
-/// contract, see ec::strict_ecdh_from_env) ReadObject enforces the
-/// symmetric key read policy; size/list/type reads are unaffected.
-pub fn handle_read(apdu: &ParsedApdu, store: &mut ObjectStore, strict: bool) -> ApduResponse {
+/// Handle READ commands for objects. ReadObject enforces the read
+/// policy on HMACKey objects (all real applet generations do, verified
+/// on SE050C applet 3.1.1 hardware); size/list/type reads are
+/// unaffected. Real applets guard AESKey objects the same way, but the
+/// simulator does not model that yet.
+pub fn handle_read(apdu: &ParsedApdu, store: &mut ObjectStore) -> ApduResponse {
     match apdu.p2 {
-        P2_DEFAULT => handle_read_object(apdu, store, strict),
+        P2_DEFAULT => handle_read_object(apdu, store),
         P2_SIZE => handle_read_size(apdu, store),
         P2_LIST => handle_read_id_list(apdu, store),
         P2_TYPE => handle_read_type(apdu, store),
@@ -192,7 +194,7 @@ fn handle_write_counter(apdu: &ParsedApdu, store: &mut ObjectStore) -> ApduRespo
     ApduResponse::success()
 }
 
-fn handle_read_object(apdu: &ParsedApdu, store: &mut ObjectStore, strict: bool) -> ApduResponse {
+fn handle_read_object(apdu: &ParsedApdu, store: &mut ObjectStore) -> ApduResponse {
     let tlvs = match apdu.parse_tlvs() {
         Ok(t) => t,
         Err(_) => return ApduResponse::error(SW_WRONG_DATA),
@@ -241,15 +243,19 @@ fn handle_read_object(apdu: &ParsedApdu, store: &mut ObjectStore, strict: bool)
                 SecureObject::UserID { value } => value.clone(),
                 SecureObject::Counter { value } => value.to_be_bytes().to_vec(),
                 SecureObject::HMACKey { key, policy } => {
-                    // Applet 7.2 refuses to export a symmetric key object
+                    // The applet refuses to export an HMACKey object
                     // unless the policy attached at creation grants
                     // POLICY_OBJ_ALLOW_READ; an object created with no
-                    // policy attached is not readable. This is the ECDH
-                    // derive target readback contract, enforced only in
-                    // strict mode so hosts that predate it keep working.
-                    if strict
-                        && policy.map_or(true,
-                            |p| p & crate::policy::POLICY_OBJ_ALLOW_READ == 0)
+                    // policy attached is not readable. This holds on every
+                    // real applet generation (observed on applet 3.1.1
+                    // SE050C hardware and applet 7.2 parts alike), so it
+                    // is enforced unconditionally, unlike the strict-mode
+                    // ECDH InObject target contract. Real applets guard
+                    // AESKey objects the same way; the simulator does not
+                    // model that yet since no tracked policy exists on
+                    // AESKey objects.
+                    if policy.map_or(true,
+                        |p| p & crate::policy::POLICY_OBJ_ALLOW_READ == 0)
                     {
                         return ApduResponse::error(SW_COMMAND_NOT_ALLOWED);
                     }
@@ -381,64 +387,68 @@ mod read_policy_tests {
     }
 
     #[test]
-    fn test_strict_read_hmackey_without_policy_returns_6986() {
-        // Applet 7.2 behavior seen on SE05x hardware: an HMACKey object
-        // created with no policy attached cannot be read back, so the
-        // ECDH shared secret export fails unless the derive target was
-        // created with a read policy.
+    fn test_read_hmackey_without_policy_returns_6986() {
+        // An HMACKey object created with no policy attached cannot be
+        // read back on any real applet generation (observed on applet
+        // 3.1.1 and 7.2 hardware alike), so the ECDH shared secret
+        // export fails unless the derive target was created with a
+        // read policy.
         let mut store = ObjectStore::new();
         store.insert([0, 0, 0, 0x66],
             SecureObject::HMACKey { key: vec![0xAB; 32], policy: None });
-        let resp = handle_read(&read_apdu([0, 0, 0, 0x66]), &mut store, true);
+        let resp = handle_read(&read_apdu([0, 0, 0, 0x66]), &mut store);
         assert_eq!(resp.sw, SW_COMMAND_NOT_ALLOWED);
     }
 
     #[test]
-    fn test_strict_read_hmackey_policy_without_read_returns_6986() {
+    fn test_read_hmackey_policy_without_read_returns_6986() {
         let mut store = ObjectStore::new();
         store.insert([0, 0, 0, 0x66],
             SecureObject::HMACKey { key: vec![0xAB; 32], policy: Some(0x0014_0000) });
-        let resp = handle_read(&read_apdu([0, 0, 0, 0x66]), &mut store, true);
+        let resp = handle_read(&read_apdu([0, 0, 0, 0x66]), &mut store);
         assert_eq!(resp.sw, SW_COMMAND_NOT_ALLOWED);
     }
 
     #[test]
-    fn test_strict_read_hmackey_with_read_policy_succeeds() {
+    fn test_read_hmackey_with_read_policy_succeeds() {
         let key = vec![0xABu8; 32];
         let mut store = ObjectStore::new();
         store.insert([0, 0, 0, 0x66],
             SecureObject::HMACKey {
                 key: key.clone(),
                 policy: Some(POLICY_OBJ_ALLOW_READ | 0x0014_0000),
             });
-        let resp = handle_read(&read_apdu([0, 0, 0, 0x66]), &mut store, true);
+        let resp = handle_read(&read_apdu([0, 0, 0, 0x66]), &mut store);
         assert_eq!(resp.sw, 0x9000);
         let tlvs = crate::tlv::parse_tlvs(&resp.data).unwrap();
         assert_eq!(tlv::find_tlv(&tlvs, TAG_1).unwrap().value, key);
     }
 
     #[test]
-    fn test_lenient_read_hmackey_without_policy_succeeds() {
-        // Hosts that predate the applet 7.2 contract keep working when
-        // strict mode is off.
-        let key = vec![0xABu8; 32];
+    fn test_read_binary_without_policy_succeeds() {
+        // Binary (file) objects stay readable with no policy attached;
+        // only symmetric key objects are read-guarded. This is what the
+        // pre-7.2 wolfSSL derive flow relies on.
+        let data = vec![0xCDu8; 32];
         let mut store = ObjectStore::new();
         store.insert([0, 0, 0, 0x66],
-            SecureObject::HMACKey { key: key.clone(), policy: None });
-        let resp = handle_read(&read_apdu([0, 0, 0, 0x66]), &mut store, false);
+            SecureObject::Binary { data: data.clone() });
+        let resp = handle_read(&read_apdu([0, 0, 0, 0x66]), &mut store);
         assert_eq!(resp.sw, 0x9000);
+        let tlvs = crate::tlv::parse_tlvs(&resp.data).unwrap();
+        assert_eq!(tlv::find_tlv(&tlvs, TAG_1).unwrap().value, data);
     }
 
     #[test]
-    fn test_strict_read_size_of_unreadable_hmackey_succeeds() {
+    fn test_read_size_of_unreadable_hmackey_succeeds() {
         // Only the object content is policy-guarded; ReadSize must keep
         // working since sss_key_store_get_key sizes its buffer with it.
         let mut store = ObjectStore::new();
         store.insert([0, 0, 0, 0x66],
             SecureObject::HMACKey { key: vec![0xAB; 32], policy: None });
         let mut apdu = read_apdu([0, 0, 0, 0x66]);
         apdu.p2 = P2_SIZE;
-        let resp = handle_read(&apdu, &mut store, true);
+        let resp = handle_read(&apdu, &mut store);
         assert_eq!(resp.sw, 0x9000);
     }
 }