+
Skip to content

Add signature verification for image volumes #9060

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

Merged

Conversation

xw19
Copy link
Member

@xw19 xw19 commented Mar 13, 2025

What type of PR is this?

/kind feature

What this PR does / why we need it:

Adds image volume verification just before mounting

Which issue(s) this PR fixes:

Fixes #9001

Special notes for your reviewer:

Does this PR introduce a user-facing change?

None

Signature verification for image volumes

@xw19 xw19 requested a review from mrunalp as a code owner March 13, 2025 05:06
@openshift-ci openshift-ci bot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Mar 13, 2025
Copy link
Contributor

openshift-ci bot commented Mar 13, 2025

@xw19: The label(s) kind/featurer cannot be applied, because the repository doesn't have them.

In response to this:

What type of PR is this?

/kind featurer

What this PR does / why we need it:

Adds image volume verification just before mounting

Which issue(s) this PR fixes:

Fixes #9001

Special notes for your reviewer:

Does this PR introduce a user-facing change?

None

Signature verification for image volumes

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@openshift-ci openshift-ci bot added the dco-signoff: yes Indicates the PR's author has DCO signed all their commits. label Mar 13, 2025
@openshift-ci openshift-ci bot requested review from klihub and littlejawa March 13, 2025 05:07
Copy link

codecov bot commented Mar 13, 2025

Codecov Report

Attention: Patch coverage is 12.12121% with 29 lines in your changes missing coverage. Please review.

Project coverage is 47.30%. Comparing base (59ba9be) to head (d94a8f3).
Report is 8 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #9060   +/-   ##
=======================================
  Coverage   47.30%   47.30%           
=======================================
  Files         162      162           
  Lines       23956    23962    +6     
=======================================
+ Hits        11332    11335    +3     
- Misses      11513    11515    +2     
- Partials     1111     1112    +1     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@xw19 xw19 changed the title Add signature verification for image volumes WIP: Add signature verification for image volumes Mar 13, 2025
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 13, 2025
@xw19
Copy link
Member Author

xw19 commented Mar 13, 2025

@saschagrunert can you provide some feedback?

@xw19 xw19 force-pushed the signature-verification-image-volumes branch from b58f930 to 7d05281 Compare March 15, 2025 10:54
@xw19
Copy link
Member Author

xw19 commented Mar 15, 2025

/kind feature

@openshift-ci openshift-ci bot added the kind/feature Categorizes issue or PR as related to a new feature. label Mar 15, 2025
@xw19
Copy link
Member Author

xw19 commented Mar 16, 2025

@cri-o/cri-o-maintainers In the implementation we need to have userSpecifiedImage mandatorily for the image to be verified else it will cause an error will this be an issue? Another optimization we could do maybe keep a list of verified images so that we don't need to check every time. This implementation only is for linux do we also need to do for BSD?

@xw19 xw19 changed the title WIP: Add signature verification for image volumes Add signature verification for image volumes Mar 18, 2025
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 18, 2025
@xw19
Copy link
Member Author

xw19 commented Mar 18, 2025

/retest

@saschagrunert
Copy link
Member

@xw19 please rebase again.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 23, 2025
@bitoku
Copy link
Contributor

bitoku commented Apr 17, 2025

Hi @xw19 ! Do you want to continue to work on this? Or do you want us to continue instead?

@xw19
Copy link
Member Author

xw19 commented Apr 17, 2025

I will do this by next week

@haircommander haircommander added this to the 1.33 milestone Apr 17, 2025
@xw19 xw19 force-pushed the signature-verification-image-volumes branch from e9d39a4 to c36b7f5 Compare April 22, 2025 16:52
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 22, 2025
@xw19 xw19 force-pushed the signature-verification-image-volumes branch 2 times, most recently from 23baab3 to 4ab6d55 Compare April 22, 2025 17:33
@xw19
Copy link
Member Author

xw19 commented Apr 23, 2025

/retest

@xw19
Copy link
Member Author

xw19 commented Apr 23, 2025

@saschagrunert Please take a look

test/policy.bats Outdated
POD_ID=$(crictl runp "$TESTDATA/sandbox_config.json")
CTR_CONFIG="$TESTDIR/config.json"
jq --arg CONTAINER_PATH "$CONTAINER_PATH" \
'.image.image = "'"$SIGNED_IMAGE"'" | .image.user_specified_image = "'"$SIGNED_IMAGE"'" | .mounts = [{
Copy link
Member

Choose a reason for hiding this comment

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

Let's do not mix test cases here. Can we use a standard image which will not be verified against any signature and focus on the mount part?

Copy link
Member Author

Choose a reason for hiding this comment

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

I am not sure how to handle this scenario.

test/policy.bats Outdated
POD_ID=$(crictl runp "$TESTDATA/sandbox_config.json")
CTR_CONFIG="$TESTDIR/config.json"
jq --arg CONTAINER_PATH "$CONTAINER_PATH" \
'.image.image = "'"$SIGNED_IMAGE"'" | .image.user_specified_image = "'"$SIGNED_IMAGE"'" | .mounts = [{
Copy link
Member

Choose a reason for hiding this comment

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

Same here, let's ensure that the signature verification of the mount part fails.

@saschagrunert saschagrunert force-pushed the signature-verification-image-volumes branch from 6f29daa to cba41a6 Compare May 13, 2025 08:38
@bitoku bitoku force-pushed the signature-verification-image-volumes branch 4 times, most recently from a5657c4 to b2510be Compare May 13, 2025 11:05
@bitoku bitoku self-assigned this May 13, 2025
@bitoku bitoku force-pushed the signature-verification-image-volumes branch from b2510be to e94ecb8 Compare May 13, 2025 12:02
@bitoku bitoku closed this May 13, 2025
@bitoku bitoku reopened this May 13, 2025
@bitoku
Copy link
Contributor

bitoku commented May 13, 2025

/retest

@haircommander
Copy link
Member

/test ci-crun-e2e

Signed-off-by: Sourav Moitra <sourav.moitr@gmail.com>
Signed-off-by: Ayato Tokubi <atokubi@redhat.com>
@bitoku bitoku force-pushed the signature-verification-image-volumes branch from e94ecb8 to d94a8f3 Compare May 13, 2025 14:50
@bitoku
Copy link
Contributor

bitoku commented May 13, 2025

@cri-o/cri-o-maintainers Can you PTAL?

@haircommander
Copy link
Member

/override ci/prow/ci-e2e-evented-pleg
/approve

LGTM, thanks both @xw19 and @bitoku !

Copy link
Contributor

openshift-ci bot commented May 13, 2025

@haircommander: Overrode contexts on behalf of haircommander: ci/prow/ci-e2e-evented-pleg

In response to this:

/override ci/prow/ci-e2e-evented-pleg
/approve

LGTM, thanks both @xw19 and @bitoku !

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 13, 2025
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 14, 2025
Copy link
Contributor

openshift-ci bot commented May 14, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: haircommander, saschagrunert, xw19

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [haircommander,saschagrunert]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@saschagrunert
Copy link
Member

/retest

1 similar comment
@bitoku
Copy link
Contributor

bitoku commented May 14, 2025

/retest

@bitoku
Copy link
Contributor

bitoku commented May 14, 2025

@cri-o/cri-o-maintainers The test failures seem like an infra issue, which happened a few days ago. Does anyone know how we fixed the issue?

https://prow.ci.openshift.org/job-history/gs/test-platform-results/pr-logs/directory/pull-ci-cri-o-cri-o-main-ci-rhel-e2e?buildId=1920929273212833792

@haircommander
Copy link
Member

/retest

done, the images we're building in gcloud for some reason don't have any packages installing, maybe having to do with the lifecycle of rhel 9.4. we may need to switch to 9.6. I deleted the offending image but a new one will currently be created every week until we fix

@bitoku
Copy link
Contributor

bitoku commented May 14, 2025

/retest

@haircommander
Copy link
Member

/override ci/prow/ci-e2e
/override ci/prow/ci-cgroupv2-e2e-features

Copy link
Contributor

openshift-ci bot commented May 14, 2025

@haircommander: Overrode contexts on behalf of haircommander: ci/prow/ci-cgroupv2-e2e-features, ci/prow/ci-e2e

In response to this:

/override ci/prow/ci-e2e
/override ci/prow/ci-cgroupv2-e2e-features

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@openshift-merge-bot openshift-merge-bot bot merged commit edadf0e into cri-o:main May 14, 2025
73 of 76 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Signature verification for image volumes
5 participants
点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载