-
Notifications
You must be signed in to change notification settings - Fork 5
Enable secure GPG signature verification for Spack buildcache installations #117
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
base: three-track-build
Are you sure you want to change the base?
Conversation
- Change mirrors.yaml to use signed: true for both eicweb and ghcr mirrors - Add GPG key setup in builder stages with support for persistent keys via secret - Export public key from builder stages for runtime verification - Import public key in runtime stages before installing from buildcache - Remove --no-check-signature from runtime SPACK_INSTALL_FLAGS - Keep --no-check-signature in builder stages (building from source) Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com>
- Add comprehensive documentation in docs/BUILDCACHE_SIGNING.md - Explain GPG key setup for production use - Document security layers and troubleshooting - Add comment clarifying temporary vs persistent key usage Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com>
- Add GPG key setup to builder_installation_custom stage - Copy and import public key from builder_installation_custom to runtime_custom - This ensures custom builds have the same security as default builds - Fix documentation placeholder Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com>
- Fix GPG private key import to use gpg --import instead of spack gpg trust - Add clearer comments explaining key management strategy - Document stage inheritance relationships - Acknowledge code duplication and explain why it's necessary Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com>
- Move GPG key initialization from eic/Dockerfile to debian/Dockerfile base layer
- Export public key to ${SPACK_ROOT}/spack-public-key.pub instead of /tmp
- Update ghcr mirror to use --signed instead of --unsigned
- Remove duplicate GPG setup from builder stages (now inherited from base)
- Simplify runtime stages to use GPG key from base layer
- Add transition strategy: try signed buildcache first, fall back to unsigned
- This allows graceful handling of unsigned artifacts from before this PR
Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com>
- Use more specific regex pattern for signature verification errors - Look for NoVerifyException and other Spack-specific error patterns - Add clearer log messages explaining the fallback behavior - Reduce false positives from generic keyword matches - Apply consistent logic to both runtime stages Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com>
- Add GPG public key existence check before trusting - Document regex pattern duplication and need to keep in sync - Expand mirror configuration comments with deployment guidance - Clarify transition strategy and safe deploy order Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com>
containers/debian/Dockerfile
Outdated
| else | ||
| # Create a temporary GPG key for signing (non-interactive) | ||
| # Note: For production, a persistent key should be provided via SPACK_SIGNING_KEY secret | ||
| spack gpg create "EIC Containers" eic-containers@github.com |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot Evaluate the security implications behind creating a trusted key that is bundled in the container which is deployed publicly (including as a base layer). Evaluate this also in the context of a provided key that is imported.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Security Evaluation of GPG Key in Public Container
I've evaluated both scenarios (temporary key and imported persistent key):
Scenario 1: Temporary Key Created in Container (No SPACK_SIGNING_KEY secret)
Security Implications:
- Private key bundled in public base layer: This creates a security vulnerability where anyone can extract the private key from the public container image
- No authenticity guarantee: Since the private key is public, anyone can sign buildcache packages pretending to be from EIC
- Integrity only, not provenance: Signatures only verify that packages haven't been modified after signing, but don't prove WHO signed them
- Attack vector: An attacker could:
- Extract the private key from the public container
- Create malicious buildcache packages
- Sign them with the extracted key
- Users would trust these packages as "legitimate"
Risk Level: HIGH for production use
Scenario 2: Persistent Key Imported via SPACK_SIGNING_KEY Secret
Security Implications:
- Private key in base layer: The imported private key is still embedded in the container layer, even though it comes from a secret
- Key still extractable: Anyone with access to the built container can extract the private key using
docker saveand layer inspection - Secret mount limitations: Docker secrets are only protected during build-time; once imported into the image, they become part of the layer
- Improved but not perfect: Better than temporary keys (key can be rotated, access controlled), but still vulnerable to extraction
Risk Level: MEDIUM-HIGH (improved over temporary, but still has exposure)
Recommended Security Improvements
Option A: Don't embed private keys in containers (RECOMMENDED)
- Only embed the public key in the container for verification
- Keep private key in CI/CD environment only (GitHub Actions runner, not in container)
- Sign buildcaches in CI before pushing to registry, outside the container build
- Container only needs public key to verify, not to sign
Option B: Use detached signing with ephemeral keys
- Generate signing key only in CI environment
- Sign buildcaches during CI workflow
- Export and store only public key in container
- Private key never touches container layers
Current Implementation Assessment:
The current design has a fundamental flaw: embedding private keys in publicly accessible container layers defeats the purpose of cryptographic signatures. The signatures prove integrity but not authenticity since anyone can obtain the signing key.
Immediate Action Items
- Short term (transition): Current approach works for migration but should be documented as temporary
- Long term: Refactor to sign buildcaches in CI environment, not within container
- Documentation: Add security warning about key exposure in current design
- Alternative: Consider using OCI registry's built-in signing (cosign/notary) instead of GPG
The transition strategy (fallback to unsigned) is actually safer than trusting signatures from publicly-available keys.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot Develop an approach that avoids local key creation (i.e. requires a secret key to be provided) and uses secret mounts in all necessary run steps of the build to ensure no key is ever stored inside the container layers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented secure key management that completely avoids storing private keys in container layers:
Key Changes:
- Required Secret: SPACK_SIGNING_KEY is now mandatory (no fallback to temporary keys)
- Temporary GNUPGHOME: Private key imported to
/tmp/gnupg-*which is cleaned before RUN completes - Secret Mounting: Private key mounted during all builder RUN steps that need signing
- Public Key Only: Container layers contain only the public key at
${SPACK_ROOT}/spack-public-key.pub - No Persistence: Private key exists only during RUN execution, never written to persistent layers
Security Model:
- Private key: GitHub Secrets → Docker secret mount → temporary GNUPGHOME → cleaned up
- Public key: Extracted once, embedded in container for verification
- Signing: Happens in builder stages with mounted secret
- Verification: Happens in runtime stages with embedded public key
The private key cannot be extracted from published containers since it's never part of any layer.
Implemented in commit 7b760bc.
- Document security limitations of embedding private keys in public containers - Explain that signatures provide integrity but not authenticity verification - Add security notice in Dockerfile about key extraction risk - Provide recommendations for production-grade security improvements - Clarify current implementation is suitable for transition/internal use - Suggest alternative approaches (CI-based signing, OCI-native signing) Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com>
- Make SPACK_SIGNING_KEY secret mandatory (no fallback to temporary keys) - Use separate GNUPGHOME to avoid persisting private key in GPG keyring - Mount private key as secret during all build steps that need signing - Only embed public key in container layers for verification - Private key exists only during RUN execution, never in final layers - Update documentation to reflect secure key management model - Remove security warnings about key exposure (no longer applicable) Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com>
Addresses security vulnerability where
--no-check-signatureallowed compromised buildcaches to serve tampered binaries without cryptographic verification.SECURITY: This implementation ensures private keys are NEVER stored in container layers. Private keys are mounted as secrets during build operations and immediately discarded.
Changes
mirrors.yaml.in
signed: truefor both eicweb and ghcr mirrors to enable automatic GPG signing during autopushcontainers/debian/Dockerfile (Base Layer)
SPACK_SIGNING_KEYsecret (mandatory, no fallback)${SPACK_ROOT}/spack-public-key.pubfor use by all derived images--unsignedto--signedcontainers/eic/Dockerfile - Builder stages
SPACK_SIGNING_KEYsecret during install steps for signingcontainers/eic/Dockerfile - Runtime stages
${SPACK_ROOT}/spack-public-key.pub--no-check-signaturefromSPACK_INSTALL_FLAGSinruntime_defaultDocumentation
docs/BUILDCACHE_SIGNING.mdwith production GPG key setup, security model, and troubleshootingSecurity Model
✅ SECURE IMPLEMENTATION - Private key never stored in container layers
What This Implementation Provides:
Key Protection Mechanisms:
How It Works
/tmp/gnupg-*) for signing${SPACK_ROOT}/spack-public-key.pubKey Management
REQUIRED:
SPACK_SIGNING_KEYsecret must be configured in GitHub repository secrets.The secret must contain a GPG private key in ASCII-armored format. Generate a key:
Breaking Change: Builds will fail without the SPACK_SIGNING_KEY secret. This is intentional to enforce secure practices.
Transition Strategy
The runtime stages implement graceful migration from unsigned to signed buildcaches:
--no-check-signature(backward compatibility)This ensures smooth migration without breaking existing workflows while preferring signed buildcaches when available.
Architecture Improvements
containers/debian/Dockerfile)✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.