From 44efbfba68bad8f2230e1f981a8876e98a06426b Mon Sep 17 00:00:00 2001 From: Lukas Burkhalter Date: Wed, 7 Jun 2023 09:33:29 +0200 Subject: [PATCH] Introduce IsVerifed field in message details IsVerifed allows to check if a signature verification has been performed. This commit additionally, updates the v6 read test with the correct testvector. --- openpgp/integration_tests/end_to_end_test.go | 6 ++++ openpgp/read.go | 37 +++++++++----------- openpgp/read_test.go | 21 +++++++++-- openpgp/read_write_test_data.go | 12 ++++--- openpgp/write_test.go | 35 ++++++++++++++++-- 5 files changed, 81 insertions(+), 30 deletions(-) diff --git a/openpgp/integration_tests/end_to_end_test.go b/openpgp/integration_tests/end_to_end_test.go index 7560a6d9d..d8364c258 100644 --- a/openpgp/integration_tests/end_to_end_test.go +++ b/openpgp/integration_tests/end_to_end_test.go @@ -122,6 +122,9 @@ func decryptionTest(t *testing.T, vector testVector, sk openpgp.EntityList) { if err != nil { t.Fatal(err) } + if !md.IsVerified { + t.Errorf("not verified despite all data read") + } stringBody := string(body) if stringBody != vector.Message { @@ -225,6 +228,9 @@ func encDecTest(t *testing.T, from testVector, testVectors []testVector) { if err != nil { t.Fatalf("Error reading encrypted contents: %s", err) } + if !md.IsVerified { + t.Errorf("not verified despite all data read") + } encryptKey, _ := pkTo[0].EncryptionKey(time.Now()) expectedEncKeyID := encryptKey.PublicKey.KeyId if len(md.EncryptedToKeyIds) != 1 || diff --git a/openpgp/read.go b/openpgp/read.go index 111a293bb..e4e9606d3 100644 --- a/openpgp/read.go +++ b/openpgp/read.go @@ -63,6 +63,7 @@ type MessageDetails struct { // been consumed. Once EOF has been seen, the following fields are // valid. (An authentication code failure is reported as a // SignatureError error when reading from UnverifiedBody.) + IsVerified bool // true if the signatures have been verified else false SignatureCandidates []*SignatureCandidate // stores state for all signatures of this message SignedBy *Key // the issuer key of the fist successfully verified signature, if any found. Signature *packet.Signature // the first successfully verified signature, if any found. @@ -270,21 +271,18 @@ FindKey: // SignatureCandidate keeps state about a signature that can be potentially verified. type SignatureCandidate struct { - // Information OPSVersion int SigType packet.SignatureType HashAlgorithm crypto.Hash PubKeyAlgo packet.PublicKeyAlgorithm IssuerKeyId uint64 - Salt []byte // v6 only IssuerFingerprint []byte // v6 only + Salt []byte // v6 only - SignedBy *Key // the key of the signer, if available. (OPS) - - SignatureError error // nil if the signature is valid or not checked. - CorrespondingSig *packet.Signature // the candidate's signature packet - - Hash, WrappedHash hash.Hash // hashes for this signature candidate (OPS) + SignedBy *Key // the key of the signer, if available. (OPS) + SignatureError error // nil if the signature is valid or not checked. + CorrespondingSig *packet.Signature // the candidate's signature packet + Hash, WrappedHash hash.Hash // hashes for this signature candidate (OPS) } func newSignatureCandidate(ops *packet.OnePassSignature) (sigCandidate *SignatureCandidate) { @@ -389,15 +387,7 @@ FindLiteralData: } } - // Check if there are signature candidates with no error - for _, candidate := range md.SignatureCandidates { - md.SignatureError = candidate.SignatureError - if candidate.SignatureError == nil { - break - } - } - - if md.IsSigned && md.SignatureError == nil { + if md.IsSigned { md.UnverifiedBody = &signatureCheckReader{packets, md, config, md.LiteralData.Body} } else if md.decrypted != nil { md.UnverifiedBody = checkReader{md} @@ -522,8 +512,9 @@ func (scr *signatureCheckReader) Read(buf []byte) (int, error) { candidate := scr.md.SignatureCandidates[sigIndex] candidate.CorrespondingSig = sig if !candidate.validate() { - candidate.SignatureError = errors.StructuralError("Signature does not match the expected ops data") - } else if candidate.SignatureError == nil { + candidate.SignatureError = errors.StructuralError("signature does not match the expected ops data") + } + if candidate.SignatureError == nil { if candidate.SignedBy == nil { candidate.SignatureError = errors.ErrUnknownIssuer scr.md.SignatureError = candidate.SignatureError @@ -556,10 +547,16 @@ func (scr *signatureCheckReader) Read(buf []byte) (int, error) { } } + if len(scr.md.SignatureCandidates) == 0 { + scr.md.SignatureError = errors.StructuralError("no signature found") + } + if scr.md.SignatureError == nil && scr.md.Signature == nil { - scr.md.SignatureError = errors.StructuralError("No matching signature found") + scr.md.SignatureError = errors.StructuralError("no matching signature found") } + scr.md.IsVerified = true + // The SymmetricallyEncrypted packet, if any, might have an // unsigned hash of its own. In order to check this we need to // close that Reader. diff --git a/openpgp/read_test.go b/openpgp/read_test.go index 12fb6ec0b..87dd9443c 100644 --- a/openpgp/read_test.go +++ b/openpgp/read_test.go @@ -139,6 +139,9 @@ func checkSignedMessage(t *testing.T, signedHex, expected string) { if err != nil { t.Errorf("error reading UnverifiedBody: %s", err) } + if !md.IsVerified { + t.Errorf("not verified despite all data read") + } if string(contents) != expected { t.Errorf("bad UnverifiedBody got:%s want:%s", string(contents), expected) } @@ -241,6 +244,9 @@ func TestSignedEncryptedMessage(t *testing.T) { if err != nil { t.Errorf("#%d: error reading UnverifiedBody: %s", i, err) } + if !md.IsVerified { + t.Errorf("not verified despite all data read") + } if string(contents) != expected { t.Errorf("#%d: bad UnverifiedBody got:%s want:%s", i, string(contents), expected) } @@ -475,6 +481,9 @@ func TestSignatureKnownNotation(t *testing.T) { t.Error(err) return } + if !md.IsVerified { + t.Errorf("not verified despite all data read") + } if md.SignatureError != nil { t.Error(md.SignatureError) return @@ -562,7 +571,9 @@ func TestSignatureV3Message(t *testing.T) { t.Error(err) return } - + if !md.IsVerified { + t.Errorf("not verified despite all data read") + } // We'll see a sig error here after reading in the UnverifiedBody above, // if there was one to see. if err = md.SignatureError; err == nil { @@ -617,10 +628,13 @@ func TestReadV6Messages(t *testing.T) { t.Error(err) return } + if !md.IsVerified { + t.Errorf("not verified despite all data read") + } if md.SignatureError != nil { t.Error("expected no signature error, got:", md.SignatureError) } - if string(contents) != "Hello, world!" { + if string(contents) != "What we need from the grocery store:\n\n- tofu\n- vegetables\n- noodles\n" { t.Errorf("inline message is wrong: %s", contents) } } @@ -937,6 +951,9 @@ func TestMultiSignedMessage(t *testing.T) { if err != nil { t.Fatal(err) } + if !md.IsVerified { + t.Errorf("not verified despite all data read") + } if md.Signature == nil { t.Error("expected a matched verified signatures, got: nil") diff --git a/openpgp/read_write_test_data.go b/openpgp/read_write_test_data.go index 5467af85f..9d5eb8c43 100644 --- a/openpgp/read_write_test_data.go +++ b/openpgp/read_write_test_data.go @@ -205,11 +205,13 @@ bhF30A+IitsxxA== // https://gitlab.com/openpgp-wg/rfc4880bis/-/merge_requests/305 const v6PrivKeyInlineSignMsg = `-----BEGIN PGP MESSAGE----- -wV0GIQYSyD8ecG9jCP4VGkF3Q6HwM3kOk+mXhIjR2zeNqZMIhRmHzxjV8bU/gXzO -WgBM85PMiVi93AZfJfhK9QmxfdNnZBjeo1VDeVZheQHgaVf7yopqR6W1FT6NOrfS -aQIHAgZhZBZTW+CwcW1g4FKlbExAf56zaw76/prQoN+bAzxpohup69LA7JW/Vp0l -yZnuSj3hcFj0DfqLTGgr4/u717J+sPWbtQBfgMfG9AOIwwrUBqsFE9zW+f1zdlYo -bhF30A+IitsxxA== +xEYGAQobIHZJX1AhiJD39eLuPBgiUU9wUA9VHYblySHkBONKU/usyxhsTwYJppfk +1S36bHIrDB8eJ8GKVnCPZSXsJ7rZrMkBy0p1AAAAAABXaGF0IHdlIG5lZWQgZnJv +bSB0aGUgZ3JvY2VyeSBzdG9yZToKCi0gdG9mdQotIHZlZ2V0YWJsZXMKLSBub29k +bGVzCsKYBgEbCgAAACkFgmOYo2MiIQbLGGxPBgmml+TVLfpscisMHx4nwYpWcI9l +JewnutmsyQAAAABpNiB2SV9QIYiQ9/Xi7jwYIlFPcFAPVR2G5ckh5ATjSlP7rCfQ +b7gKqPxbyxbhljGygHQPnqau1eBzrQD5QVplPEDnemrnfmkrpx0GmhCfokxYz9jj +FtCgazStmsuOXF9SFQE= -----END PGP MESSAGE-----` // See https://gitlab.com/openpgp-wg/rfc4880bis/-/merge_requests/274 diff --git a/openpgp/write_test.go b/openpgp/write_test.go index 2045e5163..39fa832a4 100644 --- a/openpgp/write_test.go +++ b/openpgp/write_test.go @@ -281,7 +281,7 @@ func TestEncryptWithCompression(t *testing.T) { buf := new(bytes.Buffer) var config = &packet.Config{ DefaultCompressionAlgo: packet.CompressionZIP, - CompressionConfig: &packet.CompressionConfig{-1}, + CompressionConfig: &packet.CompressionConfig{Level: -1}, } w, err := Encrypt(buf, kring[:1], nil, nil, nil /* no hints */, config) if err != nil { @@ -391,6 +391,9 @@ func TestSymmetricEncryptionV5RandomizeSlow(t *testing.T) { packets := packet.NewReader(copiedBuf) // First a SymmetricKeyEncrypted packet p, err := packets.Next() + if err != nil { + t.Errorf("error reading packet: %s", err) + } switch tp := p.(type) { case *packet.SymmetricKeyEncrypted: default: @@ -398,6 +401,9 @@ func TestSymmetricEncryptionV5RandomizeSlow(t *testing.T) { } // Then an SymmetricallyEncrypted packet version 2 p, err = packets.Next() + if err != nil { + t.Errorf("error reading packet: %s", err) + } switch tp := p.(type) { case *packet.SymmetricallyEncrypted: if tp.Version != 2 { @@ -533,6 +539,9 @@ func TestIntendedRecipientsEncryption(t *testing.T) { if err != nil { t.Errorf("error reading encrypted contents: %s", err) } + if !md.IsVerified { + t.Errorf("not verified despite all data read") + } if _, ok := md.SignatureError.(errors.SignatureError); !ok { t.Error("hidden recipient should not be in the intended recipient list") } @@ -553,6 +562,9 @@ func TestIntendedRecipientsEncryption(t *testing.T) { if err != nil { t.Errorf("error reading encrypted contents: %s", err) } + if !md.IsVerified { + t.Errorf("not verified despite all data read") + } if md.SignatureError != nil { t.Error("singature verification should pass") } @@ -612,7 +624,9 @@ func TestMultiSignEncryption(t *testing.T) { if err != nil { t.Errorf("error reading encrypted contents: %s", err) } - + if !md.IsVerified { + t.Errorf("not verified despite all data read") + } if md.Signature == nil || md.SignatureError != nil { t.Error("expected matching signature") } @@ -628,6 +642,9 @@ func TestMultiSignEncryption(t *testing.T) { if err != nil { t.Errorf("error reading encrypted contents: %s", err) } + if !md.IsVerified { + t.Errorf("not verified despite all data read") + } if md.Signature == nil || md.SignatureError != nil { t.Error("expected matching signature") } @@ -643,6 +660,9 @@ func TestMultiSignEncryption(t *testing.T) { if err != nil { t.Errorf("error reading encrypted contents: %s", err) } + if !md.IsVerified { + t.Errorf("not verified despite all data read") + } if md.Signature != nil || md.SignatureError == nil { t.Error("expected error") } @@ -687,7 +707,7 @@ func TestEncryption(t *testing.T) { } compAlgo := compAlgos[mathrand.Intn(len(compAlgos))] level := mathrand.Intn(11) - 1 - compConf := &packet.CompressionConfig{level} + compConf := &packet.CompressionConfig{Level: level} var config = &packet.Config{ DefaultCompressionAlgo: compAlgo, CompressionConfig: compConf, @@ -765,6 +785,9 @@ func TestEncryption(t *testing.T) { } if test.isSigned { + if !md.IsVerified { + t.Errorf("not verified despite all data read") + } if md.SignatureError != nil { t.Errorf("#%d: signature error: %s", i, md.SignatureError) } @@ -861,6 +884,9 @@ func TestSigning(t *testing.T) { t.Errorf("#%d: got: %q, want: %q", i, plaintext, message) } + if !md.IsVerified { + t.Errorf("not verified despite all data read") + } if md.SignatureError != nil { t.Errorf("#%d: signature error: %q", i, md.SignatureError) } @@ -965,6 +991,9 @@ FindKey: } decPackets, err := packet.Read(decrypted) + if err != nil { + return + } _, ok := decPackets.(*packet.Compressed) if !ok { return errors.InvalidArgumentError("No compressed packets found")