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.

Sign up for GitHub

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

Jump to bottom

Bring KBS Resource URI into Image-rs #119

Merged
merged 8 commits into from
Mar 10, 2023
Merged

Bring KBS Resource URI into Image-rs #119

merged 8 commits into from
Mar 10, 2023

Conversation

Xynnn007
Copy link
Member

@Xynnn007 Xynnn007 commented Feb 26, 2023 •
edited
Loading

This PR mainly aims to bring KBS Resource URI scheme into image-rs. Concrete contents of this PR includes

  • Let the following files be configurable by a uri in ImageClient.config.file_paths before doing pulling. Related codes were refactored
    • sigstore_config: Signature storage configuration file for simple signing, which describes where an image's signature is stored
    • policy_path: image security policy JSON file, s.t. policy.json
    • auth_file: credential file to access a private image registry, s.t. auth.json
  • Refactored and extended secure_channel module into resource module which gives an public api get_resource(uri). Here uri supports two schemes for now and can be extended appropriately
    • kbs://...: retrieve the resource from KBS.
    • file://...: read the resource from local file system. By default if no scheme is given, try to read using file://
  • Changed the trait for signature scheme. Deleted the method resource_manifest. For now, when a resource is needed, function crate::resource::get_resource(uri) can be called to get lazily. The code logic was simplified.
  • Some promotes for simple signing scheme. Refactoring some logic.
  • Fixed related unit tests to fit in with KBS URI scheme
  • Fixed integration tests to fit in with KBS URI scheme. Introduced a new test image for image decryption docker.io/xynnn007/busybox:encrypted-uri-key which is made using cc-kbc's key provider as it shares the same format of AnnotationPacket and KBS URI.

warning please do not merge this pr before confidential-containers/attestation-agent#129 is approved and merged. Because this pr includes personal repo of AA for integration test.

Close #116

@Xynnn007
Copy link
Member Author

enclave-cc test error is due to different attestation-agent version in ocicrypt-rs and image-rs. When confidential-containers/attestation-agent#129 is merged, AA's rev should be updated in ocicrypt-rs and then in this PR.

@Xynnn007
Copy link
Member Author

Xynnn007 commented Mar 3, 2023 •
edited
Loading

Hi @stevenhorsman @arronwy

This PR introduces three more parameters in ImageClient to specify the uri of auth file, image security policy file and sigstore configuration for simplesigning (if simple signing is to be used)

I think these parameters should be input from kata-agent as who is the caller of image-rs.

Still, a questions comes. For test we can fix these fields like kbs://example.com/... for now.

Potentially, we need to think about how we provide these uris to kata-agent as they should be specified if default value for test is not used. A naive thinking is still from commandline, which requires us to update kata-agent's related code and AS can verify whether the policy.json's uri is as expected

jialez0
jialez0 approved these changes Mar 7, 2023
View reviewed changes
Copy link
Member

@jialez0 jialez0 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@Xynnn007 Thanks a lot for this important work! Although we use the method of overwriting the resource URI to use aa_kbc_params correctly, this is a suitable method at present.

jialez0
jialez0 reviewed Mar 7, 2023
View reviewed changes
src/signature/mod.rs Show resolved Hide resolved
src/auth/mod.rs Show resolved Hide resolved
src/config.rs Show resolved Hide resolved
@arronwy
Copy link
Member

arronwy commented Mar 7, 2023

Hi @stevenhorsman @arronwy

This PR introduces three more parameters in ImageClient to specify the uri of auth file, image security policy file and sigstore configuration for simplesigning (if simple signing is to be used)

I think these parameters should be input from kata-agent as who is the caller of image-rs.

Still, a questions comes. For test we can fix these fields like kbs://example.com/... for now.

Potentially, we need to think about how we provide these uris to kata-agent as they should be specified if default value for test is not used. A naive thinking is still from commandline, which requires us to update kata-agent's related code and AS can verify whether the policy.json's uri is as expected

Thanks @Xynnn007 , to support the flexiblity for different senarios and easy to use for end user, we can expose these interfaces with default value which can backword compatibale previous release, and let the end user to config them if needed.

@Xynnn007
refactor: use ref instead of clone
a95c2b6 
This commit refactors some of the functions of
sample signing module. Mainly convert clones
into references, which helps to avoid extra
memory copies when executing

Signed-off-by: Xynnn007 <[email protected]>
@Xynnn007
Copy link
Member Author

@arronwy Yes, now the three parameters are set to

  • kbs:///default/credential/test
  • kbs:///default/sigstore-config/test
  • kbs:///default/security-policy/test

These are defined in attestation-agent https://github.com/confidential-containers/attestation-agent/blob/main/src/kbc_modules/mod.rs#L152

@Xynnn007
feat: add support for KBS Resource URI
0ffb15c 
This commit brings in KBS Resource URI support.
All the functions related to read a file from
given uri has been brought to module `resource`.

Now different resources can be fetched using the
overall api resource::get_resource()

This Api now support two basic protocol:
- file://  read file from local filesystem
- kbs:// read file from KBS

This commit also fixes features of `keywrap-grpc`,
`keywrap-ttrpc` and `keywrap-native` to let them
to be allowed to exist at the same time.

Signed-off-by: Xynnn007 <[email protected]>
@Xynnn007
Signature: apply KBS URI on signature module
bf963d2 
This commit bring out the sigstore configuration file
into ImageClient's config to specify which sigstore
configuration file for simple signing will be used.

Also, some refactoring is applied on the simple signing
related codes, including:
- Now when merging two sigstore configs, if the contents
are duplicated but not conflict, no error will be raised

Besides, `KeyPath` field in both cosign and simple signing
will use either kbs resource uri or local filesystem path
from now on. If a local fs path is used, a absolute path
is recommended

Signed-off-by: Xynnn007 <[email protected]>
@Xynnn007
test: integration test update for resource uri
a59d9e4 
Credential in integration test now is specified
by a kbs resource uri (kbs resource uri in
`get_resource` api test)

Image decryption key in integration is specified
by a kbs resource uri in image's AnnotationPacket
(kbs resource uri in `unwrap_key` api test)

Signed-off-by: Xynnn007 <[email protected]>
@Xynnn007
docs: update docs for integration test
6157cae 
Signed-off-by: Xynnn007 <[email protected]>
@Xynnn007
lint: fix clippy errors
d579c93 
Signed-off-by: Xynnn007 <[email protected]>
@Xynnn007
fix: wrong init order for ImageConfig test
d451f17 
The env should be removed first and then the config be
inited, or when the env CC_IMAGE_WORK_DIR is set, the first
assert_eq will fail

Signed-off-by: Xynnn007 <[email protected]>
@Xynnn007
dep: update aa and ocicrypt-rs rev
2597704 
related rev brings kbs resource uri scheme

Signed-off-by: Xynnn007 <[email protected]>
@Xynnn007 Xynnn007 merged commit fa6d712 into confidential-containers:main Mar 10, 2023
@Xynnn007 Xynnn007 deleted the feat-uri branch March 10, 2023 23:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jialez0 jialez0 jialez0 approved these changes

@arronwy arronwy arronwy approved these changes

@sameo sameo Awaiting requested review from sameo

@jiangliu jiangliu Awaiting requested review from jiangliu

@lumjjb lumjjb Awaiting requested review from lumjjb

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Apply on the mechinism of KBS Resource URI
3 participants
@Xynnn007 @arronwy @jialez0