Merge pull request #137 from ProtonMail/feat/explicit-stream-verify

Add a wrapper for explicit verification of decrypted streams.

CHANGELOG.md

@@ -4,6 +4,24 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## Unreleased
+
+### Changed
+
+- Changed the returned `SignatureVerificationError.Status` when trying to verify a message with no embedded signature. It used to return `constants.SIGNATURE_NO_VERIFIER` and now returns `constants.SIGNATURE_NOT_SIGNED`.
+This change impacts : 
+	- `func (sk *SessionKey) DecryptAndVerify(...)`
+	- `func (msg *PlainMessageReader) VerifySignature(...)`
+	- `func (keyRing *KeyRing) Decrypt(...)`
+
+### Added 
+- Helper to access the SignatureVerificationError explicitly when decrypting streams  in mobile apps: 
+	```go
+	func VerifySignatureExplicit(
+		reader *crypto.PlainMessageReader,
+	) (signatureVerificationError *crypto.SignatureVerificationError, err error)
+	```
+
 ## [2.2.0] 2021-06-30
 ### Added
 - Streaming API:

crypto/signature.go

@@ -99,8 +99,10 @@ 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.IsSigned {
-		md.SignedBy == nil ||
+		return newSignatureNotSigned()
+	}
+	if md.SignedBy == nil ||
 		len(verifierKey.entities) == 0 ||
 		len(verifierKey.entities.KeysById(md.SignedByKeyId)) == 0 {
 		return newSignatureNoVerifier()

helper/mobile_stream.go

@@ -180,3 +180,24 @@ func (r *Go2IOSReader) Read(max int) (result *MobileReadResult, err error) {
 	}
 	return result, nil
 }
+
+// VerifySignatureExplicit calls the reader's VerifySignature()
+// and tries to cast the returned error to a SignatureVerificationError.
+func VerifySignatureExplicit(
+	reader *crypto.PlainMessageReader,
+) (signatureVerificationError *crypto.SignatureVerificationError, err error) {
+	if reader == nil {
+		return nil, errors.New("gopenppg: the reader can't be nil")
+	}
+	err = reader.VerifySignature()
+	if err != nil {
+		castedErr := &crypto.SignatureVerificationError{}
+		isType := errors.As(err, castedErr)
+		if !isType {
+			return
+		}
+		signatureVerificationError = castedErr
+		err = nil
+	}
+	return
+}

helper/mobile_stream_test.go

@@ -5,7 +5,11 @@ import (
 	"crypto/sha256"
 	"errors"
 	"io"
+	"io/ioutil"
 	"testing"
+
+	"github.com/ProtonMail/gopenpgp/v2/constants"
+	"github.com/ProtonMail/gopenpgp/v2/crypto"
 )
 
 func cloneTestData() (a, b []byte) {
@@ -180,3 +184,163 @@ func TestMobile2GoReader(t *testing.T) {
 		t.Fatal("expected an error while reading, got nil")
 	}
 }
+
+func setUpTestKeyRing() (*crypto.KeyRing, *crypto.KeyRing, error) {
+	testKey, err := crypto.GenerateKey("test", "test@protonmail.com", "x25519", 256)
+	if err != nil {
+		return nil, nil, err
+	}
+	testPublicKey, err := testKey.ToPublic()
+	if err != nil {
+		return nil, nil, err
+	}
+	testPrivateKeyRing, err := crypto.NewKeyRing(testKey)
+	if err != nil {
+		return nil, nil, err
+	}
+	testPublicKeyRing, err := crypto.NewKeyRing(testPublicKey)
+	if err != nil {
+		return nil, nil, err
+	}
+	return testPublicKeyRing, testPrivateKeyRing, nil
+}
+
+func TestExplicitVerifyAllGoesWell(t *testing.T) {
+	data := []byte("hello")
+	pubKR, privKR, err := setUpTestKeyRing()
+	if err != nil {
+		t.Fatalf("Got an error while loading test key: %v", err)
+	}
+	defer privKR.ClearPrivateParams()
+	ciphertext, err := pubKR.Encrypt(crypto.NewPlainMessage(data), privKR)
+	if err != nil {
+		t.Fatalf("Got an error while encrypting test data: %v", err)
+	}
+	reader, err := privKR.DecryptStream(
+		bytes.NewReader(ciphertext.Data),
+		pubKR,
+		crypto.GetUnixTime(),
+	)
+	if err != nil {
+		t.Fatalf("Got an error while decrypting stream data: %v", err)
+	}
+	_, err = ioutil.ReadAll(reader)
+	if err != nil {
+		t.Fatalf("Got an error while reading decrypted data: %v", err)
+	}
+	sigErr, err := VerifySignatureExplicit(reader)
+	if sigErr != nil {
+		t.Fatalf("Got a signature error while verifying embedded sig: %v", sigErr)
+	}
+	if err != nil {
+		t.Fatalf("Got an error while verifying embedded sig: %v", err)
+	}
+}
+
+func TestExplicitVerifyTooEarly(t *testing.T) {
+	data := []byte("hello")
+	pubKR, privKR, err := setUpTestKeyRing()
+	if err != nil {
+		t.Fatalf("Got an error while loading test key: %v", err)
+	}
+	defer privKR.ClearPrivateParams()
+	ciphertext, err := pubKR.Encrypt(crypto.NewPlainMessage(data), privKR)
+	if err != nil {
+		t.Fatalf("Got an error while encrypting test data: %v", err)
+	}
+	reader, err := privKR.DecryptStream(
+		bytes.NewReader(ciphertext.Data),
+		pubKR,
+		crypto.GetUnixTime(),
+	)
+	if err != nil {
+		t.Fatalf("Got an error while decrypting stream data: %v", err)
+	}
+	buff := make([]byte, 1)
+	_, err = reader.Read(buff)
+	if err != nil {
+		t.Fatalf("Got an error while reading decrypted data: %v", err)
+	}
+	sigErr, err := VerifySignatureExplicit(reader)
+	if sigErr != nil {
+		t.Fatalf("Got a signature error while verifying embedded sig: %v", sigErr)
+	}
+	if err == nil {
+		t.Fatalf("Got no error while verifying a reader before reading it entirely")
+	}
+}
+
+func TestExplicitVerifyNoSig(t *testing.T) {
+	data := []byte("hello")
+	pubKR, privKR, err := setUpTestKeyRing()
+	if err != nil {
+		t.Fatalf("Got an error while loading test key: %v", err)
+	}
+	defer privKR.ClearPrivateParams()
+	ciphertext, err := pubKR.Encrypt(crypto.NewPlainMessage(data), nil)
+	if err != nil {
+		t.Fatalf("Got an error while encrypting test data: %v", err)
+	}
+	reader, err := privKR.DecryptStream(
+		bytes.NewReader(ciphertext.Data),
+		pubKR,
+		crypto.GetUnixTime(),
+	)
+	if err != nil {
+		t.Fatalf("Got an error while decrypting stream data: %v", err)
+	}
+	_, err = ioutil.ReadAll(reader)
+	if err != nil {
+		t.Fatalf("Got an error while reading decrypted data: %v", err)
+	}
+	sigErr, err := VerifySignatureExplicit(reader)
+	if sigErr == nil {
+		t.Fatal("Got no signature error while verifying unsigned data")
+	}
+	if sigErr.Status != constants.SIGNATURE_NOT_SIGNED {
+		t.Fatal("Signature error status was not SIGNATURE_NOT_SIGNED")
+	}
+	if err != nil {
+		t.Fatalf("Got an error while verifying embedded sig: %v", err)
+	}
+}
+
+func TestExplicitVerifyWrongVerifier(t *testing.T) {
+	data := []byte("hello")
+	pubKR, privKR, err := setUpTestKeyRing()
+	if err != nil {
+		t.Fatalf("Got an error while loading test key: %v", err)
+	}
+	defer privKR.ClearPrivateParams()
+	_, privKR2, err := setUpTestKeyRing()
+	if err != nil {
+		t.Fatalf("Got an error while loading test key: %v", err)
+	}
+	defer privKR2.ClearPrivateParams()
+	ciphertext, err := pubKR.Encrypt(crypto.NewPlainMessage(data), privKR2)
+	if err != nil {
+		t.Fatalf("Got an error while encrypting test data: %v", err)
+	}
+	reader, err := privKR.DecryptStream(
+		bytes.NewReader(ciphertext.Data),
+		pubKR,
+		crypto.GetUnixTime(),
+	)
+	if err != nil {
+		t.Fatalf("Got an error while decrypting stream data: %v", err)
+	}
+	_, err = ioutil.ReadAll(reader)
+	if err != nil {
+		t.Fatalf("Got an error while reading decrypted data: %v", err)
+	}
+	sigErr, err := VerifySignatureExplicit(reader)
+	if sigErr == nil {
+		t.Fatal("Got no signature error while verifying with wrong key")
+	}
+	if sigErr.Status != constants.SIGNATURE_NO_VERIFIER {
+		t.Fatal("Signature error status was not SIGNATURE_NO_VERIFIER")
+	}
+	if err != nil {
+		t.Fatalf("Got an error while verifying embedded sig: %v", err)
+	}
+}

helper/mobile_test.go

@@ -85,7 +85,7 @@ func TestMobileSignedMessageDecryptionWithSessionKey(t *testing.T) {
 		t.Fatal("Expected no error when decrypting, got:", err)
 	}
 
-	assert.Exactly(t, constants.SIGNATURE_NO_VERIFIER, decrypted.SignatureVerificationError.Status)
+	assert.Exactly(t, constants.SIGNATURE_NOT_SIGNED, decrypted.SignatureVerificationError.Status)
 	assert.Exactly(t, message.GetString(), decrypted.Message.GetString())
 
 	publicKey, _ = crypto.NewKeyFromArmored(readTestFile("keyring_publicKey", false))