Skip to content

Commit 2cb0c15

Browse files
authored
Merge pull request #981 from mtrmac/openpgp
INCOMPATIBLE: Replace golang.org/x/crypto/openpgp with github.com/ProtonMail/go-crypto/openpgp
2 parents 894d257 + 28e221c commit 2cb0c15

32 files changed

Lines changed: 106 additions & 7153 deletions

image/go.mod

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ go 1.25.7
88
require (
99
dario.cat/mergo v1.0.2
1010
github.com/BurntSushi/toml v1.6.0
11+
github.com/ProtonMail/go-crypto v1.4.1
1112
github.com/containers/libtrust v0.0.0-20230121012942-c1716e8a8d01
1213
github.com/containers/ocicrypt v1.3.2
1314
github.com/cyberphone/json-canonicalization v0.0.0-20241213102144-19d51d7fe467
@@ -39,7 +40,6 @@ require (
3940
go.etcd.io/bbolt v1.5.0
4041
go.podman.io/storage v1.63.0
4142
go.yaml.in/yaml/v3 v3.0.4
42-
golang.org/x/crypto v0.54.0
4343
golang.org/x/oauth2 v0.36.0
4444
golang.org/x/sync v0.22.0
4545
golang.org/x/term v0.45.0
@@ -48,7 +48,6 @@ require (
4848
require (
4949
cyphar.com/go-pathrs v0.2.5 // indirect
5050
github.com/Microsoft/go-winio v0.6.2 // indirect
51-
github.com/ProtonMail/go-crypto v1.4.1 // indirect
5251
github.com/VividCortex/ewma v1.2.0 // indirect
5352
github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d // indirect
5453
github.com/cespare/xxhash/v2 v2.3.0 // indirect
@@ -98,6 +97,7 @@ require (
9897
go.opentelemetry.io/otel v1.44.0 // indirect
9998
go.opentelemetry.io/otel/metric v1.44.0 // indirect
10099
go.opentelemetry.io/otel/trace v1.44.0 // indirect
100+
golang.org/x/crypto v0.54.0 // indirect
101101
golang.org/x/net v0.56.0 // indirect
102102
golang.org/x/sys v0.47.0 // indirect
103103
golang.org/x/text v0.40.0 // indirect

image/signature/mechanism.go

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,7 @@ import (
99
"io"
1010
"strings"
1111

12-
// This code is used only to parse the data in an explicitly-untrusted
13-
// code path, where cryptography is not relevant. For now, continue to
14-
// use this frozen deprecated implementation. When mechanism_openpgp.go
15-
// migrates to another implementation, this should migrate as well.
16-
//lint:ignore SA1019 See above
17-
"golang.org/x/crypto/openpgp" //nolint:staticcheck
12+
"github.com/ProtonMail/go-crypto/openpgp"
1813
)
1914

2015
// SigningMechanism abstracts a way to sign binary blobs and verify their signatures.

image/signature/mechanism_openpgp.go

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -10,22 +10,13 @@ import (
1010
"os"
1111
"path"
1212
"strings"
13-
"time"
1413

14+
"github.com/ProtonMail/go-crypto/openpgp"
1515
"go.podman.io/image/v5/signature/internal"
1616
"go.podman.io/storage/pkg/homedir"
17-
18-
// This is a fallback code; the primary recommendation is to use the gpgme mechanism
19-
// implementation, which is out-of-process and more appropriate for handling long-term private key material
20-
// than any Go implementation.
21-
// For this verify-only fallback, we haven't reviewed any of the
22-
// existing alternatives to choose; so, for now, continue to
23-
// use this frozen deprecated implementation.
24-
//lint:ignore SA1019 See above
25-
"golang.org/x/crypto/openpgp" //nolint:staticcheck
2617
)
2718

28-
// A GPG/OpenPGP signing mechanism, implemented using x/crypto/openpgp.
19+
// A GPG/OpenPGP signing mechanism, implemented using github.com/ProtonMail/go-crypto/openpgp.
2920
type openpgpSigningMechanism struct {
3021
keyring openpgp.EntityList
3122
}
@@ -155,18 +146,6 @@ func (m *openpgpSigningMechanism) Verify(unverifiedSignature []byte) (contents [
155146
if md.SignedBy == nil {
156147
return nil, "", internal.NewInvalidSignatureError(fmt.Sprintf("Key not found for key ID %x in signature", md.SignedByKeyId))
157148
}
158-
if md.Signature != nil {
159-
if md.Signature.SigLifetimeSecs != nil {
160-
expiry := md.Signature.CreationTime.Add(time.Duration(*md.Signature.SigLifetimeSecs) * time.Second)
161-
if time.Now().After(expiry) {
162-
return nil, "", internal.NewInvalidSignatureError(fmt.Sprintf("Signature expired on %s", expiry))
163-
}
164-
}
165-
} else if md.SignatureV3 == nil {
166-
// Coverage: If md.SignedBy != nil, the final md.UnverifiedBody.Read() either sets one of md.Signature or md.SignatureV3,
167-
// or sets md.SignatureError.
168-
return nil, "", internal.NewInvalidSignatureError("Unexpected openpgp.MessageDetails: neither Signature nor SignatureV3 is set")
169-
}
170149

171150
// Uppercase the fingerprint to be compatible with gpgme
172151
return content, strings.ToUpper(fmt.Sprintf("%x", md.SignedBy.Entity.PrimaryKey.Fingerprint)), nil

image/signature/mechanism_test.go

Lines changed: 101 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,26 @@ const (
1717
testGPGHomeDirectory = "./fixtures"
1818
)
1919

20-
// Many of the tests use two fixtures: V4 signature packets (*.signature), and V3 signature packets (*.signature-v3)
20+
// Many of the tests use two fixtures: V4 signature packets (*.signature), and V3 signature packets (*.signature-v3).
21+
// Note that V3 signature packets are not supported by openpgpSigningMechanism.
2122

22-
// fixtureVariants loads V3 and V4 signature fixture variants based on the v4 fixture path, and returns a map which makes it easy to test both.
23-
func fixtureVariants(t *testing.T, v4Path string) map[string][]byte {
23+
type fixtureVariant struct {
24+
path string
25+
bytes []byte
26+
isV3 bool
27+
}
28+
29+
// fixtureVariants loads V3 and V4 signature fixture variants based on the v4 fixture path.
30+
func fixtureVariants(t *testing.T, v4Path string) []fixtureVariant {
2431
v4, err := os.ReadFile(v4Path)
2532
require.NoError(t, err)
2633
v3Path := v4Path + "-v3"
2734
v3, err := os.ReadFile(v3Path)
2835
require.NoError(t, err)
29-
return map[string][]byte{v4Path: v4, v3Path: v3}
36+
return []fixtureVariant{
37+
{path: v4Path, bytes: v4, isV3: false},
38+
{path: v3Path, bytes: v3, isV3: true},
39+
}
3040
}
3141

3242
func TestSigningNotSupportedError(t *testing.T) {
@@ -56,9 +66,9 @@ func TestNewGPGSigningMechanismInDirectory(t *testing.T) {
5666
mech, err = newGPGSigningMechanismInDirectory("")
5767
require.NoError(t, err)
5868
defer mech.Close()
59-
for version, signature := range signatures {
60-
_, _, err := mech.Verify(signature)
61-
assert.Error(t, err, version)
69+
for _, variant := range signatures {
70+
_, _, err := mech.Verify(variant.bytes)
71+
assert.Error(t, err, variant.path)
6272
}
6373

6474
// Similarly, using a newly created empty directory makes TestKeyFingerprint
@@ -67,9 +77,9 @@ func TestNewGPGSigningMechanismInDirectory(t *testing.T) {
6777
mech, err = newGPGSigningMechanismInDirectory(emptyDir)
6878
require.NoError(t, err)
6979
defer mech.Close()
70-
for version, signature := range signatures {
71-
_, _, err := mech.Verify(signature)
72-
assert.Error(t, err, version)
80+
for _, variant := range signatures {
81+
_, _, err := mech.Verify(variant.bytes)
82+
assert.Error(t, err, variant.path)
7383
}
7484

7585
// If pubring.gpg is unreadable in the directory, either initializing
@@ -82,29 +92,33 @@ func TestNewGPGSigningMechanismInDirectory(t *testing.T) {
8292
mech, err = newGPGSigningMechanismInDirectory(unreadableDir)
8393
if err == nil {
8494
defer mech.Close()
85-
for version, signature := range signatures {
86-
_, _, err := mech.Verify(signature)
87-
assert.Error(t, err, version)
95+
for _, variant := range signatures {
96+
_, _, err := mech.Verify(variant.bytes)
97+
assert.Error(t, err, variant.path)
8898
}
8999
}
90100

91101
// Setting the directory parameter to testGPGHomeDirectory makes the key available.
92102
mech, err = newGPGSigningMechanismInDirectory(testGPGHomeDirectory)
93103
require.NoError(t, err)
94104
defer mech.Close()
95-
for version, signature := range signatures {
96-
_, _, err := mech.Verify(signature)
97-
assert.NoError(t, err, version)
105+
for _, variant := range signatures {
106+
_, _, err := mech.Verify(variant.bytes)
107+
if !variant.isV3 { // V3 signatures might be entirely unsupported and rejected.
108+
assert.NoError(t, err, variant.path)
109+
}
98110
}
99111

100112
// If we use the default directory mechanism, GNUPGHOME is respected.
101113
t.Setenv("GNUPGHOME", testGPGHomeDirectory)
102114
mech, err = newGPGSigningMechanismInDirectory("")
103115
require.NoError(t, err)
104116
defer mech.Close()
105-
for version, signature := range signatures {
106-
_, _, err := mech.Verify(signature)
107-
assert.NoError(t, err, version)
117+
for _, variant := range signatures {
118+
_, _, err := mech.Verify(variant.bytes)
119+
if !variant.isV3 { // V3 signatures might be entirely unsupported and rejected.
120+
assert.NoError(t, err, variant.path)
121+
}
108122
}
109123
}
110124

@@ -116,9 +130,9 @@ func TestNewEphemeralGPGSigningMechanism(t *testing.T) {
116130
assert.Empty(t, keyIdentities)
117131
// Try validating a signature when the key is unknown.
118132
signatures := fixtureVariants(t, "./fixtures/invalid-blob.signature")
119-
for version, signature := range signatures {
120-
_, _, err := mech.Verify(signature)
121-
require.Error(t, err, version)
133+
for _, variant := range signatures {
134+
_, _, err := mech.Verify(variant.bytes)
135+
require.Error(t, err, variant.path)
122136
}
123137

124138
// Successful import
@@ -129,11 +143,15 @@ func TestNewEphemeralGPGSigningMechanism(t *testing.T) {
129143
defer mech.Close()
130144
assert.Equal(t, []string{TestKeyFingerprint}, keyIdentities)
131145
// After import, the signature should validate.
132-
for version, signature := range signatures {
133-
content, signingFingerprint, err := mech.Verify(signature)
134-
require.NoError(t, err, version)
135-
assert.Equal(t, []byte("This is not JSON\n"), content, version)
136-
assert.Equal(t, TestKeyFingerprint, signingFingerprint, version)
146+
for _, variant := range signatures {
147+
content, signingFingerprint, err := mech.Verify(variant.bytes)
148+
if !variant.isV3 { // V3 signatures might be entirely unsupported and rejected.
149+
require.NoError(t, err, variant.path)
150+
}
151+
if err == nil {
152+
assert.Equal(t, []byte("This is not JSON\n"), content, variant.path)
153+
assert.Equal(t, TestKeyFingerprint, signingFingerprint, variant.path)
154+
}
137155
}
138156

139157
// Import of a key with a subkey
@@ -239,34 +257,46 @@ func TestGPGSigningMechanismVerify(t *testing.T) {
239257
require.NoError(t, err)
240258
defer mech.Close()
241259

260+
// For extra paranoia, test that we return nil data on error.
261+
242262
// Successful verification
243263
signatures := fixtureVariants(t, "./fixtures/invalid-blob.signature")
244-
for variant, signature := range signatures {
245-
content, signingFingerprint, err := mech.Verify(signature)
246-
require.NoError(t, err, variant)
247-
assert.Equal(t, []byte("This is not JSON\n"), content, variant)
248-
assert.Equal(t, TestKeyFingerprint, signingFingerprint, variant)
264+
for _, variant := range signatures {
265+
content, signingFingerprint, err := mech.Verify(variant.bytes)
266+
if !variant.isV3 { // V3 signatures might be entirely unsupported and rejected.
267+
require.NoError(t, err, variant.path)
268+
}
269+
if err == nil {
270+
assert.Equal(t, []byte("This is not JSON\n"), content, variant.path)
271+
assert.Equal(t, TestKeyFingerprint, signingFingerprint, variant.path)
272+
} else {
273+
assertSigningError(t, content, signingFingerprint, err)
274+
}
249275
}
250276
// Successful verification of a signature using a subkey
251277
signatures = fixtureVariants(t, "./fixtures/subkey.signature")
252-
for variant, signature := range signatures {
253-
content, signingFingerprint, err := mech.Verify(signature)
254-
require.NoError(t, err, variant)
255-
assert.Equal(t, []byte(`{"critical":{"identity":{"docker-reference":"testing/manifest:latest"},"image":{"docker-manifest-digest":"sha256:20bf21ed457b390829cdbeec8795a7bea1626991fda603e0d01b4e7f60427e55"},"type":"atomic container signature"},"optional":{}}`), content, variant)
256-
if signingFingerprint != TestKeyFingerprintPrimaryWithSubkey {
257-
assert.Equal(t, TestKeyFingerprintSubkeyWithSubkey, signingFingerprint, variant)
258-
withLookup, ok := mech.(signingMechanismWithVerificationIdentityLookup)
259-
require.True(t, ok, variant)
260-
261-
primaryKey, err := withLookup.keyIdentityForVerificationKeyIdentity(signingFingerprint)
262-
require.NoError(t, err, variant)
263-
signingFingerprint = primaryKey
278+
for _, variant := range signatures {
279+
content, signingFingerprint, err := mech.Verify(variant.bytes)
280+
if !variant.isV3 { // V3 signatures might be entirely unsupported and rejected.
281+
require.NoError(t, err, variant.path)
282+
}
283+
if err == nil {
284+
assert.Equal(t, []byte(`{"critical":{"identity":{"docker-reference":"testing/manifest:latest"},"image":{"docker-manifest-digest":"sha256:20bf21ed457b390829cdbeec8795a7bea1626991fda603e0d01b4e7f60427e55"},"type":"atomic container signature"},"optional":{}}`), content, variant.path)
285+
if signingFingerprint != TestKeyFingerprintPrimaryWithSubkey {
286+
assert.Equal(t, TestKeyFingerprintSubkeyWithSubkey, signingFingerprint, variant.path)
287+
withLookup, ok := mech.(signingMechanismWithVerificationIdentityLookup)
288+
require.True(t, ok, variant.path)
289+
290+
primaryKey, err := withLookup.keyIdentityForVerificationKeyIdentity(signingFingerprint)
291+
require.NoError(t, err, variant.path)
292+
signingFingerprint = primaryKey
293+
}
294+
assert.Equal(t, TestKeyFingerprintPrimaryWithSubkey, signingFingerprint, variant.path)
295+
} else {
296+
assertSigningError(t, content, signingFingerprint, err)
264297
}
265-
assert.Equal(t, TestKeyFingerprintPrimaryWithSubkey, signingFingerprint, variant)
266298
}
267299

268-
// For extra paranoia, test that we return nil data on error.
269-
270300
// Completely invalid signature.
271301
content, signingFingerprint, err := mech.Verify([]byte{})
272302
assertSigningError(t, content, signingFingerprint, err)
@@ -296,23 +326,23 @@ func TestGPGSigningMechanismVerify(t *testing.T) {
296326

297327
// Corrupt signature
298328
signatures = fixtureVariants(t, "./fixtures/corrupt.signature")
299-
for version, signature := range signatures {
300-
content, signingFingerprint, err := mech.Verify(signature)
301-
assertSigningError(t, content, signingFingerprint, err, version)
329+
for _, variant := range signatures {
330+
content, signingFingerprint, err := mech.Verify(variant.bytes)
331+
assertSigningError(t, content, signingFingerprint, err, variant.path)
302332
}
303333

304334
// Valid signature with an unknown key
305335
signatures = fixtureVariants(t, "./fixtures/unknown-key.signature")
306-
for version, signature := range signatures {
307-
content, signingFingerprint, err := mech.Verify(signature)
308-
assertSigningError(t, content, signingFingerprint, err, version)
336+
for _, variant := range signatures {
337+
content, signingFingerprint, err := mech.Verify(variant.bytes)
338+
assertSigningError(t, content, signingFingerprint, err, variant.path)
309339
}
310340

311341
// Valid signature with a revoked subkey
312342
signatures = fixtureVariants(t, "./fixtures/subkey-revoked.signature")
313-
for version, signature := range signatures {
314-
content, signingFingerprint, err := mech.Verify(signature)
315-
assertSigningError(t, content, signingFingerprint, err, version)
343+
for _, variant := range signatures {
344+
content, signingFingerprint, err := mech.Verify(variant.bytes)
345+
assertSigningError(t, content, signingFingerprint, err, variant.path)
316346
}
317347

318348
// The various GPG/GPGME failures cases are not obviously easy to reach.
@@ -367,11 +397,11 @@ func TestGPGSigningMechanismUntrustedSignatureContents(t *testing.T) {
367397

368398
// A valid signature
369399
signatures := fixtureVariants(t, "./fixtures/invalid-blob.signature")
370-
for version, signature := range signatures {
371-
content, shortKeyID, err := mech.UntrustedSignatureContents(signature)
372-
require.NoError(t, err, version)
373-
assert.Equal(t, []byte("This is not JSON\n"), content, version)
374-
assert.Equal(t, TestKeyShortID, shortKeyID, version)
400+
for _, variant := range signatures {
401+
content, shortKeyID, err := mech.UntrustedSignatureContents(variant.bytes)
402+
require.NoError(t, err, variant.path)
403+
assert.Equal(t, []byte("This is not JSON\n"), content, variant.path)
404+
assert.Equal(t, TestKeyShortID, shortKeyID, variant.path)
375405
}
376406

377407
// Completely invalid signature.
@@ -403,19 +433,19 @@ func TestGPGSigningMechanismUntrustedSignatureContents(t *testing.T) {
403433

404434
// Corrupt signature
405435
signatures = fixtureVariants(t, "./fixtures/corrupt.signature")
406-
for version, signature := range signatures {
407-
content, shortKeyID, err := mech.UntrustedSignatureContents(signature)
408-
require.NoError(t, err, version)
409-
assert.Equal(t, []byte(`{"critical":{"identity":{"docker-reference":"testing/manifest"},"image":{"docker-manifest-digest":"sha256:20bf21ed457b390829cdbeec8795a7bea1626991fda603e0d01b4e7f60427e55"},"type":"atomic container signature"},"optional":{"creator":"atomic ","timestamp":1458239713}}`), content, version)
410-
assert.Equal(t, TestKeyShortID, shortKeyID, version)
436+
for _, variant := range signatures {
437+
content, shortKeyID, err := mech.UntrustedSignatureContents(variant.bytes)
438+
require.NoError(t, err, variant.path)
439+
assert.Equal(t, []byte(`{"critical":{"identity":{"docker-reference":"testing/manifest"},"image":{"docker-manifest-digest":"sha256:20bf21ed457b390829cdbeec8795a7bea1626991fda603e0d01b4e7f60427e55"},"type":"atomic container signature"},"optional":{"creator":"atomic ","timestamp":1458239713}}`), content, variant.path)
440+
assert.Equal(t, TestKeyShortID, shortKeyID, variant.path)
411441
}
412442

413443
// Valid signature with an unknown key
414444
signatures = fixtureVariants(t, "./fixtures/unknown-key.signature")
415-
for version, signature := range signatures {
416-
content, shortKeyID, err := mech.UntrustedSignatureContents(signature)
417-
require.NoError(t, err, version)
418-
assert.Equal(t, []byte(`{"critical":{"identity":{"docker-reference":"testing/manifest"},"image":{"docker-manifest-digest":"sha256:20bf21ed457b390829cdbeec8795a7bea1626991fda603e0d01b4e7f60427e55"},"type":"atomic container signature"},"optional":{"creator":"atomic 0.1.13-dev","timestamp":1464633474}}`), content, version)
419-
assert.Equal(t, "5F9470E3BC6C3B55", shortKeyID, version)
445+
for _, variant := range signatures {
446+
content, shortKeyID, err := mech.UntrustedSignatureContents(variant.bytes)
447+
require.NoError(t, err, variant.path)
448+
assert.Equal(t, []byte(`{"critical":{"identity":{"docker-reference":"testing/manifest"},"image":{"docker-manifest-digest":"sha256:20bf21ed457b390829cdbeec8795a7bea1626991fda603e0d01b4e7f60427e55"},"type":"atomic container signature"},"optional":{"creator":"atomic 0.1.13-dev","timestamp":1464633474}}`), content, variant.path)
449+
assert.Equal(t, "5F9470E3BC6C3B55", shortKeyID, variant.path)
420450
}
421451
}

0 commit comments

Comments
 (0)