core: derive key: fix out-of-bounds write in SM2-KEP derivation

[secretpack]

What

In syscall_cryp_derive_key() the TEE_ALG_SM2_KEP branch passes so->info.maxObjectSize as the number of bytes sm2_kdf() writes into the derived key's secret buffer, but never bounds it against sk->alloc_size.

For a TEE_TYPE_GENERIC_SECRET object sk->alloc_size is fixed at 4096 / 8 = 512 bytes, while maxObjectSize may be requested up to 4096.

A caller that allocates the output object with a size in (512, 4096] and runs an SM2-KEP TEE_DeriveKey() makes sm2_kdf() write past the allocation up to a ~3.5 KiB heap overflow.

The sibling key-derivation branches (HKDF, Concat KDF, PBKDF2) already reject a derived length larger than sk->alloc_size; the SM2-KEP branch was missed.

Fix

Add the out_len > sk->alloc_size check to the SM2-KEP branch, matching the other derivation branches. out_len (= `maxd, so the GENERIC_SECRET known-answer test (regression_4014, 16-byte output) is unaffected.

Testing

• Builds for QEMUv8 (CFG_CRYPTO_SM2_KEP=y)
• xtest regression_4014 (SM2-KEP KAT) passes (16-byte output, < 512).

[jenswikl]

Please look into the CI error(s).

[secretpack]

I've force-pushed a corrected version of this commit.

The first version also converted the length with / 8 (bits -> bytes).

That was wrong: in the SM2-KEP path so->info.maxObjectSize is already used as a byte count.

The GENERIC_SECRET KAT in regression_4014 allocates the output object with the secret's byte length (16) and expects that many bytes back, so dividing by 8 shrank the derived key to 2 bytes and broke regression_4014.1 on QEMUv8.

The actual issue is only a missing bound: out_len (= maxObjectSize, up to 4096) is written by sm2_kdf() into a GENERIC_SECRET buffer whose alloc_size is fixed at 4096 / 8 = 512 bytes, so a request larger than 512 overflows the
allocation.

The commit now leaves out_len unchanged and only adds the out_len > sk->alloc_size check, matching the HKDF / Concat-KDF / PBKDF2 branches. regression_4014 uses a 16-byte output, well under 512, so it is unaffected.

I also corrected the commit authorship so the DCO check passes.

[jenswikl]

Reviewed-by: Jens Wiklander <jens.wiklander@oss.qualcomm.com>

Edit: wait with that R-B. I have a few comments.

[jenswikl]

Reviewed-by: Jens Wiklander <jens.wiklander@oss.qualcomm.com>

[jenswikl]

It seems that .out_len still has the wrong value. However, I think we can take that in a separate PR to fix the bits-vs-bytes confusion, since that will involve xtest case 4014. Would you mind fixing that too (later), or shall I?

[secretpack]

Thanks for the review — glad the bounds check (the actual OOB-write guard) is good to merge as-is.

I'd be happy to take the bits-vs-bytes cleanup as a separate follow-up.

To makesure I get it right this time (the earlier /8 attempt broke regression_4014 precisely because I only touched the derive side):
my understanding is that so->info.maxObjectSize for the SM2-KEP GENERIC_SECRET output should be in bits per the GP convention, so out_len should be maxObjectSize / 8 bytes and regression_4014 needs to allocate the output object with the bit size (8 * sizeof(gmt_003_part5_b2_shared_secret)) rather than the byte size, so the KAT still matches.

Does that match your intent? If so, I'll prepare the optee_os + optee_test change together in a new PR.
(And if you'd rather take it yourself, that's completely fine. just let me know.)

[jenswikl]

That's how I understand it too.

[jenswikl]

Please apply my R-B, and I'll merge this.

[secretpack]

Please apply my R-B, and I'll merge this.

Done, applied your Reviewed-by, thanks!