Move hardware signing out of experimental #21003

riyazdf · 2016-03-07T19:42:33Z

Moves pkcs11 hardware signing for notary out of the experimental build tag and to the default docker build (relevant notary issue for tracking: notaryproject/notary#591).

I'm not the most familiar with exactly how our packaging works but I think I've covered the necessary changes.

Here's a cute aardvark

Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>

riyazdf · 2016-03-08T18:14:47Z

Moving out of WIP, did manual testing with a make cross binary:

Pull/push without DCT
Pull/push with DCT enabled (but no Yubikey 4)
Pull/push with DCT enabled and Yubikey 4 hardware signing

thaJeztah · 2016-03-09T11:56:06Z

Two failures on Windows; flaky tests? https://jenkins.dockerproject.org/job/Docker-PRs-WoW-TP4/2314/console

04:04:38 PASS: docker_cli_build_test.go:1051: DockerSuite.TestBuildAddMultipleFilesToFileWithWhitespace 0.034s
04:04:49 
04:04:49 ----------------------------------------------------------------------
04:04:49 FAIL: docker_cli_build_test.go:2636: DockerSuite.TestBuildAddMultipleLocalFileWithCache
04:04:49 
04:04:49 docker_cli_build_test.go:2653:
04:04:49     c.Fatal(err)
04:04:49 ... Error: failed to build the image: Sending build context to Docker daemon 3.072 kB

04:04:49 Step 1 : FROM busybox
04:04:49  ---> b69195d14dbe
04:04:49 Step 2 : MAINTAINER dockerio
04:04:49  ---> Running in cea6eea57600
04:04:49  ---> 81b03eff5fa6
04:04:49 Removing intermediate container cea6eea57600
04:04:49 Step 3 : ADD foo Dockerfile /usr/lib/bla/
04:04:49  ---> b0352f78d4eb
04:04:49 Removing intermediate container 8c2afdc80059
04:04:49 Step 4 : RUN sh -c "[ $(cat /usr/lib/bla/foo) = "hello" ]"
04:04:49  ---> Running in 04f30c654e0a
04:04:49 Cannot start container 04f30c654e0a59524d8e061ec2dc6e0b34c8b6cc3548b1621aa806dc6b2eaf1c: HCSShim::CreateProcessInComputeSystem failed in Win32: The specified path is invalid. (0xa1) id=04f30c654e0a59524d8e061ec2dc6e0b34c8b6cc3548b1621aa806dc6b2eaf1c params={ cmd /S /C sh -c "[ $(cat /usr/lib/bla/foo) = "hello" ]"  map[] false [0 0]}
04:04:49

04:04:38 PASS: docker_cli_build_test.go:1051: DockerSuite.TestBuildAddMultipleFilesToFileWithWhitespace 0.034s
04:04:49 
04:04:49 ----------------------------------------------------------------------
04:04:49 FAIL: docker_cli_build_test.go:2636: DockerSuite.TestBuildAddMultipleLocalFileWithCache
04:04:49 
04:04:49 docker_cli_build_test.go:2653:
04:04:49     c.Fatal(err)
04:04:49 ... Error: failed to build the image: Sending build context to Docker daemon 3.072 kB

04:04:49 Step 1 : FROM busybox
04:04:49  ---> b69195d14dbe
04:04:49 Step 2 : MAINTAINER dockerio
04:04:49  ---> Running in cea6eea57600
04:04:49  ---> 81b03eff5fa6
04:04:49 Removing intermediate container cea6eea57600
04:04:49 Step 3 : ADD foo Dockerfile /usr/lib/bla/
04:04:49  ---> b0352f78d4eb
04:04:49 Removing intermediate container 8c2afdc80059
04:04:49 Step 4 : RUN sh -c "[ $(cat /usr/lib/bla/foo) = "hello" ]"
04:04:49  ---> Running in 04f30c654e0a
04:04:49 Cannot start container 04f30c654e0a59524d8e061ec2dc6e0b34c8b6cc3548b1621aa806dc6b2eaf1c: HCSShim::CreateProcessInComputeSystem failed in Win32: The specified path is invalid. (0xa1) id=04f30c654e0a59524d8e061ec2dc6e0b34c8b6cc3548b1621aa806dc6b2eaf1c params={ cmd /S /C sh -c "[ $(cat /usr/lib/bla/foo) = "hello" ]"  map[] false [0 0]}
04:04:49

riyazdf · 2016-03-10T01:11:27Z

@thaJeztah it seems that these are unrelated/flaky failures? I haven't had these issues with local testing and building. Should we maybe retry that build? Let me know if there's another way I can help

thaJeztah · 2016-03-10T10:49:08Z

@riyazdf yeah, I already restarted them after that comment, but now it looks like it failed again due to an issue on that node. I'll trigger it again.

Note that that build is running on a Windows Daemon on Windows TP4, so not you have that installed locally

riyazdf · 2016-03-10T22:43:11Z

@thaJeztah all green after a retry :)

calavera · 2016-03-11T00:04:45Z

LGTM

calavera · 2016-03-11T21:14:58Z

hack/make/.build-deb/rules

-	# if we are building experimental we recommend yubico-piv-tool
-	echo 'yubico:Recommends=$(shell [ "$DOCKER_EXPERIMENTAL" ] && echo "yubico-piv-tool (>= 1.1.0~)")' >> debian/docker-engine.substvars
+	# recommend yubico-piv-tool since we include pkcs11 by default
+	echo 'yubico:Recommends=$(echo "yubico-piv-tool (>= 1.1.0~)")' >> debian/docker-engine.substvars


I think we don't need the echo here anymore, do we @jfrazelle? Something like this should be enough:

echo 'yubico:Recommends="yubico-piv-tool (>= 1.1.0~)"' >> debian/docker-engine.substvars

thaJeztah · 2016-03-15T10:43:54Z

ping @jfrazelle could you have a look?

jessfraz · 2016-03-15T15:59:15Z

Dockerfile.s390x

@@ -133,7 +133,7 @@ RUN useradd --create-home --gid docker unprivilegeduser

 VOLUME /var/lib/docker
 WORKDIR /go/src/github.com/docker/docker
-ENV DOCKER_BUILDTAGS apparmor selinux
+ENV DOCKER_BUILDTAGS apparmor pkcs11 selinux


You should be sure this will work for things like Z before adding

Do we support the docker CLI on those systems? Bear in mind trust all happens on the CLI side.

Ya I just want to make sure ping @estesp

On Tuesday, March 15, 2016, David Lawrence notifications@github.com wrote:

In Dockerfile.s390x
#21003 (comment):

@@ -133,7 +133,7 @@ RUN useradd --create-home --gid docker unprivilegeduser

VOLUME /var/lib/docker
WORKDIR /go/src/github.com/docker/docker
-ENV DOCKER_BUILDTAGS apparmor selinux
+ENV DOCKER_BUILDTAGS apparmor pkcs11 selinux

Do we support the docker CLI on those systems? Bear in mind trust all
happens on the CLI side.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/docker/docker/pull/21003/files#r56207250

Jessie Frazelle
4096R / D4C4 DD60 0D66 F65A 8EFC 511E 18F3 685C 0022 BFF3
pgp.mit.edu http://pgp.mit.edu/pks/lookup?op=get&search=0x18F3685C0022BFF3

I think we need to ask the teams building with gccgo--I have a vague recollection that there was an issue with pkcs11 build via gccgo? @clnperez or @tophj-ibm do you have any info here?

I don't recall anything, and this works with both gccgo and power. Is pkcs11 used for anything other than notary, because you might not want to include it with gccgo seeing as we aren't building notary there.

I just tested this PR on ppc64le with our old dockerfile before we switched from gccgo to golang/gc -- but with the pcks11 buildtag added. The gccgo build itself works fine. @tophj-ibm said the gccgo build on x86 works. @brahmaroutu might be able to test out z.

@tophj-ibm good point, since we aren't building notary on gccgo I'll remove pkcs11.

As @endophage mentioned above we're only using pkcs11 on the CLI-side, so just keep me posted with whether we need it in this Dockerfile and I'll update accordingly :)

Thanks @jfrazelle @riyazdf, looks like no issues on Z and it builds fine.

Thank you @brahmaroutu for checking! I'll leave this Dockerfile as is then :)

Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>

tiborvass · 2016-03-17T17:31:15Z

code LGTM but couldn't test it.

jessfraz · 2016-03-17T18:10:20Z

LGTM

On Thu, Mar 17, 2016 at 10:33 AM, Tibor Vass notifications@github.com
wrote:

code LGTM but couldn't test it.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#21003 (comment)

Jessie Frazelle
4096R / D4C4 DD60 0D66 F65A 8EFC 511E 18F3 685C 0022 BFF3
pgp.mit.edu http://pgp.mit.edu/pks/lookup?op=get&search=0x18F3685C0022BFF3

Move hardware signing out of experimental

This reverts commit e6d3a98, reversing changes made to d3afe34. Signed-off-by: cyli <cyli@twistedmatrix.com> Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

This reverts commit e6d3a98, reversing changes made to d3afe34. Signed-off-by: cyli <cyli@twistedmatrix.com> (cherry picked from commit dd33d18)

Move pkcs11 out of experimental, into GA

37fa75b

Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>

GordonTheTurtle added the status/0-triage label Mar 7, 2016

thaJeztah added this to the 1.11.0 milestone Mar 7, 2016

thaJeztah added status/1-design-review and removed status/0-triage labels Mar 7, 2016

Update packaging for yubico

2c3e9e5

Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>

riyazdf changed the title ~~WIP: move hardware signing out of experimental~~ Move hardware signing out of experimental Mar 8, 2016

calavera added status/2-code-review and removed status/1-design-review labels Mar 11, 2016

calavera reviewed Mar 11, 2016
View reviewed changes

jessfraz reviewed Mar 15, 2016
View reviewed changes

Improve messaging and binary generation for pkcs11

f7fa83c

Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>

tiborvass added a commit that referenced this pull request Mar 17, 2016

Merge pull request #21003 from riyazdf/hardware-signing-ga

e6d3a98

Move hardware signing out of experimental

tiborvass merged commit e6d3a98 into moby:master Mar 17, 2016

riyazdf deleted the hardware-signing-ga branch March 17, 2016 18:27

thaJeztah added the impact/changelog label Mar 18, 2016

riyazdf mentioned this pull request Mar 28, 2016

move hardware signing out of experimental, remove yubico-piv-tool deps #21591

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move hardware signing out of experimental #21003

Move hardware signing out of experimental #21003

riyazdf commented Mar 7, 2016

riyazdf commented Mar 8, 2016

thaJeztah commented Mar 9, 2016

riyazdf commented Mar 10, 2016

thaJeztah commented Mar 10, 2016

riyazdf commented Mar 10, 2016

calavera commented Mar 11, 2016

calavera Mar 11, 2016

jessfraz Mar 15, 2016

thaJeztah commented Mar 15, 2016

jessfraz Mar 15, 2016

endophage Mar 15, 2016

jessfraz Mar 15, 2016

estesp Mar 15, 2016

tophj-ibm Mar 16, 2016

clnperez Mar 16, 2016

riyazdf Mar 16, 2016

brahmaroutu Mar 16, 2016

riyazdf Mar 17, 2016

tiborvass commented Mar 17, 2016

jessfraz commented Mar 17, 2016

Move hardware signing out of experimental #21003

Move hardware signing out of experimental #21003

Conversation

riyazdf commented Mar 7, 2016

riyazdf commented Mar 8, 2016

thaJeztah commented Mar 9, 2016

riyazdf commented Mar 10, 2016

thaJeztah commented Mar 10, 2016

riyazdf commented Mar 10, 2016

calavera commented Mar 11, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thaJeztah commented Mar 15, 2016

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

tiborvass commented Mar 17, 2016

jessfraz commented Mar 17, 2016