diff --git a/ProposalChanges.md b/ProposalChanges.md index 8dd86a8..953a9f8 100644 --- a/ProposalChanges.md +++ b/ProposalChanges.md @@ -63,13 +63,6 @@ type pgpKeyObject struct { } ``` -## Dropped -### Signature -``` -type Signature struct { - md *openpgp.MessageDetails -} -``` ### SignedString ``` // SignedString wraps string with Signature @@ -78,6 +71,23 @@ type SignedString struct { Signed *Signature } ``` +is now +``` +// ClearTextMessage, split signed clear text message container +type ClearTextMessage struct { + Data []byte + Signature []byte +} + +``` + +## Dropped +### Signature +``` +type Signature struct { + md *openpgp.MessageDetails +} +``` ## New ### PGPMessage ``` @@ -87,6 +97,7 @@ type PGPMessage struct { Data []byte } ``` + ### PGPSignature ``` // PGPSignature stores a PGP-encoded detached signature. @@ -96,13 +107,31 @@ type PGPSignature struct { } ``` +### SignatureVerificationError +``` +// SignatureVerificationError is returned from Decrypt and VerifyDetached functions when signature verification fails +type SignatureVerificationError struct { + Status int + Message string +} +``` + # API changes ## armor.go ### ReadClearSignedMessage -Added signature info to returned info. +Moved to crypto package. Changed to return ClearTextMessage. ``` ReadClearSignedMessage(signedMessage string) (string, error): +* NewClearTextMessageFromArmored(signedMessage string) (*ClearTextMessage, error) +``` +In addition, were added: +``` +* NewClearTextMessage(data []byte, signature []byte) *ClearTextMessage +* (msg *ClearTextMessage) GetBinary() []byte +* (msg *ClearTextMessage) GetString() string +* (msg *ClearTextMessage) GetSignature() []byte +* (msg *ClearTextMessage) GetArmored() (string, error) ``` ## attachment.go @@ -247,10 +276,10 @@ Dropped, now the procedure is split in two parts. ``` ### DecryptString, Decrypt, DecryptArmored -Same as Encrypt* +Same as Encrypt*. If signature verification fails it will return a SignatureVerificationError. ``` (kr *KeyRing) DecryptString(encrypted string) (SignedString, error): -* (if binary data) func (keyRing *KeyRing) Decrypt(message *PGPMessage, verifyKey *KeyRing, verifyTime int64) (*PlainMessage, *Verification, error) +* (if binary data) func (keyRing *KeyRing) Decrypt(message *PGPMessage, verifyKey *KeyRing, verifyTime int64) (*PlainMessage, error) * (if plain text, wrapped) (helper) DecryptMessageArmored(privateKey, passphrase, ciphertext string) (plaintext string, err error) * (if plain text, wrapped, verified) (helper) DecryptVerifyMessageArmored(publicKey, privateKey, passphrase, ciphertext string) (plaintext string, err error) ``` @@ -271,10 +300,10 @@ Replaced by signing methods. ``` ### VerifyString -Same as signing. +Same as signing. Returns SignatureVerificationError if the verification fails. ``` (kr *KeyRing) VerifyString(message, signature string, sign *KeyRing) (err error): -* (to verify) (keyRing *KeyRing) VerifyDetached(message *PlainMessage, signature *PGPSignature, verifyTime int64) (*Verification, error) +* (to verify) (keyRing *KeyRing) VerifyDetached(message *PlainMessage, signature *PGPSignature, verifyTime int64) error ``` ### Unlock @@ -342,7 +371,7 @@ See Decrypt* (pm *PmCrypto) DecryptMessage(encryptedText string, privateKey *KeyRing, passphrase string) (string, error): (pm *PmCrypto) DecryptMessageStringKey(encryptedText string, privateKey string, passphrase string) (string, error): (pm *PmCrypto) DecryptMessageVerify(encryptedText string, verifierKey *KeyRing, privateKeyRing *KeyRing, passphrase string, verifyTime int64) (*models.DecryptSignedVerify, error) : -* (if binary data) (keyRing *KeyRing) Decrypt(message *PGPMessage, verifyKey *KeyRing, verifyTime int64) (*PlainMessage, *Verification, error) +* (if binary data) (keyRing *KeyRing) Decrypt(message *PGPMessage, verifyKey *KeyRing, verifyTime int64) (*PlainMessage, error) * (if plain text, wrapped) (helper) DecryptMessageArmored(privateKey, passphrase, ciphertext string) (plaintext string, err error) * (if plain text, wrapped, verified) (helper) DecryptVerifyMessageArmored(publicKey, privateKey, passphrase, ciphertext string) (plaintext string, err error) ``` @@ -437,7 +466,7 @@ See signature_test.go for use examples. ``` (pm *PmCrypto) VerifyTextSignDetachedBinKey(signature string, plaintext string, publicKey *KeyRing, verifyTime int64) (bool, error): (pm *PmCrypto) VerifyBinSignDetachedBinKey(signature string, plainData []byte, publicKey *KeyRing, verifyTime int64) (bool, error): -* (to verify) (keyRing *KeyRing) VerifyDetached(message *PlainMessage, signature *PGPSignature, verifyTime int64) (*Verification, error) +* (to verify) (keyRing *KeyRing) VerifyDetached(message *PlainMessage, signature *PGPSignature, verifyTime int64) (error) * (if PGP SIGNED MESSAGE) (helper) VerifyCleartextMessage(keyRing *crypto.KeyRing, armored string, verifyTime int64) (string, error) * (if PGP SIGNED MESSAGE) (helper) VerifyCleartextMessageArmored(publicKey, armored string, verifyTime int64) (string, error) ``` diff --git a/README.md b/README.md index fafa2d5..87b49ab 100644 --- a/README.md +++ b/README.md @@ -172,12 +172,10 @@ pgpMessage, err := publicKeyRing.Encrypt(binMessage, privateKeyRing) // Armored message in pgpMessage.GetArmored() // pgpMessage can be obtained from NewPGPMessageFromArmored(ciphertext) -message, verification, err := privateKeyRing.Decrypt(pgpMessage, publicKeyRing, pgp.GetUnixTime()) +message, err := privateKeyRing.Decrypt(pgpMessage, publicKeyRing, pgp.GetUnixTime()) // Original data in message.GetString() -if verification.IsValid() { - // verification success -} +// `err` can be a SignatureVerificationError ``` ### Generate key @@ -242,9 +240,9 @@ message := NewPlaintextMessage("Verified message") pgpSignature, err := NewPGPSignatureFromArmored(signature) signingKeyRing, err := pgp.BuildKeyRingArmored(pubkey) -verification, err := signingKeyRing.VerifyDetached(message, pgpSignature, pgp.GetUnixTime()) +err := signingKeyRing.VerifyDetached(message, pgpSignature, pgp.GetUnixTime()) -if verification.IsValid() { +if err == nil { // verification success } ``` @@ -287,9 +285,9 @@ message := NewPlainMessage("Verified message") pgpSignature, err := NewPGPSignatureFromArmored(signature) signingKeyRing, err := pgp.BuildKeyRingArmored(pubkey) -verification, err := signingKeyRing.VerifyDetached(message, pgpSignature, pgp.GetUnixTime()) +err := signingKeyRing.VerifyDetached(message, pgpSignature, pgp.GetUnixTime()) -if verification.IsValid() { +if err == nil { // verification success } ``` diff --git a/armor/armor.go b/armor/armor.go index 778edc1..574af0f 100644 --- a/armor/armor.go +++ b/armor/armor.go @@ -4,7 +4,6 @@ package armor import ( "bytes" - "errors" "io" "io/ioutil" @@ -12,7 +11,6 @@ import ( "github.com/ProtonMail/gopenpgp/internal" "golang.org/x/crypto/openpgp/armor" - "golang.org/x/crypto/openpgp/clearsign" ) // ArmorKey armors input as a public key. @@ -51,33 +49,3 @@ func Unarmor(input string) ([]byte, error) { } return ioutil.ReadAll(b.Body) } - -// ReadClearSignedMessage returns the message body and unarmored signature from a clearsigned message. -func ReadClearSignedMessage(signedMessage string) (string, []byte, error) { - modulusBlock, rest := clearsign.Decode([]byte(signedMessage)) - if len(rest) != 0 { - return "", nil, errors.New("pmapi: extra data after modulus") - } - - signature, err := ioutil.ReadAll(modulusBlock.ArmoredSignature.Body) - if err != nil { - return "", nil, err - } - - return string(modulusBlock.Bytes), signature, nil -} - -// ArmorClearSignedMessage armors plaintext and signature with the PGP SIGNED MESSAGE armoring -func ArmorClearSignedMessage(plaintext []byte, signature []byte) (string, error) { - armSignature, err := ArmorWithType(signature, constants.PGPSignatureHeader) - if err != nil { - return "", err - } - - str := "-----BEGIN PGP SIGNED MESSAGE-----\r\nHash:SHA512\r\n\r\n" - str += string(plaintext) - str += "\r\n" - str += armSignature - - return str, nil -} diff --git a/crypto/attachment_test.go b/crypto/attachment_test.go index 63215b6..03eb490 100644 --- a/crypto/attachment_test.go +++ b/crypto/attachment_test.go @@ -38,7 +38,7 @@ func TestAttachmentSetKey(t *testing.T) { assert.Exactly(t, testSymmetricKey, symmetricKey) } -func TestAttachnentEncryptDecrypt(t *testing.T) { +func TestAttachmentEncryptDecrypt(t *testing.T) { var testAttachmentCleartext = "cc,\ndille." var message = NewPlainMessage([]byte(testAttachmentCleartext)) @@ -55,7 +55,7 @@ func TestAttachnentEncryptDecrypt(t *testing.T) { assert.Exactly(t, message, redecData) } -func TestAttachnentEncrypt(t *testing.T) { +func TestAttachmentEncrypt(t *testing.T) { var testAttachmentCleartext = "cc,\ndille." var message = NewPlainMessage([]byte(testAttachmentCleartext)) @@ -66,7 +66,7 @@ func TestAttachnentEncrypt(t *testing.T) { pgpMessage := NewPGPMessage(append(encSplit.GetKeyPacket(), encSplit.GetDataPacket()...)) - redecData, _, err := testPrivateKeyRing.Decrypt(pgpMessage, nil, 0) + redecData, err := testPrivateKeyRing.Decrypt(pgpMessage, nil, 0) if err != nil { t.Fatal("Expected no error while decrypting attachment, got:", err) } @@ -74,7 +74,7 @@ func TestAttachnentEncrypt(t *testing.T) { assert.Exactly(t, message, redecData) } -func TestAttachnentDecrypt(t *testing.T) { +func TestAttachmentDecrypt(t *testing.T) { var testAttachmentCleartext = "cc,\ndille." var message = NewPlainMessage([]byte(testAttachmentCleartext)) diff --git a/crypto/keyring.go b/crypto/keyring.go index 8dcdc93..c61ee3b 100644 --- a/crypto/keyring.go +++ b/crypto/keyring.go @@ -322,20 +322,16 @@ func (keyRing *KeyRing) UnlockJSONKeyRing(jsonData []byte) (newKeyRing *KeyRing, return nil, err } - token, _, err := keyRing.Decrypt(message, nil, 0) + token, err := keyRing.Decrypt(message, nil, 0) if err != nil { return nil, err } - ver, err := keyRing.VerifyDetached(token, signature, 0) + err = keyRing.VerifyDetached(token, signature, 0) if err != nil { return nil, err } - if !ver.IsValid() { - return nil, errors.New("gopenpgp: unable to verify token") - } - err = newKeyRing.Unlock(token.GetBinary()) if err != nil { return nil, errors.New("gopenpgp: wrong token") diff --git a/crypto/keyring_message.go b/crypto/keyring_message.go index 37f8edf..aa076ed 100644 --- a/crypto/keyring_message.go +++ b/crypto/keyring_message.go @@ -3,18 +3,11 @@ package crypto import ( "bytes" "crypto" - "errors" "io" "io/ioutil" - "math" - "time" "golang.org/x/crypto/openpgp" - pgpErrors "golang.org/x/crypto/openpgp/errors" "golang.org/x/crypto/openpgp/packet" - - "github.com/ProtonMail/gopenpgp/constants" - "github.com/ProtonMail/gopenpgp/internal" ) // Encrypt encrypts a PlainMessage, outputs a PGPMessage. @@ -36,13 +29,10 @@ func (keyRing *KeyRing) Encrypt(message *PlainMessage, privateKey *KeyRing) (*PG // verifyTime : Time at verification (necessary only if verifyKey is not nil) func (keyRing *KeyRing) Decrypt( message *PGPMessage, verifyKey *KeyRing, verifyTime int64, -) (*PlainMessage, *Verification, error) { - decrypted, verifyStatus, err := asymmetricDecrypt(message.NewReader(), keyRing, verifyKey, verifyTime) - if err != nil { - return nil, nil, err - } +) (*PlainMessage, error) { + decrypted, err := asymmetricDecrypt(message.NewReader(), keyRing, verifyKey, verifyTime) - return NewPlainMessage(decrypted), newVerification(verifyStatus), nil + return NewPlainMessage(decrypted), err } // SignDetached generates and returns a PGPSignature for a given PlainMessage @@ -63,18 +53,16 @@ func (keyRing *KeyRing) SignDetached(message *PlainMessage) (*PGPSignature, erro } // VerifyDetached verifies a PlainMessage with embedded a PGPSignature -// and returns a Verification with the filled Verified field. +// and returns a SignatureVerificationError if fails func (keyRing *KeyRing) VerifyDetached( message *PlainMessage, signature *PGPSignature, verifyTime int64, -) (*Verification, error) { - var err error - verifyVal, err := verifySignature( +) (error) { + return verifySignature( keyRing.GetEntities(), message.NewReader(), signature.GetBinary(), verifyTime, ) - return newVerification(verifyVal), err } // ------ INTERNAL FUNCTIONS ------- @@ -123,7 +111,7 @@ func asymmetricEncrypt(data []byte, publicKey *KeyRing, privateKey *KeyRing, isB // Core for decryption+verification functions func asymmetricDecrypt( encryptedIO io.Reader, privateKey *KeyRing, verifyKey *KeyRing, verifyTime int64, -) (plaintext []byte, verified int, err error) { +) (plaintext []byte, err error) { privKeyEntries := privateKey.GetEntities() var additionalEntries openpgp.EntityList @@ -139,12 +127,12 @@ func asymmetricDecrypt( messageDetails, err := openpgp.ReadMessage(encryptedIO, privKeyEntries, nil, config) if err != nil { - return nil, constants.SIGNATURE_NOT_SIGNED, err + return nil, err } body, err := ioutil.ReadAll(messageDetails.UnverifiedBody) if err != nil { - return nil, constants.SIGNATURE_NOT_SIGNED, err + return nil, err } if verifyKey != nil { @@ -152,104 +140,8 @@ func asymmetricDecrypt( } if verifyKey != nil { - verifyStatus, verifyError := verifyDetailsSignature(messageDetails, verifyKey) - - if verifyStatus == constants.SIGNATURE_FAILED { - return nil, verifyStatus, errors.New(verifyError) - } - - return body, verifyStatus, nil + return body, verifyDetailsSignature(messageDetails, verifyKey) } - return body, constants.SIGNATURE_NOT_SIGNED, nil -} - -// processSignatureExpiration handles signature time verification manually, so we can add a margin to the -// creationTime check. -func processSignatureExpiration(md *openpgp.MessageDetails, verifyTime int64) { - if md.SignatureError == pgpErrors.ErrSignatureExpired { - if verifyTime > 0 { - created := md.Signature.CreationTime.Unix() - expires := int64(math.MaxInt64) - if md.Signature.SigLifetimeSecs != nil { - expires = int64(*md.Signature.SigLifetimeSecs) + created - } - if created-internal.CreationTimeOffset <= verifyTime && verifyTime <= expires { - md.SignatureError = nil - } - } else { - // verifyTime = 0: time check disabled, everything is okay - md.SignatureError = nil - } - } -} - -// Verify signature from message details -func verifyDetailsSignature(md *openpgp.MessageDetails, verifierKey *KeyRing) (int, string) { - if md.IsSigned { - if md.SignedBy != nil { - if len(verifierKey.entities) > 0 { - matches := verifierKey.entities.KeysById(md.SignedByKeyId) - if len(matches) > 0 { - if md.SignatureError == nil { - return constants.SIGNATURE_OK, "" - } - return constants.SIGNATURE_FAILED, md.SignatureError.Error() - } - } else { - return constants.SIGNATURE_NO_VERIFIER, "" - } - } else { - return constants.SIGNATURE_NO_VERIFIER, "" - } - } - - return constants.SIGNATURE_NOT_SIGNED, "" -} - -// verifySignature verifies if a signature is valid with the entity list -func verifySignature( - pubKeyEntries openpgp.EntityList, origText io.Reader, signature []byte, verifyTime int64) (int, error) { - config := &packet.Config{} - if verifyTime == 0 { - config.Time = func() time.Time { - return time.Unix(0, 0) - } - } else { - config.Time = func() time.Time { - return time.Unix(verifyTime+internal.CreationTimeOffset, 0) - } - } - signatureReader := bytes.NewReader(signature) - - signer, err := openpgp.CheckDetachedSignature(pubKeyEntries, origText, signatureReader, config) - - if err == pgpErrors.ErrSignatureExpired && signer != nil { - if verifyTime > 0 { // if verifyTime = 0: time check disabled, everything is okay - // Maybe the creation time offset pushed it over the edge - // Retry with the actual verification time - config.Time = func() time.Time { - return time.Unix(verifyTime, 0) - } - - _, err = signatureReader.Seek(0, io.SeekStart) - if err != nil { - return constants.SIGNATURE_FAILED, err - } - - signer, err = openpgp.CheckDetachedSignature(pubKeyEntries, origText, signatureReader, config) - if err != nil { - return constants.SIGNATURE_FAILED, err - } - } - } - - if signer == nil { - return constants.SIGNATURE_FAILED, errors.New("gopenpgp: signer is empty") - } - // if signer.PrimaryKey.KeyId != signed.PrimaryKey.KeyId { - // // t.Errorf("wrong signer got:%x want:%x", signer.PrimaryKey.KeyId, 0) - // return false, errors.New("signer is nil") - // } - return constants.SIGNATURE_OK, nil + return body, nil } diff --git a/crypto/message.go b/crypto/message.go index 6e10472..4bf725a 100644 --- a/crypto/message.go +++ b/crypto/message.go @@ -14,6 +14,7 @@ import ( "github.com/ProtonMail/gopenpgp/constants" "github.com/ProtonMail/gopenpgp/internal" + "golang.org/x/crypto/openpgp/clearsign" "golang.org/x/crypto/openpgp/packet" ) @@ -27,12 +28,6 @@ type PlainMessage struct { TextType bool } -// Verification for a PlainMessage -type Verification struct { - // If the decoded message was correctly signed. See constants.SIGNATURE* for all values. - Verified int -} - // PGPMessage stores a PGP-encrypted message. type PGPMessage struct { // The content of the message @@ -52,6 +47,12 @@ type PGPSplitMessage struct { KeyPacket []byte } +// ClearTextMessage, split signed clear text message container +type ClearTextMessage struct { + Data []byte + Signature []byte +} + // ---- GENERATORS ----- // NewPlainMessage generates a new binary PlainMessage ready for encryption, @@ -72,13 +73,6 @@ func NewPlainMessageFromString(text string) *PlainMessage { } } -// newVerification returns a new instance of *Verification with the specified value -func newVerification(value int) *Verification { - return &Verification{ - Verified: value, - } -} - // NewPGPMessage generates a new PGPMessage from the unarmored binary data. func NewPGPMessage(data []byte) *PGPMessage { return &PGPMessage{ @@ -147,6 +141,29 @@ func NewPGPSignatureFromArmored(armored string) (*PGPSignature, error) { }, nil } +// NewClearTextMessage generates a new ClearTextMessage from data and signature +func NewClearTextMessage(data []byte, signature []byte) *ClearTextMessage { + return &ClearTextMessage{ + Data: data, + Signature: signature, + } +} + +// NewClearTextMessageFromArmored returns the message body and unarmored signature from a clearsigned message. +func NewClearTextMessageFromArmored(signedMessage string) (*ClearTextMessage, error) { + modulusBlock, rest := clearsign.Decode([]byte(signedMessage)) + if len(rest) != 0 { + return nil, errors.New("pmapi: extra data after modulus") + } + + signature, err := ioutil.ReadAll(modulusBlock.ArmoredSignature.Body) + if err != nil { + return nil, err + } + + return NewClearTextMessage(modulusBlock.Bytes, signature), nil +} + // ---- MODEL METHODS ----- // GetBinary returns the binary content of the message as a []byte @@ -164,19 +181,6 @@ func (msg *PlainMessage) GetBase64() string { return base64.StdEncoding.EncodeToString(msg.Data) } -// GetVerification returns the verification status of a verification, -// to use after the KeyRing.Decrypt* or KeyRing.Verify* functions. -// The int value returned is to compare to constants.SIGNATURE*. -func (ver *Verification) GetVerification() int { - return ver.Verified -} - -// IsValid returns true if the message is signed and the signature is valid. -// To use after the KeyRing.Decrypt* or KeyRing.Verify* functions. -func (ver *Verification) IsValid() bool { - return ver.Verified == constants.SIGNATURE_OK -} - // NewReader returns a New io.Reader for the bianry data of the message func (msg *PlainMessage) NewReader() io.Reader { return bytes.NewReader(msg.GetBinary()) @@ -317,6 +321,36 @@ func (msg *PGPSignature) GetArmored() (string, error) { return armor.ArmorWithType(msg.Data, constants.PGPSignatureHeader) } +// GetBinary returns the unarmored signed data as a []byte +func (msg *ClearTextMessage) GetBinary() []byte { + return msg.Data +} + +// GetString returns the unarmored signed data as a string +func (msg *ClearTextMessage) GetString() string { + return string(msg.Data) +} + +// GetSignature returns the unarmored binary signature as a []byte +func (msg *ClearTextMessage) GetSignature() []byte { + return msg.Signature +} + +// GetArmored armors plaintext and signature with the PGP SIGNED MESSAGE armoring +func (msg *ClearTextMessage) GetArmored() (string, error) { + armSignature, err := armor.ArmorWithType(msg.GetSignature(), constants.PGPSignatureHeader) + if err != nil { + return "", err + } + + str := "-----BEGIN PGP SIGNED MESSAGE-----\r\nHash:SHA512\r\n\r\n" + str += msg.GetString() + str += "\r\n" + str += armSignature + + return str, nil +} + // ---- UTILS ----- // IsPGPMessage checks if data if has armored PGP message format. diff --git a/crypto/message_test.go b/crypto/message_test.go index 8161c43..4b437a0 100644 --- a/crypto/message_test.go +++ b/crypto/message_test.go @@ -5,7 +5,6 @@ import ( "strings" "testing" - "github.com/ProtonMail/gopenpgp/constants" "github.com/stretchr/testify/assert" ) @@ -67,13 +66,11 @@ func TestTextMessageEncryption(t *testing.T) { t.Fatal("Expected no error when encrypting, got:", err) } - decrypted, ver, err := testPrivateKeyRing.Decrypt(ciphertext, testPublicKeyRing, pgp.GetUnixTime()) + decrypted, err := testPrivateKeyRing.Decrypt(ciphertext, testPublicKeyRing, pgp.GetUnixTime()) if err != nil { t.Fatal("Expected no error when decrypting, got:", err) } assert.Exactly(t, message.GetString(), decrypted.GetString()) - assert.Exactly(t, constants.SIGNATURE_OK, ver.GetVerification()) - assert.Exactly(t, true, ver.IsValid()) } func TestBinaryMessageEncryption(t *testing.T) { @@ -94,13 +91,11 @@ func TestBinaryMessageEncryption(t *testing.T) { t.Fatal("Expected no error when encrypting, got:", err) } - decrypted, ver, err := testPrivateKeyRing.Decrypt(ciphertext, testPublicKeyRing, pgp.GetUnixTime()) + decrypted, err := testPrivateKeyRing.Decrypt(ciphertext, testPublicKeyRing, pgp.GetUnixTime()) if err != nil { t.Fatal("Expected no error when decrypting, got:", err) } assert.Exactly(t, message.GetBinary(), decrypted.GetBinary()) - assert.Exactly(t, constants.SIGNATURE_OK, ver.GetVerification()) - assert.Exactly(t, true, ver.IsValid()) } func TestIssue11(t *testing.T) { @@ -126,11 +121,10 @@ func TestIssue11(t *testing.T) { t.Fatal("Expected no error while unlocking private keyring, got:", err) } - plainMessage, verification, err := myKeyring.Decrypt(pgpMessage, senderKeyring, 0) + plainMessage, err := myKeyring.Decrypt(pgpMessage, senderKeyring, 0) if err != nil { t.Fatal("Expected no error while decrypting/verifying, got:", err) } assert.Exactly(t, "message from sender", plainMessage.GetString()) - assert.Exactly(t, true, verification.IsValid()) } diff --git a/crypto/mime.go b/crypto/mime.go index a93b3b2..91521d9 100644 --- a/crypto/mime.go +++ b/crypto/mime.go @@ -8,7 +8,6 @@ import ( "strings" gomime "github.com/ProtonMail/go-mime" - "github.com/ProtonMail/gopenpgp/constants" "golang.org/x/crypto/openpgp" "golang.org/x/crypto/openpgp/packet" @@ -28,13 +27,13 @@ type MIMECallbacks interface { func (keyRing *KeyRing) DecryptMIMEMessage( message *PGPMessage, verifyKey *KeyRing, callbacks MIMECallbacks, verifyTime int64, ) { - decryptedMessage, verification, err := keyRing.Decrypt(message, verifyKey, verifyTime) + decryptedMessage, err := keyRing.Decrypt(message, verifyKey, verifyTime) if err != nil { callbacks.OnError(err) return } - body, verified, attachments, attachmentHeaders, err := pgp.parseMIME(decryptedMessage.GetString(), verifyKey) + body, attachments, attachmentHeaders, err := pgp.parseMIME(decryptedMessage.GetString(), verifyKey) if err != nil { callbacks.OnError(err) return @@ -45,28 +44,23 @@ func (keyRing *KeyRing) DecryptMIMEMessage( callbacks.OnAttachment(attachmentHeaders[i], []byte(attachments[i])) } callbacks.OnEncryptedHeaders("") - if verification.GetVerification() != constants.SIGNATURE_NOT_SIGNED { - callbacks.OnVerified(verification.GetVerification()) - } else { - callbacks.OnVerified(verified) - } } // ----- INTERNAL FUNCTIONS ----- func (pgp GopenPGP) parseMIME( mimeBody string, verifierKey *KeyRing, -) (*gomime.BodyCollector, int, []string, []string, error) { +) (*gomime.BodyCollector, []string, []string, error) { mm, err := mail.ReadMessage(strings.NewReader(mimeBody)) if err != nil { - return nil, 0, nil, nil, err + return nil, nil, nil, err } config := &packet.Config{DefaultCipher: packet.CipherAES256, Time: pgp.getTimeGenerator()} h := textproto.MIMEHeader(mm.Header) mmBodyData, err := ioutil.ReadAll(mm.Body) if err != nil { - return nil, 0, nil, nil, err + return nil, nil, nil, err } printAccepter := gomime.NewMIMEPrinter() @@ -82,11 +76,12 @@ func (pgp GopenPGP) parseMIME( signatureCollector := newSignatureCollector(mimeVisitor, pgpKering, config) err = gomime.VisitAll(bytes.NewReader(mmBodyData), h, signatureCollector) + if err == nil && verifierKey != nil { + err = signatureCollector.verified; + } - verified := signatureCollector.verified - body := bodyCollector - atts := attachmentsCollector.GetAttachments() - attHeaders := attachmentsCollector.GetAttHeaders() - - return body, verified, atts, attHeaders, err + return bodyCollector, + attachmentsCollector.GetAttachments(), + attachmentsCollector.GetAttHeaders(), + err } diff --git a/crypto/mime_test.go b/crypto/mime_test.go index 26022c5..1a1800d 100644 --- a/crypto/mime_test.go +++ b/crypto/mime_test.go @@ -1,10 +1,9 @@ package crypto import ( - "github.com/ProtonMail/gopenpgp/internal" - "github.com/stretchr/testify/assert" - "io/ioutil" "testing" + + "github.com/stretchr/testify/assert" ) // Corresponding key in testdata/mime_privateKey @@ -38,21 +37,8 @@ func TestDecrypt(t *testing.T) { callbacks := Callbacks{ Testing: t, } + privateKeyRing, _ := pgp.BuildKeyRingArmored(readTestFile("mime_privateKey", false)) - block, err := internal.Unarmor(readTestFile("mime_publicKey", false)) - if err != nil { - t.Fatal("Cannot unarmor public key: ", err) - } - - publicKeyUnarmored, _ := ioutil.ReadAll(block.Body) - - block, err = internal.Unarmor(readTestFile("mime_privateKey", false)) - if err != nil { - t.Fatal("Cannot unarmor private key:", err) - } - - privateKeyUnarmored, _ := ioutil.ReadAll(block.Body) - privateKeyRing, _ := pgp.BuildKeyRing(privateKeyUnarmored) err = privateKeyRing.UnlockWithPassphrase(privateKeyPassword) if err != nil { t.Fatal("Cannot unlock private key:", err) @@ -65,13 +51,13 @@ func TestDecrypt(t *testing.T) { privateKeyRing.DecryptMIMEMessage( message, - pgp.BuildKeyRingNoError(publicKeyUnarmored), + nil, &callbacks, pgp.GetUnixTime()) } func TestParse(t *testing.T) { - body, _, atts, attHeaders, err := pgp.parseMIME(readTestFile("mime_testMessage", false), nil) + body, atts, attHeaders, err := pgp.parseMIME(readTestFile("mime_testMessage", false), nil) if err != nil { t.Error("Expected no error while parsing message, got:", err) diff --git a/crypto/signature.go b/crypto/signature.go new file mode 100644 index 0000000..cfd4cf1 --- /dev/null +++ b/crypto/signature.go @@ -0,0 +1,141 @@ +package crypto + +import ( + "bytes" + "fmt" + "io" + "math" + "time" + + "golang.org/x/crypto/openpgp" + "golang.org/x/crypto/openpgp/packet" + pgpErrors "golang.org/x/crypto/openpgp/errors" + + "github.com/ProtonMail/gopenpgp/constants" + "github.com/ProtonMail/gopenpgp/internal" +) + +// SignatureVerificationError is returned from Decrypt and VerifyDetached functions when signature verification fails +type SignatureVerificationError struct { + Status int + Message string +} + +// Error is the base method for all errors +func (e SignatureVerificationError) Error() string { + return fmt.Sprintf("Signature Verification Error: %v", e.Message) +} + +// ------------------ +// Internal functions +// ------------------ + +// newSignatureFailed creates a new SignatureVerificationError, type SIGNATURE_FAILED +func newSignatureFailed() SignatureVerificationError { + return SignatureVerificationError { + constants.SIGNATURE_FAILED, + "Invalid signature", + } +} + +// newSignatureNotSigned creates a new SignatureVerificationError, type SIGNATURE_NOT_SIGNED +func newSignatureNotSigned() SignatureVerificationError { + return SignatureVerificationError { + constants.SIGNATURE_NOT_SIGNED, + "Missing signature", + } +} + +// newSignatureNoVerifier creates a new SignatureVerificationError, type SIGNATURE_NO_VERIFIER +func newSignatureNoVerifier() SignatureVerificationError { + return SignatureVerificationError { + constants.SIGNATURE_NO_VERIFIER, + "No matching signature", + } +} + +// processSignatureExpiration handles signature time verification manually, so we can add a margin to the +// creationTime check. +func processSignatureExpiration(md *openpgp.MessageDetails, verifyTime int64) { + if md.SignatureError == pgpErrors.ErrSignatureExpired { + if verifyTime > 0 { + created := md.Signature.CreationTime.Unix() + expires := int64(math.MaxInt64) + if md.Signature.SigLifetimeSecs != nil { + expires = int64(*md.Signature.SigLifetimeSecs) + created + } + if created-internal.CreationTimeOffset <= verifyTime && verifyTime <= expires { + md.SignatureError = nil + } + } else { + // verifyTime = 0: time check disabled, everything is okay + md.SignatureError = nil + } + } +} + +// verifyDetailsSignature verifies signature from message details +func verifyDetailsSignature(md *openpgp.MessageDetails, verifierKey *KeyRing) error { + if md.IsSigned { + if md.SignedBy != nil { + if len(verifierKey.entities) > 0 { + matches := verifierKey.entities.KeysById(md.SignedByKeyId) + if len(matches) > 0 { + if md.SignatureError == nil { + return nil + } + return newSignatureFailed() + } + } else { + return newSignatureNoVerifier() + } + } else { + return newSignatureNoVerifier() + } + } + + return newSignatureNoVerifier() +} + +// verifySignature verifies if a signature is valid with the entity list +func verifySignature(pubKeyEntries openpgp.EntityList, origText io.Reader, signature []byte, verifyTime int64) error { + config := &packet.Config{} + if verifyTime == 0 { + config.Time = func() time.Time { + return time.Unix(0, 0) + } + } else { + config.Time = func() time.Time { + return time.Unix(verifyTime+internal.CreationTimeOffset, 0) + } + } + signatureReader := bytes.NewReader(signature) + + signer, err := openpgp.CheckDetachedSignature(pubKeyEntries, origText, signatureReader, config) + + if err == pgpErrors.ErrSignatureExpired && signer != nil { + if verifyTime > 0 { // if verifyTime = 0: time check disabled, everything is okay + // Maybe the creation time offset pushed it over the edge + // Retry with the actual verification time + config.Time = func() time.Time { + return time.Unix(verifyTime, 0) + } + + _, err = signatureReader.Seek(0, io.SeekStart) + if err != nil { + return newSignatureFailed() + } + + signer, err = openpgp.CheckDetachedSignature(pubKeyEntries, origText, signatureReader, config) + if err != nil { + return newSignatureFailed() + } + } + } + + if signer == nil { + return newSignatureFailed() + } + + return nil +} diff --git a/crypto/signature_collector.go b/crypto/signature_collector.go index 37282a1..4e35012 100644 --- a/crypto/signature_collector.go +++ b/crypto/signature_collector.go @@ -8,7 +8,6 @@ import ( "net/textproto" gomime "github.com/ProtonMail/go-mime" - "github.com/ProtonMail/gopenpgp/constants" "golang.org/x/crypto/openpgp" "golang.org/x/crypto/openpgp/packet" @@ -20,7 +19,7 @@ type SignatureCollector struct { keyring openpgp.KeyRing target gomime.VisitAcceptor signature string - verified int + verified error } func newSignatureCollector( @@ -52,7 +51,7 @@ func (sc *SignatureCollector) Accept( } } if len(multiparts) != 2 { - sc.verified = constants.SIGNATURE_NOT_SIGNED + sc.verified = newSignatureNotSigned() // Invalid multipart/signed format just pass along _, err = ioutil.ReadAll(rawBody) if err != nil { @@ -97,12 +96,12 @@ func (sc *SignatureCollector) Accept( _, err = openpgp.CheckArmoredDetachedSignature(sc.keyring, rawBody, bytes.NewReader(buffer), sc.config) if err != nil { - sc.verified = constants.SIGNATURE_FAILED + sc.verified = newSignatureFailed() } else { - sc.verified = constants.SIGNATURE_OK + sc.verified = nil } } else { - sc.verified = constants.SIGNATURE_NO_VERIFIER + sc.verified = newSignatureNoVerifier() } return nil } diff --git a/crypto/signature_test.go b/crypto/signature_test.go index 8eb3ea3..c46569c 100644 --- a/crypto/signature_test.go +++ b/crypto/signature_test.go @@ -48,20 +48,20 @@ func TestSignTextDetached(t *testing.T) { } func TestVerifyTextDetachedSig(t *testing.T) { - signedMessage, err := signingKeyRing.VerifyDetached(message, textSignature, testTime) - if err != nil { + verificationError := signingKeyRing.VerifyDetached(message, textSignature, testTime) + if verificationError != nil { t.Fatal("Cannot verify plaintext signature:", err) } - - assert.Exactly(t, constants.SIGNATURE_OK, signedMessage.GetVerification()) } func TestVerifyTextDetachedSigWrong(t *testing.T) { fakeMessage := NewPlainMessageFromString("wrong text") - signedMessage, err := signingKeyRing.VerifyDetached(fakeMessage, textSignature, testTime) + verificationError := signingKeyRing.VerifyDetached(fakeMessage, textSignature, testTime) - assert.EqualError(t, err, "gopenpgp: signer is empty") - assert.Exactly(t, constants.SIGNATURE_FAILED, signedMessage.GetVerification()) + assert.EqualError(t, verificationError, "Signature Verification Error: Invalid signature") + + err, _ := verificationError.(SignatureVerificationError) + assert.Exactly(t, constants.SIGNATURE_FAILED, err.Status) } func TestSignBinDetached(t *testing.T) { @@ -81,10 +81,8 @@ func TestSignBinDetached(t *testing.T) { } func TestVerifyBinDetachedSig(t *testing.T) { - signedMessage, err := signingKeyRing.VerifyDetached(message, binSignature, testTime) - if err != nil { + verificationError := signingKeyRing.VerifyDetached(message, binSignature, testTime) + if verificationError != nil { t.Fatal("Cannot verify binary signature:", err) } - - assert.Exactly(t, constants.SIGNATURE_OK, signedMessage.GetVerification()) } diff --git a/helper/cleartext.go b/helper/cleartext.go index b5cfb72..a602eb9 100644 --- a/helper/cleartext.go +++ b/helper/cleartext.go @@ -1,10 +1,8 @@ package helper import ( - "errors" "strings" - "github.com/ProtonMail/gopenpgp/armor" "github.com/ProtonMail/gopenpgp/crypto" "github.com/ProtonMail/gopenpgp/internal" ) @@ -47,28 +45,24 @@ func SignCleartextMessage(keyRing *crypto.KeyRing, text string) (string, error) return "", err } - return armor.ArmorClearSignedMessage(message.GetBinary(), signature.GetBinary()) + return crypto.NewClearTextMessage(message.GetBinary(), signature.GetBinary()).GetArmored() } // VerifyCleartextMessage verifies PGP-compliant armored signed plain text given the public keyring // and returns the text or err if the verification fails func VerifyCleartextMessage(keyRing *crypto.KeyRing, armored string, verifyTime int64) (string, error) { - text, signatureData, err := armor.ReadClearSignedMessage(armored) + clearTextMessage, err := crypto.NewClearTextMessageFromArmored(armored) if err != nil { return "", err } - message := crypto.NewPlainMessageFromString(text) - signature := crypto.NewPGPSignature(signatureData) - ver, err := keyRing.VerifyDetached(message, signature, verifyTime) + message := crypto.NewPlainMessageFromString(clearTextMessage.GetString()) + signature := crypto.NewPGPSignature(clearTextMessage.GetSignature()) + err = keyRing.VerifyDetached(message, signature, verifyTime) if err != nil { return "", err } - if !ver.IsValid() { - return "", errors.New("gopenpgp: unable to verify attachment") - } - return message.GetString(), nil } diff --git a/helper/helper.go b/helper/helper.go index f74f06a..4d0c141 100644 --- a/helper/helper.go +++ b/helper/helper.go @@ -130,7 +130,7 @@ func DecryptMessageArmored( return "", err } - if message, _, err = privateKeyRing.Decrypt(pgpMessage, nil, 0); err != nil { + if message, err = privateKeyRing.Decrypt(pgpMessage, nil, 0); err != nil { return "", err } @@ -146,7 +146,6 @@ func DecryptVerifyMessageArmored( var publicKeyRing, privateKeyRing *crypto.KeyRing var pgpMessage *crypto.PGPMessage var message *crypto.PlainMessage - var verification *crypto.Verification if publicKeyRing, err = pgp.BuildKeyRingArmored(publicKey); err != nil { return "", err @@ -164,14 +163,10 @@ func DecryptVerifyMessageArmored( return "", err } - if message, verification, err = privateKeyRing.Decrypt(pgpMessage, publicKeyRing, pgp.GetUnixTime()); err != nil { + if message, err = privateKeyRing.Decrypt(pgpMessage, publicKeyRing, pgp.GetUnixTime()); err != nil { return "", err } - if !verification.IsValid() { - return "", errors.New("gopenpgp: unable to verify message") - } - return message.GetString(), nil } @@ -222,7 +217,6 @@ func DecryptVerifyAttachment( var publicKeyRing, privateKeyRing *crypto.KeyRing var detachedSignature *crypto.PGPSignature var message *crypto.PlainMessage - var verification *crypto.Verification var packets = crypto.NewPGPSplitMessage(keyPacket, dataPacket) @@ -246,11 +240,7 @@ func DecryptVerifyAttachment( return nil, err } - if verification, err = publicKeyRing.VerifyDetached(message, detachedSignature, pgp.GetUnixTime()); err != nil { - return nil, errors.New("gopenpgp: unable to verify attachment") - } - - if !verification.IsValid() { + if publicKeyRing.VerifyDetached(message, detachedSignature, pgp.GetUnixTime()) != nil { return nil, errors.New("gopenpgp: unable to verify attachment") } diff --git a/helper/helper_test.go b/helper/helper_test.go index 1ca316c..cd00ce2 100644 --- a/helper/helper_test.go +++ b/helper/helper_test.go @@ -70,7 +70,7 @@ func TestArmoredTextMessageEncryptionVerification(t *testing.T) { testMailboxPassword, // Password defined in base_test armored, ) - assert.EqualError(t, err, "gopenpgp: unable to verify message") + assert.EqualError(t, err, "Signature Verification Error: No matching signature") decrypted, err := DecryptVerifyMessageArmored( readTestFile("keyring_publicKey", false), @@ -78,6 +78,7 @@ func TestArmoredTextMessageEncryptionVerification(t *testing.T) { testMailboxPassword, // Password defined in base_test armored, ) + if err != nil { t.Fatal("Expected no error when decrypting, got:", err) }