Add support for validating signatures from PGP subkeys

signature/docker.go

@@ -51,6 +51,24 @@ func SignDockerManifest(m []byte, dockerReference string, mech SigningMechanism,
 // using mech.
 func VerifyDockerManifestSignature(unverifiedSignature, unverifiedManifest []byte,
 	expectedDockerReference string, mech SigningMechanism, expectedKeyIdentity string) (*Signature, error) {
+	_, signerKeyIdentity, err := mech.Verify(unverifiedSignature)
+	if err != nil {
+		return nil, err
+	}
+
+	// If signerKeyIdentity is different from expectedKeyIdentity, check if it's a valid subkey
+	if signerKeyIdentity != expectedKeyIdentity {
+		// Implementation depends on mechanism-specific functionality to verify subkey relationships
+		// This could be a new method on the SigningMechanism interface or mechanism-specific implementation
+		isValidSubkey, err := isKeyOrValidSubkey(mech, signerKeyIdentity, expectedKeyIdentity)
+		if err != nil {
+			return nil, err
+		}
+		if !isValidSubkey {
+			return nil, fmt.Errorf("signature by key %s does not match expected key %s",
+				signerKeyIdentity, expectedKeyIdentity)
+		}
+	}
 	sig, _, err := VerifyImageManifestSignatureUsingKeyIdentityList(unverifiedSignature, unverifiedManifest, expectedDockerReference, mech, []string{expectedKeyIdentity})
 	return sig, err
 }

signature/fixtures/.gitignore

@@ -1,6 +1,8 @@
 /*.gpg~
 /.gpg-v21-migrated
 /private-keys-v1.d
+/openpgp-revocs.d
 /random_seed
 /gnupg_spawn_agent_sentinel.lock
+/tofu.db
 /.#*

signature/fixtures/dir-img-valid-3/manifest.json

@@ -0,0 +1,26 @@
+{
+    "schemaVersion": 2,
+    "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
+    "config": {
+        "mediaType": "application/vnd.docker.container.image.v1+json",
+        "size": 7023,
+        "digest": "sha256:b5b2b2c507a0944348e0303114d8d93aaaa081732b86451d9bce1f432a537bc7"
+    },
+    "layers": [
+        {
+            "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip",
+            "size": 32654,
+            "digest": "sha256:e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7fc331f"
+        },
+        {
+            "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip",
+            "size": 16724,
+            "digest": "sha256:3c3a4604a545cdc127456d94e421cd355bca5b528f4a9c1905b15da2eb4a4c6b"
+        },
+        {
+            "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip",
+            "size": 73109,
+            "digest": "sha256:ec4b8955958665577945c89419d1af06b5f7636b4ac3da7f12184802ad867736"
+        }
+    ]
+}

signature/fixtures_info_test.go

@@ -19,6 +19,10 @@ const (
 	TestKeyFingerprintWithPassphrase = "F2B501009F78B0B340221A12A3CD242DA6028093"
 	// TestPassphrase is the passphrase for TestKeyFingerprintWithPassphrase.
 	TestPassphrase = "WithPassphrase123"
+	// TestKeyFingerprintWithSubkey is the fingerpring of the private key with a signing subkey in this directory.
+	TestKeyFingerprintWithSubkey = "92697D73924E7ADC91C14A001065C1A47B86FD50"
+	// TestKeyFingerprintWithRevokedSubkey is the fingerprint of the private key with a revoked signing subkey in this directory.
+	TestKeyFingerprintWithRevokedSubkey = "F501EDF1A5CD65421FCB51B57EB92DC581193558"
 )
 
 var (

signature/mechanism.go

@@ -27,7 +27,9 @@ type SigningMechanism interface {
 	// Sign creates a (non-detached) signature of input using keyIdentity.
 	// Fails with a SigningNotSupportedError if the mechanism does not support signing.
 	Sign(input []byte, keyIdentity string) ([]byte, error)
-	// Verify parses unverifiedSignature and returns the content and the signer's identity
+	// Verify parses unverifiedSignature and returns the content and the signer's identity.
+	// For GPG signatures, the signer's identity is the fingerprint of the key or subkey
+	// that created the signature, not necessarily the primary key fingerprint.
 	Verify(unverifiedSignature []byte) (contents []byte, keyIdentity string, err error)
 	// UntrustedSignatureContents returns UNTRUSTED contents of the signature WITHOUT ANY VERIFICATION,
 	// along with a short identifier of the key used for signing.

signature/mechanism_gpgme.go

@@ -204,3 +204,59 @@ func (m *gpgmeSigningMechanism) Verify(unverifiedSignature []byte) (contents []b
 func (m *gpgmeSigningMechanism) UntrustedSignatureContents(untrustedSignature []byte) (untrustedContents []byte, shortKeyIdentifier string, err error) {
 	return gpgUntrustedSignatureContents(untrustedSignature)
 }
+
+// isSubkeyOf checks if signerKeyIdentity is a subkey of expectedKeyIdentity
+func (m *gpgmeSigningMechanism) isSubkeyOf(signerKeyIdentity, expectedKeyIdentity string) (bool, error) {
+	// Load the key for expectedKeyIdentity
+	key, err := m.ctx.GetKey(expectedKeyIdentity, false)
+	if err != nil {
+		return false, err
+	}
+
+	// Check if signerKeyIdentity is a subkey of the primary key
+	// Get the first subkey (which should be the primary key itself)
+	subkey := key.SubKeys()
+
+	// Skip the primary key if needed and iterate through all subkeys
+	for subkey != nil {
+		// Check if the current subkey's fingerprint matches the signer's identity
+		if subkey.Fingerprint() == signerKeyIdentity {
+			return true, nil
+		}
+
+		// Move to the next subkey
+		subkey = subkey.Next()
+	}
+
+	// If we didn't find it as a subkey, check if it might be the primary key itself
+	// (in case the expectedKeyIdentity is actually a subkey)
+	key, err = m.ctx.GetKey(signerKeyIdentity, false)
+	if err != nil {
+		return false, err
+	}
+
+	// Get the primary key fingerprint
+	primarySubkey := key.SubKeys()
+	if primarySubkey != nil && primarySubkey.Fingerprint() == expectedKeyIdentity {
+		return true, nil
+	}
+
+	return false, nil
+}
+
+// isKeyOrValidSubkey checks if the signerKeyIdentity is either the same as the expectedKeyIdentity, or is a valid
+// subkey of the expectedKeyIdentity
+func isKeyOrValidSubkey(mech SigningMechanism, signerKeyIdentity, expectedKeyIdentity string) (bool, error) {
+	// If they're the same, no need to check subkey relationship
+	if signerKeyIdentity == expectedKeyIdentity {
+		return true, nil
+	}
+
+	// For gpgme mechanism, check subkey relationships
+	if gpgmeMech, ok := mech.(*gpgmeSigningMechanism); ok {
+		return gpgmeMech.isSubkeyOf(signerKeyIdentity, expectedKeyIdentity)
+	}
+
+	// For other mechanisms, only accept exact matches
+	return false, nil
+}

signature/mechanism_openpgp.go

@@ -166,7 +166,8 @@ func (m *openpgpSigningMechanism) Verify(unverifiedSignature []byte) (contents [
 	}
 
 	// Uppercase the fingerprint to be compatible with gpgme
-	return content, strings.ToUpper(fmt.Sprintf("%x", md.SignedBy.PublicKey.Fingerprint)), nil
+	keyIdentity = strings.ToUpper(fmt.Sprintf("%x", md.SignedBy.PublicKey.Fingerprint))
+	return content, keyIdentity, nil
 }
 
 // UntrustedSignatureContents returns UNTRUSTED contents of the signature WITHOUT ANY VERIFICATION,
@@ -177,3 +178,101 @@ func (m *openpgpSigningMechanism) Verify(unverifiedSignature []byte) (contents [
 func (m *openpgpSigningMechanism) UntrustedSignatureContents(untrustedSignature []byte) (untrustedContents []byte, shortKeyIdentifier string, err error) {
 	return gpgUntrustedSignatureContents(untrustedSignature)
 }
+
+// isSubkeyOf checks if signerKeyIdentity is a subkey of expectedKeyIdentity
+func (m *openpgpSigningMechanism) isSubkeyOf(signerKeyIdentity, expectedKeyIdentity string) (bool, error) {
+	// Convert fingerprints to lowercase for comparison
+	signerFingerprint := strings.ToLower(signerKeyIdentity)
+	expectedFingerprint := strings.ToLower(expectedKeyIdentity)
+
+	// If they're the same, it's a match
+	if signerFingerprint == expectedFingerprint {
+		return true, nil
+	}
+
+	// Find the entity with the expected fingerprint
+	var expectedEntity *openpgp.Entity
+	var signerEntity *openpgp.Entity
+
+	for _, entity := range m.keyring {
+		// Check if this entity's primary key matches the expected fingerprint
+		if entity.PrimaryKey != nil {
+			primaryFingerprint := strings.ToLower(fmt.Sprintf("%x", entity.PrimaryKey.Fingerprint))
+			if primaryFingerprint == expectedFingerprint {
+				expectedEntity = entity
+			}
+			if primaryFingerprint == signerFingerprint {
+				signerEntity = entity
+			}
+		}
+
+		// Also check subkeys
+		for _, subkey := range entity.Subkeys {
+			if subkey.PublicKey != nil {
+				subkeyFingerprint := strings.ToLower(fmt.Sprintf("%x", subkey.PublicKey.Fingerprint))
+				if subkeyFingerprint == expectedFingerprint {
+					expectedEntity = entity
+				}
+				if subkeyFingerprint == signerFingerprint {
+					signerEntity = entity
+				}
+			}
+		}
+	}
+
+	// If we couldn't find either key, return false
+	if expectedEntity == nil || signerEntity == nil {
+		return false, nil
+	}
+
+	// Case 1: Both keys belong to the same entity - this is valid
+	if expectedEntity == signerEntity {
+		return true, nil
+	}
+
+	// Case 2: The signer is a subkey of the expected entity
+	if expectedEntity.PrimaryKey != nil {
+		expectedPrimaryFingerprint := strings.ToLower(fmt.Sprintf("%x", expectedEntity.PrimaryKey.Fingerprint))
+		if expectedPrimaryFingerprint == expectedFingerprint {
+			// The expected key is a primary key, check if signer is one of its subkeys
+			for _, subkey := range expectedEntity.Subkeys {
+				if subkey.PublicKey != nil {
+					subkeyFingerprint := strings.ToLower(fmt.Sprintf("%x", subkey.PublicKey.Fingerprint))
+					if subkeyFingerprint == signerFingerprint {
+						return true, nil
+					}
+				}
+			}
+		}
+	}
+
+	// Case 3: The expected key is a subkey, and the signer is the primary key of the same entity
+	for _, subkey := range signerEntity.Subkeys {
+		if subkey.PublicKey != nil {
+			subkeyFingerprint := strings.ToLower(fmt.Sprintf("%x", subkey.PublicKey.Fingerprint))
+			if subkeyFingerprint == expectedFingerprint {
+				// The expected key is a subkey of the signer's entity
+				return true, nil
+			}
+		}
+	}
+
+	return false, nil
+}
+
+// isKeyOrValidSubkey checks if the signerKeyIdentity is either the same as the expectedKeyIdentity, or is a valid
+// subkey of the expectedKeyIdentity
+func isKeyOrValidSubkey(mech SigningMechanism, signerKeyIdentity, expectedKeyIdentity string) (bool, error) {
+	// If they're the same, no need to check subkey relationship
+	if signerKeyIdentity == expectedKeyIdentity {
+		return true, nil
+	}
+
+	// For openpgp mechanism, check subkey relationships
+	if openpgpMech, ok := mech.(*openpgpSigningMechanism); ok {
+		return openpgpMech.isSubkeyOf(signerKeyIdentity, expectedKeyIdentity)
+	}
+
+	// For other mechanisms, only accept exact matches
+	return false, nil
+}

signature/mechanism_test.go

@@ -143,7 +143,16 @@ func TestNewEphemeralGPGSigningMechanism(t *testing.T) {
 	mech, keyIdentities, err = NewEphemeralGPGSigningMechanism(bytes.Join([][]byte{keyBlob, keyBlob}, nil))
 	require.NoError(t, err)
 	defer mech.Close()
-	assert.Equal(t, []string{TestKeyFingerprintWithPassphrase, TestKeyFingerprint, TestKeyFingerprintWithPassphrase, TestKeyFingerprint}, keyIdentities)
+	assert.Equal(t, []string{
+		TestKeyFingerprintWithPassphrase,
+		TestKeyFingerprint,
+		TestKeyFingerprintWithSubkey,
+		TestKeyFingerprintWithRevokedSubkey,
+		TestKeyFingerprintWithPassphrase,
+		TestKeyFingerprint,
+		TestKeyFingerprintWithSubkey,
+		TestKeyFingerprintWithRevokedSubkey,
+	}, keyIdentities)
 
 	// Two keys from two blobs:
 	keyBlob1, err := os.ReadFile("./fixtures/public-key-1.gpg")

signature/policy_eval_signedby.go

@@ -6,7 +6,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"slices"
 
 	"github.com/containers/image/v5/internal/multierr"
 	"github.com/containers/image/v5/internal/private"
@@ -52,8 +51,15 @@ func (pr *prSignedBy) isSignatureAuthorAccepted(ctx context.Context, image priva
 
 	signature, err := verifyAndExtractSignature(mech, sig, signatureAcceptanceRules{
 		validateKeyIdentity: func(keyIdentity string) error {
-			if slices.Contains(trustedIdentities, keyIdentity) {
-				return nil
+			// Iterate over the keys, validate if the provided keyIdentity is either the key itself or a valid
+			// subkey
+			for _, allowedKeyIdentity := range trustedIdentities {
+				if isValid, err := isKeyOrValidSubkey(mech, keyIdentity, allowedKeyIdentity); err != nil {
+					// Throwing an error shouldn't exit the program, we should just move to the next key
+					continue
+				} else if isValid {
+					return nil
+				}
 			}
 			// Coverage: We use a private GPG home directory and only import trusted keys, so this should
 			// not be reachable.

signature/policy_eval_signedby_test.go

@@ -207,6 +207,15 @@ func TestPRSignedByIsSignatureAuthorAccepted(t *testing.T) {
 	require.NoError(t, err)
 	sar, parsedSig, err = pr.isSignatureAuthorAccepted(context.Background(), image, sig)
 	assertSARRejectedPolicyRequirement(t, sar, parsedSig, err)
+
+	// An invalid signature from a revoked signing subkey
+	image = dirImageMock(t, "fixtures/dir-img-rev-subkey", "testing/manifest:latest")
+	sig, err = os.ReadFile("fixtures/dir-img-rev-subkey/signature-1")
+	require.NoError(t, err)
+	pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key-4.gpg", prm)
+	require.NoError(t, err)
+	sar, parsedSig, err = pr.isSignatureAuthorAccepted(context.Background(), image, sig)
+	assertSARRejected(t, sar, parsedSig, err)
 }
 
 // createInvalidSigDir creates a directory suitable for dirImageMock, in which image.Signatures()
@@ -277,4 +286,11 @@ func TestPRSignedByIsRunningImageAllowed(t *testing.T) {
 	require.NoError(t, err)
 	allowed, err = pr.isRunningImageAllowed(context.Background(), image)
 	assertRunningRejectedPolicyRequirement(t, allowed, err)
+
+	// 1 valid signature from a subkey
+	image = dirImageMock(t, "fixtures/dir-img-valid-3", "testing/manifest:latest")
+	pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key-3.gpg", prm)
+	require.NoError(t, err)
+	allowed, err = pr.isRunningImageAllowed(context.Background(), image)
+	assertRunningAllowed(t, allowed, err)
 }