KBS: Add attestation policy selection - #1521
Conversation
722bf3c to
de04b7f
Compare
fitzthum
left a comment
fitzthum
left a comment
There was a problem hiding this comment.
Code looks fine. A couple design comments, but I think this is workable. At least it's disabled by default.
| are specific to a deployment and must be known to the KBC in advance. An | ||
| unmapped `id` is rejected, and omitting the field selects a default policy. | ||
|
|
||
| The KBC picks the `id` itself, so doing so is not an authorization decision. The |
There was a problem hiding this comment.
I don't really understand what is meant by authorization decision. The policy hint must exist in the map configured by the admin, but this is still a highly significant field. It can completely shift the meaning of the attestation result.
Should consumers of the attestation token now be checking the policy id to make sure it was the right one? Ofc they don't know the real policy id or content, just the hint.
There was a problem hiding this comment.
Oh. That means selection of the id does not mean any authorization to access the KBS, just a hint. All authorization work is to filter the AS token based on kbs policy
There was a problem hiding this comment.
The user does not need to know. It does not matter. It matters for KBS to know it. The token provisioner hinted by iss and the policy id can be used to generally index a policy. The hint is enough for the user. But, if the user and the attestation admin/trustee admin, he needs to know it
There was a problem hiding this comment.
I am just a little worried about cases where we switched between two policies with different meanings. Keep in mind that this field is not measured so it could be modified in transit. In this case does the user need to inspect the attestation token to make sure the right policy was used?
There was a problem hiding this comment.
It can be modified during transit. But note that AS will sincerely evaluate the evidences based on the "malwared" policy id(s). Then, it's responsibility of KBS, to check if the token has proper policy id(s).
There was a problem hiding this comment.
i still think this is a little sketchy, but I guess we do not enable this feature by default so I can live with it
de04b7f to
252fe13
Compare
The RCAR implementation gave the Attestation Service no say over which policy to apply: both CoCo AS backends hardcoded "default", so a deployment could not evaluate different guests against different policies without editing that one policy in place. Widen the Attest::verify interface to carry the policies to evaluate with, in preparation for letting a client select them. Passing None keeps each backend on the behaviour it has today, which for the CoCo AS backends is the "default" policy and for Intel TA is the configured policy_ids list, so this commit is a no-op at runtime. The literal that the CoCo backends share moves to their common module rather than being repeated. Signed-off-by: Xynnn007 <xynnn@linux.alibaba.com> Co-authored-by: Cursor <cursoragent@cursor.com>
fitzthum
left a comment
fitzthum
left a comment
There was a problem hiding this comment.
LGTM
like i said, i don't totally love this feature, but i guess it's ok to give people the option.
maybe another review from @confidential-containers/trustee-maintainers would be good
|
cc @mythi I guess you will be interested in this multiple policy selection part. |
252fe13 to
0385096
Compare
A KBC could not indicate which attestation policy should judge its evidence, so all guests of a deployment shared one policy. Let the KBC name an "policy selector" in the extra-params of the RCAR Request, which KBS resolves through the new policy_id_map configuration. Resolving through an administrator-declared table is preferred over letting the KBC name Attestation Service policy IDs directly. It keeps a client-controlled string away from the AS policy storage keys, avoids turning the AS policy namespace into a public contract that guest images depend on, and bounds the reachable policies to a whitelist. An unknown id is rejected at /auth rather than falling back, so a typo cannot silently change which policy applies. Omitting the field keeps the existing behaviour, and the map is empty by default, so the feature is inert until an administrator opts in. An id mapped to an empty list is rejected at startup: it would leave the evidence unevaluated, and Intel TA accepts that silently by disabling policy matching altogether. Note that a KBC picking its own id is not an authorization decision. The policy that was applied is reported in the attestation token, so a resource policy can restrict what a given appraisal releases; the default resource policy does not do so yet. Signed-off-by: Xynnn007 <xynnn@linux.alibaba.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Describe the "policy-selector" field of the RCAR Request in the protocol document, including that its accepted values are deployment specific and that an unmapped id is rejected, both of which a KBC implementation has to know. State that selecting an id is not an authorization decision so that readers do not mistake it for one. Document policy_id_map for each attestation service type, and note that for Intel TA a selected id replaces the configured policy_ids. Signed-off-by: Xynnn007 <xynnn@linux.alibaba.com> Co-authored-by: Cursor <cursoragent@cursor.com>
0385096 to
4d61017
Compare
|
Seem like I cannot get copilot to review this PR. |
2 participants
This patch mainly introduces a static config that maps id field to a set of policy ids. By default, if no id is given from client side, the "default" policy will still be used.
For ITA, in previous version it has its own config to specify the policy ids. In this new version, if any id->policy ids is given by the request, it will use the mapped policy ids. If not, use the policy ids in the ITA config.
We do not bump a protocol version here as it's add-only feature and do not break.
Fixes #282