From 4c2b1ef089ebdd8fe4b9c624c13b5de8002dfcf8 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Mon, 3 Aug 2026 14:59:50 +0100 Subject: [PATCH 1/2] SE050Sim: support direct ECDH variant, never export HMACKey objects Real SE051 applet 7.2.0 hardware settled two behaviors the simulator modeled wrongly: ReadObject on an HMACKey object fails with SW 0x6986 even when the object's attributes confirm an attached POLICY_OBJ_ALLOW_READ, so the read-policy exception honored since the strict-mode enforcement landed does not exist on silicon. Refuse HMACKey reads unconditionally; the policy recorded at WriteSymmKey stays as attribute bookkeeping. The Tag7-less ECDHGenerateSharedSecret form is how a host obtains an ECDH secret on applet >= 7.2: the applet returns it in the response (big endian for Montgomery curves, byte swapped by the SDK around the call) and touches no object. The simulator rejected Tag7-less requests with SW_WRONG_DATA; accept them in strict and lenient mode alike, with the strict InObject target contract unchanged for Tag7 requests. Rewrite the sdk-test P-256 and X25519 ECDH tests to the direct variant, since the derive-into-object-and-read-back flow they encoded is impossible on real parts, and drop their derive-target policy globals. Verified: 52 cargo tests; wolfCrypt-vs-simulator docker run against the wolfSSL direct-APDU port passes in strict and lenient modes; sdk-test 31/31; hardware cross-check on SE051 applet 7.2.0 (direct ECDH passes, policy-attached HMACKey read denied 0x6986). --- SE050Sim/sdk-test/test_se050.c | 192 ++++++++---------- SE050Sim/se050-sim/src/handlers/aes.rs | 5 +- SE050Sim/se050-sim/src/handlers/ec.rs | 139 ++++++++++--- .../se050-sim/src/handlers/object_mgmt.rs | 50 ++--- SE050Sim/se050-sim/src/object_store/types.rs | 5 +- 5 files changed, 224 insertions(+), 167 deletions(-) diff --git a/SE050Sim/sdk-test/test_se050.c b/SE050Sim/sdk-test/test_se050.c index d0332bf..d6a656f 100644 --- a/SE050Sim/sdk-test/test_se050.c +++ b/SE050Sim/sdk-test/test_se050.c @@ -58,26 +58,6 @@ 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 @@ -346,13 +326,15 @@ static void test_ecdh(const char *name, uint32_t obj_a, uint32_t obj_b, { TEST_BEGIN(name); sss_status_t status; - sss_se05x_object_t key_a, key_b, derived_key; - sss_se05x_derive_key_t derive_ctx; + sss_se05x_object_t key_a, key_b; uint8_t shared[64] = {0}; size_t shared_len = sizeof(shared); - size_t shared_bits = 0; - uint8_t dummy[64] = {0}; + uint8_t pub_b[256]; + size_t pub_b_len = sizeof(pub_b); + size_t pub_b_bits = sizeof(pub_b) * 8; + size_t point_len; + smStatus_t sm; /* Generate two key pairs */ cleanup_object(obj_a); @@ -375,36 +357,27 @@ static void test_ecdh(const char *name, uint32_t obj_a, uint32_t obj_b, status = sss_key_store_generate_key(&g_ks, &key_b, key_bits, NULL); ASSERT_OK(status, "key_b generate"); - /* Compute ECDH(A_priv, B_pub) */ - sss_key_object_init(&derived_key, &g_ks); - status = sss_key_object_allocate_handle(&derived_key, obj_ss, - kSSS_KeyPart_Default, kSSS_CipherType_HMAC, key_bytes, - kKeyObject_Mode_Transient); - ASSERT_OK(status, "derived allocate"); - - status = sss_derive_key_context_init(&derive_ctx, &g_ctx.session, - &key_a, kAlgorithm_SSS_ECDH, kMode_SSS_ComputeSharedSecret); - ASSERT_OK(status, "derive_key_context_init"); - - sss_key_store_erase_key(&g_ks, &derived_key); - /* Applet 7.2+ stores the shared secret into a pre-existing HMACKey - * 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, &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"); - - sss_derive_key_context_free(&derive_ctx); - - /* Read shared secret. sss_key_store_get_key has no HMAC read case, so - * read the object back as AES type (both are a plain ReadObject). */ - derived_key.cipherType = kSSS_CipherType_AES; - status = sss_key_store_get_key(&g_ks, &derived_key, - shared, &shared_len, &shared_bits); - ASSERT_OK(status, "get shared secret"); + /* Compute ECDH(A_priv, B_pub) via the direct APDU. The applet never + * exports a symmetric key object, not even with an attached read + * policy (verified on SE051 applet 7.2.0 hardware), so deriving into + * an SE05x object cannot be read back; the direct variant returns + * the shared secret in the response, as the middleware itself does + * when the derived key object lives in a host keystore. */ + status = sss_key_store_get_key(&g_ks, &key_b, + pub_b, &pub_b_len, &pub_b_bits); + ASSERT_OK(status, "get key_b public"); + point_len = (size_t)(1 + 2 * key_bytes); + if (pub_b_len < point_len || pub_b[pub_b_len - point_len] != 0x04) { + TEST_FAIL("key_b public point not found"); + return; + } + sm = Se05x_API_ECDHGenerateSharedSecret(&g_session->s_ctx, obj_a, + pub_b + pub_b_len - point_len, point_len, shared, &shared_len); + if (sm != SM_OK) { + TEST_FAILF("direct ECDH failed: 0x%04x", (unsigned)sm); + return; + } + ASSERT_EQ(shared_len, (size_t)key_bytes, "shared secret length"); /* Should not be all zeros */ uint8_t zeros[64] = {0}; @@ -413,10 +386,9 @@ static void test_ecdh(const char *name, uint32_t obj_a, uint32_t obj_b, /* Cleanup */ sss_key_store_erase_key(&g_ks, &key_a); sss_key_store_erase_key(&g_ks, &key_b); - sss_key_store_erase_key(&g_ks, &derived_key); sss_key_object_free(&key_a); sss_key_object_free(&key_b); - sss_key_object_free(&derived_key); + (void)obj_ss; TEST_PASS(); } @@ -1408,8 +1380,7 @@ static void test_x25519_ecdh(void) { TEST_BEGIN("X25519-ECDH"); sss_status_t status; - sss_se05x_object_t key_a, key_b, derived_a, derived_b; - sss_se05x_derive_key_t derive_ctx; + sss_se05x_object_t key_a, key_b; uint32_t id_a = OBJ_ID_BASE + 60; uint32_t id_b = OBJ_ID_BASE + 61; @@ -1418,9 +1389,13 @@ static void test_x25519_ecdh(void) uint8_t shared_a[32] = {0}, shared_b[32] = {0}; size_t shared_a_len = sizeof(shared_a), shared_b_len = sizeof(shared_b); - size_t shared_bits = 0; uint8_t zeros[32] = {0}; - uint8_t dummy[32] = {0}; + uint8_t pub_der[64]; + uint8_t point[32]; + size_t pub_len, pub_bits; + smStatus_t sm; + int i; + uint8_t swp; cleanup_object(id_a); cleanup_object(id_b); @@ -1445,55 +1420,54 @@ static void test_x25519_ecdh(void) status = sss_key_store_generate_key(&g_ks, &key_b, 256, NULL); ASSERT_OK(status, "x25519 key_b generate"); - /* ECDH(A_priv, B_pub) */ - sss_key_object_init(&derived_a, &g_ks); - status = sss_key_object_allocate_handle(&derived_a, id_ss_a, - kSSS_KeyPart_Default, kSSS_CipherType_HMAC, 32, - kKeyObject_Mode_Transient); - ASSERT_OK(status, "derived_a allocate"); - - /* 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, &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, - &key_a, kAlgorithm_SSS_ECDH, kMode_SSS_ComputeSharedSecret); - ASSERT_OK(status, "derive_a context_init"); - status = sss_derive_key_dh(&derive_ctx, &key_b, &derived_a); - ASSERT_OK(status, "derive_a dh"); - sss_derive_key_context_free(&derive_ctx); - - derived_a.cipherType = kSSS_CipherType_AES; - status = sss_key_store_get_key(&g_ks, &derived_a, - shared_a, &shared_a_len, &shared_bits); - ASSERT_OK(status, "get shared_a"); + /* ECDH(A_priv, B_pub) via the direct APDU (see test_ecdh); the + * applet speaks big endian for Montgomery keys, so the peer point + * and the returned secret are byte swapped around each call. */ + pub_len = sizeof(pub_der); + pub_bits = sizeof(pub_der) * 8; + status = sss_key_store_get_key(&g_ks, &key_b, + pub_der, &pub_len, &pub_bits); + ASSERT_OK(status, "get key_b public"); + if (pub_len < 32) { TEST_FAIL("key_b public too short"); return; } + memcpy(point, pub_der + pub_len - 32, 32); + for (i = 0; i < 16; i++) { + swp = point[i]; point[i] = point[31 - i]; point[31 - i] = swp; + } + sm = Se05x_API_ECDHGenerateSharedSecret(&g_session->s_ctx, id_a, + point, sizeof(point), shared_a, &shared_a_len); + if (sm != SM_OK) { + TEST_FAILF("direct ECDH a failed: 0x%04x", (unsigned)sm); + return; + } + for (i = 0; i < 16; i++) { + swp = shared_a[i]; shared_a[i] = shared_a[31 - i]; + shared_a[31 - i] = swp; + } /* ECDH(B_priv, A_pub) */ - sss_key_object_init(&derived_b, &g_ks); - status = sss_key_object_allocate_handle(&derived_b, id_ss_b, - kSSS_KeyPart_Default, kSSS_CipherType_HMAC, 32, - kKeyObject_Mode_Transient); - ASSERT_OK(status, "derived_b allocate"); - - status = sss_key_store_set_key(&g_ks, &derived_b, dummy, - 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, - &key_b, kAlgorithm_SSS_ECDH, kMode_SSS_ComputeSharedSecret); - ASSERT_OK(status, "derive_b context_init"); - status = sss_derive_key_dh(&derive_ctx, &key_a, &derived_b); - ASSERT_OK(status, "derive_b dh"); - sss_derive_key_context_free(&derive_ctx); - - derived_b.cipherType = kSSS_CipherType_AES; - status = sss_key_store_get_key(&g_ks, &derived_b, - shared_b, &shared_b_len, &shared_bits); - ASSERT_OK(status, "get shared_b"); + pub_len = sizeof(pub_der); + pub_bits = sizeof(pub_der) * 8; + status = sss_key_store_get_key(&g_ks, &key_a, + pub_der, &pub_len, &pub_bits); + ASSERT_OK(status, "get key_a public"); + if (pub_len < 32) { TEST_FAIL("key_a public too short"); return; } + memcpy(point, pub_der + pub_len - 32, 32); + for (i = 0; i < 16; i++) { + swp = point[i]; point[i] = point[31 - i]; point[31 - i] = swp; + } + sm = Se05x_API_ECDHGenerateSharedSecret(&g_session->s_ctx, id_b, + point, sizeof(point), shared_b, &shared_b_len); + if (sm != SM_OK) { + TEST_FAILF("direct ECDH b failed: 0x%04x", (unsigned)sm); + return; + } + for (i = 0; i < 16; i++) { + swp = shared_b[i]; shared_b[i] = shared_b[31 - i]; + shared_b[31 - i] = swp; + } + + (void)id_ss_a; + (void)id_ss_b; /* Shared secrets must be non-zero and equal */ ASSERT_MEM_NEQ(shared_a, zeros, 32, "shared_a is all zeros"); @@ -1503,12 +1477,8 @@ static void test_x25519_ecdh(void) /* Cleanup */ sss_key_store_erase_key(&g_ks, &key_a); sss_key_store_erase_key(&g_ks, &key_b); - sss_key_store_erase_key(&g_ks, &derived_a); - sss_key_store_erase_key(&g_ks, &derived_b); sss_key_object_free(&key_a); sss_key_object_free(&key_b); - sss_key_object_free(&derived_a); - sss_key_object_free(&derived_b); TEST_PASS(); } diff --git a/SE050Sim/se050-sim/src/handlers/aes.rs b/SE050Sim/se050-sim/src/handlers/aes.rs index 16e6339..958a737 100644 --- a/SE050Sim/se050-sim/src/handlers/aes.rs +++ b/SE050Sim/se050-sim/src/handlers/aes.rs @@ -84,8 +84,9 @@ pub fn handle_write_aes_key(apdu: &ParsedApdu, store: &mut ObjectStore) -> ApduR /// Handle WRITE HMAC key command (WriteSymmKey with P1=HMAC). /// Policy(opt), Tag1=obj_id(4B), Tag3=key_data. HMAC keys have no fixed /// length, so any non-empty Tag3 value is accepted as-is. An attached -/// policy is recorded with the object; the applet 7.2 read-policy -/// contract (see handle_read_object) keys off it. +/// policy is validated and recorded with the object (it is part of the +/// object's attributes on a real applet); note it cannot make the +/// object readable (see object_mgmt::handle_read). pub fn handle_write_hmac_key(apdu: &ParsedApdu, store: &mut ObjectStore) -> ApduResponse { let tlvs = match apdu.parse_tlvs() { Ok(t) => t, diff --git a/SE050Sim/se050-sim/src/handlers/ec.rs b/SE050Sim/se050-sim/src/handlers/ec.rs index d8522dc..4ee91fd 100644 --- a/SE050Sim/se050-sim/src/handlers/ec.rs +++ b/SE050Sim/se050-sim/src/handlers/ec.rs @@ -463,27 +463,34 @@ 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. 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). +/// SE050_SIM_STRICT_ECDH=1 to enforce it. The direct (Tag7-less) +/// variant is valid in both modes, and ReadObject on HMACKey objects is +/// always refused regardless of this switch or any attached policy, as +/// on real applets (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) } /// Handle ECDH shared secret generation. /// INS=Crypto, P1=EC, P2=DH(0x0F) -/// Tag1=privateKeyID(4B), Tag2=peerPublicKey, Tag7=sharedSecretOutputID(4B) -/// Per the SE05x IoT applet spec (7.2+, see Se05x_API_ECDHGenerateSharedSecret -/// in the Plug & Trust middleware), the Tag7 target must reference an existing -/// HMACKey object whose size equals the shared secret exactly; otherwise the -/// applet returns SW_CONDITIONS_NOT_SATISFIED. The shared secret overwrites -/// that object's value. +/// Tag1=privateKeyID(4B), Tag2=peerPublicKey, Tag7=sharedSecretOutputID(4B, +/// optional). +/// Without Tag7 this is the direct variant +/// (Se05x_API_ECDHGenerateSharedSecret): the shared secret is returned in +/// the response Tag1 and no object is created; big endian for Montgomery +/// curves, as on real applets. This is the only way a host can obtain the +/// secret on applet >= 7.2, which refuses to export symmetric key objects +/// regardless of policy (verified on SE051 applet 7.2.0 hardware). +/// With Tag7 this is the InObject variant: the target must reference an +/// existing HMACKey object whose size equals the shared secret exactly, +/// otherwise the applet returns SW_CONDITIONS_NOT_SATISFIED; the secret +/// overwrites that object's value (little endian for Montgomery, matching +/// the SDK convention). /// With `strict` false (the default, see strict_ecdh_from_env) a missing -/// target instead falls back to the legacy simulator behavior of implicitly -/// creating a Binary object, so hosts that predate the applet 7.2 contract -/// keep working. A pre-existing HMACKey target gets the exact-size contract -/// in both modes. +/// InObject target instead falls back to the legacy simulator behavior of +/// implicitly creating a Binary object, so hosts that predate the applet +/// 7.2 contract keep working. A pre-existing HMACKey target gets the +/// exact-size contract in both modes. pub fn handle_ecdh(apdu: &ParsedApdu, store: &mut ObjectStore, strict: bool) -> ApduResponse { let tlvs = match apdu.parse_tlvs() { Ok(t) => t, @@ -504,13 +511,19 @@ pub fn handle_ecdh(apdu: &ParsedApdu, store: &mut ObjectStore, strict: bool) -> _ => return ApduResponse::error(SW_WRONG_DATA), }; - let output_id = match tlv::find_tlv(&tlvs, TAG_7) { + // Tag7 absent selects the direct variant + // (Se05x_API_ECDHGenerateSharedSecret): the shared secret is returned + // in the response and no object is touched. Tag7 present selects the + // InObject variant. Both are valid on every real applet generation; + // the strict target contract only applies to the InObject form. + let output_id: Option<[u8; 4]> = match tlv::find_tlv(&tlvs, TAG_7) { Some(t) if t.value.len() == 4 => { let mut id = [0u8; 4]; id.copy_from_slice(&t.value); - id + Some(id) } - _ => return ApduResponse::error(SW_WRONG_DATA), + Some(_) => return ApduResponse::error(SW_WRONG_DATA), + None => None, }; let key_obj = match store.get(&key_id) { @@ -522,12 +535,19 @@ pub fn handle_ecdh(apdu: &ParsedApdu, store: &mut ObjectStore, strict: bool) -> // object; the applet refuses to create it implicitly. In lenient mode a // missing (or non-HMACKey) target keeps the legacy implicit-create // behavior instead (target None). - let target = match store.get(&output_id) { - Some(SecureObject::HMACKey { key, policy }) => Some((key.len(), *policy)), - _ if strict => return ApduResponse::error(SW_CONDITIONS_NOT_SATISFIED), - _ => None, + let target = match output_id { + Some(oid) => match store.get(&oid) { + Some(SecureObject::HMACKey { key, policy }) => + Some((key.len(), *policy)), + _ if strict => return ApduResponse::error(SW_CONDITIONS_NOT_SATISFIED), + _ => None, + }, + None => None, }; + let is_mont = matches!(&key_obj, + SecureObject::ECKeyPair { curve: ECCurve::Curve25519, .. }); + let shared_secret = match &key_obj { SecureObject::ECKeyPair { curve: ECCurve::NistP224, private_key, .. } => { p224_ecdh(private_key, peer_pubkey) @@ -546,6 +566,22 @@ pub fn handle_ecdh(apdu: &ParsedApdu, store: &mut ObjectStore, strict: bool) -> match shared_secret { Some(secret) => { + let oid = match output_id { + None => { + // Direct variant: the secret goes out in the response. + // The applet speaks big endian for Montgomery curves + // (the middleware byte swaps around the call), while + // the InObject store below keeps the little endian + // convention the SDK reads back. + let mut resp = secret; + if is_mont { + resp.reverse(); + } + return ApduResponse::success_with_tlvs( + &[Tlv::new(TAG_1, &resp)]); + } + Some(oid) => oid, + }; match target { Some((len, policy)) => { if secret.len() != len { @@ -553,12 +589,12 @@ pub fn handle_ecdh(apdu: &ParsedApdu, store: &mut ObjectStore, strict: bool) -> } // The derive overwrites the object's value; the policy // attached at creation stays with the object. - store.insert(output_id, SecureObject::HMACKey { key: secret, policy }); + store.insert(oid, SecureObject::HMACKey { key: secret, policy }); } None => { // Legacy lenient behavior: implicitly create the target // as a Binary object. - store.insert(output_id, SecureObject::Binary { data: secret }); + store.insert(oid, SecureObject::Binary { data: secret }); } } ApduResponse::success() @@ -753,6 +789,63 @@ mod tests { } } + #[test] + fn test_ecdh_direct_no_tag7_returns_secret_both_modes() { + // Without Tag7 the applet returns the shared secret in the + // response and touches no object, in strict and lenient mode + // alike. This is how hosts on applet >= 7.2 obtain the secret. + for strict in [true, false] { + let (mut apdu, mut store, expected) = ecdh_inobject_fixture(); + // Rebuild the request without the Tag7 TLV. + let mut data = tlv_bytes(TAG_1, &[0, 0, 0, 0x64]); + data.extend(tlv_bytes(TAG_2, &apdu.data[8..8 + 65])); + apdu.data = data; + let count_before = store.count(); + let resp = handle_ecdh(&apdu, &mut store, strict); + assert_eq!(resp.sw, 0x9000, "strict={}", strict); + let tlvs = crate::tlv::parse_tlvs(&resp.data).unwrap(); + assert_eq!(tlv::find_tlv(&tlvs, TAG_1).unwrap().value, expected, + "strict={}", strict); + assert_eq!(store.count(), count_before, "no object may be created"); + assert!(store.get(&[0, 0, 0, 0x66]).is_none()); + } + } + + #[test] + fn test_ecdh_direct_x25519_secret_is_big_endian() { + // The applet speaks big endian for Montgomery curves; the SDK + // swaps around the direct call. Keys are stored reversed (BE). + let sk = x25519_dalek::StaticSecret::random_from_rng(&mut OsRng); + let pk = x25519_dalek::PublicKey::from(&sk); + let peer_sk = x25519_dalek::StaticSecret::random_from_rng(&mut OsRng); + let peer_pk = x25519_dalek::PublicKey::from(&peer_sk); + let mut expected_be = sk.diffie_hellman(&peer_pk).to_bytes().to_vec(); + expected_be.reverse(); + + let mut priv_be = sk.to_bytes().to_vec(); + priv_be.reverse(); + let mut pub_be = pk.to_bytes().to_vec(); + pub_be.reverse(); + let mut peer_be = peer_pk.to_bytes().to_vec(); + peer_be.reverse(); + + let mut store = ObjectStore::new(); + store.insert([0, 0, 0, 0x64], SecureObject::ECKeyPair { + curve: ECCurve::Curve25519, + private_key: priv_be, + public_key: pub_be, + }); + let mut data = tlv_bytes(TAG_1, &[0, 0, 0, 0x64]); + data.extend(tlv_bytes(TAG_2, &peer_be)); + let apdu = ParsedApdu { + cla: 0x80, ins: 0x03, p1: P1_EC, p2: P2_DH, data, le: None, + }; + let resp = handle_ecdh(&apdu, &mut store, true); + assert_eq!(resp.sw, 0x9000); + let tlvs = crate::tlv::parse_tlvs(&resp.data).unwrap(); + assert_eq!(tlv::find_tlv(&tlvs, TAG_1).unwrap().value, expected_be); + } + #[test] fn test_ecdh_preserves_target_policy() { // The policy attached when the derive target was created must diff --git a/SE050Sim/se050-sim/src/handlers/object_mgmt.rs b/SE050Sim/se050-sim/src/handlers/object_mgmt.rs index 635b731..e5def5a 100644 --- a/SE050Sim/se050-sim/src/handlers/object_mgmt.rs +++ b/SE050Sim/se050-sim/src/handlers/object_mgmt.rs @@ -34,11 +34,12 @@ pub fn handle_write(apdu: &ParsedApdu, store: &mut ObjectStore) -> 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. +/// Handle READ commands for objects. ReadObject always refuses HMACKey +/// objects, as every real applet generation does regardless of any +/// attached read policy (verified on SE051 applet 7.2.0 and 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), @@ -242,24 +243,16 @@ fn handle_read_object(apdu: &ParsedApdu, store: &mut ObjectStore) -> ApduRespons SecureObject::AESKey { key } => key.clone(), SecureObject::UserID { value } => value.clone(), SecureObject::Counter { value } => value.to_be_bytes().to_vec(), - SecureObject::HMACKey { key, policy } => { - // 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 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); - } - key.clone() + SecureObject::HMACKey { .. } => { + // The applet never exports an HMACKey object, not even + // with POLICY_OBJ_ALLOW_READ attached at creation: + // verified on SE051 applet 7.2.0 hardware, where + // ReadObject fails with SW 0x6986 although the object + // attributes confirm the read policy, and observed + // identically on SE050C applet 3.1.1. Real applets + // guard AESKey objects the same way; the simulator + // does not model that yet. + return ApduResponse::error(SW_COMMAND_NOT_ALLOWED); } }; ApduResponse::success_with_tlvs(&[Tlv::new(TAG_1, &data)]) @@ -410,18 +403,17 @@ mod read_policy_tests { } #[test] - fn test_read_hmackey_with_read_policy_succeeds() { - let key = vec![0xABu8; 32]; + fn test_read_hmackey_with_read_policy_still_denied() { + // Hardware ground truth (SE051 applet 7.2.0): even an attached + // POLICY_OBJ_ALLOW_READ does not make an HMACKey object readable. let mut store = ObjectStore::new(); store.insert([0, 0, 0, 0x66], SecureObject::HMACKey { - key: key.clone(), + key: vec![0xAB; 32], policy: Some(POLICY_OBJ_ALLOW_READ | 0x0014_0000), }); 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); + assert_eq!(resp.sw, SW_COMMAND_NOT_ALLOWED); } #[test] diff --git a/SE050Sim/se050-sim/src/object_store/types.rs b/SE050Sim/se050-sim/src/object_store/types.rs index 9b42f67..fea1b7b 100644 --- a/SE050Sim/se050-sim/src/object_store/types.rs +++ b/SE050Sim/se050-sim/src/object_store/types.rs @@ -112,8 +112,9 @@ pub enum SecureObject { key: Vec, /// Union of the AR headers from the TAG_POLICY TLV attached at /// creation; None when the object was created with no policy. - /// The applet 7.2 read-policy contract keys off this (see - /// crate::policy). + /// Kept as part of the object's attributes; note ReadObject + /// refuses HMACKey objects regardless of this value, as real + /// applets do (see crate::policy and object_mgmt::handle_read). #[serde(default)] policy: Option, }, From 895ff41b2447efe952e902c467c5ef479f3eda37 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Mon, 3 Aug 2026 15:07:13 +0100 Subject: [PATCH 2/2] SE050Sim: sdk-test and direct-variant test cleanups from review Zero-init the sss_key_store_get_key bit-length out-parameters for consistency with the rest of the file, drop the unreachable returns after TEST_FAIL/TEST_FAILF (the macros return), and extract the peer public key in the Tag7-less unit test by TLV parsing instead of fixed offsets. --- SE050Sim/sdk-test/test_se050.c | 14 +++++--------- SE050Sim/se050-sim/src/handlers/ec.rs | 7 +++++-- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/SE050Sim/sdk-test/test_se050.c b/SE050Sim/sdk-test/test_se050.c index d6a656f..f51f7bd 100644 --- a/SE050Sim/sdk-test/test_se050.c +++ b/SE050Sim/sdk-test/test_se050.c @@ -332,7 +332,7 @@ static void test_ecdh(const char *name, uint32_t obj_a, uint32_t obj_b, size_t shared_len = sizeof(shared); uint8_t pub_b[256]; size_t pub_b_len = sizeof(pub_b); - size_t pub_b_bits = sizeof(pub_b) * 8; + size_t pub_b_bits = 0; size_t point_len; smStatus_t sm; @@ -369,13 +369,11 @@ static void test_ecdh(const char *name, uint32_t obj_a, uint32_t obj_b, point_len = (size_t)(1 + 2 * key_bytes); if (pub_b_len < point_len || pub_b[pub_b_len - point_len] != 0x04) { TEST_FAIL("key_b public point not found"); - return; } sm = Se05x_API_ECDHGenerateSharedSecret(&g_session->s_ctx, obj_a, pub_b + pub_b_len - point_len, point_len, shared, &shared_len); if (sm != SM_OK) { TEST_FAILF("direct ECDH failed: 0x%04x", (unsigned)sm); - return; } ASSERT_EQ(shared_len, (size_t)key_bytes, "shared secret length"); @@ -1424,11 +1422,11 @@ static void test_x25519_ecdh(void) * applet speaks big endian for Montgomery keys, so the peer point * and the returned secret are byte swapped around each call. */ pub_len = sizeof(pub_der); - pub_bits = sizeof(pub_der) * 8; + pub_bits = 0; status = sss_key_store_get_key(&g_ks, &key_b, pub_der, &pub_len, &pub_bits); ASSERT_OK(status, "get key_b public"); - if (pub_len < 32) { TEST_FAIL("key_b public too short"); return; } + if (pub_len < 32) { TEST_FAIL("key_b public too short"); } memcpy(point, pub_der + pub_len - 32, 32); for (i = 0; i < 16; i++) { swp = point[i]; point[i] = point[31 - i]; point[31 - i] = swp; @@ -1437,7 +1435,6 @@ static void test_x25519_ecdh(void) point, sizeof(point), shared_a, &shared_a_len); if (sm != SM_OK) { TEST_FAILF("direct ECDH a failed: 0x%04x", (unsigned)sm); - return; } for (i = 0; i < 16; i++) { swp = shared_a[i]; shared_a[i] = shared_a[31 - i]; @@ -1446,11 +1443,11 @@ static void test_x25519_ecdh(void) /* ECDH(B_priv, A_pub) */ pub_len = sizeof(pub_der); - pub_bits = sizeof(pub_der) * 8; + pub_bits = 0; status = sss_key_store_get_key(&g_ks, &key_a, pub_der, &pub_len, &pub_bits); ASSERT_OK(status, "get key_a public"); - if (pub_len < 32) { TEST_FAIL("key_a public too short"); return; } + if (pub_len < 32) { TEST_FAIL("key_a public too short"); } memcpy(point, pub_der + pub_len - 32, 32); for (i = 0; i < 16; i++) { swp = point[i]; point[i] = point[31 - i]; point[31 - i] = swp; @@ -1459,7 +1456,6 @@ static void test_x25519_ecdh(void) point, sizeof(point), shared_b, &shared_b_len); if (sm != SM_OK) { TEST_FAILF("direct ECDH b failed: 0x%04x", (unsigned)sm); - return; } for (i = 0; i < 16; i++) { swp = shared_b[i]; shared_b[i] = shared_b[31 - i]; diff --git a/SE050Sim/se050-sim/src/handlers/ec.rs b/SE050Sim/se050-sim/src/handlers/ec.rs index 4ee91fd..dde6aef 100644 --- a/SE050Sim/se050-sim/src/handlers/ec.rs +++ b/SE050Sim/se050-sim/src/handlers/ec.rs @@ -796,9 +796,12 @@ mod tests { // alike. This is how hosts on applet >= 7.2 obtain the secret. for strict in [true, false] { let (mut apdu, mut store, expected) = ecdh_inobject_fixture(); - // Rebuild the request without the Tag7 TLV. + // Rebuild the request without the Tag7 TLV, extracting the + // peer public key by TLV parsing rather than fixed offsets. + let orig = crate::tlv::parse_tlvs(&apdu.data).unwrap(); + let peer = tlv::find_tlv(&orig, TAG_2).unwrap().value.clone(); let mut data = tlv_bytes(TAG_1, &[0, 0, 0, 0x64]); - data.extend(tlv_bytes(TAG_2, &apdu.data[8..8 + 65])); + data.extend(tlv_bytes(TAG_2, &peer)); apdu.data = data; let count_before = store.count(); let resp = handle_ecdh(&apdu, &mut store, strict);