Security: Cross-validate KeyMaterialType in NewHandleWithNoSecrets to prevent spoofing

[Ashutosh0x]

Summary

Fix NewHandleWithNoSecrets / ReadWithNoSecrets to cross-validate the actual KeyMaterialType after parsing, preventing attackers from importing symmetric secret key material by spoofing key_material_type to ASYMMETRIC_PUBLIC.

Vulnerability (Issue #43)

hasSecrets() trusts the serialized KeyData.key_material_type enum to decide whether a keyset contains secret material. This field is attacker-controlled in the input bytes.

Attack path:

1. Serialize a tinkpb.Keyset containing an HMAC key with attacker-chosen key bytes
2. Set KeyData.key_material_type to ASYMMETRIC_PUBLIC
3. Load through ReadWithNoSecrets or NewHandleWithNoSecrets
4. hasSecrets() accepts because it only checks the spoofed enum
5. HMAC parser accepts the key, mac.New(handle) returns a working primitive
6. Attacker now has a handle with known symmetric key material enabling tag forgery

Fix

After parsing keys via newKeysetHandleFromProto, re-serialize each parsed key through the trusted serializer and verify the actual KeyMaterialType. Also check if any parsed key implements the privateKey interface. If any key's trusted type indicates secret material, reject the keyset with an error.

This is a defense-in-depth approach — the trusted parser knows the real key type regardless of what the attacker claimed in the serialized enum.

Impact

Callers using ReadWithNoSecrets / NewHandleWithNoSecrets as a trust boundary for importing externally-provided "public" keysets can be tricked into accepting symmetric key material. For HMAC, this enables tag forgery for any authentication signal derived from that MAC.

Fixes #43

[Ashutosh0x]

Hi @morambro @juergw — this PR fixes the KeyMaterialType spoofing vulnerability reported in #43 (originally reported through g.co/vulnz and redirected here by Google VRP).

The core issue is that hasSecrets() trusts the serialized key_material_type enum, which is attacker-controlled. A symmetric key (e.g., HMAC) serialized with key_material_type=ASYMMETRIC_PUBLIC passes the no-secrets check, and the HMAC parser happily parses it into a working MAC primitive with attacker-known key material.

My fix adds a post-parse cross-validation step: after parsing via newKeysetHandleFromProto, each key is re-serialized through the trusted serializer and its actual KeyMaterialType is checked. If any key turns out to be SYMMETRIC or ASYMMETRIC_PRIVATE despite the input claiming otherwise, the keyset is rejected.

This is a defense-in-depth approach that works regardless of whether individual key parsers enforce type consistency. Happy to adjust the approach if you prefer a different strategy!

[morambro]

Hi Ashutosh0x

The core issue is that hasSecrets() trusts the serialized key_material_type enum, which is attacker-controlled. A symmetric key (e.g., HMAC) serialized with key_material_type=ASYMMETRIC_PUBLIC passes the no-secrets check, and the HMAC parser happily parses it into a working MAC primitive with attacker-known key material.

Overall, I am not sure what the attack is here: what are the attacker's capabilities? What's the attacker goal? From your description, it seems the attacker has both control on the keyset (to change key_material_type) and arbitrary code execution on a victim application? An application that reads a no-secret keyset and gets a MAC from it is buggy.

My fix adds a post-parse cross-validation step: after parsing via newKeysetHandleFromProto, each key is re-serialized through the trusted serializer and its actual KeyMaterialType is checked. If any key turns out to be SYMMETRIC or ASYMMETRIC_PRIVATE despite the input claiming otherwise, the keyset is rejected.

This is a defense-in-depth approach that works regardless of whether individual key parsers enforce type consistency. Happy to adjust the approach if you prefer a different strategy!

I think I'd rather fix each protoserialization as suggested here: #43 (comment)