Skip to content

Commit 16b1b24

Browse files
committed
Also accept subkey signatures in simple signing
This does not change behavior for mechanisms which already always return the primary key fingerprint (after a recent change, openpgp); for the others, add support for signingMechanismWithVerificationIdentityLookup . Signed-off-by: Miloslav Trmač <mitr@redhat.com>
1 parent 1d69b2d commit 16b1b24

File tree

5 files changed

+50
-3
lines changed

5 files changed

+50
-3
lines changed
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
../image.manifest.json
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
../subkey.signature

signature/policy_eval_signedby_test.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,4 +277,11 @@ func TestPRSignedByIsRunningImageAllowed(t *testing.T) {
277277
require.NoError(t, err)
278278
allowed, err = pr.isRunningImageAllowed(context.Background(), image)
279279
assertRunningRejectedPolicyRequirement(t, allowed, err)
280+
281+
// A valid signature using a subkey
282+
image = dirImageMock(t, "fixtures/dir-img-valid-subkey", "testing/manifest:latest")
283+
pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key-with-subkey.gpg", prm)
284+
require.NoError(t, err)
285+
allowed, err = pr.isRunningImageAllowed(context.Background(), image)
286+
assertRunningAllowed(t, allowed, err)
280287
}

signature/simple.go

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,21 @@ func verifyAndExtractSignature(mech SigningMechanism, unverifiedSignature []byte
222222
return nil, "", err
223223
}
224224
if !slices.Contains(rules.acceptedKeyIdentities, keyIdentity) {
225-
return nil, "", internal.NewInvalidSignatureError(fmt.Sprintf("signature by key %s is not accepted", keyIdentity))
225+
withLookup, ok := mech.(signingMechanismWithVerificationIdentityLookup)
226+
if !ok {
227+
return nil, "", internal.NewInvalidSignatureError(fmt.Sprintf("signature by key %s is not accepted", keyIdentity))
228+
}
229+
230+
primaryKey, err := withLookup.keyIdentityForVerificationKeyIdentity(keyIdentity)
231+
if err != nil {
232+
// Coverage: This only fails if lookup by keyIdentity fails, but we just found and used that key.
233+
// Or maybe on some unexpected I/O error.
234+
return nil, "", err
235+
}
236+
if !slices.Contains(rules.acceptedKeyIdentities, primaryKey) {
237+
return nil, "", internal.NewInvalidSignatureError(fmt.Sprintf("signature by key %s of %s is not accepted", keyIdentity, primaryKey))
238+
}
239+
keyIdentity = primaryKey
226240
}
227241

228242
var unmatchedSignature untrustedSignature

signature/simple_test.go

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -319,12 +319,26 @@ func TestVerifyAndExtractSignature(t *testing.T) {
319319
assert.Equal(t, TestKeyFingerprint, keyIdentity)
320320
assert.Equal(t, signatureData, *recorded)
321321
}
322+
// Successful verification using a subkey
323+
sig2, err := os.ReadFile("./fixtures/subkey.signature")
324+
require.NoError(t, err)
325+
sig2Data := tuple{
326+
signedDockerReference: "testing/manifest:latest",
327+
signedDockerManifestDigest: TestImageManifestDigest,
328+
}
329+
recorded, recordingRules := setupRecording(sig2Data, TestKeyFingerprintPrimaryWithSubkey)
330+
sig, keyIdentity, err := verifyAndExtractSignature(mech, sig2, recordingRules)
331+
require.NoError(t, err)
332+
assert.Equal(t, sig2Data.signedDockerReference, sig.DockerReference)
333+
assert.Equal(t, sig2Data.signedDockerManifestDigest, sig.DockerManifestDigest)
334+
assert.Equal(t, TestKeyFingerprintPrimaryWithSubkey, keyIdentity)
335+
assert.Equal(t, sig2Data, *recorded)
322336

323337
// For extra paranoia, test that we return a nil signature object and a "" key identity on error.
324338

325339
// Completely invalid signature.
326-
recorded, recordingRules := setupRecording(signatureData, TestKeyFingerprint)
327-
sig, keyIdentity, err := verifyAndExtractSignature(mech, []byte{}, recordingRules)
340+
recorded, recordingRules = setupRecording(signatureData, TestKeyFingerprint)
341+
sig, keyIdentity, err = verifyAndExtractSignature(mech, []byte{}, recordingRules)
328342
assert.Error(t, err)
329343
assert.Nil(t, sig)
330344
assert.Equal(t, "", keyIdentity)
@@ -345,6 +359,16 @@ func TestVerifyAndExtractSignature(t *testing.T) {
345359
assert.Equal(t, "", keyIdentity)
346360
assert.Equal(t, tuple{}, *recorded)
347361

362+
// Valid signature with a revoked subkey
363+
sig2, err = os.ReadFile("./fixtures/subkey-revoked.signature")
364+
require.NoError(t, err)
365+
recorded, recordingRules = setupRecording(sig2Data, TestKeyFingerprintPrimaryWithRevokedSubkey) // sig2Data describes subkey-revoked.signature as well.
366+
sig, keyIdentity, err = verifyAndExtractSignature(mech, sig2, recordingRules)
367+
assert.Error(t, err)
368+
assert.Nil(t, sig)
369+
assert.Equal(t, "", keyIdentity)
370+
assert.Equal(t, tuple{}, *recorded)
371+
348372
// Valid signature of non-JSON: used acceptedKeyIdentities only
349373
invalidBlobSignature, err := os.ReadFile("./fixtures/invalid-blob.signature")
350374
require.NoError(t, err)

0 commit comments

Comments
 (0)