Image signature verification workflow test #14124

mfojtik · 2017-05-10T09:29:23Z

@miminar @soltysh as promised

This will basically:

Take the origin:latest image (which should be the local image we built using make release
Add skopeo into it and generate dummy GPG keys and commit it as "signer" image
Use that image for a "sign&push" job (I know Maciej, I should use Job this ;-)
Once the signed:latest is present, check the signature using describe
Use the "signer" image once again to run oc adm verify-image-signature and verify the identity of the image
Check the image is now "verified"

I'm pretty hesitating to add this into "comformance" as the test takes ~1minute to run, thoughts?

mfojtik · 2017-05-10T09:33:15Z

[test][testextended][extended:core(image signature workflow)]

mfojtik · 2017-05-10T11:35:43Z

test/extended/registry/signature.go

+
+	g.It("can push a signed image to openshift registry and verify it", func() {
+		g.By("building an signer image that know how to sign images")
+		_, err := oc.Run("new-build").Args("--docker-image", "openshift/origin:latest", "--to", "signer:latest", "-D", "-").InputString(`FROM openshift/origin:latest


@bparees @smarterclayton is there a way to create a build config without creating an image stream?
The intent of this 'rebuild' is to add skopeo and generate gpg keys on top of the current openshift/origin:latest image that is already present on the node (we build it using make release...).

I think the new-build here will create image stream and import the openshift/origin:latest from the dockerhub which is not what I want ;-)

also I thought the --docker-image will do what I want, but it still creates the image stream for the builder...

you can do it, but not via new-build/new-app. you'd have to define the build yourself using a dockerimage reference.

We should have oc create buildconfig :D

I've created #14138.

i will create the fixture for now :-)

miminar

Can this include some negative scenarios? To name a few: verifying with a different key, verifying with different expected-identity, verifying with no key, tagging another image over the verified istag (which is present in expected-identity) and verifying again, serving a wrong manifest to the verifier, …

miminar · 2017-05-10T11:57:58Z

test/extended/registry/signature.go

+	e2e "k8s.io/kubernetes/test/e2e/framework"
+)
+
+var _ = g.Describe("image signature workflow", func() {


Add '[imageapis].

miminar · 2017-05-10T12:03:08Z

test/extended/registry/signature.go

+ENV GNUPGHOME="/var/lib/origin/gnupg"
+RUN yum install -y skopeo && yum clean all
+RUN chmod 0777 -R /var/lib/origin && mkdir -p $GNUPGHOME && chmod 0600 $GNUPGHOME && \
+rm -f /dev/random; ln -sf /dev/urandom /dev/random && \


Ugh?

Maybe worth a comment.

@miminar containers lack entropy ;-)

miminar · 2017-05-10T12:17:25Z

test/extended/registry/signature.go

+		err = exutil.WaitForAnImageStreamTag(oc, oc.Namespace(), "signed", "latest")
+		o.Expect(err).NotTo(o.HaveOccurred())
+
+		g.By("checking the signature is present on the image and it state is unverified")


s/on/in/ s/it/its/

miminar · 2017-05-10T12:26:37Z

test/extended/registry/signature.go

+		o.Expect(err).NotTo(o.HaveOccurred())
+
+		g.By("signing the origin-pod:latest image and pushing it into signed:latest")
+		// TODO: Note that the skopeo command is expecting the KUBERNETES_MASTER and the


This should either go without TODO: or state what needs to be done.

miminar · 2017-05-10T12:31:57Z

test/extended/registry/signature.go

+		// image-signer and image-auditor roles.
+		signAndPushCmd := fmt.Sprintf(`export KUBERNETES_MASTER=https://$KUBERNETES_SERVICE_HOST:$KUBERNETES_SERVICE_PORT;`+
+			`oc login --token=%s $KUBERNETES_MASTER --certificate-authority=/run/secrets/kubernetes.io/serviceaccount/ca.crt && `+
+			`skopeo --debug --tls-verify=false copy --sign-by joe@foo.bar --dest-creds %s:%s --dest-tls-verify=false docker://docker.io/openshift/origin-pod:latest atomic:%s/%s/signed:latest`,


Why is this signing docker.io/openshift/origin-pod:latest? Can we use some local image? Is this pulling the image or just a manifest?

@miminar this pod does not have access to local docker (!) it uses skopeo transport. i don't want it to have privileged access.

soltysh

Nit, lgtm

soltysh · 2017-05-11T11:31:02Z

test/extended/registry/signature.go

+
+	g.It("can push a signed image to openshift registry and verify it", func() {
+		g.By("building an signer image that know how to sign images")
+		_, err := oc.Run("new-build").Args("--docker-image", "openshift/origin:latest", "--to", "signer:latest", "-D", "-").InputString(`FROM openshift/origin:latest


We should have oc create buildconfig :D

soltysh · 2017-05-11T11:34:38Z

test/extended/registry/signature.go

+		// Note that we need to replace the /dev/random with /dev/urandom to get more entropy
+		// into container so we can successfully generate the GPG keypair.
+		g.By("creating dummy GPG key")
+		out, err := pod.Exec("rm -f /dev/random; ln -sf /dev/urandom /dev/random && " +


@soltysh without this the container will just hang forever as it does not have enough entropy... the same hack is used in the skopeo integration testing...

I'm not saying you can't do this, I'm just expressing my level of sadness with this hack, sigh...

soltysh · 2017-05-11T11:38:34Z

test/extended/util/framework.go

+	waitErr := wait.PollImmediate(1*time.Second, 3*time.Minute, func() (bool, error) {
+		var err error
+		out, err = r.client.Run("exec").Args(r.podName, "--", "/bin/bash", "-c", script).Output()
+		framework.Logf("output:\n%s\n", out)


Drop this line, not needed here. If you want to log it, make sure the caller does that for you, since you're returning that output.

sure. (this was for debugging the test only)

miminar

Few more nits.

miminar · 2017-05-11T11:54:46Z

test/extended/registry/signature.go

+		o.Expect(err).NotTo(o.HaveOccurred())
+		o.Expect(out).To(o.ContainSubstring("Logged in"))
+
+		// Sign and copy the origin-pod image into targed image stream tag


s/targed/target/

miminar · 2017-05-11T11:58:58Z

test/extended/registry/signature.go

+		// TODO: Fix skopeo to pickup the Kubernetes environment variables (remove the $KUBERNETES_MASTER)
+		g.By("signing the origin-pod:latest image and pushing it into openshift registry")
+		_, err = pod.Exec("KUBERNETES_MASTER=https://$KUBERNETES_SERVICE_HOST:$KUBERNETES_SERVICE_PORT GNUPGHOME=/var/lib/origin/gnupg " +
+			"skopeo --debug --tls-verify=false copy --sign-by joe@foo.bar --dest-creds " + user + ":" + token + " --dest-tls-verify=false docker://docker.io/openshift/origin-pod:latest atomic:" + signedImage)


Now that the pod is privileged, you could sign a local image instead of remote one.

@miminar i have no idea how to plumb the docker socket to skopeo in container... i guess it will make the test pretty complicated...

@miminar also skopeo does not use the docker to transport images, it has custom functions to deal with that.

miminar · 2017-05-11T12:11:08Z

test/extended/util/framework.go

+// Exec executes a single command or a script in the running pod. It returns the command
+// output and error if the command finished with non-zero status code or the command took
+// longer then 3 minutes to run.
+func (r *podExecutor) Exec(script string) (string, error) {


The godoc could mention that the script is a script interpreted by bash.

miminar · 2017-05-11T12:14:13Z

test/extended/registry/signature.go

@@ -0,0 +1,120 @@
+package registry


IMHO this should belong rather to test/extended/imageapis. But I admit it's ambiguous. Tests in the imageapis directories work with registry as well. And the same goes for tests in images and builds directory.

yeah, i don't think either of these packages is a great location for this test, but I would like to have it in registry as the verification fetching manifest from registry and I want to add additional test in (near) future for the signature endpoint in registry (don't want to have the signature test splitted in two packages)

miminar · 2017-05-11T12:14:58Z

test/extended/registry/signature.go

+	e2e "k8s.io/kubernetes/test/e2e/framework"
+)
+
+var _ = g.Describe("[imageapis] image signature workflow", func() {


Since it involves registry, there's no harm adding [registry] as well. Sorry I missed that earlier.

openshift-bot · 2017-05-11T12:21:35Z

continuous-integration/openshift-jenkins/test Waiting: Determining build queue position

mfojtik · 2017-05-11T13:45:46Z

@miminar success \o/

mfojtik · 2017-05-15T11:54:43Z

@miminar @soltysh moved the build into its own fixture (which is btw. cool because we don't have dockerfile test anyway ;-) addressed comments, ptal.

next step will be registry signature endpoint test (part of this test)

openshift-bot · 2017-05-15T12:05:42Z

Evaluated for origin testextended up to 1abda05

openshift-bot · 2017-05-15T12:37:32Z

continuous-integration/openshift-jenkins/testextended SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin_extended/393/) (Base Commit: b581bfe) (Extended Tests: core(image signature workflow))

miminar

LGTM

mfojtik · 2017-05-16T09:45:07Z

[merge]

mfojtik · 2017-05-17T14:16:55Z

[test]

openshift-bot · 2017-05-17T14:21:34Z

Evaluated for origin test up to 1abda05

openshift-bot · 2017-05-17T16:30:27Z

continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/1523/) (Base Commit: 71b89dc)

mfojtik · 2017-05-18T12:45:17Z

flake #14241

[merge]

mfojtik · 2017-05-19T09:04:25Z

flake: #14236 (fixed)

[merge]

openshift-bot · 2017-05-19T09:05:28Z

Evaluated for origin merge up to 1abda05

openshift-bot · 2017-05-19T10:37:38Z

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_origin/704/) (Base Commit: ff5f7c5) (Image: devenv-rhel7_6240)

mfojtik changed the title ~~Image signature verification worflow test~~ Image signature verification workflow test May 10, 2017

mfojtik force-pushed the signature-test branch 2 times, most recently from bbe0ef2 to d09429f Compare May 10, 2017 11:33

mfojtik commented May 10, 2017

View reviewed changes

miminar suggested changes May 10, 2017

View reviewed changes

mfojtik force-pushed the signature-test branch 5 times, most recently from 0c1998c to 1833540 Compare May 11, 2017 10:37

soltysh approved these changes May 11, 2017

View reviewed changes

mfojtik force-pushed the signature-test branch from 1833540 to 81b69c3 Compare May 11, 2017 11:44

miminar suggested changes May 11, 2017

View reviewed changes

Image signature verification worflow test

1abda05

mfojtik force-pushed the signature-test branch from 81b69c3 to 1abda05 Compare May 15, 2017 11:53

miminar approved these changes May 16, 2017

View reviewed changes

openshift-bot merged commit 2099ae7 into openshift:master May 19, 2017

mfojtik deleted the signature-test branch September 5, 2018 21:08

bparees mentioned this pull request Jan 30, 2019

remove oapi #21782

Merged

Image signature verification workflow test #14124

Image signature verification workflow test #14124

Conversation

mfojtik commented May 10, 2017

mfojtik commented May 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miminar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

soltysh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miminar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openshift-bot commented May 11, 2017 • edited Loading

mfojtik commented May 11, 2017

mfojtik commented May 15, 2017

openshift-bot commented May 15, 2017

openshift-bot commented May 15, 2017

miminar left a comment

Choose a reason for hiding this comment

mfojtik commented May 16, 2017

mfojtik commented May 17, 2017

openshift-bot commented May 17, 2017

openshift-bot commented May 17, 2017

mfojtik commented May 18, 2017

mfojtik commented May 19, 2017

openshift-bot commented May 19, 2017

openshift-bot commented May 19, 2017 • edited Loading

openshift-bot commented May 11, 2017 •

edited

Loading

openshift-bot commented May 19, 2017 •

edited

Loading