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

Isolated sign #41

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
284 changes: 203 additions & 81 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,14 @@ on:
- '' # without this, it's technically "required" 🙃
- 2022
- 2019
pruneArtifact:
required: true
type: boolean
default: true
run-name: '${{ inputs.bashbrewArch }}: ${{ inputs.firstTag }} (${{ inputs.buildId }})'
permissions:
contents: read
actions: write # for https://github.com/andymckay/cancel-action (see usage below)
id-token: write # for AWS KMS signing (see usage below)
concurrency:
group: ${{ github.event.inputs.buildId }}
cancel-in-progress: false
Expand All @@ -34,9 +37,20 @@ defaults:
env:
BUILD_ID: ${{ inputs.buildId }}
BASHBREW_ARCH: ${{ inputs.bashbrewArch }}

# the image we'll run to access the signing tool
# https://explore.ggcr.dev/?repo=docker/image-signer-verifier
IMAGE_SIGNER: 'docker/image-signer-verifier:0.6.9@sha256:e38be2b9e6f010cf3432a2772b0e800feee572f7733c6df81e21293cb3e977e0'

# Docker Hub repository we'll push the (signed) attestation artifacts to
REFERRERS_REPO: oisupport/referrers
Comment on lines +40 to +46
Copy link
Member

Choose a reason for hiding this comment

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

These should move down to be part of jobs.sign.env or better yet even jobs.sign.steps[].env for the specific step that needs them (given there's only the one).

Copy link
Member Author

Choose a reason for hiding this comment

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

There is only one of the iteration but it would be used more than once once we have the VSA in. It's not as obvious here but I wanted to do it like this to minimize the changrs

jobs:
build:
name: Build ${{ inputs.buildId }}
outputs:
json: ${{ steps.json.outputs.json }}
artifactUrl: ${{ steps.oci.outputs.artifact-url }}
artifactSha256: ${{ steps.sha256.outputs.sha256 }}
runs-on: ${{ inputs.bashbrewArch == 'windows-amd64' && format('windows-{0}', inputs.windowsVersion) || 'ubuntu-latest' }}
steps:

Expand Down Expand Up @@ -148,135 +162,243 @@ jobs:
fi
eval "$shell"

# TODO signing prototype (see above where "shouldSign" is populated)
# save the build as an "artifact" so we can sign it in a separate step (and minimize the exposure of the signing credentials); also save a checksum so we can be sure it transmits between the steps accurately
# TODO what do we do here for "classic" builds like Windows? (no "temp" OCI layout currently)
- name: Generate Artifact
id: sha256
run: |
tar -cvf temp.tar -C build temp
sha256="$(sha256sum temp.tar | cut -d' ' -f1)"
echo "sha256=$sha256" >> "$GITHUB_OUTPUT"
- name: Upload Artifact
id: oci
uses: actions/upload-artifact@v4
with:
name: build-oci
path: |
temp.tar
retention-days: 5

sign:
name: Sign
needs: build
if: fromJSON(needs.build.outputs.json).shouldSign
runs-on: ubuntu-latest
permissions:
contents: read
id-token: write # for AWS KMS signing (see usage below)
steps:
- uses: actions/checkout@v4
with:
sparse-checkout-cone-mode: 'false'
sparse-checkout: |
.scripts/oci.jq
.scripts/provenance.jq
Comment on lines +194 to +196
Copy link
Member

Choose a reason for hiding this comment

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

In this repository, .scripts is a submodule -- does this work across a submodule boundary? 🤔

On the other hand, I'm not personally super concerned about this, and think a checkout just like the one above would be fine. If we want something like the intent of this, I think we should front-load it to the previous step instead, which is really the step that performed the "build" anyhow so arguably should be validating the build and generating provenance.json.

Edit: with my following suggested changes, this becomes irrelevant and we can drop actions/checkout from this step completely, which I think is even stronger

- name: Download Artifact
uses: actions/download-artifact@v4
with:
name: build-oci
- name: Extract Artifact
env:
sha256: ${{ needs.build.outputs.artifactSha256 }}
run: |
sha256sum <<<"$sha256 *temp.tar" --strict --check -
tar -xvf temp.tar
[ -d temp ] # basic "valid JSON" check
- name: Configure AWS (for signing)
if: fromJSON(steps.json.outputs.json).shouldSign
# https://github.com/aws-actions/configure-aws-credentials/releases
uses: aws-actions/configure-aws-credentials@010d0da01d0b5a38af31e9c3470dbfdabdecca3a # v4.0.1
with:
aws-region: ${{ github.ref_name == 'main' && secrets.AWS_KMS_PROD_REGION || secrets.AWS_KMS_STAGE_REGION }}
role-to-assume: ${{ github.ref_name == 'main' && secrets.AWS_KMS_PROD_ROLE_ARN || secrets.AWS_KMS_STAGE_ROLE_ARN }}
# TODO figure out if there's some way we could make our secrets ternaries here more DRY without major headaches 🙈
- name: Generate Provenance
Copy link
Member

Choose a reason for hiding this comment

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

This whole step goes away 👀

env:
json: ${{ needs.build.outputs.json }}
GITHUB_CONTEXT: ${{ toJson(github) }}
run: |
image-digest() {
local dir="$1/blobs"
img=$(
grep -R --include "*" '"mediaType":\s"application/vnd.oci.image.layer.' "$dir" \
| head -n 1 \
| cut -d ':' -f1
)
[ "$(cat $img | jq -r '.mediaType')" = "application/vnd.oci.image.manifest.v1+json" ] || exit 1
echo $img | rev | cut -d '/' -f2,1 --output-delimiter ':' | rev
}

digest=$(image-digest temp)

echo $json | jq -L.scripts --argjson github '${{ env.GITHUB_CONTEXT }}' --argjson runner '${{ toJson(runner) }}' --arg digest ${digest} '
include "provenance";
github_actions_provenance($github; $runner; $digest)
' >> provenance.json
- name: Sign
if: fromJSON(steps.json.outputs.json).shouldSign
env:
AWS_KMS_REGION: ${{ github.ref_name == 'main' && secrets.AWS_KMS_PROD_REGION || secrets.AWS_KMS_STAGE_REGION }}
AWS_KMS_KEY_ARN: ${{ github.ref_name == 'main' && secrets.AWS_KMS_PROD_KEY_ARN || secrets.AWS_KMS_STAGE_KEY_ARN }}

DOCKER_HUB_USERNAME: ${{ secrets.DOCKER_HUB_SIGNING_USERNAME }}
DOCKER_HUB_PASSWORD: ${{ secrets.DOCKER_HUB_SIGNING_PASSWORD }}
run: |
cd build
validate-oci-layout() {
Copy link
Member

Choose a reason for hiding this comment

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

We can drop this entirely now (handled elsewhere).

local dir="$1"
jq -L.scripts -s '
include "oci";
validate_oci_layout | true
' "$dir/oci-layout" "$dir/index.json" || return "$?"
local manifest
manifest="$dir/blobs/$(jq -r '.manifests[0].digest | sub(":"; "/")' "$dir/index.json")" || return "$?"
jq -L.scripts -s '
include "oci";
if length != 1 then
error("unexpected image index document count: \(length)")
else .[0] end
| validate_oci_index

# TODO more validation?
' "$manifest" || return "$?"
}

args=(
dockerArgs=(
--interactive
--rm
--read-only
--workdir /tmp # see "--tmpfs" below (TODO the signer currently uses PWD as TMPDIR -- something to fix in the future so we can drop this --workdir and only keep --tmpfs perhaps adding --env TMPDIR=/tmp if necessary)
)
if [ -t 0 ] && [ -t 1 ]; then
args+=( --tty )
dockerArgs+=( --tty )
fi

user="$(id -u)"
args+=( --tmpfs "/tmp:uid=$user" )
dockerArgs+=( --tmpfs "/tmp:uid=$user" )
user+=":$(id -g)"
args+=( --user "$user" )
dockerArgs+=( --user "$user" )

awsEnvs=( "${!AWS_@}" )
args+=( "${awsEnvs[@]/#/--env=}" )

# some very light assumption verification (see TODO in --mount below)
validate-oci-layout() {
local dir="$1"
jq -s '
if length != 1 then
error("unexpected 'oci-layout' document count: " + length)
else .[0] end
| if .imageLayoutVersion != "1.0.0" then
error("unsupported imageLayoutVersion: " + .imageLayoutVersion)
else . end
' "$dir/oci-layout" || return "$?"
jq -s '
if length != 1 then
error("unexpected 'index.json' document count: " + length)
else .[0] end

| if .schemaVersion != 2 then
error("unsupported schemaVersion: " + .schemaVersion)
else . end
| if .mediaType != "application/vnd.oci.image.index.v1+json" and .mediaType then # TODO drop the second half of this validation: https://github.com/moby/buildkit/issues/4595
error("unsupported index mediaType: " + .mediaType)
else . end
| if .manifests | length != 1 then
error("expected only one manifests entry, not " + (.manifests | length))
else . end

| .manifests[0] |= (
if .mediaType != "application/vnd.oci.image.index.v1+json" then
error("unsupported descriptor mediaType: " + .mediaType)
else . end
# TODO validate .digest somehow (`crane validate`?) - would also be good to validate all descriptors recursively
| if .size < 0 then
error("invalid descriptor size: " + .size)
else . end
)
' "$dir/index.json" || return "$?"
local manifest
manifest="$dir/blobs/$(jq -r '.manifests[0].digest | sub(":"; "/")' "$dir/index.json")" || return "$?"
jq -s '
if length != 1 then
error("unexpected image index document count: " + length)
else .[0] end
| if .schemaVersion != 2 then
error("unsupported schemaVersion: " + .schemaVersion)
else . end
| if .mediaType != "application/vnd.oci.image.index.v1+json" then
error("unsupported image index mediaType: " + .mediaType)
else . end
dockerArgs+=( "${awsEnvs[@]/#/--env=}" )

# TODO more validation?
' "$manifest" || return "$?"
}
validate-oci-layout temp

mkdir signed
# Login to Docker Hub
export DOCKER_CONFIG="$PWD/.docker"
mkdir "$DOCKER_CONFIG"
trap 'find "$DOCKER_CONFIG" -type f -exec shred -fuvz "{}" + || :; rm -rf "$DOCKER_CONFIG"' EXIT
docker login --username "$DOCKER_HUB_USERNAME" --password-stdin <<<"$DOCKER_HUB_PASSWORD"
unset DOCKER_HUB_USERNAME DOCKER_HUB_PASSWORD

args+=(
--mount "type=bind,src=$PWD/temp,dst=/doi-build/unsigned" # TODO this currently assumes normalized_builder == "buildkit" and !should_use_docker_buildx_driver -- we need to factor that in later (although this signs the attestations, not the image, so buildkit/buildx is the only builder whose output we *can* sign right now)
--mount "type=bind,src=$PWD/signed,dst=/doi-build/signed"
# Create signatures
dockerArgs+=(
--mount "type=bind,src=$PWD/temp,dst=/doi-build/image,ro" # TODO this currently assumes normalized_builder == "buildkit" and !should_use_docker_buildx_driver -- we need to factor that in later (although this signs the attestations, not the image, so buildkit/buildx is the only builder whose output we *can* sign right now)
--mount "type=bind,src=$PWD/provenance.json,dst=/doi-build/provenance.json,ro"
--mount "type=bind,src=$DOCKER_CONFIG,dst=/docker-config,ro"
--env DOCKER_CONFIG=/docker-config # TODO verify that image-signer supports this environment variable correctly

# https://explore.ggcr.dev/?repo=docker/image-signer-verifier
docker/image-signer-verifier:0.3.3@sha256:a5351e6495596429bacea85fbf8f41a77ce7237c26c74fd7c3b94c3e6d409c82

sign
"$IMAGE_SIGNER"
Copy link
Member

Choose a reason for hiding this comment

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

Since this is the only place it's referenced, we could move this back here (ie, no env: at all anymore). That has the benefit of putting the pin directly next to the code that uses it (which helps encourage us to verify the usage when updating the pinning):

Suggested change
"$IMAGE_SIGNER"
docker/image-signer-verifier:0.6.9@sha256:e38be2b9e6f010cf3432a2772b0e800feee572f7733c6df81e21293cb3e977e0

Copy link
Member Author

Choose a reason for hiding this comment

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

Same as previous one. It will be used more than once when we add VSA.

)

--envelope-style oci-content-descriptor
kmsArg=(
# kms key used to sign attestation artifacts
--kms="AWS"
--kms-region="$AWS_KMS_REGION"
--kms-key-ref="$AWS_KMS_KEY_ARN"

--aws_region "$AWS_KMS_REGION"
--aws_arn "awskms:///$AWS_KMS_KEY_ARN"
--referrers-dest="$REFERRERS_REPO" # repo to store attestation artifacts and provenance
)

--input oci:///doi-build/unsigned
--output oci:///doi-build/signed
# Sign buildkit statements
signArgs=(
"${kmsArg[@]}"
--input=oci:///doi-build/image
--keep=true # keep preserves the unsigned attestations generated by buildkit
Copy link
Member

Choose a reason for hiding this comment

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

I recall discussions around not needing this anymore because nothing we're doing munges the input -- is that accurate? Is this still necessary?

Copy link
Member Author

Choose a reason for hiding this comment

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

@kipz do you know?

Choose a reason for hiding this comment

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

The --keep parameter should be made optional (if it isn't already) since having a mutated index is optional for signing using referrers. Too bad we open sourced everything but image-signer-verifier, I'd be able to verify and potentially help here.

)

docker run "${args[@]}"
docker run "${dockerArgs[@]}" sign "${signArgs[@]}"

validate-oci-layout signed
# Attach and sign provenance
provArgs=(
"${kmsArg[@]}"
--image=oci:///doi-build/image
--statement="/doi-build/provenance.json"
)
docker run "${dockerArgs[@]}" attest "${provArgs[@]}"

push:
name: Push
needs:
- build
- sign
LaurentGoderre marked this conversation as resolved.
Show resolved Hide resolved
# - verify
if: ${{ always() }}
Copy link
Member

Choose a reason for hiding this comment

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

We don't want this to still run on failure, right? I guess this is how you're managing the issue with the if: I noted above? 😭

Copy link
Member Author

Choose a reason for hiding this comment

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

We want to clear the artifact on failure and there is an extra setting to not clear it for debugging (that's what the of does)

Copy link
Member Author

@LaurentGoderre LaurentGoderre Nov 11, 2024

Choose a reason for hiding this comment

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

Ok, collapsing both comments into this one, I was able to use if: ${{ always() && inputs.pruneArtifact }} which ignores build failures but respect the boolean fort pruning.

runs-on: ubuntu-latest
steps:
- run: ${{ (needs.sign.result == 'skipped' && needs.build.result == 'success') || needs.verify.result == 'success' || 'exit 1' }}
- name: Download Artifact
uses: actions/download-artifact@v4
with:
name: build-oci
- name: Extract Artifact
env:
sha256: ${{ needs.build.outputs.artifactSha256 }}
run: |
sha256sum <<<"$sha256 *temp.tar" --strict --check -
tar -xvf temp.tar
[ -d temp ] # basic "valid JSON" check
- name: Tools
run: |
mkdir .gha-bin
echo "$PWD/.gha-bin" >> "$GITHUB_PATH"

# TODO validate that "signed" still has all the original layer blobs from "temp" (ie, that the attestation manifest *just* has some new layers and everything else is unchanged)
case "${RUNNER_ARCH}" in \
X64) ARCH='amd64';; \
esac

rm -rf temp
mv signed temp
_download() {
local target="$1"; shift
local url="$1"; shift
wget --timeout=5 -O "$target" "$url" --progress=dot:giga
}

# https://doi-janky.infosiftr.net/job/wip/job/crane
_download ".gha-bin/crane" "https://doi-janky.infosiftr.net/job/wip/job/crane/lastSuccessfulBuild/artifact/crane-$ARCH"
# TODO checksum verification ("checksums.txt")
chmod +x ".gha-bin/crane"
".gha-bin/crane" version
- name: Push
env:
DOCKER_HUB_USERNAME: ${{ secrets.DOCKER_HUB_USERNAME }}
DOCKER_HUB_PASSWORD: ${{ secrets.DOCKER_HUB_PASSWORD }}
json: ${{ needs.build.outputs.json }}
run: |
export DOCKER_CONFIG="$PWD/.docker"
mkdir "$DOCKER_CONFIG"
trap 'find "$DOCKER_CONFIG" -type f -exec shred -fuvz "{}" + || :; rm -rf "$DOCKER_CONFIG"' EXIT
docker login --username "$DOCKER_HUB_USERNAME" --password-stdin <<<"$DOCKER_HUB_PASSWORD"
unset DOCKER_HUB_USERNAME DOCKER_HUB_PASSWORD

cd build
shell="$(jq <<<"$json" -r '.commands.push')"
eval "$shell"

clean:
name: Cleanup
needs:
- build
- sign
- push
if: ${{ always() && inputs.pruneArtifact }}
runs-on: ubuntu-latest
steps:
- name: Clean Up Artifact
env:
ARTIFACT_URL: ${{ needs.build.outputs.artifactUrl }}
TOKEN: ${{ github.token }}
run: |
url="${ARTIFACT_URL/\/\/github\.com///api.github.com/repos}"
url="${url/runs\/*\/artifacts/artifacts}" # Translate web URL to API url
curl -L \
-X DELETE \
-H "Accept: application/vnd.github+json" \
-H "Authorization: Bearer $TOKEN" \
-H "X-GitHub-Api-Version: 2022-11-28" \
"$url"