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

cosign verify-dockerfile is dangerous #648

Closed
mattmoor opened this issue Sep 11, 2021 · 13 comments
Closed

cosign verify-dockerfile is dangerous #648

mattmoor opened this issue Sep 11, 2021 · 13 comments
Labels
no-issue-activity

Comments

@mattmoor
Copy link
Member

Description

Today cosign verify-dockerfile is dangerous because it verifies and allows FROM image:tag vs. FROM image@sha256:deadbeef. This is dangerous because even if what's currently tagged on the registry is signed properly, there is a race before the FROM is evaluated (what if it changes!), or (with docker build) it's possible that what is in the local cache(!) is what's actually used, and not what was verified! 😬

I would propose the following two changes to eliminate this danger:

  1. Change cosign verify-dockerfile to reject Dockerfiles that don't use FROM <digest>
  2. Introduce cosign resolve-dockerfile that allows FROM <tag>, but rewrites the Dockerfile to FROM <verified digest>.

cc @dlorenc
@mattmoor
Copy link
Member Author

cc @mlieberman85 who I was telling about this race the other day.

@dlorenc
Copy link
Member

dlorenc commented Sep 11, 2021

+1, cc @dekkagaijin I thought we already had a bug to do the in place resolution actually

@developer-guy
Copy link
Member

we (w/@erkanzileli w/@Dentrax) would love to do that issue if you let us do this @mattmoor @dlorenc

@dekkagaijin
Copy link
Member

The behavior is consistent with plain-ol verify, at least.

It's becoming more and more apparent that we shouldn't have added image discovery functionality to cosign itself. I'm uncomfortable with the idea of adding & supporting this kind of relatively advanced functionality rather than creating built-for-purpose tooling that can be composed with cosign to achieve the same effect.

See also:
#491 (comment)
#527 (comment)

@dekkagaijin
Copy link
Member

Here's a first step to begin breaking out image discovery from signature creation/verification: #662

@dekkagaijin dekkagaijin mentioned this issue Sep 15, 2021
Closed
@mattmoor mattmoor mentioned this issue Sep 16, 2021
Open
@dekkagaijin dekkagaijin mentioned this issue Sep 17, 2021
Closed
@mattmoor mattmoor mentioned this issue Sep 24, 2021
Closed
@Dentrax Dentrax mentioned this issue Nov 30, 2021
Open
@AkihiroSuda AkihiroSuda mentioned this issue Nov 30, 2021
Merged
Dentrax added a commit to Dentrax/cosign that referenced this issue Dec 1, 2021
@Dentrax @developer-guy
feat(dockerfile): add resolve command
a471954 
Fixes sigstore#648
Fixes sigstore#707

Signed-off-by: Furkan <furkan.turkal@trendyol.com>
Co-authored-by: Batuhan <batuhan.apaydin@trendyol.com>
@Dentrax Dentrax mentioned this issue Dec 1, 2021
Closed
Dentrax added a commit to Dentrax/cosign that referenced this issue Feb 15, 2022
@Dentrax @developer-guy
feat(dockerfile): add resolve command
c5398b2 
Fixes sigstore#648
Fixes sigstore#707

Signed-off-by: Furkan <furkan.turkal@trendyol.com>
Co-authored-by: Batuhan <batuhan.apaydin@trendyol.com>
Dentrax added a commit to Dentrax/cosign that referenced this issue May 25, 2022
@Dentrax @developer-guy
feat(dockerfile): add resolve command
48cdd17 
Fixes sigstore#648
Fixes sigstore#707

Signed-off-by: Furkan <furkan.turkal@trendyol.com>
Co-authored-by: Batuhan <batuhan.apaydin@trendyol.com>
Dentrax added a commit to Dentrax/cosign that referenced this issue Jun 18, 2022
@Dentrax @developer-guy
feat(dockerfile): add resolve command
4e38dab 
Fixes sigstore#648
Fixes sigstore#707

Signed-off-by: Furkan <furkan.turkal@trendyol.com>
Co-authored-by: Batuhan <batuhan.apaydin@trendyol.com>
@github-actions
Copy link

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 5 days.

Dentrax added a commit to Dentrax/cosign that referenced this issue Sep 14, 2022
@Dentrax @developer-guy
feat(dockerfile): add resolve command
499c147 
Fixes sigstore#648
Fixes sigstore#707

Signed-off-by: Furkan <furkan.turkal@trendyol.com>
Co-authored-by: Batuhan <batuhan.apaydin@trendyol.com>
Dentrax added a commit to Dentrax/cosign that referenced this issue Oct 5, 2022
@Dentrax @developer-guy
feat(dockerfile): add resolve command
218d828 
Fixes sigstore#648
Fixes sigstore#707

Signed-off-by: Furkan <furkan.turkal@trendyol.com>
Co-authored-by: Batuhan <batuhan.apaydin@trendyol.com>
@github-actions
Copy link

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions
Copy link

This issue was closed because it has been stalled for 5 days with no activity.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 20, 2022
Dentrax added a commit to Dentrax/cosign that referenced this issue Jan 3, 2023
@Dentrax @developer-guy
feat(dockerfile): add resolve command
18bdbd2 
Fixes sigstore#648
Fixes sigstore#707

Signed-off-by: Furkan <furkan.turkal@trendyol.com>
Co-authored-by: Batuhan <batuhan.apaydin@trendyol.com>
@znewman01 znewman01 reopened this Feb 13, 2023
@github-actions
Copy link

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions
Copy link

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions
Copy link

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions
Copy link

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions
Copy link

This issue was closed because it has been stalled for 5 days with no activity.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Oct 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
no-issue-activity
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

cmd: dockerfile resolve Dentrax/cosign
5 participants
@znewman01 @dekkagaijin @dlorenc @mattmoor @developer-guy