Skip to content
Merged
Show file tree
Hide file tree
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
132 changes: 132 additions & 0 deletions .github/tests/buildkit-mtls-client.test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
# Client half of the buildkitd mutual-TLS invariant (CEL-1513).
#
# The server half lives in CellarNode/crossplane-gcloud
# (scripts/verify_buildkit_mtls.py). That one proves buildkitd demands a client
# certificate; this one proves the only caller actually presents one, over the
# exact contract documented in crossplane-gcloud docs/buildkit-mtls.md:
#
# docker buildx create --name k8s-buildkit --driver remote \
# --driver-opt cacert=/run/buildkit/certs/ca.crt \
# --driver-opt cert=/run/buildkit/certs/tls.crt \
# --driver-opt key=/run/buildkit/certs/tls.key \
# tcp://buildkitd.buildkit.svc.cluster.local:1234 --use
#
# The regression it exists for is a silent downgrade. Dropping the --driver-opt
# flags, or letting the fallback swallow a failed handshake once the server is
# TLS-only, leaves every build green while the connection goes back to
# unauthenticated plaintext. Nothing else in CI notices.

require "yaml"

# Merge-order gate. The rollout is three steps and this constant is which one
# we are on:
#
# true - step 1 of 3. This repo lands FIRST, before crossplane-gcloud#78.
# The certificate does not exist in the cluster yet, so the step must
# still be able to reach a plaintext buildkitd.
# false - step 3 of 3. #78 has soaked; the fallback is deleted so a
# server-side TLS regression fails the build instead of downgrading
# it. Flip this in the same commit that deletes the branch.
PLAINTEXT_FALLBACK_EXPECTED = true

CERT_DIR = "/run/buildkit/certs".freeze
BUILDKITD_ADDR = "tcp://buildkitd.buildkit.svc.cluster.local:1234".freeze

workflow_path = File.expand_path("../workflows/deploy-backend.yaml", __dir__)
workflow = YAML.safe_load(File.read(workflow_path), aliases: true)

build = workflow.fetch("jobs").fetch("build")
connect = build.fetch("steps").find { |step| step.fetch("name", "") == "Connect to in-cluster buildkitd" }
abort "deploy-backend must have a 'Connect to in-cluster buildkitd' step" if connect.nil?

run = connect.fetch("run", "")
env = connect.fetch("env", {})

# --- endpoint and certificate paths are data, not inlined literals -----------

abort "buildkitd address must be passed as env data" unless env.fetch("BUILDKITD_ADDR", nil) == BUILDKITD_ADDR
abort "buildkitd certificate directory must be passed as env data" unless env.fetch("BUILDKITD_CERT_DIR", nil) == CERT_DIR
abort "buildkitd endpoint must not be inlined in the script" if run.include?(BUILDKITD_ADDR)

# --- the mTLS invocation itself ---------------------------------------------

abort "buildkitd connection must use the remote driver" unless run.include?("--driver remote")

{
"cacert" => "ca.crt",
"cert" => "tls.crt",
"key" => "tls.key",
}.each do |opt, file|
# cert-manager's key names, absolute paths. buildx rejects relative ones and
# renaming a key silently produces a builder that cannot handshake.
abort "buildkitd client must pass --driver-opt #{opt}=<certdir>/#{file}" unless
run.include?(%(--driver-opt "#{opt}=${BUILDKITD_CERT_DIR}/#{file}"))
end

# All three files, or none. A partial mount must not select the TLS path and
# then fail the handshake on a missing key.
["ca.crt", "tls.crt", "tls.key"].each do |file|
abort "buildkitd client must require #{file} to be readable before choosing TLS" unless
run.include?(%([ -r "${BUILDKITD_CERT_DIR}/#{file}" ]))
end

# The condition is pinned whole, not just scanned for its parts. Certificate
# presence must be the ONLY thing gating the TLS branch: an extra conjunct
# (`if false && [ -r ... ]`, `... && [ "$SOMETHING" = 1 ]`) leaves every string
# this file greps for intact while making the branch unreachable, and the
# fallback below then quietly carries every build over plaintext. Reformatting
# the guard is expected to fail here — re-read it, then update this regex.
tls_guard = /
^[ ]*if[ ]\[[ ]-r[ ]"\$\{BUILDKITD_CERT_DIR\}\/ca\.crt"[ ]\][ ]\\\n
[ ]*&&[ ]\[[ ]-r[ ]"\$\{BUILDKITD_CERT_DIR\}\/tls\.crt"[ ]\][ ]\\\n
[ ]*&&[ ]\[[ ]-r[ ]"\$\{BUILDKITD_CERT_DIR\}\/tls\.key"[ ]\];[ ]then$
/x
abort "the TLS branch must be gated on certificate presence and nothing else" unless run.match?(tls_guard)

# `docker buildx create` records an endpoint without dialling it, so create
# exiting 0 proves nothing. Only bootstrap performs the handshake.
abort "buildkitd connection must be proven with 'inspect --bootstrap'" unless run.include?("docker buildx inspect --bootstrap k8s-buildkit")

# --- merge-order gate --------------------------------------------------------

fallback_markers = [
"connected=false",
%(if [ "$connected" != true ]; then),
]
Comment on lines +92 to +95

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: The test pins the fallback markers (connected=false, if [ "$connected" != true ]; then) and that the TLS attempt precedes the fallback, but it never requires the TLS-success assignment connected=true in the workflow. Deleting connected=true (workflow line 153) passes this test: the guard is still all three cert files, the driver-opt flags are present, and the fallback still bootstraps. On today's plaintext server the build stays green (a silent downgrade while certs are present, mid-sync), and after crossplane-gcloud#78 switches the server to TLS-only every normal build would fall through to the plaintext fallback and fail at bootstrap — the exact silent-downgrade then late-break this test exists to prevent. Add connected=true to the required markers so the fallback is proven gated on TLS success.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/tests/buildkit-mtls-client.test.rb, line 92:

<comment>The test pins the fallback markers (`connected=false`, `if [ "$connected" != true ]; then`) and that the TLS attempt precedes the fallback, but it never requires the TLS-success assignment `connected=true` in the workflow. Deleting `connected=true` (workflow line 153) passes this test: the guard is still all three cert files, the driver-opt flags are present, and the fallback still bootstraps. On today's plaintext server the build stays green (a silent downgrade while certs are present, mid-sync), and after crossplane-gcloud#78 switches the server to TLS-only every normal build would fall through to the plaintext fallback and fail at bootstrap — the exact silent-downgrade then late-break this test exists to prevent. Add `connected=true` to the required markers so the fallback is proven gated on TLS success.</comment>

<file context>
@@ -0,0 +1,132 @@
+
+# --- merge-order gate --------------------------------------------------------
+
+fallback_markers = [
+  "connected=false",
+  %(if [ "$connected" != true ]; then),
</file context>
Suggested change
fallback_markers = [
"connected=false",
%(if [ "$connected" != true ]; then),
]
fallback_markers = [
&quot;connected=false&quot;,
&quot;connected=true&quot;,
%(if [ &quot;$connected&quot; != true ]; then),
]


if PLAINTEXT_FALLBACK_EXPECTED
fallback_markers.each do |marker|
abort "step 1 of 3 must keep the plaintext fallback (missing: #{marker})" unless run.include?(marker)
end

# The fallback exists so a plaintext server still works. It must never be
# the only path: the TLS attempt has to come first.
tls_at = run.index("--driver-opt")
fallback_at = run.index(%(if [ "$connected" != true ]; then))
abort "the mTLS attempt must precede the plaintext fallback" unless tls_at && fallback_at && tls_at < fallback_at

# An unbootstrapped fallback would defer a total connection failure to the
# first `docker buildx build`, where it reads as a build error.
abort "the plaintext fallback must bootstrap too, so a dead endpoint fails here" unless
run.match?(/if \[ "\$connected" != true \]; then\s+recreate\s+docker buildx inspect --bootstrap/)
else
# Step 3 of 3: no branch may reach buildkitd without the client credential.
fallback_markers.each do |marker|
abort "step 3 of 3 must delete the plaintext fallback (still present: #{marker})" if run.include?(marker)
end
abort "step 3 of 3 must not leave a bare plaintext create" if
run.match?(/recreate\s*$/) || run.include?("recreate\n")
end

# --- fork PRs must never see a client certificate ---------------------------

# The credential is mounted by ARC into job pods in the cluster. Fork PRs are
# untrusted and build on GitHub-hosted runners, which have no cluster access
# and therefore no certificate; if that job ever moved to self-hosted it would
# hand arbitrary PR code a working buildkitd credential.
build_fork = workflow.fetch("jobs").fetch("build-fork")
abort "fork builds must stay on GitHub-hosted runners" unless build_fork.fetch("runs-on") == "ubuntu-latest"
abort "fork builds must not connect to in-cluster buildkitd" if
build_fork.fetch("steps").any? { |step| step.fetch("run", "").include?("--driver remote") }

puts "BuildKit mTLS client contract passed (plaintext fallback: #{PLAINTEXT_FALLBACK_EXPECTED})"
72 changes: 67 additions & 5 deletions .github/workflows/deploy-backend.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,74 @@ jobs:
fi
fi

# Plain TCP is accepted here: private single-tenant cluster, and the
# legacy GKE dataplane doesn't enforce NetworkPolicy. Revisit (TLS via
# --driver-opt cacert/cert/key, or DPv2 + NetworkPolicy) if the cluster
# ever becomes multi-tenant.
# CEL-1513 step 1 of 3 — mutual TLS on the shared buildkitd, with a
# plaintext fallback. Server side is CellarNode/crossplane-gcloud#78
# (runbook: docs/buildkit-mtls.md there).
#
# buildkitd applies TLS per listener and one daemon cannot serve
# plaintext and TLS on the same TCP port, so the server cutover is
# atomic and it spans two repos. This step lands FIRST and is correct on
# both sides of it:
#
# - server plaintext (today): the client leaf is not in the cluster
# yet, /run/buildkit/certs is absent, the guard is false, and the
# plaintext create runs exactly as before.
# - server TLS (after #78 syncs): ARC's container-hook extension mounts
# the client leaf into every job pod at /run/buildkit/certs, the
# guard is true, and the mTLS builder bootstraps.
# - mid-sync, either order: whichever path fails the handshake falls
# through to the other, and if neither connects the step fails here
# rather than at the first build.
#
# `docker buildx create` does not dial the daemon — it only records the
# endpoint. `inspect --bootstrap` is what performs the handshake, so
# both paths are keyed on it and not on create's exit status.
#
# Certificate file names are cert-manager's (ca.crt / tls.crt / tls.key)
# and the paths must be absolute. No `servername` override: the server
# leaf's SAN list covers the FQDN dialled below.
#
# STEP 3 of 3 removes the fallback branch, leaving only the mTLS path.
# Until it does, a server-side regression that dropped --tlscacert would
# silently downgrade CI to plaintext instead of failing the build.
# .github/tests/buildkit-mtls-client.test.rb pins that transition: flip
# PLAINTEXT_FALLBACK_EXPECTED there in the same commit.
- name: Connect to in-cluster buildkitd
run: docker buildx create --name k8s-buildkit --driver remote tcp://buildkitd.buildkit.svc.cluster.local:1234 --use
env:
BUILDKITD_ADDR: tcp://buildkitd.buildkit.svc.cluster.local:1234
BUILDKITD_CERT_DIR: /run/buildkit/certs
run: |
set -euo pipefail

recreate() {
docker buildx rm k8s-buildkit >/dev/null 2>&1 || true
docker buildx create --name k8s-buildkit --driver remote "$@" \
"$BUILDKITD_ADDR" --use
}

connected=false

if [ -r "${BUILDKITD_CERT_DIR}/ca.crt" ] \
&& [ -r "${BUILDKITD_CERT_DIR}/tls.crt" ] \
&& [ -r "${BUILDKITD_CERT_DIR}/tls.key" ]; then
echo "buildkitd client certificate found - connecting with mutual TLS"
recreate \
--driver-opt "cacert=${BUILDKITD_CERT_DIR}/ca.crt" \
--driver-opt "cert=${BUILDKITD_CERT_DIR}/tls.crt" \
--driver-opt "key=${BUILDKITD_CERT_DIR}/tls.key"
if docker buildx inspect --bootstrap k8s-buildkit; then
connected=true
else
echo "::warning title=buildkitd mTLS handshake failed::Falling back to plaintext (CEL-1513 step 1 of 3). If buildkitd is already TLS-only this build will fail at connect - re-run it."
fi
else
echo "no buildkitd client certificate at ${BUILDKITD_CERT_DIR} - using plaintext (CEL-1513 step 1 of 3)"
fi

if [ "$connected" != true ]; then

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: Custom agent: Flag Security Vulnerabilities

When the client certificates are absent or mTLS bootstrap fails, this fallback re-creates k8s-buildkit against the tcp:// endpoint and continues after only a warning. That leaves the shared BuildKit connection unauthenticated; remove the plaintext fallback before relying on this workflow for the CEL-1513 hardening.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/deploy-backend.yaml, line 161:

<comment>When the client certificates are absent or mTLS bootstrap fails, this fallback re-creates `k8s-buildkit` against the `tcp://` endpoint and continues after only a warning. That leaves the shared BuildKit connection unauthenticated; remove the plaintext fallback before relying on this workflow for the CEL-1513 hardening.</comment>

<file context>
@@ -94,12 +94,74 @@ jobs:
+            echo "no buildkitd client certificate at ${BUILDKITD_CERT_DIR} - using plaintext (CEL-1513 step 1 of 3)"
+          fi
+
+          if [ "$connected" != true ]; then
+            recreate
+            docker buildx inspect --bootstrap k8s-buildkit
</file context>

recreate
docker buildx inspect --bootstrap k8s-buildkit
fi

- name: Authenticate to Google Cloud
id: auth
Expand Down
32 changes: 32 additions & 0 deletions .github/workflows/validate-backend-deploy.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
name: Validate shared backend deploy workflow

on:
pull_request:
paths:
- .github/tests/buildkit-mtls-client.test.rb
- .github/workflows/deploy-backend.yaml
- .github/workflows/validate-backend-deploy.yaml
push:
branches: [main]
paths:
- .github/tests/buildkit-mtls-client.test.rb
- .github/workflows/deploy-backend.yaml
- .github/workflows/validate-backend-deploy.yaml

permissions:
contents: read

jobs:
buildkit-mtls-client:
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262
with:
persist-credentials: false

# Client half of the CEL-1513 invariant. The server half is
# scripts/verify_buildkit_mtls.py in CellarNode/crossplane-gcloud; both
# halves have to hold for the control to exist.
- name: Test BuildKit mTLS client contract
run: ruby .github/tests/buildkit-mtls-client.test.rb
Loading