EAR Support

[fitzthum]

This is the very beginning of this PR. I still need to fix some issues with the underlying crate, add support for verifying tokens in the KBS, and fixup all the tests, examples, and default values.

See commit messages for more info.

Fixes: #353

Progress so far:

• Miscellaneous Fixups
• Implement EAR token generation in the AS
• Implement EAR verification in KBS
• Update docker compose
• Update k8s (hard to test fully without building the images, but should be smooth when we do)
• Fix integration tests
• FIxup RVPS
• Fixup policy_ids
• Improve default policy to validate TCB of all platforms (well, for a couple of platforms. the rest will be added by others hopefully)
• Update docs and examples

[fitzthum]

This is ready for review. I tried to make this as simple as possible, but there are a lot of interlocking pieces. Please read the commit messages. They will hopefully explain what is happening and highlight a couple of significant changes (like not flattening the tcb claims anymore, not supporting multiple policies, etc)

We still have some issues to resolve in the underlying crate before we can merge.

• Appraisal is not Send veraison/rust-ear#19 - without this we can't create extensions, which we aren't using anyway yet. we could probably go ahead without a fix, but we have to pin the crate to a hash right before they added extensions or it won't compile (and we will need more commits on top of this for the other issues)
• JWT has no expiration field veraison/rust-ear#20 - without this the tokens never expire. interestingly guest-components is fine with this. the code works fine, but it's a security problem since the KBS can't verify that the token hasn't expired.
• RawValue has no variant for null veraison/rust-ear#21 - without this JSON deserialization of some values into EAR RawValues causes everything to explode. this may even be triggered by the TDX or SGX claims, which would be bad.

[Xynnn007]

Thanks Tobin. Nice work about code, tests and documents. I am excited to see that the design that leaves the trusted anchor calculation to users.

I have not dived into the code too deeply, but only for a quick catch for the architecture design.

This PR abondons former AS token format. Maybe we could leave the token plugins still. Probably the difficulty is where should OPA lie. My suggestion is to make OPA a common lib for both legacy AS token and EAR.

In this way, the token type could be configured in the launch toml of AS. Such as

[token]
type = "ear"
# other configs for ear

or

[token]
type = "simple"
# other configs for simple

This can be implemented by

#[serde(tag = "type")]
pub enum AttestationTokenConfig {
    Ear(EarConfig),

    Simple(SimpleConfig),
}

pub type AttestationTokenBroker = Arc<dyn AttestationTokenBrokerTrait + Send + Sync>;

impl TryFrom<AttestationTokenConfig> for AttestationTokenBroker {
    type Error = Error;

    fn try_from(value: AttestationTokenConfig) -> Result<Self> {
        match value {
            AttestationTokenConfig::Ear(config) => {
                let ear_broker = Arc::new(EarBroker::new(config)?);
                Ok(ear_broker as _)
            }
            AttestationTokenConfig::Simple(config) => {
                let simple_broker = Arc::new(SimpleBroker::new(config)?);
                Ok(simple_broker as _)
            }
        }
    }
}

This would give enough flexibility to downstream users to define their different token format, although they might not be standard as EAR. But for CoCo, we can support EAR by default.

[jialez0]

This PR abondons former AS token format. Maybe we could leave the token plugins still. Probably the difficulty is where should OPA lie. My suggestion is to make OPA a common lib for both legacy AS token and EAR.

In this way, the token type could be configured in the launch toml of AS. Such as

[token]
type = "ear"
# other configs for ear

or

type = "simple"
# other configs for simple

This can be implemented by

#[serde(tag = "type")]
pub enum AttestationTokenConfig {
    Ear(EarConfig),

    Simple(SimpleConfig),
}

pub type AttestationTokenBroker = Arc<dyn AttestationTokenBrokerTrait + Send + Sync>;

impl TryFrom<AttestationTokenConfig> for AttestationTokenBroker {
    type Error = Error;

    fn try_from(value: AttestationTokenConfig) -> Result<Self> {
        match value {
            AttestationTokenConfig::Ear(config) => {
                let ear_broker = Arc::new(EarBroker::new(config)?);
                Ok(ear_broker as _)
            }
            AttestationTokenConfig::Simple(config) => {
                let simple_broker = Arc::new(SimpleBroker::new(config)?);
                Ok(simple_broker as _)
            }
        }
    }
}

This would give enough flexibility to downstream users to define their different token format, although they might not be standard as EAR. But for CoCo, we can support EAR by default.

I concur that the format for generating tokens should be modular. We can default to supporting EAR, but simultaneously, retain the previous simple AS token mode as a plugin option. This approach will provide ample room for customization to the users of this project.

[fitzthum]

@Xynnn007 @jialez0 You are both right that this PR makes our token provisioning less generic. In fact, it's no longer generic at all. There are some reasons for this.

First, with EAR tokens the policy is fundamentally tied to the token. Our CoCo token supports just about any policy claims, but with EAR we need to generate an Appraisal. This means that 1) the policy is specific to the type of token we use. 2) A bunch of the code surrounding the policy is specific to the type of token we are using.

The first thing isn't a huge problem. It's not a great user experience to require totally different policies for different tokens, but at least that is configurable at runtime. The second thing is a bit more of a problem. There is code in both the policy engine and lib.rs that is specific to the type of policy that we are using. For instance, the policy engine parses the claims differently, and lib.rs composes the token differently. Supporting generic tokens would require changing some abstractions and adding complexity.

One option would be to move most of the logic into the token generation code. The policy engine would always process all the claims and return the full results. The results would be provided as-is to the token broker. The token broker would also get the TCB claims and any other pieces of the token. I don't really like broadening the scope of the token broker like this. If we want to keep the token broker abstraction the same, we would probably have to add a bunch of conditional logic to the policy engine and lib.rs, which also doesn't seem like a great idea.

It's possible to keep both tokens, but there would be costs. Keep in mind that we would need to create more tests, more examples, extend the docs, etc. Besides the costs, I actually think there are advantages to supporting fewer things. So far we have tried to make Trustee as generic as possible. In general I think this is good, but currently I think we are in danger of giving users too many ways to configure things without showing them any particular way that actually works end-to-end. I think part of the reason why we have some gaps with policies, the RVPS, etc is that we haven't been prescriptive enough. I think EAR tokens are a good fit because they are supposed to be generic, but they are more opinionated.

Anyway, this change wasn't an accident, but I understand that it is kind of scary. I am open to trying to support both, but my question for you is what is the value of keeping the CoCo token?

[fitzthum]

Another way we could potentially compromise is by keeping the CoCo token but populating the policy results with an EAR-like Appraisal. I'm not sure if this really makes sense tho.

[Xynnn007]

Hi @fitzthum

Anyway, this change wasn't an accident, but I understand that it is kind of scary. I am open to trying to support both, but my question for you is what is the value of keeping the CoCo token?

Actually we are trying to define some own token format in downstream pre-productions benefits from the original plugin design.

Let's go back to your worries. The core issues are 1. token generation code and 2. user interface.

For 1,
I tried to look into the code about the design of current policy module. Some ideas:

• Change the API of policy engine. That takes policy id, data and input as parameters, just as OPA defines. We could have only regorus as backend and abondon the extensibility for policy engine (I have a strong hunch we need to do this because so far there is no indication that we will have a strategic replacement other than rego)
• Change the AttestationTokenBroker's issue API to receive parameter reference_data_map, tcb_claims and policy_id. Then the module itself decides how to interact with regorus.
• To help KBS recognize the concrete token type, we could add another return value of AS's evaluate function.
  For 2,
  The current config of AS looks like

{
    "work_dir": "/opt/confidential-containers/attestation-service",
    "policy_engine": "opa",
    "rvps_config": {
	    "remote_addr":"http://rvps:50003"
    },
    "attestation_token_broker": "Simple",
    "attestation_token_config": {
        "duration_min": 5
    }
}

After some change, it would be

{
    "work_dir": "/opt/confidential-containers/attestation-service",
    "policy_engine": "opa",
    "rvps_config": {
	    "remote_addr":"http://rvps:50003"
    },
    "attestation_token_broker": {
        "type": "XXX",
        ...// Other options
    }
    "attestation_token_config": {
        "duration_min": 5
    }
}

The default one we could set as EAR, together with all other e2e examples. Although we provide "too" many ways to run trustee, I believe most users will directly use the default configuration of our e2e test (probably EAR or Simple token), which needs to be ensured to run normally by us. Some advanced users might want do extension, then the value of extensibility values.

[fitzthum]

Actually we are trying to define some own token format in downstream pre-productions benefits from the original plugin design.

Ok. This is a valid consideration. Ideally we could converge on one token, but I'm not sure what your requirements are and they might not be public. Keep in mind that the EAR token is in some ways very similar to our CoCo token. For instance, they are both JWT. The EAR token stores a public key in a similar way but with a different path. Consumers can somewhat ignore parsing the AR4SI Trust Claims by just looking at the status field of the Appraisal.

We could have only regorus as backend and abondon the extensibility for policy engine

I think we should try to keep the policy engine modular. My opinion of rego/opa is declining and I would love to provide another option (as long as we don't invent it ourselves).

Btw, this PR also switches from evaluating multiple policies with the AS, to only taking one policy id. If we keep support for the CoCo token, can it just take one policy or does it need to do multiple?

[Xynnn007]

Btw, this PR also switches from evaluating multiple policies with the AS, to only taking one policy id. If we keep support for the CoCo token, can it just take one policy or does it need to do multiple?

One policy is ok for now.

[Xynnn007]

Thanks @fitzthum ! Last comments

[Xynnn007]

Ok. Could you help to mark a TODO comment or an issue to record this?

[fitzthum]

Found a small issue. Fixing. We should be good to go v soon

[fitzthum]

Ok, we're back. I think this is good to go. I will merge tomorrowish if there is no more feedback.

One thing to note is that now that we are using the JWK verifier for Ear tokens, we can set insecure_key to true. This wasn't possible with EAR until the changes to support the JWT. I would prefer to have insecure_key always be set to false, but for this PR I now have it matching what we do for the simple tokens since they use the same implementation. Setting up the cert chain is really orthogonal to using EAR, so let's keep tracking that in #598.

Don't worry the Makefile test does check that the full cert chain verification works with Ear.

[fitzthum]

Time to merge. I will keep a close eye on the CI and make sure I didn't leave any configs behind. Tomorrow I will post some issues with next steps for improving the policy and making tests that actually check the TCB.