feat(BRIDGE-120): use appropriate address key when importing / saving draft.

This commit is contained in:
Xavier Michelon
2024-10-09 07:44:12 +02:00
parent bee2642aec
commit 7457fb06d2
4 changed files with 279 additions and 9 deletions

View File

@ -31,6 +31,7 @@ import (
"github.com/ProtonMail/gluon/connector" "github.com/ProtonMail/gluon/connector"
"github.com/ProtonMail/gluon/imap" "github.com/ProtonMail/gluon/imap"
"github.com/ProtonMail/gluon/reporter" "github.com/ProtonMail/gluon/reporter"
"github.com/ProtonMail/gluon/rfc5322"
"github.com/ProtonMail/gluon/rfc822" "github.com/ProtonMail/gluon/rfc822"
"github.com/ProtonMail/go-proton-api" "github.com/ProtonMail/go-proton-api"
"github.com/ProtonMail/gopenpgp/v2/crypto" "github.com/ProtonMail/gopenpgp/v2/crypto"
@ -69,6 +70,8 @@ type Connector struct {
syncState *SyncState syncState *SyncState
} }
var errNoSenderAddressMatch = errors.New("no matching sender found in address list")
func NewConnector( func NewConnector(
addrID string, addrID string,
apiClient APIClient, apiClient APIClient,
@ -107,6 +110,7 @@ func NewConnector(
labels: labels, labels: labels,
addressMode: addressMode, addressMode: addressMode,
log: logrus.WithFields(logrus.Fields{ log: logrus.WithFields(logrus.Fields{
"pkg": "imapservice",
"gluon-connector": addressMode, "gluon-connector": addressMode,
"addr-id": addrID, "addr-id": addrID,
"user-id": userID, "user-id": userID,
@ -676,20 +680,18 @@ func (s *Connector) importMessage(
) (imap.Message, []byte, error) { ) (imap.Message, []byte, error) {
var full proton.FullMessage var full proton.FullMessage
// addr is primary for combined mode or active for split mode
addr, ok := s.identityState.GetAddress(s.addrID)
if !ok {
return imap.Message{}, nil, fmt.Errorf("could not find address")
}
p, err2 := parser.New(bytes.NewReader(literal)) p, err2 := parser.New(bytes.NewReader(literal))
if err2 != nil { if err2 != nil {
return imap.Message{}, nil, fmt.Errorf("failed to parse literal: %w", err2) return imap.Message{}, nil, fmt.Errorf("failed to parse literal: %w", err2)
} }
isDraft := slices.Contains(labelIDs, proton.DraftsLabel) isDraft := slices.Contains(labelIDs, proton.DraftsLabel)
addr, err := s.getImportAddress(p, isDraft)
if err != nil {
return imap.Message{}, nil, err
}
if err := s.identityState.WithAddrKR(s.addrID, func(_, addrKR *crypto.KeyRing) error { if err := s.identityState.WithAddrKR(addr.ID, func(_, addrKR *crypto.KeyRing) error {
primaryKey, errKey := addrKR.FirstKey() primaryKey, errKey := addrKR.FirstKey()
if errKey != nil { if errKey != nil {
return fmt.Errorf("failed to get primary key for import: %w", errKey) return fmt.Errorf("failed to get primary key for import: %w", errKey)
@ -716,7 +718,7 @@ func (s *Connector) importMessage(
} }
str, err := s.client.ImportMessages(ctx, primaryKey, 1, 1, []proton.ImportReq{{ str, err := s.client.ImportMessages(ctx, primaryKey, 1, 1, []proton.ImportReq{{
Metadata: proton.ImportMetadata{ Metadata: proton.ImportMetadata{
AddressID: s.addrID, AddressID: addr.ID,
LabelIDs: labelIDs, LabelIDs: labelIDs,
Unread: proton.Bool(unread), Unread: proton.Bool(unread),
Flags: flags, Flags: flags,
@ -874,3 +876,75 @@ func stripPlusAlias(a string) string {
func equalAddresses(a, b string) bool { func equalAddresses(a, b string) bool {
return strings.EqualFold(stripPlusAlias(a), stripPlusAlias(b)) return strings.EqualFold(stripPlusAlias(a), stripPlusAlias(b))
} }
func (s *Connector) getImportAddress(p *parser.Parser, isDraft bool) (proton.Address, error) {
// addr is primary for combined mode or active for split mode
address, ok := s.identityState.GetAddress(s.addrID)
if !ok {
return proton.Address{}, errors.New("could not find account address")
}
inCombinedMode := s.addressMode == usertypes.AddressModeCombined
if !inCombinedMode {
return address, nil
}
senderAddr, err := s.getSenderProtonAddress(p)
if err != nil {
if !errors.Is(err, errNoSenderAddressMatch) {
s.log.WithError(err).Warn("Could not get import address")
}
// We did not find a match, so we use the default address.
return address, nil
}
if senderAddr.ID == address.ID {
return address, nil
}
// GODT-3185 / BRIDGE-120 In combined mode, in certain cases we adapt the address used for encryption.
// - draft with non-default address in combined mode: using sender address
// - import with non-default address in combined mode: using sender address
// - import with non-default disabled address in combined mode: using sender address
isSenderAddressDisabled := (!bool(senderAddr.Send)) || (senderAddr.Status != proton.AddressStatusEnabled)
if isDraft && isSenderAddressDisabled {
return address, nil
}
return senderAddr, nil
}
func (s *Connector) getSenderProtonAddress(p *parser.Parser) (proton.Address, error) {
// Step 1: extract sender email address from message
if (p == nil) || (p.Root() == nil) || (p.Root().Header.Len() == 0) {
return proton.Address{}, errors.New("invalid message encountered while trying to extract sender address")
}
addrField := p.Root().Header.Get("From")
if len(addrField) == 0 {
addrField = p.Root().Header.Get("Sender")
}
if len(addrField) == 0 {
return proton.Address{}, errors.New("no sender found in message headers")
}
sender, err := rfc5322.ParseAddressList(addrField)
if (err != nil) || (len(sender) == 0) {
return proton.Address{}, fmt.Errorf("invalid sender address in message: %w", err)
}
addrStr := sender[0].Address
// Step 2: match email with the user address list.
addressList := s.identityState.GetAddresses()
index := slices.IndexFunc(addressList, func(a proton.Address) bool {
return equalAddresses(a.Email, addrStr)
})
if index < 0 {
return proton.Address{}, errNoSenderAddressMatch
}
return addressList[index], nil
}

View File

@ -20,11 +20,15 @@ package tests
import ( import (
"bufio" "bufio"
"bytes" "bytes"
"context"
"encoding/json" "encoding/json"
"errors"
"fmt" "fmt"
"net/http" "net/http"
"strings" "strings"
"github.com/ProtonMail/go-proton-api"
"github.com/ProtonMail/gopenpgp/v2/crypto"
"github.com/cucumber/godog" "github.com/cucumber/godog"
) )
@ -116,7 +120,7 @@ func (s *scenario) checkParsedMultipartFormForFile(method, path, file string, ha
} }
if _, ok := req.MultipartForm.File[file]; hasFile != ok { if _, ok := req.MultipartForm.File[file]; hasFile != ok {
return fmt.Errorf("Multipart file in bug report is %t, want it to be %t", ok, hasFile) return fmt.Errorf("multipart file in bug report is %t, want it to be %t", ok, hasFile)
} }
return nil return nil
@ -240,3 +244,57 @@ func (s *scenario) theMessageUsedKeyForSending(address string) error {
return nil return nil
} }
func (s *scenario) theKeyForAddressWasUsedToImport(address string) error {
// Response does not include the address ID, only the messageID, so we extract the messageID from the response body
call, err := s.t.getLastCallExcludingHTTPOverride("POST", "/mail/v4/messages/import")
if err != nil {
return err
}
var resp struct {
Responses []struct{ Response struct{ MessageID string } }
}
if err := json.Unmarshal(call.ResponseBody, &resp); err != nil {
return err
}
return s.checkMessageIsEncryptedForAddress(resp.Responses[0].Response.MessageID, address)
}
func (s *scenario) theKeyForAddressWasUsedToCreateDraft(address string) error {
call, err := s.t.getLastCallExcludingHTTPOverride("POST", "/mail/v4/messages")
if err != nil {
return err
}
var resp struct{ Message struct{ ID string } }
if err := json.Unmarshal(call.ResponseBody, &resp); err != nil {
return err
}
return s.checkMessageIsEncryptedForAddress(resp.Message.ID, address)
}
func (s *scenario) checkMessageIsEncryptedForAddress(messageID string, address string) error {
user := s.t.getUserByAddress(address)
addrID := user.getAddrID(address)
return s.t.withClient(context.Background(), user.getName(), func(ctx context.Context, client *proton.Client) error {
message, err := client.GetMessage(ctx, messageID)
if err != nil {
return err
}
// Check 1: the address associated with the message is the one we expect.
if message.AddressID != addrID {
return errors.New("the message is not encrypted with the specified address")
}
// Check 2: we indeed encrypted the message with this address' key ring.
return s.t.withAddrKR(ctx, client, user.name, addrID, func(_ context.Context, kr *crypto.KeyRing) error {
_, err := message.Decrypt(kr)
return err
})
})
}

View File

@ -0,0 +1,136 @@
Feature: IMAP import messages
Background:
Given there exists an account with username "[user:user]" and password "password"
And the account "[user:user]" has additional address "[alias:secondary]@[domain]"
And the account "[user:user]" has additional disabled address "[alias:disabled]@[domain]"
Then it succeeds
When bridge starts
And the user logs in with username "[user:user]" and password "password"
And user "[user:user]" finishes syncing
And user "[user:user]" connects and authenticates IMAP client "1"
Then it succeeds
@skip-black
Scenario: Messages imported with default address as sender are encrypted with the default address key
When IMAP client "1" appends the following message to "INBOX":
"""
From: Bridge Test <[user:user]@[domain]>
Date: 01 Jan 1980 00:00:00 +0000
To: Internal Bridge <bridgetest@example.com>
Received: by 2002:0:0:0:0:0:0:0 with SMTP id 0123456789abcdef; Wed, 30 Dec 2020 01:23:45 0000
Subject: Basic text/plain message
Content-Type: text/plain
Hello
"""
Then it succeeds
And the key for address "[user:user]@[domain]" was used to import
@skip-black
Scenario: Messages imported with alias as sender are encrypted with secondary address key
When IMAP client "1" appends the following message to "INBOX":
"""
From: Bridge Test <[alias:secondary]@[domain]>
Date: 01 Jan 1980 00:00:00 +0000
To: Internal Bridge <bridgetest@example.com>
Received: by 2002:0:0:0:0:0:0:0 with SMTP id 0123456789abcdef; Wed, 30 Dec 2020 01:23:45 0000
Subject: Basic text/plain message
Content-Type: text/plain
Hello
"""
Then it succeeds
And the key for address "[alias:secondary]@[domain]" was used to import
@skip-black
Scenario: Messages imported with a disabled alias as sender are encrypted with the disabled address key
When IMAP client "1" appends the following message to "INBOX":
"""
From: Bridge Test <[alias:disabled]@[domain]>
Date: 01 Jan 1980 00:00:00 +0000
To: Internal Bridge <bridgetest@example.com>
Received: by 2002:0:0:0:0:0:0:0 with SMTP id 0123456789abcdef; Wed, 30 Dec 2020 01:23:45 0000
Subject: Basic text/plain message
Content-Type: text/plain
Hello
"""
Then it succeeds
And the key for address "[alias:disabled]@[domain]" was used to import
@skip-black
Scenario: Messages imported with an unknown address as sender are encrypted with primary address key
When IMAP client "1" appends the following message to "INBOX":
"""
From: Bridge Test <bridgeqa@example.com>
Date: 01 Jan 1980 00:00:00 +0000
To: Internal Bridge <bridgetest@example.com>
Received: by 2002:0:0:0:0:0:0:0 with SMTP id 0123456789abcdef; Wed, 30 Dec 2020 01:23:45 0000
Subject: Basic text/plain message
Content-Type: text/plain
Hello
"""
Then it succeeds
And the key for address "[user:user]@[domain]" was used to import
@skip-black
Scenario: Drafts imported with default address as sender are encrypted with the default address key
When IMAP client "1" appends the following message to "Drafts":
"""
From: Bridge Test <[user:user]@[domain]>
Date: 01 Jan 1980 00:00:00 +0000
To: Internal Bridge <bridgetest@example.com>
Subject: Basic text/plain message
Content-Type: text/plain
Hello
"""
Then it succeeds
And the key for address "[user:user]@[domain]" was used to create draft
@skip-black
Scenario: Drafts imported with alias as sender are encrypted with secondary key
When IMAP client "1" appends the following message to "Drafts":
"""
From: Bridge Test <[alias:secondary]@[domain]>
Date: 01 Jan 1980 00:00:00 +0000
To: Internal Bridge <bridgetest@example.com>
Subject: Basic text/plain message
Content-Type: text/plain
Hello
"""
Then it succeeds
And the key for address "[alias:secondary]@[domain]" was used to create draft
@skip-black
Scenario: Drafts imported with a disabled alias as sender are encrypted with the disabled address key
When IMAP client "1" appends the following message to "Drafts":
"""
From: Bridge Test <[alias:disabled]@[domain]>
Date: 01 Jan 1980 00:00:00 +0000
To: Internal Bridge <bridgetest@example.com>
Subject: Basic text/plain message
Content-Type: text/plain
Hello
"""
Then it succeeds
And the key for address "[user:user]@[domain]" was used to create drafts
@skip-black
Scenario: Drafts imported with an unknown address as sender are encrypted with primary address key
When IMAP client "1" appends the following message to "Drafts":
"""
From: Bridge Test <bridgeqa@example.com>
Date: 01 Jan 1980 00:00:00 +0000
To: Internal Bridge <bridgetest@example.com>
Subject: Basic text/plain message
Content-Type: text/plain
Hello
"""
Then it succeeds
And the key for address "[user:user]@[domain]" was used to create draft

View File

@ -39,6 +39,8 @@ func (s *scenario) steps(ctx *godog.ScenarioContext) {
ctx.Step(`^bridge IMAP port is (\d+)`, s.bridgeIMAPPortIs) ctx.Step(`^bridge IMAP port is (\d+)`, s.bridgeIMAPPortIs)
ctx.Step(`^bridge SMTP port is (\d+)`, s.bridgeSMTPPortIs) ctx.Step(`^bridge SMTP port is (\d+)`, s.bridgeSMTPPortIs)
ctx.Step(`^the message used "([^"]*)" key for sending$`, s.theMessageUsedKeyForSending) ctx.Step(`^the message used "([^"]*)" key for sending$`, s.theMessageUsedKeyForSending)
ctx.Step(`^the key for address "([^"]*)" was used to import`, s.theKeyForAddressWasUsedToImport)
ctx.Step(`^the key for address "([^"]*)" was used to create draft`, s.theKeyForAddressWasUsedToCreateDraft)
// ==== SETUP ==== // ==== SETUP ====
ctx.Step(`^there exists an account with username "([^"]*)" and password "([^"]*)"$`, s.thereExistsAnAccountWithUsernameAndPassword) ctx.Step(`^there exists an account with username "([^"]*)" and password "([^"]*)"$`, s.thereExistsAnAccountWithUsernameAndPassword)