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

Merge Attestation-Service and KBS #173

Merged
merged 165 commits into from
Nov 10, 2023

Conversation

Xynnn007
Copy link
Member

@Xynnn007 Xynnn007 commented Nov 3, 2023

@Xynnn007 Xynnn007 force-pushed the merge-as branch 4 times, most recently from e7fdc4d to a992d26 Compare November 3, 2023 03:44
@Xynnn007
Copy link
Member Author

Xynnn007 commented Nov 3, 2023

Let's wait for the PR confidential-containers/guest-components#388 to be merged and then continue with this PR.

@Xynnn007
Copy link
Member Author

Xynnn007 commented Nov 3, 2023

Things have been fixed:

  • CODEOWNERS/LICENCE
  • import relationship

We still need the following things:

@Xynnn007 Xynnn007 marked this pull request as ready for review November 3, 2023 05:51
@Xynnn007 Xynnn007 changed the title Merge as Merge Attestation-Service and KBS Nov 3, 2023
@Xynnn007 Xynnn007 marked this pull request as draft November 3, 2023 09:08
@Xynnn007 Xynnn007 force-pushed the merge-as branch 2 times, most recently from 1862cb9 to e20a0ad Compare November 3, 2023 09:31
@mythi
Copy link
Contributor

mythi commented Nov 3, 2023

follow confidential-containers/confidential-containers#169

I've added my comments to the issue.

@Xynnn007 Xynnn007 force-pushed the merge-as branch 2 times, most recently from 5d4a9ef to 232df61 Compare November 6, 2023 07:04
@Xynnn007 Xynnn007 marked this pull request as ready for review November 9, 2023 08:27
@fitzthum
Copy link
Member

My vote for the name would be to simply stick with kbs for now. If we come up with a good name that covers all of the components we can change it later. So far I don't think we have a great alternative.

Copy link
Member

@fitzthum fitzthum left a comment

Choose a reason for hiding this comment

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

LGTM

Xynnn007 and others added 12 commits November 10, 2023 09:38
1.A Crate that implements the Attestation Service core functionality..

Fixes: #1
Signed-off-by: zhouliang121 <[email protected]>
1.Enable the basic compile check for AS.
2.Fixed warning errors that were detected by the "cargo fmt --all --
  --check" and "cargo clippy -- -D warnings"

Fixes: confidential-containers#5
Signed-off-by: zhouliang121 <[email protected]>
1.Implemented the Attestation Service's server application.

Fixes: confidential-containers#3
Signed-off-by: zhouliang121 <[email protected]>
The policy engine used in the Attestation Service is the powerful
and flexible Open Policy Agent(OPA), it can provide a flexible,
fine-grained control for the owner of Attestation Service.

This commit optimizes the integration logic of OPA and Attestation
Service, and turns the policy engine into a general functional
component on the upper layer of the verifier driver, which is used
to verify whether the TCB status of various HW-TEE meets the
owner's expectations.

Signed-off-by: Jiale Zhang <[email protected]>
Disable the check for clippy::derive_partial_eq_without_eq, which is triggered
by generated code
The maintainer of tonic recommends this:
tokio-rs/prost#661 (comment)

Signed-off-by: stevenhorsman <[email protected]>
Basic architecture of the RVPS, including

- Pre-Processor
- Extractors
- Cache

Also define the following formats:

- Provenance Message
- Reference Value (Stored inside the RVPS)
- Trusted Digests (Proveded for AS)

Signed-off-by: Xynnn007 <[email protected]>
chendave and others added 24 commits November 10, 2023 09:38
CCA verifier is actually done by forwarding the evidence to Verasion[1].

[1] https://github.com/veraison/services

Signed-off-by: Dave Chen <[email protected]>
Bumps [actions/checkout](https://github.com/actions/checkout) from 3 to 4.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v3...v4)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [github.com/open-policy-agent/opa](https://github.com/open-policy-agent/opa) from 0.55.0 to 0.56.0.
- [Release notes](https://github.com/open-policy-agent/opa/releases)
- [Changelog](https://github.com/open-policy-agent/opa/blob/main/CHANGELOG.md)
- [Commits](open-policy-agent/opa@v0.55.0...v0.56.0)

---
updated-dependencies:
- dependency-name: github.com/open-policy-agent/opa
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
The new version of csv-rs will support openssl 3.1.x

Signed-off-by: fangbaoshun <[email protected]>
Now we use ubuntu 20.04 as base image, the openssl version is 1.1.1
where the location of `libssl.so` is different from version 3.0+
`csv-rs` crate now can only find `libssl.so` with version 3.0+

Signed-off-by: Jiale Zhang <[email protected]>
According to the RMM spec, A7.2.3 Attestation token format [1], the CCA token
has a client challenge sealed in the token, the challenge is generated
when the CCA token is got from AA (attestation agent).

When the CCA token is validated by the veraison, a session nonce is created as well,
semantically, the session nonce should be equal with the client challenge, this logic
has been validated inside veraison service [2].

While the freshness also implies the session nonce key should equal with the nonce
enveloped in the ear which is the format of attestation result [3], by this, we have:
```
client challenge in token == session nonce == ear's nonce
```
and thus avoid the replay attack.

NOTE: If the "client challenge in token != session nonce", vts service from Veraison
takes it as a warning, this is debatable and should be further discuss with Veraison
team, `rust-client` should return an error instead.

fix: https://github.com/confidential-containers/attestation-service/issues/127

> WARN    vts     {"detail":["freshness: realm challenge (00000000000...) does not match session nonce
> ... "detail-type":"error","error":"bad evidence"}

Signed-off-by: Dave Chen <[email protected]>

[1] https://documentation-service.arm.com/static/63a16f163f28e5456434c719?token=
[2] https://github.com/veraison/services/blob/dfb068204473cad9c412337d5abef7ad88b8bc3b/scheme/cca-ssd-platform/evidence_handler.go#L120-L126
[3] https://github.com/veraison/docs/blob/main/architecture/verifier/freshness.md
DateTime::<Tz>::from_utc is deprecated now.

Signed-off-by: Dave Chen <[email protected]>
Signed-off-by: Xynnn007 <[email protected]>
Signed-off-by: Xynnn007 <[email protected]>
Signed-off-by: Xynnn007 <[email protected]>
Signed-off-by: Xynnn007 <[email protected]>
Signed-off-by: Xynnn007 <[email protected]>
8dd91e0 version has fixed the bug of csv

Signed-off-by: Xynnn007 <[email protected]>
This commit disabled the checking of docs. Because for now we do not
have enough inline docs so that the lint checker will raise a lot of
error information.

Also, make the coverage of Makefile of kbs only kbs repo.

Signed-off-by: Xynnn007 <[email protected]>
@Xynnn007 Xynnn007 merged commit 5a832c9 into confidential-containers:main Nov 10, 2023
8 of 9 checks passed
@Xynnn007 Xynnn007 deleted the merge-as branch November 10, 2023 02:26
@mythi
Copy link
Contributor

mythi commented Nov 10, 2023

LGTM

@fitzthum did you follow to the conversation in the issue? There were objections to this change and now it's merged without a concensus.

@Xynnn007
Copy link
Member Author

LGTM

@fitzthum did you follow to the conversation in the issue? There were objections to this change and now it's merged without a concensus.

@mythi

I must have done something bad that click the rebase button before reaching a concensus with you. We can revert the merging if needed.

Also, a better way is that let's keep talking about that in the issue or in slack (I recommend) to reach a consensus.

1 similar comment
@Xynnn007
Copy link
Member Author

LGTM

@fitzthum did you follow to the conversation in the issue? There were objections to this change and now it's merged without a concensus.

@mythi

I must have done something bad that click the rebase button before reaching a concensus with you. We can revert the merging if needed.

Also, a better way is that let's keep talking about that in the issue or in slack (I recommend) to reach a consensus.

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.