Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sample: always enable sample attester #426

Merged

Conversation

fitzthum
Copy link
Member

@fitzthum fitzthum commented Jan 2, 2024

Rather than setting an environment variable to enable the sample attester, always enable it as a fallback.

See confidential-containers/confidential-containers#184 for reasons

I don't think it makes much sense to enable the sample attester based on an environment variable. Having it always available as a fallback shouldn't cause any security problems. The KBS/AS shouldn't be fooled by the sample evidence in a real deployment.

@fitzthum fitzthum requested a review from jialez0 as a code owner January 2, 2024 21:32
@fitzthum fitzthum force-pushed the always-detect-sample branch from d4c1abd to 7f45726 Compare January 2, 2024 21:45
Copy link
Member

@Xynnn007 Xynnn007 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@@ -78,10 +78,6 @@ pub trait Attester {

// Detect which TEE platform the KBC running environment is.
pub fn detect_tee_type() -> Option<Tee> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should also change the result to Tee rather than Option as at least sample will be returned.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got rid of the option. I kept the interface the same at the level of the EvidenceProvider, but adjusted things up to that.

@fitzthum fitzthum force-pushed the always-detect-sample branch 4 times, most recently from ae6eb91 to a05a908 Compare January 4, 2024 16:00
Rather than setting an environment variable to enable
the sample attester, always enable it as a fallback.

Signed-off-by: Tobin Feldman-Fitzthum <[email protected]>
@fitzthum fitzthum force-pushed the always-detect-sample branch from a05a908 to 006e1ff Compare January 4, 2024 16:13
@Xynnn007
Copy link
Member

Xynnn007 commented Jan 4, 2024

We might also think about in real TEE, if no legal platform is detected, the attester will downgrade to sample attester and do rcar.

At AS side, it might directly use sample verifier without a explicit approval of the user. The user should define a rule in policy on AS side to ban sample verifier claims.

Or, a non-tee might be leveraged by an attacker to do a successful attestation.

with environment variable, we can ensure that the logic of not setting env by checking the measurement of the guest image.

@fitzthum
Copy link
Member Author

fitzthum commented Jan 4, 2024

At AS side, it might directly use sample verifier without a explicit approval of the user. The user should define a rule in policy on AS side to ban sample verifier claims.

Yes, I think we should setup the AS/KBS so that the default policy rejects the sample attester. Then for tests we can manually enable it.

@fitzthum fitzthum merged commit 7ddecc7 into confidential-containers:main Jan 5, 2024
9 checks passed
@fitzthum fitzthum mentioned this pull request Jan 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants