From 40f0e16777e8daf646e54294cbdca7e215dffbd8 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Sat, 27 Dec 2025 22:07:42 -0500 Subject: [PATCH] skopeo: add `--require-signed` In bootc, we want the ability to assert that signature verification is enforced. Add a new top-level `--require-signed` switch. When passed, we use the new `RequireSignatureVerification()` method to ensure that signature verification is enforced. Part of https://github.com/containers/skopeo/issues/1829. Signed-off-by: Jonathan Lebon --- cmd/skopeo/main.go | 14 +++++- docs/skopeo.1.md | 4 ++ integration/copy_test.go | 100 +++++++++++++++++++++++++++++++++++++-- 3 files changed, 113 insertions(+), 5 deletions(-) diff --git a/cmd/skopeo/main.go b/cmd/skopeo/main.go index 22039f59..db7099d9 100644 --- a/cmd/skopeo/main.go +++ b/cmd/skopeo/main.go @@ -31,6 +31,7 @@ type globalOptions struct { registriesConfPath string // Path to the "registries.conf" file tmpDir string // Path to use for big temporary files userAgentPrefix string // Prefix to add to the user agent string + requireSigned bool // Require any pulled image to be signed } // requireSubcommand returns an error if no sub command is provided @@ -81,6 +82,7 @@ func createApp() (*cobra.Command, *globalOptions) { rootCommand.PersistentFlags().BoolVar(&opts.debug, "debug", false, "enable debug output") rootCommand.PersistentFlags().StringVar(&opts.policyPath, "policy", "", "Path to a trust policy file") rootCommand.PersistentFlags().BoolVar(&opts.insecurePolicy, "insecure-policy", false, "run the tool without any policy check") + rootCommand.PersistentFlags().BoolVar(&opts.requireSigned, "require-signed", false, "require any pulled image to be signed") rootCommand.PersistentFlags().StringVar(&opts.registriesDirPath, "registries.d", "", "use registry configuration files in `DIR` (e.g. for container signature storage)") rootCommand.PersistentFlags().StringVar(&opts.overrideArch, "override-arch", "", "use `ARCH` instead of the architecture of the machine for choosing images") rootCommand.PersistentFlags().StringVar(&opts.overrideOS, "override-os", "", "use `OS` instead of the running OS for choosing images") @@ -135,6 +137,9 @@ func (opts *globalOptions) before(cmd *cobra.Command, args []string) error { if opts.tlsVerify.Present() { logrus.Warn("'--tls-verify' is deprecated, please set this on the specific subcommand") } + if opts.insecurePolicy && opts.requireSigned { + return fmt.Errorf("--insecure-policy and --require-signed are mutually exclusive") + } return nil } @@ -166,7 +171,14 @@ func (opts *globalOptions) getPolicyContext() (*signature.PolicyContext, error) if err != nil { return nil, err } - return signature.NewPolicyContext(policy) + pc, err := signature.NewPolicyContext(policy) + if err != nil { + return nil, err + } + if opts.requireSigned { + pc.RequireSignatureVerification(true) + } + return pc, nil } // commandTimeoutContext returns a context.Context and a cancellation callback based on opts. diff --git a/docs/skopeo.1.md b/docs/skopeo.1.md index 0fafe318..d78fefcb 100644 --- a/docs/skopeo.1.md +++ b/docs/skopeo.1.md @@ -92,6 +92,10 @@ Path to a policy.json file to use for verifying signatures and deciding whether Use registry configuration files in _dir_ (e.g. for container signature storage), overriding the default path. +**--require-signed** + +Require that any pulled image must be signed regardless of what the default or provided trust policy file says. + **--tmpdir** _dir_ Directory used to store temporary files. Defaults to /var/tmp. diff --git a/integration/copy_test.go b/integration/copy_test.go index 2a8232ce..ecc275db 100644 --- a/integration/copy_test.go +++ b/integration/copy_test.go @@ -12,6 +12,7 @@ import ( "net/http" "net/http/httptest" "os" + "os/exec" "path/filepath" "strings" "testing" @@ -42,10 +43,11 @@ func TestCopy(t *testing.T) { type copySuite struct { suite.Suite - cluster *openshiftCluster - registry *testRegistryV2 - s1Registry *testRegistryV2 - gpgHome string + cluster *openshiftCluster + registry *testRegistryV2 + s1Registry *testRegistryV2 + gpgHome string + fingerprint string } var ( @@ -90,6 +92,12 @@ func (s *copySuite) SetupSuite() { []byte(out), 0o600) require.NoError(t, err) } + + // Get fingerprint for the personal key (used by some tests) + lines, err := exec.Command(gpgBinary, "--homedir", s.gpgHome, "--with-colons", "--no-permission-warning", "--fingerprint", "personal@example.com").Output() + require.NoError(t, err) + s.fingerprint, err = findFingerprint(lines) + require.NoError(t, err) } func (s *copySuite) TearDownSuite() { @@ -1285,3 +1293,87 @@ func (s *copySuite) TestCopyFailsWhenReferenceIsInvalid() { t := s.T() assertSkopeoFails(t, `.*Invalid image name.*`, "copy", "unknown:transport", "unknown:test") } + +func (s *copySuite) TestInsecurePolicyAndRequireSignedConflict() { + t := s.T() + assertSkopeoFails(t, ".*--insecure-policy and --require-signed are mutually exclusive.*", + "--insecure-policy", "--require-signed", "inspect", "dir:/nonexistent") +} + +func (s *copySuite) TestRequireSignedAcceptsSignedImage() { + t := s.T() + mech, err := signature.NewGPGSigningMechanism() + require.NoError(t, err) + defer mech.Close() + if err := mech.SupportsSigning(); err != nil { + t.Skipf("Signing not supported: %v", err) + } + + srcDir := t.TempDir() + + // get an image to work with + assertSkopeoSucceeds(t, "", "copy", "--retry-times", "3", + testFQIN64, "dir:"+srcDir) + + // first, sanity-check that without --require-signed, we can copy it since by default, `dir:` is insecureAcceptAnything + destDir1 := t.TempDir() + assertSkopeoSucceeds(t, "", "copy", "dir:"+srcDir, "dir:"+destDir1) + + // now verify that copying fails with --require-signed + destDir2 := t.TempDir() + assertSkopeoFails(t, ".*Source image rejected: No signature verification policy found for image.*", + "--require-signed", "copy", + "dir:"+srcDir, "dir:"+destDir2) + + // sign the image + manifestPath := filepath.Join(srcDir, "manifest.json") + signaturePath := filepath.Join(srcDir, "signature-1") + dockerReference := "localhost/test:latest" + + assertSkopeoSucceeds(t, "", "standalone-sign", + "-o", signaturePath, + manifestPath, dockerReference, s.fingerprint) + + // sanity-check signature file is there + _, err = os.Stat(signaturePath) + require.NoError(t, err) + + // create a basic policy that requires signatures + policy := map[string]any{ + "default": []map[string]any{{ + "type": "signedBy", + "keyType": "GPGKeys", + "keyPath": filepath.Join(s.gpgHome, "personal-pubkey.gpg"), + "signedIdentity": map[string]any{ + "type": "exactRepository", + "dockerRepository": dockerReference, + }, + }}, + } + policyJSON, err := json.Marshal(policy) + require.NoError(t, err) + + policyFile, err := os.CreateTemp("", "policy-*.json") + require.NoError(t, err) + t.Cleanup(func() { os.Remove(policyFile.Name()) }) + _, err = policyFile.Write(policyJSON) + require.NoError(t, err) + err = policyFile.Close() + require.NoError(t, err) + + // now copying with --require-signed should pass + destDir3 := t.TempDir() + assertSkopeoSucceeds(t, "", "--policy", policyFile.Name(), "--require-signed", "copy", + "dir:"+srcDir, "dir:"+destDir3) + + // Delete the signature and sanity-check that copying fails. This doesn't + // strictly test --require-signed, but rather the PolicyRequirements logic, but + // it makes the test feel complete. + err = os.Remove(signaturePath) + require.NoError(t, err) + + destDir4 := t.TempDir() + assertSkopeoFails(t, ".*Source image rejected: A signature was required, but no signature exists.*", + "--policy", policyFile.Name(), "--require-signed", "copy", + "dir:"+srcDir, "dir:"+destDir4) +}