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.

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

release: correctly handle binary signing for "make releaseall" #3309

Merged
merged 1 commit into from
Dec 9, 2021
Merged

release: correctly handle binary signing for "make releaseall" #3309

merged 1 commit into from
Dec 9, 2021

Conversation

cyphar
Copy link
Member

@cyphar cyphar commented Dec 7, 2021

My GPG keys are not available inside the container, so it makes little
sense to try to sign the binaries inside the container's release.sh. The
solution is to split things into separate build and sign stages, with
signing ocurring after the in-Docker build.

Fixes #3038
Signed-off-by: Aleksa Sarai cyphar@cyphar.com

@cyphar cyphar added this to the 1.1.0 milestone Dec 7, 2021
My GPG keys are not available inside the container, so it makes little
sense to try to sign the binaries inside the container's release.sh. The
solution is to split things into separate build and sign stages, with
signing ocurring after the in-Docker build.

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
@cyphar cyphar requested a review from a team December 8, 2021 06:18
@cyphar cyphar mentioned this pull request Dec 8, 2021

# Print usage information.
function usage() {
echo "usage: release_sign.sh [-S <gpg-key-id>] [-r <release-dir>]" >&2
Copy link
Contributor

Choose a reason for hiding this comment

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

This lacks some options (-H, -v)

@kolyshkin
Copy link
Contributor

Hmm, can't we just do something like this instead (not tested)?

diff --git a/Makefile b/Makefile
index aeb62f8c..c4aff886 100644
--- a/Makefile
+++ b/Makefile
@@ -44,6 +44,7 @@ releaseall: release
 release: runcimage
        $(CONTAINER_ENGINE) run $(CONTAINER_ENGINE_RUN_FLAGS) \
                --rm -v $(CURDIR):/go/src/$(PROJECT) \
+               -v ~/.gnupg:/root/.gnupg:ro \
                -e RELEASE_ARGS=$(RELEASE_ARGS) \
                $(RUNC_IMAGE) make localrelease

@kolyshkin
Copy link
Contributor

Hmm, can't we just do something like this instead (not tested)?

Theoretically speaking, some malicious image can stole private keys. OTOH if we trust this image in building runc binary which we are releasing, it is to be trusted.

Copy link
Contributor

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

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

LGTM (in case you want to merge this as is, which is fine by me)

@cyphar
Copy link
Member Author

cyphar commented Dec 8, 2021

I use a hardware token for my GPG keys, so passing the keydir won't work.

@AkihiroSuda
Copy link
Member

Two LGTMs, is this ready to merge?

@cyphar
Copy link
Member Author

cyphar commented Dec 9, 2021

Yup. The help thing is a minor issue, I can fix it some other time.

@cyphar cyphar closed this in 1b747a4 Dec 9, 2021
@cyphar cyphar merged commit 1b747a4 into opencontainers:master Dec 9, 2021
@cyphar cyphar deleted the release-scripts branch December 9, 2021 06:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Include ARM64 static binary in the official release artifact
3 participants