diff --git a/signature/fixtures/corrupt.signature-v3 b/signature/fixtures/corrupt.signature-v3 new file mode 100644 index 0000000000..51f986b961 Binary files /dev/null and b/signature/fixtures/corrupt.signature-v3 differ diff --git a/signature/fixtures/invalid-blob.signature-v3 b/signature/fixtures/invalid-blob.signature-v3 new file mode 100644 index 0000000000..246bdd9a8e Binary files /dev/null and b/signature/fixtures/invalid-blob.signature-v3 differ diff --git a/signature/fixtures/unknown-key.signature b/signature/fixtures/unknown-key.signature index 2277b130da..393ace4a92 100644 Binary files a/signature/fixtures/unknown-key.signature and b/signature/fixtures/unknown-key.signature differ diff --git a/signature/fixtures/unknown-key.signature-v3 b/signature/fixtures/unknown-key.signature-v3 new file mode 100644 index 0000000000..67f429b0d8 Binary files /dev/null and b/signature/fixtures/unknown-key.signature-v3 differ diff --git a/signature/mechanism_openpgp.go b/signature/mechanism_openpgp.go index e383bde3e6..eccd610c9d 100644 --- a/signature/mechanism_openpgp.go +++ b/signature/mechanism_openpgp.go @@ -132,11 +132,17 @@ func (m *openpgpSigningMechanism) Verify(unverifiedSignature []byte) (contents [ if md.SignedBy == nil { return nil, "", InvalidSignatureError{msg: fmt.Sprintf("Invalid GPG signature: %#v", md.Signature)} } - if md.Signature.SigLifetimeSecs != nil { - expiry := md.Signature.CreationTime.Add(time.Duration(*md.Signature.SigLifetimeSecs) * time.Second) - if time.Now().After(expiry) { - return nil, "", InvalidSignatureError{msg: fmt.Sprintf("Signature expired on %s", expiry)} + if md.Signature != nil { + if md.Signature.SigLifetimeSecs != nil { + expiry := md.Signature.CreationTime.Add(time.Duration(*md.Signature.SigLifetimeSecs) * time.Second) + if time.Now().After(expiry) { + return nil, "", InvalidSignatureError{msg: fmt.Sprintf("Signature expired on %s", expiry)} + } } + } else if md.SignatureV3 == nil { + // Coverage: If md.SignedBy != nil, the final md.UnverifiedBody.Read() either sets one of md.Signature or md.SignatureV3, + // or sets md.SignatureError. + return nil, "", InvalidSignatureError{msg: "Unexpected openpgp.MessageDetails: neither Signature nor SignatureV3 is set"} } // Uppercase the fingerprint to be compatible with gpgme diff --git a/signature/mechanism_test.go b/signature/mechanism_test.go index 53f2b3d9f0..a6587c61c4 100644 --- a/signature/mechanism_test.go +++ b/signature/mechanism_test.go @@ -17,6 +17,18 @@ const ( testGPGHomeDirectory = "./fixtures" ) +// Many of the tests use two fixtures: V4 signature packets (*.signature), and V3 signature packets (*.signature-v3) + +// 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. +func fixtureVariants(t *testing.T, v4Path string) map[string][]byte { + v4, err := ioutil.ReadFile(v4Path) + require.NoError(t, err) + v3Path := v4Path + "-v3" + v3, err := ioutil.ReadFile(v3Path) + require.NoError(t, err) + return map[string][]byte{v4Path: v4, v3Path: v3} +} + func TestSigningNotSupportedError(t *testing.T) { // A stupid test just to keep code coverage s := "test" @@ -40,13 +52,14 @@ func TestNewGPGSigningMechanismInDirectory(t *testing.T) { // Test that using the default directory (presumably in user’s home) // cannot use TestKeyFingerprint. - signature, err := ioutil.ReadFile("./fixtures/invalid-blob.signature") - require.NoError(t, err) + signatures := fixtureVariants(t, "./fixtures/invalid-blob.signature") mech, err = newGPGSigningMechanismInDirectory("") require.NoError(t, err) defer mech.Close() - _, _, err = mech.Verify(signature) - assert.Error(t, err) + for version, signature := range signatures { + _, _, err := mech.Verify(signature) + assert.Error(t, err, version) + } // Similarly, using a newly created empty directory makes TestKeyFingerprint // unavailable @@ -56,8 +69,10 @@ func TestNewGPGSigningMechanismInDirectory(t *testing.T) { mech, err = newGPGSigningMechanismInDirectory(emptyDir) require.NoError(t, err) defer mech.Close() - _, _, err = mech.Verify(signature) - assert.Error(t, err) + for version, signature := range signatures { + _, _, err := mech.Verify(signature) + assert.Error(t, err, version) + } // If pubring.gpg is unreadable in the directory, either initializing // the mechanism fails (with openpgp), or it succeeds (sadly, gpgme) and @@ -71,16 +86,20 @@ func TestNewGPGSigningMechanismInDirectory(t *testing.T) { mech, err = newGPGSigningMechanismInDirectory(unreadableDir) if err == nil { defer mech.Close() - _, _, err = mech.Verify(signature) + for version, signature := range signatures { + _, _, err := mech.Verify(signature) + assert.Error(t, err, version) + } } - assert.Error(t, err) // Setting the directory parameter to testGPGHomeDirectory makes the key available. mech, err = newGPGSigningMechanismInDirectory(testGPGHomeDirectory) require.NoError(t, err) defer mech.Close() - _, _, err = mech.Verify(signature) - assert.NoError(t, err) + for version, signature := range signatures { + _, _, err := mech.Verify(signature) + assert.NoError(t, err, version) + } // If we use the default directory mechanism, GNUPGHOME is respected. origGNUPGHOME := os.Getenv("GNUPGHOME") @@ -89,8 +108,10 @@ func TestNewGPGSigningMechanismInDirectory(t *testing.T) { mech, err = newGPGSigningMechanismInDirectory("") require.NoError(t, err) defer mech.Close() - _, _, err = mech.Verify(signature) - assert.NoError(t, err) + for version, signature := range signatures { + _, _, err := mech.Verify(signature) + assert.NoError(t, err, version) + } } func TestNewEphemeralGPGSigningMechanism(t *testing.T) { @@ -100,10 +121,11 @@ func TestNewEphemeralGPGSigningMechanism(t *testing.T) { defer mech.Close() assert.Empty(t, keyIdentities) // Try validating a signature when the key is unknown. - signature, err := ioutil.ReadFile("./fixtures/invalid-blob.signature") - require.NoError(t, err) - content, signingFingerprint, err := mech.Verify(signature) - require.Error(t, err) + signatures := fixtureVariants(t, "./fixtures/invalid-blob.signature") + for version, signature := range signatures { + _, _, err := mech.Verify(signature) + require.Error(t, err, version) + } // Successful import keyBlob, err := ioutil.ReadFile("./fixtures/public-key.gpg") @@ -113,10 +135,12 @@ func TestNewEphemeralGPGSigningMechanism(t *testing.T) { defer mech.Close() assert.Equal(t, []string{TestKeyFingerprint}, keyIdentities) // After import, the signature should validate. - content, signingFingerprint, err = mech.Verify(signature) - require.NoError(t, err) - assert.Equal(t, []byte("This is not JSON\n"), content) - assert.Equal(t, TestKeyFingerprint, signingFingerprint) + for version, signature := range signatures { + content, signingFingerprint, err := mech.Verify(signature) + require.NoError(t, err, version) + assert.Equal(t, []byte("This is not JSON\n"), content, version) + assert.Equal(t, TestKeyFingerprint, signingFingerprint, version) + } // Two keys: Read the binary-format pubring.gpg, and concatenate it twice. // (Using two copies of public-key.gpg, in the ASCII-armored format, works with @@ -176,10 +200,10 @@ func TestGPGSigningMechanismSign(t *testing.T) { // The various GPG/GPGME failures cases are not obviously easy to reach. } -func assertSigningError(t *testing.T, content []byte, fingerprint string, err error) { - assert.Error(t, err) - assert.Nil(t, content) - assert.Empty(t, fingerprint) +func assertSigningError(t *testing.T, content []byte, fingerprint string, err error, msgAndArgs ...interface{}) { + assert.Error(t, err, msgAndArgs...) + assert.Nil(t, content, msgAndArgs...) + assert.Empty(t, fingerprint, msgAndArgs...) } func TestGPGSigningMechanismVerify(t *testing.T) { @@ -188,30 +212,31 @@ func TestGPGSigningMechanismVerify(t *testing.T) { defer mech.Close() // Successful verification - signature, err := ioutil.ReadFile("./fixtures/invalid-blob.signature") - require.NoError(t, err) - content, signingFingerprint, err := mech.Verify(signature) - require.NoError(t, err) - assert.Equal(t, []byte("This is not JSON\n"), content) - assert.Equal(t, TestKeyFingerprint, signingFingerprint) + signatures := fixtureVariants(t, "./fixtures/invalid-blob.signature") + for variant, signature := range signatures { + content, signingFingerprint, err := mech.Verify(signature) + require.NoError(t, err, variant) + assert.Equal(t, []byte("This is not JSON\n"), content, variant) + assert.Equal(t, TestKeyFingerprint, signingFingerprint, variant) + } // For extra paranoia, test that we return nil data on error. // Completely invalid signature. - content, signingFingerprint, err = mech.Verify([]byte{}) + content, signingFingerprint, err := mech.Verify([]byte{}) assertSigningError(t, content, signingFingerprint, err) content, signingFingerprint, err = mech.Verify([]byte("invalid signature")) assertSigningError(t, content, signingFingerprint, err) // Literal packet, not a signature - signature, err = ioutil.ReadFile("./fixtures/unsigned-literal.signature") + signature, err := ioutil.ReadFile("./fixtures/unsigned-literal.signature") // Not fixtureVariants, the “literal data” packet does not have V3/V4 versions. require.NoError(t, err) content, signingFingerprint, err = mech.Verify(signature) assertSigningError(t, content, signingFingerprint, err) // Encrypted data, not a signature. - signature, err = ioutil.ReadFile("./fixtures/unsigned-encrypted.signature") + signature, err = ioutil.ReadFile("./fixtures/unsigned-encrypted.signature") // Not fixtureVariants, the “public-key encrypted session key” does not have V3/V4 versions. require.NoError(t, err) content, signingFingerprint, err = mech.Verify(signature) assertSigningError(t, content, signingFingerprint, err) @@ -219,22 +244,24 @@ func TestGPGSigningMechanismVerify(t *testing.T) { // FIXME? Is there a way to create a multi-signature so that gpgme_op_verify returns multiple signatures? // Expired signature - signature, err = ioutil.ReadFile("./fixtures/expired.signature") + signature, err = ioutil.ReadFile("./fixtures/expired.signature") // Not fixtureVariants, V3 signature packets don’t support expiration. require.NoError(t, err) content, signingFingerprint, err = mech.Verify(signature) assertSigningError(t, content, signingFingerprint, err) // Corrupt signature - signature, err = ioutil.ReadFile("./fixtures/corrupt.signature") - require.NoError(t, err) - content, signingFingerprint, err = mech.Verify(signature) - assertSigningError(t, content, signingFingerprint, err) + signatures = fixtureVariants(t, "./fixtures/corrupt.signature") + for version, signature := range signatures { + content, signingFingerprint, err := mech.Verify(signature) + assertSigningError(t, content, signingFingerprint, err, version) + } // Valid signature with an unknown key - signature, err = ioutil.ReadFile("./fixtures/unknown-key.signature") - require.NoError(t, err) - content, signingFingerprint, err = mech.Verify(signature) - assertSigningError(t, content, signingFingerprint, err) + signatures = fixtureVariants(t, "./fixtures/unknown-key.signature") + for version, signature := range signatures { + content, signingFingerprint, err := mech.Verify(signature) + assertSigningError(t, content, signingFingerprint, err, version) + } // The various GPG/GPGME failures cases are not obviously easy to reach. } @@ -245,12 +272,13 @@ func TestGPGSigningMechanismUntrustedSignatureContents(t *testing.T) { defer mech.Close() // A valid signature - signature, err := ioutil.ReadFile("./fixtures/invalid-blob.signature") - require.NoError(t, err) - content, shortKeyID, err := mech.UntrustedSignatureContents(signature) - require.NoError(t, err) - assert.Equal(t, []byte("This is not JSON\n"), content) - assert.Equal(t, TestKeyShortID, shortKeyID) + signatures := fixtureVariants(t, "./fixtures/invalid-blob.signature") + for version, signature := range signatures { + content, shortKeyID, err := mech.UntrustedSignatureContents(signature) + require.NoError(t, err, version) + assert.Equal(t, []byte("This is not JSON\n"), content, version) + assert.Equal(t, TestKeyShortID, shortKeyID, version) + } // Completely invalid signature. _, _, err = mech.UntrustedSignatureContents([]byte{}) @@ -260,19 +288,19 @@ func TestGPGSigningMechanismUntrustedSignatureContents(t *testing.T) { assert.Error(t, err) // Literal packet, not a signature - signature, err = ioutil.ReadFile("./fixtures/unsigned-literal.signature") + signature, err := ioutil.ReadFile("./fixtures/unsigned-literal.signature") // Not fixtureVariants, the “literal data” packet does not have V3/V4 versions. require.NoError(t, err) - content, shortKeyID, err = mech.UntrustedSignatureContents(signature) + content, shortKeyID, err := mech.UntrustedSignatureContents(signature) assert.Error(t, err) // Encrypted data, not a signature. - signature, err = ioutil.ReadFile("./fixtures/unsigned-encrypted.signature") + signature, err = ioutil.ReadFile("./fixtures/unsigned-encrypted.signature") // Not fixtureVariants, the “public-key encrypted session key” does not have V3/V4 versions. require.NoError(t, err) content, shortKeyID, err = mech.UntrustedSignatureContents(signature) assert.Error(t, err) // Expired signature - signature, err = ioutil.ReadFile("./fixtures/expired.signature") + signature, err = ioutil.ReadFile("./fixtures/expired.signature") // Not fixtureVariants, V3 signature packets don’t support expiration. require.NoError(t, err) content, shortKeyID, err = mech.UntrustedSignatureContents(signature) require.NoError(t, err) @@ -280,18 +308,20 @@ func TestGPGSigningMechanismUntrustedSignatureContents(t *testing.T) { assert.Equal(t, TestKeyShortID, shortKeyID) // Corrupt signature - signature, err = ioutil.ReadFile("./fixtures/corrupt.signature") - require.NoError(t, err) - content, shortKeyID, err = mech.UntrustedSignatureContents(signature) - require.NoError(t, err) - 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) - assert.Equal(t, TestKeyShortID, shortKeyID) + signatures = fixtureVariants(t, "./fixtures/corrupt.signature") + for version, signature := range signatures { + content, shortKeyID, err := mech.UntrustedSignatureContents(signature) + require.NoError(t, err, version) + 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) + assert.Equal(t, TestKeyShortID, shortKeyID, version) + } // Valid signature with an unknown key - signature, err = ioutil.ReadFile("./fixtures/unknown-key.signature") - require.NoError(t, err) - content, shortKeyID, err = mech.UntrustedSignatureContents(signature) - require.NoError(t, err) - 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) - assert.Equal(t, "E5476D1110D07803", shortKeyID) + signatures = fixtureVariants(t, "./fixtures/unknown-key.signature") + for version, signature := range signatures { + content, shortKeyID, err := mech.UntrustedSignatureContents(signature) + require.NoError(t, err, version) + 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) + assert.Equal(t, "BB75E91990DF8F7E", shortKeyID, version) + } }