diff --git a/crypto/message_test.go b/crypto/message_test.go index 9dce8cf..9548d8d 100644 --- a/crypto/message_test.go +++ b/crypto/message_test.go @@ -143,6 +143,35 @@ func TestSignedMessageDecryption(t *testing.T) { assert.Exactly(t, readTestFile("message_plaintext", true), decrypted.GetString()) } +func TestSHA256SignedMessageDecryption(t *testing.T) { + pgpMessage, err := NewPGPMessageFromArmored(readTestFile("message_sha256_signed", false)) + if err != nil { + t.Fatal("Expected no error when unarmoring, got:", err) + } + + decrypted, err := keyRingTestPrivate.Decrypt(pgpMessage, keyRingTestPrivate, 0) + if err != nil { + t.Fatal("Expected no error when decrypting, got:", err) + } + assert.Exactly(t, readTestFile("message_plaintext", true), decrypted.GetString()) +} + +func TestSHA1SignedMessageDecryption(t *testing.T) { + pgpMessage, err := NewPGPMessageFromArmored(readTestFile("message_sha1_signed", false)) + if err != nil { + t.Fatal("Expected no error when unarmoring, got:", err) + } + + decrypted, err := keyRingTestPrivate.Decrypt(pgpMessage, keyRingTestPrivate, 0) + if err == nil { + t.Fatal("Expected verification error when decrypting") + } + if err.Error() != "Signature Verification Error: Insecure signature" { + t.Fatal("Expected verification error when decrypting, got:", err) + } + assert.Exactly(t, readTestFile("message_plaintext", true), decrypted.GetString()) +} + func TestMultipleKeyMessageEncryption(t *testing.T) { var message = NewPlainMessageFromString("plain text") assert.Exactly(t, 3, len(keyRingTestMultiple.entities)) diff --git a/crypto/signature.go b/crypto/signature.go index 637386d..8fb0f34 100644 --- a/crypto/signature.go +++ b/crypto/signature.go @@ -2,6 +2,7 @@ package crypto import ( "bytes" + "crypto" "fmt" "io" "math" @@ -15,6 +16,13 @@ import ( "github.com/ProtonMail/gopenpgp/v2/internal" ) +var allowedHashes = []crypto.Hash{ + crypto.SHA224, + crypto.SHA256, + crypto.SHA384, + crypto.SHA512, +} + // SignatureVerificationError is returned from Decrypt and VerifyDetached // functions when signature verification fails. type SignatureVerificationError struct { @@ -40,6 +48,15 @@ func newSignatureFailed() SignatureVerificationError { } } +// newSignatureInsecure creates a new SignatureVerificationError, type +// SignatureFailed, with a message describing the signature as insecure. +func newSignatureInsecure() SignatureVerificationError { + return SignatureVerificationError{ + constants.SIGNATURE_FAILED, + "Insecure signature", + } +} + // newSignatureNotSigned creates a new SignatureVerificationError, type // SignatureNotSigned. func newSignatureNotSigned() SignatureVerificationError { @@ -81,20 +98,21 @@ func processSignatureExpiration(md *openpgp.MessageDetails, verifyTime int64) { // verifyDetailsSignature verifies signature from message details. func verifyDetailsSignature(md *openpgp.MessageDetails, verifierKey *KeyRing) error { - if md.IsSigned { - if md.SignedBy == nil || len(verifierKey.entities) == 0 { - return newSignatureNoVerifier() - } - matches := verifierKey.entities.KeysById(md.SignedByKeyId) - if len(matches) > 0 { - if md.SignatureError == nil { - return nil - } - return newSignatureFailed() - } + if !md.IsSigned || + md.SignedBy == nil || + len(verifierKey.entities) == 0 || + len(verifierKey.entities.KeysById(md.SignedByKeyId)) == 0 { + return newSignatureNoVerifier() } - - return newSignatureNoVerifier() + if md.SignatureError != nil { + return newSignatureFailed() + } + if md.Signature == nil || + md.Signature.Hash < allowedHashes[0] || + md.Signature.Hash > allowedHashes[len(allowedHashes)-1] { + return newSignatureInsecure() + } + return nil } // verifySignature verifies if a signature is valid with the entity list. @@ -111,7 +129,7 @@ func verifySignature(pubKeyEntries openpgp.EntityList, origText io.Reader, signa } signatureReader := bytes.NewReader(signature) - signer, err := openpgp.CheckDetachedSignature(pubKeyEntries, origText, signatureReader, config) + signer, err := openpgp.CheckDetachedSignatureAndHash(pubKeyEntries, origText, signatureReader, allowedHashes, config) if err == pgpErrors.ErrSignatureExpired && signer != nil && verifyTime > 0 { // if verifyTime = 0: time check disabled, everything is okay @@ -126,7 +144,7 @@ func verifySignature(pubKeyEntries openpgp.EntityList, origText io.Reader, signa return newSignatureFailed() } - signer, err = openpgp.CheckDetachedSignature(pubKeyEntries, origText, signatureReader, config) + signer, err = openpgp.CheckDetachedSignatureAndHash(pubKeyEntries, origText, signatureReader, allowedHashes, config) if err != nil { return newSignatureFailed() } diff --git a/crypto/testdata/message_sha1_signed b/crypto/testdata/message_sha1_signed new file mode 100644 index 0000000..029d871 --- /dev/null +++ b/crypto/testdata/message_sha1_signed @@ -0,0 +1,25 @@ +-----BEGIN PGP MESSAGE----- +Version: OpenPGP.js v4.10.4 +Comment: https://openpgpjs.org + +wcBMA0fcZ7XLgmf2AQf/SRGI1T68u5cqeSbznX2T8mpHliVIXj1Oifa1cqaA +spPxpuFMYctAS1J3yOVaFMtNSlFaGR3ftQb09d1Lx9JXW31tshWO+7EAZ44j +yqS3y4KUqJPm9W73UIEUk0z49fiIDAnOTfMkSai1Y1KtSUtD+4ZRdkYRqEV0 +WfUlExkjDu98aBEzr4GPMefl8+VC17n6jOpOikT1rUJbelOIhpW2T9sEy0mw +X200eaiVY1Ge9OTOEpLxY+pmFfdVZpmoOzSCn3C3uxNPT5uDzwBqFBUOAJHA +oZNrZx67Nl/swa+Y7Mcr4z1iIMKbwn37Zk29zzat9tfJRhDTAOk/6Z5KjOPB +qNLBeQFoUgbkNmoLDEbi6EYU9POQxkovnIfBGhJI5XkJlh/Dqe2iaysrNIHv +JIhtqZ1Zq3NNsKdOsfCTkkQFZwJc8gsK72aRXPDMn+qXm9O6Tv3qXZL+Eewz ++RT+OI2xtc44xpY6tLPTmNYlVyqGC2yAUIbFeqct3A/98g1uZn617RGtHBiY +ql1L4YeB/3mJpY4pK4Syota95vbNhJo5Sai9qnh+KTB8UlEbjmg6ULdH2tVG +F7ET/IbjCs1D2wgdjs2fn/TIpO5sbVjo4VmxxnYdWWvf36r7QTmHXh1mAkKt +EqSxQg/VUaeW/ew41zfO+xYJLGnpkvgcfyrzgHqrq0wEMe/+V+r8s1lRJ1NC +8jv6vEyhH/cS/qTK7zhs8rS7JB0ywefuJKyu3XcBUSceQOmwG7GqhXkYYaAT +yscUK82tOxw562ccxxgXtQ11EXrJw4+lOe2hVkp6y+MRYGFQY4VntpnxbrpY +gPikwUOpDXbluzzn5pXgKXsMfdhxcYLL8NaBq6xMLRR+MNoO+fQ8WVR7HWcE +BdYLl4vRXdwCBW1Bg1ndmF050d6otcPHZ54xaEqZDugtv3schPrqXaNSGMg3 +DTceof/a4KUaAfJxpP7BfCTrFu+CyLhNMS5RUeKNVEXxmXnlF6g8x6gEKbJA +ave6M75z/Pluqpz6ZtYxKJPsutX4WPoGahY/Gu6aRkp1RcE7VFsSlv2sZq4A +t7zowuG+7Y+PdQzOhNkxs17D5vhjJYqF+rmFwQ/TU9V6 +=nhYM +-----END PGP MESSAGE----- diff --git a/crypto/testdata/message_sha256_signed b/crypto/testdata/message_sha256_signed new file mode 100644 index 0000000..623e7f2 --- /dev/null +++ b/crypto/testdata/message_sha256_signed @@ -0,0 +1,25 @@ +-----BEGIN PGP MESSAGE----- +Version: OpenPGP.js v4.10.4 +Comment: https://openpgpjs.org + +wcBMA0fcZ7XLgmf2AQf/adxZ10qXLL0qVcYq7iDTpGNEKbR8UWcWqN6S6GnG +XJwVretWIGLjJiPdh7irZcG993rCJr43P6uZg8AD/+7G/ceVFSRw2+dYHZmp +bq3e89py+zGO/m3o+41RF+3ekH3LHZsb/x4Ob+ZZM3UqOX9cvFiKKN1MjyH2 +JkHWhQLtZVxDKwa77QBvs5fu8kzEdVFjA1RQhRbQpkuTMkfOEfcsuqHrWJrG +T2eliNU5VzIBxfojLOSAF1ueMP3QMrB/vMimEnO140MJHf4mwj92+a9BxyXs +UAbgKC79BsrF5CsND7+ANsoTJTv2eDxf6bVEfCk8HwT5X/C4hOgJ9ILOw8K2 +Y9LBeQH9RMVQej1Az9glJY7S3FlTL9IUndFPD5Mb/+zNjNv6Mg5UquxmeZZx +S8RJDjAOTC2Rcv9jlELh6leS3meY+lpXar/ao+FL/1eB8zKkGsZqstfhmA14 +S5I5R4EgLmjkt+Ibg3Kj2sswWchB9eDdfvW/VwrhXU45Hv+EeKS/Pn+5qf05 +E5j7PQ3+EtdMTLIT3cSJe1a7zHlLR49hAzJ3Z5wPSmIzFYNE7RvnRA/7MMv/ +amlHAdIjDejDFIHR4LsT5e0GyMcAaDAOyGZRxM9fPo7PmZhz7mvoY4MGaBZ/ +icrv91IunxWo7QjLzubx4lVJM6xi2375Mntk4ZI6JNTblodOXEchTCmzNQxE +jJkzLnlMU8/gTNLJh43pgy2F8+QVYjqZT61mnScBIUidQVNeqSOiHyLCsAgS +Q2XAzIgf7HO+UBqHfrDG9wtNtUKVRrGhMwNiVXIO8VvdrKsYrWdmCgvKXZn8 +cdS9991BJQSavQRjf6rZRit+4iBzx3elYOtdN6zu3JrF1Mjm7pgDfij8Z1o1 +2hpUDNWPCa1idijPp9wvy03lXuVhB1U+rFb+ASslYoXyOASiZuKGAyxXaJwR +5VOY1EKExsSzh74w2bvtFyO/BjF5OFqtL2j3CEu3CWPiYlFPZzF+/jf8v/QQ +jfB1gs9u/Yz9ao9BU00eooLud3cS5mr8jeO/ugKh3rAqQEShsmoillyuNWlr +Dusky1Y9txR4llJOdcdIyhMhnUdMd8yVF4FfhlGgVirY +=oTYf +-----END PGP MESSAGE-----