Skip to content

Commit 77893fc

Browse files
committed
Add support for validating signatures from PGP subkeys
This change should update the gpgme and openpgp implmenetations to support validating signatures made by signing subkeys. It will close an open issue that was referenced specifically in [podman](containers/podman#16406), but should resolve this across a number of tools that use this library. Signed-off-by: Danny Grove <danny@dannygrove.com>
1 parent 539b1c5 commit 77893fc

13 files changed

Lines changed: 227 additions & 10 deletions

signature/docker.go

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,27 @@ func SignDockerManifest(m []byte, dockerReference string, mech SigningMechanism,
5151
// using mech.
5252
func VerifyDockerManifestSignature(unverifiedSignature, unverifiedManifest []byte,
5353
expectedDockerReference string, mech SigningMechanism, expectedKeyIdentity string) (*Signature, error) {
54-
sig, _, err := VerifyImageManifestSignatureUsingKeyIdentityList(unverifiedSignature, unverifiedManifest, expectedDockerReference, mech, []string{expectedKeyIdentity})
55-
return sig, err
56-
}
54+
_, signerKeyIdentity, err := mech.Verify(unverifiedSignature)
55+
if err != nil {
56+
return nil, err
57+
}
58+
59+
// If signerKeyIdentity is different from expectedKeyIdentity, check if it's a valid subkey
60+
if signerKeyIdentity != expectedKeyIdentity {
61+
// Implementation depends on mechanism-specific functionality to verify subkey relationships
62+
// This could be a new method on the SigningMechanism interface or mechanism-specific implementation
63+
isValidSubkey, err := isKeyOrValidSubkey(mech, signerKeyIdentity, expectedKeyIdentity)
64+
if err != nil {
65+
return nil, err
66+
}
67+
if !isValidSubkey {
68+
return nil, fmt.Errorf("signature by key %s does not match expected key %s",
69+
signerKeyIdentity, expectedKeyIdentity)
70+
}
71+
}
72+
sig, _, err := VerifyImageManifestSignatureUsingKeyIdentityList(unverifiedSignature, unverifiedManifest, expectedDockerReference, mech, []string{expectedKeyIdentity})
73+
return sig, err
74+
}
5775

5876
// VerifyImageManifestSignatureUsingKeyIdentityList checks that unverifiedSignature uses one of the expectedKeyIdentities
5977
// to sign unverifiedManifest as expectedDockerReference, using mech. Returns the verified signature and the key identity that
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
{
2+
"schemaVersion": 2,
3+
"mediaType": "application/vnd.docker.distribution.manifest.v2+json",
4+
"config": {
5+
"mediaType": "application/vnd.docker.container.image.v1+json",
6+
"size": 7023,
7+
"digest": "sha256:b5b2b2c507a0944348e0303114d8d93aaaa081732b86451d9bce1f432a537bc7"
8+
},
9+
"layers": [
10+
{
11+
"mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip",
12+
"size": 32654,
13+
"digest": "sha256:e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7fc331f"
14+
},
15+
{
16+
"mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip",
17+
"size": 16724,
18+
"digest": "sha256:3c3a4604a545cdc127456d94e421cd355bca5b528f4a9c1905b15da2eb4a4c6b"
19+
},
20+
{
21+
"mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip",
22+
"size": 73109,
23+
"digest": "sha256:ec4b8955958665577945c89419d1af06b5f7636b4ac3da7f12184802ad867736"
24+
}
25+
]
26+
}
405 Bytes
Binary file not shown.
907 Bytes
Binary file not shown.

signature/fixtures/pubring.gpg

959 Bytes
Binary file not shown.

signature/fixtures/trustdb.gpg

80 Bytes
Binary file not shown.

signature/fixtures_info_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ const (
1919
TestKeyFingerprintWithPassphrase = "E3EB7611D815211F141946B5B0CDE60B42557346"
2020
// TestPassphrase is the passphrase for TestKeyFingerprintWithPassphrase.
2121
TestPassphrase = "WithPassphrase123"
22+
// TestKeyFingerprintWithSubkey is the fingerprint of the private key with a signing subkey in this directory.
23+
TestKeyFingerprintWithSubkey = "D3163372E124602D6D7C9FF202C264661307033F"
2224
)
2325

2426
var (

signature/mechanism.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,9 @@ type SigningMechanism interface {
2727
// Sign creates a (non-detached) signature of input using keyIdentity.
2828
// Fails with a SigningNotSupportedError if the mechanism does not support signing.
2929
Sign(input []byte, keyIdentity string) ([]byte, error)
30-
// Verify parses unverifiedSignature and returns the content and the signer's identity
30+
// Verify parses unverifiedSignature and returns the content and the signer's identity.
31+
// For GPG signatures, the signer's identity is the fingerprint of the key or subkey
32+
// that created the signature, not necessarily the primary key fingerprint.
3133
Verify(unverifiedSignature []byte) (contents []byte, keyIdentity string, err error)
3234
// UntrustedSignatureContents returns UNTRUSTED contents of the signature WITHOUT ANY VERIFICATION,
3335
// along with a short identifier of the key used for signing.

signature/mechanism_gpgme.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,3 +204,59 @@ func (m *gpgmeSigningMechanism) Verify(unverifiedSignature []byte) (contents []b
204204
func (m *gpgmeSigningMechanism) UntrustedSignatureContents(untrustedSignature []byte) (untrustedContents []byte, shortKeyIdentifier string, err error) {
205205
return gpgUntrustedSignatureContents(untrustedSignature)
206206
}
207+
208+
// isSubkeyOf checks if signerKeyIdentity is a subkey of expectedKeyIdentity
209+
func (m *gpgmeSigningMechanism) isSubkeyOf(signerKeyIdentity, expectedKeyIdentity string) (bool, error) {
210+
// Load the key for expectedKeyIdentity
211+
key, err := m.ctx.GetKey(expectedKeyIdentity, false)
212+
if err != nil {
213+
return false, err
214+
}
215+
216+
// Check if signerKeyIdentity is a subkey of the primary key
217+
// Get the first subkey (which should be the primary key itself)
218+
subkey := key.SubKeys()
219+
220+
// Skip the primary key if needed and iterate through all subkeys
221+
for subkey != nil {
222+
// Check if the current subkey's fingerprint matches the signer's identity
223+
if subkey.Fingerprint() == signerKeyIdentity {
224+
return true, nil
225+
}
226+
227+
// Move to the next subkey
228+
subkey = subkey.Next()
229+
}
230+
231+
// If we didn't find it as a subkey, check if it might be the primary key itself
232+
// (in case the expectedKeyIdentity is actually a subkey)
233+
key, err = m.ctx.GetKey(signerKeyIdentity, false)
234+
if err != nil {
235+
return false, err
236+
}
237+
238+
// Get the primary key fingerprint
239+
primarySubkey := key.SubKeys()
240+
if primarySubkey != nil && primarySubkey.Fingerprint() == expectedKeyIdentity {
241+
return true, nil
242+
}
243+
244+
return false, nil
245+
}
246+
247+
// isKeyOrValidSubkey checks if the signerKeyIdentity is either the same as the expectedKeyIdentity, or is a valid
248+
// subkey of the expecctedKeyIdentity
249+
func isKeyOrValidSubkey(mech SigningMechanism, signerKeyIdentity, expectedKeyIdentity string) (bool, error) {
250+
// If they're the same, no need to check subkey relationship
251+
if signerKeyIdentity == expectedKeyIdentity {
252+
return true, nil
253+
}
254+
255+
// For gpgme mechanism, check subkey relationships
256+
if gpgmeMech, ok := mech.(*gpgmeSigningMechanism); ok {
257+
return gpgmeMech.isSubkeyOf(signerKeyIdentity, expectedKeyIdentity)
258+
}
259+
260+
// For other mechanisms, only accept exact matches
261+
return false, nil
262+
}

signature/mechanism_openpgp.go

Lines changed: 100 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,8 @@ func (m *openpgpSigningMechanism) Verify(unverifiedSignature []byte) (contents [
166166
}
167167

168168
// Uppercase the fingerprint to be compatible with gpgme
169-
return content, strings.ToUpper(fmt.Sprintf("%x", md.SignedBy.PublicKey.Fingerprint)), nil
169+
keyIdentity = strings.ToUpper(fmt.Sprintf("%x", md.SignedBy.PublicKey.Fingerprint))
170+
return content, keyIdentity, nil
170171
}
171172

172173
// UntrustedSignatureContents returns UNTRUSTED contents of the signature WITHOUT ANY VERIFICATION,
@@ -177,3 +178,101 @@ func (m *openpgpSigningMechanism) Verify(unverifiedSignature []byte) (contents [
177178
func (m *openpgpSigningMechanism) UntrustedSignatureContents(untrustedSignature []byte) (untrustedContents []byte, shortKeyIdentifier string, err error) {
178179
return gpgUntrustedSignatureContents(untrustedSignature)
179180
}
181+
182+
// isSubkeyOf checks if signerKeyIdentity is a subkey of expectedKeyIdentity
183+
func (m *openpgpSigningMechanism) isSubkeyOf(signerKeyIdentity, expectedKeyIdentity string) (bool, error) {
184+
// Convert fingerprints to lowercase for comparison
185+
signerFingerprint := strings.ToLower(signerKeyIdentity)
186+
expectedFingerprint := strings.ToLower(expectedKeyIdentity)
187+
188+
// If they're the same, it's a match
189+
if signerFingerprint == expectedFingerprint {
190+
return true, nil
191+
}
192+
193+
// Find the entity with the expected fingerprint
194+
var expectedEntity *openpgp.Entity
195+
var signerEntity *openpgp.Entity
196+
197+
for _, entity := range m.keyring {
198+
// Check if this entity's primary key matches the expected fingerprint
199+
if entity.PrimaryKey != nil {
200+
primaryFingerprint := strings.ToLower(fmt.Sprintf("%x", entity.PrimaryKey.Fingerprint))
201+
if primaryFingerprint == expectedFingerprint {
202+
expectedEntity = entity
203+
}
204+
if primaryFingerprint == signerFingerprint {
205+
signerEntity = entity
206+
}
207+
}
208+
209+
// Also check subkeys
210+
for _, subkey := range entity.Subkeys {
211+
if subkey.PublicKey != nil {
212+
subkeyFingerprint := strings.ToLower(fmt.Sprintf("%x", subkey.PublicKey.Fingerprint))
213+
if subkeyFingerprint == expectedFingerprint {
214+
expectedEntity = entity
215+
}
216+
if subkeyFingerprint == signerFingerprint {
217+
signerEntity = entity
218+
}
219+
}
220+
}
221+
}
222+
223+
// If we couldn't find either key, return false
224+
if expectedEntity == nil || signerEntity == nil {
225+
return false, nil
226+
}
227+
228+
// Case 1: Both keys belong to the same entity - this is valid
229+
if expectedEntity == signerEntity {
230+
return true, nil
231+
}
232+
233+
// Case 2: The signer is a subkey of the expected entity
234+
if expectedEntity.PrimaryKey != nil {
235+
expectedPrimaryFingerprint := strings.ToLower(fmt.Sprintf("%x", expectedEntity.PrimaryKey.Fingerprint))
236+
if expectedPrimaryFingerprint == expectedFingerprint {
237+
// The expected key is a primary key, check if signer is one of its subkeys
238+
for _, subkey := range expectedEntity.Subkeys {
239+
if subkey.PublicKey != nil {
240+
subkeyFingerprint := strings.ToLower(fmt.Sprintf("%x", subkey.PublicKey.Fingerprint))
241+
if subkeyFingerprint == signerFingerprint {
242+
return true, nil
243+
}
244+
}
245+
}
246+
}
247+
}
248+
249+
// Case 3: The expected key is a subkey, and the signer is the primary key of the same entity
250+
for _, subkey := range signerEntity.Subkeys {
251+
if subkey.PublicKey != nil {
252+
subkeyFingerprint := strings.ToLower(fmt.Sprintf("%x", subkey.PublicKey.Fingerprint))
253+
if subkeyFingerprint == expectedFingerprint {
254+
// The expected key is a subkey of the signer's entity
255+
return true, nil
256+
}
257+
}
258+
}
259+
260+
return false, nil
261+
}
262+
263+
// isKeyOrValidSubkey checks if the signerKeyIdentity is either the same as the expectedKeyIdentity, or is a valid
264+
// subkey of the expectedKeyIdentity
265+
func isKeyOrValidSubkey(mech SigningMechanism, signerKeyIdentity, expectedKeyIdentity string) (bool, error) {
266+
// If they're the same, no need to check subkey relationship
267+
if signerKeyIdentity == expectedKeyIdentity {
268+
return true, nil
269+
}
270+
271+
// For openpgp mechanism, check subkey relationships
272+
if openpgpMech, ok := mech.(*openpgpSigningMechanism); ok {
273+
return openpgpMech.isSubkeyOf(signerKeyIdentity, expectedKeyIdentity)
274+
}
275+
276+
// For other mechanisms, only accept exact matches
277+
return false, nil
278+
}

0 commit comments

Comments
 (0)