Add cosign verify-manifest command
#490
Conversation
Signed-off-by: Joshua Hansen <[email protected]>
developer-guy
force-pushed
the
feature/verify-manifest
branch
from
22c4fe0
to
36c8e1e
Compare
Signed-off-by: Batuhan Apaydın <[email protected]>
cmd/cosign/cli/verify_manifest.go
Outdated
| LongHelp: `Verify all signature of images in a Kubernetes resource manifest by checking claims | ||
| against the transparency log. | ||
|
|
||
| Shell-like variables in the Dockerfile's FROM lines will be substituted with values from the OS ENV. |
There was a problem hiding this comment.
I believe this is coming from the verify-dockerfile command description.
There was a problem hiding this comment.
Updated the help docs to reflect.
|
|
||
| func getImagesFromYamlManifest(manifest string) ([]string, error) { | ||
| var images []string | ||
| re := regexp.MustCompile(`image:\s?(?P<Image>.*)\s?`) |
There was a problem hiding this comment.
suggestion: Perhaps it could be more robust to decode the manifest and validate that the extensions are among those identified in the issue (Pod, DaemonSet...), then use the decoded object to get the image value for all the containers (e.g. obj.Spec.Containers[*].Image).
Otherwise you could have a some resources that might not be among the identified list, e.g. CRs.
There was a problem hiding this comment.
Let me know if you want me to help you with this.
There was a problem hiding this comment.
@hectorj2f yes, this was a consideration as mentioned in #437 (comment) - I think it would be better, but as different Kinds come out, it binds us more to the K8s API for parsing unless we still just walk down the tree looking for "image" keys (but guaranteed valid YAML) which would address your other comment.
This regex also doesn't account for quoted string variants, so either way this naive approach needs to change in some fashion.
There was a problem hiding this comment.
Agreed. There's prior art for ducktyping PodSpec-like Objects, but I don't think it's worth blocking merge for a "perfect solution
There was a problem hiding this comment.
There's prior art for ducktyping PodSpec-like Objects.
Yes, you could use runtime.Raw and decode the object to the list of supported types here.
Sure, the current implementation should not be blocked by this. Once this PR is merged, I could open a PR with my proposed changes, so you can have a look at them.
|
|
||
| manifestPath := args[0] | ||
|
|
||
| err := isExtensionAllowed(manifestPath) |
There was a problem hiding this comment.
suggestion: try to decode the yaml file to verify it is a valid.
There was a problem hiding this comment.
(YAML or JSON)
Eventually we should probably just use a canonical k8s parser
|
|
||
| func getImagesFromYamlManifest(manifest string) ([]string, error) { | ||
| var images []string | ||
| re := regexp.MustCompile(`image:\s?(?P<Image>.*)\s?`) |
There was a problem hiding this comment.
Agreed. There's prior art for ducktyping PodSpec-like Objects, but I don't think it's worth blocking merge for a "perfect solution
|
|
||
| manifestPath := args[0] | ||
|
|
||
| err := isExtensionAllowed(manifestPath) |
There was a problem hiding this comment.
(YAML or JSON)
Eventually we should probably just use a canonical k8s parser
|
@joshes You need to sign off your commits. |
Signed-off-by: Joshua Hansen <[email protected]>
Signed-off-by: Joshua Hansen <[email protected]>
joshes
force-pushed
the
feature/verify-manifest
branch
from
84b0849
to
cc0131b
Compare
Signed and pushed, thanks. |
|
Amazing work! Let's get this in and follow up :) |
Adds support for verifying kubernetes resources (manifests) and addresses #437
It's a naive approach, but wanted to get feedback early on, especially around YAML parsing via regex and JSON support as discussed in the ticket.