Refactor kbs client

[Xynnn007]

This PR mainly aims to tidy & refactor the code in kbs_protocol. Now KBS protocol supports both passport & background check model to access a resource. So the different ways are included in this PR

Also, some work aiming to tidy the deps/crypto was introduced.

To assert that the new kbs_protocol works, a unit test using a CoCo community published kbs image is impled.

see #292

cc @jialez0 @mkulke

[mkulke]

I have only looked at the kbs client part so far, I will look at the passport part seperately:

I found that the Attester and the EvidenceProvider traits are very similar. Is there a reason to keep the Attester trait in the current form?

but if we still want to keep both traits we can create a "trait alias", it's a tiny bit of ceremony, but the evidence_provider code would be redundant, plz see this commit for reference

[Xynnn007]

I found that the Attester and the EvidenceProvider traits are very similar. Is there a reason to keep the Attester trait in the current form?

but if we still want to keep both traits we can create a "trait alias", it's a tiny bit of ceremony, but the evidence_provider code would be redundant, plz see this commit for reference

Thanks for the reference! My first idea was that the EvidenceProvider and Attester are different:

1. Attester are native driver/plugins and their tee type can be detected directly via a function detect_tee_type() rather than a method tee.detect_tee_type().
2. EvidenceProvider is a higher level abstraction that defines how the underlying evidence is generated and how tee type can be detected. This design comes from "a remote server(another process, or IMDS) will tell the concrete tee type" discussed with you.

I discussed with @jialez0 and the detect_tee_type() should not be a trait function in Attester for we should firstly know which Tee is and then create an Attester object using the Tee enum.

If we do not need to "know which tee is via communicating with a remote server", Attester and EvidenceProvider can be the same. I'd like to combine them.
If we do need, they are different.

What is your opinion?

[mkulke]

I discussed with @jialez0 and the detect_tee_type() should not be a trait function in Attester for we should firstly know which Tee is and then create an Attester object using the Tee enum.

yes, I agree. this makes sense. In this case EvidenceProvider and Attester probably have to be different traits, since they are providing different things. I understand that KbsClient._tee is populated lazily in background_check#rcar_handshake() (and initialized with None).

Looking from the user side, a kbs client impl could look like this:

use kbs_protocol::{EvidenceProvider, KbsClientCapabilities};
use kbs_types::Tee;
use std::error::Error;

struct MyEvidenceProvider;

#[async_trait::async_trait]
impl EvidenceProvider for MyEvidenceProvider {
    async fn get_evidence(&self, _d: Vec<u8>) -> Result<String, Box<dyn Error>> {
        todo!();
    }
    async fn get_tee_type(&self) -> Result<Tee, Box<dyn Error>> {
        todo!();
    }
}

#[tokio::main]
async fn main() {
    let provider = Box::new(MyEvidenceProvider);
    let mut client =
        kbs_protocol::KbsClientBuilder::with_evidence_provider(provider, "https://kbs.com")
            .build()
            .unwrap();
    let resource_uri = "one/two/key".try_into().unwrap();
    let _ = client.get_resource(resource_uri).await.unwrap();
}

[fitzthum]

Nice job. Some comments. Please bear with me as I attempt to understand all the changes.

[fitzthum]

Do we need to import PassportClientCapabilities here?

[Xynnn007]

yes, PassportClientCapabilities defines get_token ability.

Also I find the names passport and background check are very confusing in the code. Let me rename them and just tell "how to perform a passport-model access or a background-check-model access in code"

[fitzthum]

Ok, I was getting confused by those. I think it makes more sense now.

I notice that the passport feature doesn't have any attester associated with it. I assume we will be adding some new ones there?

[Xynnn007]

The passport feature Client does not need to care about attester. As attesters in KBS Client only helps to generate evidence in order to get token. The passport feature Client does not assume the origin of the token, and it just call get_token from TokenProvider.

[jialez0]

We should implement ToString or some thing like for this struct, so that we can get the original content of JWT.

[fitzthum]

Ok, I think I've wrapped my head around this. Let me try to summarize.

This PR gives us two methods to request a resource from a KBS. One is the RCAR method that we have been using, which is based on "evidence". The other uses a token instead of evidence. When we use a token, we skip over the typical challenge and attestation steps of RCAR by providing the token in the request and immediately getting a response.

Now to make things a bit more complicated, there is also a token involved in a normal RCAR request. The KBS sends a token to the client once attestation is successful. Here when we use the TokenProvider, we don't use this token from the KBS. Instead we use one generated by a TokenProvider. It doesn't seem like we've really implemented this yet, but presumably these tokens will come from the HW evidence of certain platforms.

Whether you have a platform that gives you "evidence" or a platform that gives you a token, you can connect to the KBS and get a resource. In both cases you can also get a token. In the RCAR case the token comes from the KBS. In the TokenProvider case, it comes directly from the TokenProvider.

What is the difference between a token and evidence? A token represents the attestation of the entire workload, whereas evidence is typically the initial state of the enclave. Hence the alternate terms passport and background check. A token is a passport and evidence is more like a background check. Again, you can get a passport/token either from RCAR or TokenProvider and you can connect to a KBS with RCAR or TokenProvider. With TokenProvider you can get a passport without connecting to the KBS.

Is this summary correct?

[Xynnn007]

@fitzthum Nice summury and I think it is correct. About the TokenProvider I want to say more.

When I firstly designed this, two scenarios were considered:

• A KBS Client connects to a KBS using a token. The token is manually set by the user. Typically for testing, this is TestTokenProvider
• When we design the architecture of CDH and new AA, we decouple the "getting token" part and "using token to access resource" part. The "getting token" part should belong to AA, as it involves remote attestation. The "using token to access resource" should belong to CDH, as it cares about resource access. In this way, a programming problem is how to get a new token if the "using token to access resource" part finds the token is expired? Yes, call AA to generate a new one.

The two scenarios are abstractly the same, s.t. we need a TokenProvider which is responsible for "generating new tokens".
We will refactor AA later. At that time I'll add this TokenProvider which will call AA from CDH via ttRPC.

We should also be careful. The trait TokenProvider makes it possible for a component inside Tee to get an token from somewhere else. Try to think about this: a malwared Tee gets a correct token to retrieve resource. So when we apply this crate, we must avoid insecure usages. This is beyond of the crate, but the overall design of CoCo/other platforms.

[fitzthum]

Ok, LGTM.

Looks like there is a merge conflict.

@mkulke can you take another look so we can merge and get the CDH stuff going

[fitzthum]

When we design the architecture of CDH and new AA, we decouple the "getting token" part and "using token to access resource" part. The "getting token" part should belong to AA, as it involves remote attestation. The "using token to access resource" should belong to CDH, as it cares about resource access. In this way, a programming problem is how to get a new token if the "using token to access resource" part finds the token is expired? Yes, call AA to generate a new one.

Hmmm. I guess I have misunderstood part of the design of the CDH. I had thought that the AA would not make any network calls. Instead it would just be responsible for getting the hardware measurement, which would be used by the KBS either as evidence or a token.

What you're saying is that we are going to use something more like the RATS passport model where the KBS will give the AA a token and then the CDH will use that token for future requests?

[Xynnn007]

What you're saying is that we are going to use something more like the RATS passport model where the KBS will give the AA a token and then the CDH will use that token for future requests?

Yes

[Xynnn007]

Looks like there is a merge conflict.

Rebased and fixed the conflict.

cc @mythi the conflict is about the commit

[mythi]

Looks like there is a merge conflict.

Rebased and fixed the conflict.

cc @mythi the conflict is about the commit

yeah, looks like it was merged yesterday ahead of this so a rebase was needed. Thanks!

[Xynnn007]

@mkulke Thanks for the suggestions. I've fixed them. PTAL