Zero key for FIPS

[Wenxing-hou]

Reference: #1260

2023/1/7 Commit 1: Zero key for FIPS when encode and decode.
2023/1/13 Commit 2: Fix unit test because of the salt change.
2023/1/9 Commit 3: Zero salt1 in libspdm_generate_session_data_key.

Signed-off-by: Wenxing Hou wenxing.hou@intel.com

[jyao1]

we need zero key in any return path.

[Wenxing-hou]

Thanks. I have added the salt zero in every return path.

[jyao1]

we need zero key in any return path.

[steven-bellock]

Why does the key need to be on the stack at all? Why not use

secured_message_context->handshake_secret.request_handshake_encryption_key

directly?

[jyao1]

Yes, I think we can change to uint8_t *key.

[Wenxing-hou]

Yes, I think we can change to uint8_t *key.

Thanks. I have changed the key from array to pointer.

[jyao1]

Please remove /*zero salt for security*/. No need to say it every where.

[Wenxing-hou]

Thanks. I have removed the comments except the first comment.

[steven-bellock]

Can also make the salt a pointer.

[jyao1]

If so, the XOR sequence_number need to be: *(uint64_t *)salt ^ (sequence_number - 1) ^ sequence_number

Please test it between old version and new version.

[Wenxing-hou]

Thanks. I have changed the salt to pointer and added a new commit to fix the unit test.

[Wenxing-hou]

And I have tested pass the emu.
the array salt is old version; the pointer salt is new version. The following test has passed.

new version requester <---> new version responder
old version requester <---> new version responder
new version requester <---> old version responder

[steven-bellock]

The salt should stay const, like the key.
Also we'll need to declare uint8_t iv[LIBSPDM_MAX_AEAD_IV_SIZE], that actually gets fed into libspdm_aead_encryption.

[jyao1]

I don't think we need define uint8_t iv[LIBSPDM_MAX_AEAD_IV_SIZE]. That will go back to original problem.

[steven-bellock]

This (and the 2.3 version) is misleading as once the assignment occurs this value is no longer the salt, it's the IV. So here I would zero-extend sequence_number and XOR it with salt and assign it (libspdm_copy_mem) to iv[].

[jyao1]

If the name is confusing, I think we can rename to uint8_t *iv. I don't see the need to cache the iv[].

[steven-bellock]

Ok. Feel free to merge this and I'll file an issue.