se-attester: use SHA-512 for report data in IBM SEL

[BbolroC]

This PR uses more secure SHA-512 algorithm for report data in IBM SEL instead of the fallback algorithm SHA-384.
As small refactor, it also extracts the process_runtime_digest() helper function from the SE attester into a general-purpose utility function.

Depends-on: confidential-containers/trustee#1338

Signed-off-by: Hyounggyu Choi Hyounggyu.Choi@ibm.com

[mythi]

Depends-on: confidential-containers/trustee#1338

The Trustee PR is also set to depend on this one. To avoid the circular dependency, can this be mode so that se-attester uses sha384 hash algo with old Trustee that does not give you selected-hash-algorithm. Or is it like this already?

could process_runtime_digest() be made such that it can also be used in get_evidence() to deduplicate the logic.

[BbolroC]

Depends-on: confidential-containers/trustee#1338

The Trustee PR is also set to depend on this one. To avoid the circular dependency, can this be mode so that se-attester uses sha384 hash algo with old Trustee that does not give you selected-hash-algorithm. Or is it like this already?

Yeah, I am aware of it, unfortunately. For this, the attestation service should keep memorizing which hash algorithm is selected for the extra_params for each attestation challenge and pass the info to a verifier for validation of the report data, which leads to non-marginal changes. I think we can deal with it separately. Or does it make sense to make a fallback for the validation in the verifier side? What do you think @Xynnn007 about this?

could process_runtime_digest() be made such that it can also be used in get_evidence() to deduplicate the logic.

Good idea! This also makes the unit tests look nice and clear. I will update the PR. Thanks!

[Xynnn007]

Yeah, I am aware of it, unfortunately. For this, the attestation service should keep memorizing which hash algorithm is selected for the extra_params for each attestation challenge and pass the info to a verifier for validation of the report data, which leads to non-marginal changes. I think we can deal with it separately. Or does it make sense to make a fallback for the validation in the verifier side? What do you think @Xynnn007 about this?

Let's first distinguish a few concepts before looking at what the current PR is doing.

First, the current Attester abstracts the TEE's report retrieval capability into a black box. For architectures like TDX/SNP, there's a challenge parameter called report_data, which is a short data value, typically a few tens of bytes long. IBM SE's black box uses the UserData field for this.

As for runtime_data, it's a large JSON structure, a concept at the KBS protocol level. We bind the entire runtime_data to specific evidence when retrieving evidence. However, since report_data is short, we hash the entire runtime_data and use the hash value as report_data to call get_report. The hash algorithm can be chosen during kbs protocol handshake.

So, in this PR, the change is for low-level black box to implement IBM SE's report_data functionality - we can treat it as what the Intel CPU circuits inside is doing - changing report field from 48 bytes to 64 bytes - thus a breaking change but I think it's ok. It's not a good idea to support both modes in report_data ability level as it's low level black box, but we need to ensure that the compatibility for attester/verifier in a same version.

[Xynnn007]

LGTM! Thanks @BbolroC

Note that this is a breaking change for IBM SE architecture.
Also, with a deep diving to code, I realize now SE could now support composite attestation. The SeAttestationRequest.runtime_data_digest can be set in the kbs_protocol for IBM SE. This work can be done a separate PR.

[Xynnn007]

btw let's merge this when confidential-containers/trustee#1338 is ready also, for there are breaking changes and we need the compability for both side.

[mythi]

@BbolroC I think b9f82d4 needs to be the first commit in this PR.

[Xynnn007]

Note that confidential-containers/trustee#1338 gets merged

[mythi]

Looks OK. Have you tested this with existing Trustee releases?

[BbolroC]

Looks OK. Have you tested this with existing Trustee releases?

Yes, I tested it with the latest push for both. It works. But not working with the current Trustee (the version before confidential-containers/trustee#1338 is merged) as @Xynnn007 explained above.