Fix the example attestation policy for Azure vTPM

[esposem]

See #1386 (comment)

Initial implementation of what discussed in the issue. We still need to discuss the other values (marked as TODO)

[mkulke]

not sure I understand. why would we want to hardcode values in the policy? those values will change. For SNP and similar values in other TEEs the value is queried from the reference values, no?

[esposem]

The only one I hardcoded are the ones that can just increase and not completely change. But yeah I am not fully convinced either, I just wanted to write down an initial draft to show you what I meant.

Otherwise as you said we need to figure these reference values, but I have no clue how.

[mkulke]

The only one I hardcoded are the ones that can just increase and not completely change. But yeah I am not fully convinced either, I just wanted to write down an initial draft to show you what I meant.

Otherwise as you said we need to figure these reference values, but I have no clue how.

I guess this is something that still needs to be answered/solved project-wide. AFAIU this part is left as an exercise for the user today. it looks like the confidential-containers/reference-values is a good place to start this effort.

I would suggest remaining consistent with other TEEs here. setting some hard invariants is probably fine (we might want to assert that SMT is enabled + allowed, for example, because peerpods doesn't expose a toggle for smt=off, although I'm still unsure this is the correct approach). putting concrete firmware, microcode etc version in the policy is the wrong approach, though, I'm pretty sure.

[esposem]

So if I understand correctly, you are suggesting to remove most of the fields? That would mean that for SNP we get:

executables := 3 if {
	input["az-snp-vtpm"]

	input["az-snp-vtpm"].tpm.pcr11 in query_reference_value("snp_pcr11")
}

hardware := 2 if {
	input["az-snp-vtpm"]
}

# For the 'configuration' trust claim 2 stands for
# "The configuration is a known and approved config."
#
# For this, we compare all the configuration fields.
configuration := 2 if {
	input["az-snp-vtpm"]

	# smt is disabled only if the CVM has 1 vcpu per core (--v-cpus-per-core=1)
	input["az-snp-vtpm"].platform_smt_enabled == "true"
	input["az-snp-vtpm"].policy_smt_allowed in query_reference_value("smt_allowed")
}

Which is ok but the hardware configuration is left empty. I guess for a public cloud this could be ok.

I wonder why were these fields added in the first place though...

[mkulke]

So if I understand correctly, you are suggesting to remove most of the fields?

no, I would suggest leaving the fields as they are and populate reference values instead. the snp tcb fields are absolutely worth checking, we don't want to discard them.

the only fields where you could argue are the smt ones (b/c of limitation in peerpod configuration) but again, this might change, and this could also be set in the reference values.

If CoCo a) doesn't provide sane defaults for SNP (or other TEEs') reference values and b) offers no convenient way for a user to import those, then this is a TDB for the project. and I understand Ding is working on that. In any case, I think the default policy is the wrong place to put those defaults.

[esposem]

Ok so basically you are suggesting to wait if I understand correctly then.

If CoCo a) doesn't provide sane defaults for SNP (or other TEEs') reference values and b) offers no convenient way for a user to import those, then this is a TDB for the project.

Completely agree

[fitzthum]

When it comes to the TCB values, I'd like us to have a similar approach for the SNP and AZ SNP platforms. I've been hoping AMD would provide some guidance on how to handle these fields or where to get the reference values.

[Xynnn007]

A simple idea is that if the fields are included inside the evidence, and can be changed based on different deployments, we should provide ways to check them than remove them.

Back to the PR, two questions from my side is

1. Are fields like reported_tcb_bootloader, reported_tcb_microcode, ... determined by the different deployments?
2. Is the responsibility to provide these reference values for those fields AMD's or Azure's, than CoCo's?

If either one is "yes", a more practical way to me might be documents about how to write attestation policies or how to choose reference values, than hardcode them inside the policies. Other archs have similar fields, and we would need a systematic documents for advanced users to configure these advanced fields in future, which will need help from different arch owners, to help perfect this guide. With the guide, advanced users can edit attestation policy / or set reference values themselves in any shape they want, like mixing simple equals and comparations to accomplish the goal.

[mkulke]

If either one is "yes", a more practical way to me might be documents about how to write attestation policies or how to choose reference values, than hardcode them inside the policies. Other archs have similar fields, and we would need a systematic documents for advanced users to configure these advanced fields in future, which will need help from different arch owners, to help perfect this guide. With the guide, advanced users can edit attestation policy / or set reference values themselves in any shape they want, like mixing simple equals and comparations to accomplish the goal.

Documentattion for sophisticated users makes sense in any case. Beyond that we might also want a streamlined/casual deployment story for potential adopters. (think: helm apply -f coco.yaml) This one could have reasonable defaults that resemble a production setup more closely. But I'm not sure how much of this is in the scope of downstream products rather than the CoCo project.

For example, currently many scripts, guides, e2e tests result in a setup in which the relying party components and the coco runtime are deployed into the same env, which is nonsensical but pragmatic: casual visitors might not have two discrete environments (+ networking glue) available just to test coco.

[esposem]

Yeah I think at this point this falls back on the hardware vendor rather than the cloud. AMD/Intel should provide some public (upstream) way to figure these values, as Tobin said. In general the CoCo software part is easy: built & measured upstream and downstream forks can provide their own reference values like PCRs, but the hardware doesn't follow this process. So IMO either the CoCo project doesn't provide any default att-policy/refval for hardware and just relies on the measurable software artifacts, or we figure a way to get these info from the hardware vendors.

[Xynnn007]

tbo I am not sure what do field like reported_tcb_bootloader actually mean. Are they about the hardware firmware?

For TDX, all hardware firmware related fields will be check against Intel's PC(C)S service, which is called in tdx-verifier, rather than doing policy check in trustee, and they maintain a machanism of "Security Version" to reflect the whole of these fields.

[mkulke]

tbo I am not sure what do field like reported_tcb_bootloader actually mean. Are they about the hardware firmware?

For TDX, all hardware firmware related fields will be check against Intel's PC(C)S service, which is called in tdx-verifier, rather than doing policy check in trustee, and they maintain a machanism of "Security Version" to reflect the whole of these fields.

yes, those are about firmware. and there a several variants of the TCB fields with complicated semantics. I understand TDX is more opinionated, but you'll need a collateral service. For SNP you have to make those decisions yourself, and the values will vary from cloud/region/SKU, some might update the firmware, some might get a custom fix via microcode update, etc...

[Xynnn007]

tbo I am not sure what do field like reported_tcb_bootloader actually mean. Are they about the hardware firmware?

For TDX, all hardware firmware related fields will be check against Intel's PC(C)S service, which is called in tdx-verifier, rather than doing policy check in trustee, and they maintain a machanism of "Security Version" to reflect the whole of these fields.

yes, those are about firmware. and there a several variants of the TCB fields with complicated semantics. I understand TDX is more opinionated, but you'll need a collateral service. For SNP you have to make those decisions yourself, and the values will vary from cloud/region/SKU, some might update the firmware, some might get a custom fix via microcode update, etc...

thanks for the explaination. if so, it is good to keep codebase as is for users to configure themselves. Just like other floks said, AMD should provide more information about the rv.

[fitzthum]

There are a few loose ends with the TCB values. First, I think we might actually be checking the wrong set of TCBs. I think the reported tcb corresponds to the the vcek, but there is another set with stricter guarantees regarding updates and rollbacks.

It might make sense to specify a minimum reported tcb level, but we need to make sure that these SVN fields actually can be compared numerically. Currently, we just check if the version is in the list of allowed versions. That's not ideal.

Another thing we could think about is having different trust levels depending on if/how we check the tcb version. This is a cool use of AR4SI, but it could ultimately be misleading/confusing since the levels are eventually collapsed into a status.

cc @amd-aliem @hgowda-amd

[esposem]

@mkulke @fitzthum I know we are trying to figure a solution but for the time being maybe it's better to comment them out?
Alternatively happy to also apply this fix only into the trustee-operator.

[mkulke]

@mkulke @fitzthum I know we are trying to figure a solution but for the time being maybe it's better to comment them out? Alternatively happy to also apply this fix only into the trustee-operator.

I think it is reasonable to comment the measurement claim, because this is per spec not a claim that a user can rely on to remain stable (otoh, maybe they still want to assert a certain value and don't want to accept paravisor updates).

SMT is hardcoded like you mentioned in peerpods currently, but that might change, and also maybe people want to use trustee outside of peerpods, so it's arguable whether we shouldn't have this claim.

Finally, commenting out the TCB and policy values doesn't make sense in any scenario IMO, those are the core of an attestation policy.

It's unfortunate that we don't have authoritative reference values for those, but the same also applies to snp.

So, yeah, I would suggest implementing a more practical policy (works with peerpods + azure today) somewhere downstream and leave the default policy as-is in trustee, for now.

[esposem]

Ok then closing this one. Thanks for the feedback!

[hgowda-amd]

Sorry got this PR pretty late. Just wanted to add a small note that AMD is working on figuring out how to collect, manage reference values. Since, we need to get a lot of buy ins internally, the work is moving slowly but surely. Will keep the community posted once we have a pretty solid solution for the SNP reference values.