confidential-containers / trustee

Attestation and Secret Delivery Components
Apache License 2.0
56 stars 81 forks source link

Refactoring KBS #217

Open Xynnn007 opened 10 months ago

Xynnn007 commented 10 months ago

After merging AS code into KBS, it is a good time to refactor KBS codes.

Before v0.8.0, we use KBS to return an attestation token from CoCo AS that includes a tee pubkey inside as a claim. This will make the things coupled, because it requires that any token provisioned by an attestation service behind KBS should have a claim named tee-pubkey. However, except CoCo-AS, other attestation services like Amber does not have such a claim.

To let KBS to work with different kinds of attestation services, it is a good practice to decouple KBS protocol semantics from the attestation token. That is to say, the attestation service only helps to check the base TCB of the TEE, while other information like tee pubkey, the nonce, etc are authentication semantics higher than attestation. KBS should leverage the returned attestation token from the attestation behind to check if the step Attestation succeeds, and then decide to provision a token/credential of itself. This new token is endorsed by KBS, showing the TEE client has passed the KBS protocol and mainly used for the TEE to retrieve the resources of KBS and other privileged operations.

mythi commented 9 months ago

To let KBS to work with different kinds of attestation services, it is a good practice to decouple KBS protocol semantics from the attestation token.

I don't think the KBS protocol is coupled with the AS today. The challenges are mostly with the current implementation and I've summarized the gaps in #162.

You mentioned tee-pubkey claim. I've checked Intel Trust Authority and Azure Attestation. Both have a similar JWT token claim about "TEE held data". Intel Trust Authority based flow can be made to work just doing:

diff --git a/src/api/src/http/resource.rs b/src/api/src/http/resource.rs
index 26ac847..4b4f7de 100644
--- a/src/api/src/http/resource.rs
+++ b/src/api/src/http/resource.rs
@@ -47,7 +47,7 @@ pub(crate) async fn get_resource(
     })?;

     let pkey_value = claims
-        .get("tee-pubkey")
+        .get("attester_runtime_data")
         .ok_or(Error::AttestationClaimsParseFailed(String::from(
             "No `tee-pubkey` in the attestation claims",
         )))?;

while other information like tee pubkey, the nonce, etc are authentication semantics higher than attestation.

We must also have a nonce originated from the attestation service so that each quote it verifies can be considered "fresh".

Xynnn007 commented 9 months ago

I don't think the KBS protocol is coupled with the AS today.

please see the call stack here:

You mentioned tee-pubkey claim. I've checked Intel Trust Authority and Azure Attestation. Both have a similar JWT token claim about "TEE held data". Intel Trust Authority based flow can be made to work just doing:

IMO, CoCo-AS/Amber/MAA are all verifiers, while the KBS is a relying party that includes two functionalities:

There might be two reasons I think it would be better to refactor:

  1. The two abilities are different and it can be better for us to decouple the parts.

  2. Now the authorization is using a tee-pubkey that will be used to encrypt the data, which will bring security issue as https://github.com/confidential-containers/kbs/issues/143 shows. To resolve that, having a public key cert leveraging mTLS and DH key exchange will help. However in this context, an AS-provisioned token does have enough ability to work as a TLS cert.

mythi commented 9 months ago

I don't think the KBS protocol is coupled with the AS today.

please see the call stack here:

My comment was: the KBS protocol specification itself is fine and it has no coupling to CoCo AS but also Intel Trust Authority can be made to work by following the spec. Maybe we are talking about the same thing: the kbs implementation (and guest-components' attesters + kbs_protocol) needs rework to better support other AS'es. I totally agree and we can start from #162 issues.

What you write about authentication and authorization issues sound something that first needs to be addressed in the KBS protocol spec and once updates to the spec are made, the implementation would follow.