Silverfish/proton-bridge
1
0
Fork 1
mirror of https://github.com/ProtonMail/proton-bridge.git synced 2025-12-23 18:36:46 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Other: Bump go-smtp version to fix race condition

Browse Source 
There was a race condition internal to the go-smtp library.
In order to fix it, a version bump was necessary.
However, this significantly changed the library interface.
This commit is contained in:
James Houlahan
2022-10-21 11:07:53 +02:00
parent 974735d415
commit 0f125196a6
13 changed files with 210 additions and 316 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
internal/bridge/bridge.go
View File

@ -64,8 +64,7 @@ type Bridge struct {
imapListener net.Listener
// smtpServer is the bridge's SMTP server.
smtpServer *smtp.Server
smtpBackend *smtpBackend
smtpServer *smtp.Server
// updater is the bridge's updater.
updater Updater
@ -131,8 +130,6 @@ func New( //nolint:funlen
return nil, nil, fmt.Errorf("failed to get Gluon directory: %w", err)
}
smtpBackend := newSMTPBackend()
imapServer, err := newIMAPServer(gluonDir, curVersion, tlsConfig, logIMAPClient, logIMAPServer)
if err != nil {
return nil, nil, fmt.Errorf("failed to create IMAP server: %w", err)
@ -159,7 +156,6 @@ func New( //nolint:funlen
// Service stuff
tlsConfig,
imapServer,
smtpBackend,
focusService,
// Logging stuff
@ -191,11 +187,10 @@ func newBridge(
tlsConfig *tls.Config,
imapServer *gluon.Server,
smtpBackend *smtpBackend,
focusService *focus.Service,
logIMAPClient, logIMAPServer, logSMTP bool,
) *Bridge {
return &Bridge{
bridge := &Bridge{
vault: vault,
users: safe.NewMap[string, *user.User](nil),
@ -205,10 +200,8 @@ func newBridge(
proxyCtl: proxyCtl,
identifier: identifier,
tlsConfig: tlsConfig,
imapServer: imapServer,
smtpServer: newSMTPServer(smtpBackend, tlsConfig, logSMTP),
smtpBackend: smtpBackend,
tlsConfig: tlsConfig,
imapServer: imapServer,
updater: updater,
curVersion: curVersion,
@ -226,6 +219,10 @@ func newBridge(
stopCh: make(chan struct{}),
}
bridge.smtpServer = newSMTPServer(&smtpBackend{bridge}, tlsConfig, logSMTP)
return bridge
}
func (bridge *Bridge) init(tlsReporter TLSReporter) error {

16
internal/bridge/smtp.go
View File

@ -26,7 +26,6 @@ import (
"github.com/ProtonMail/proton-bridge/v2/internal/logging"
"github.com/ProtonMail/proton-bridge/v2/internal/constants"
"github.com/emersion/go-sasl"
"github.com/emersion/go-smtp"
"github.com/sirupsen/logrus"
)
@ -67,7 +66,7 @@ func (bridge *Bridge) restartSMTP() error {
return err
}
bridge.smtpServer = newSMTPServer(bridge.smtpBackend, bridge.tlsConfig, bridge.logSMTP)
bridge.smtpServer = newSMTPServer(&smtpBackend{bridge}, bridge.tlsConfig, bridge.logSMTP)
return bridge.serveSMTP()
}
@ -99,18 +98,5 @@ func newSMTPServer(smtpBackend *smtpBackend, tlsConfig *tls.Config, shouldLog bo
smtpServer.Debug = logging.NewSMTPDebugLogger()
}
smtpServer.EnableAuth(sasl.Login, func(conn *smtp.Conn) sasl.Server {
return sasl.NewLoginServer(func(address, password string) error {
user, err := conn.Server().Backend.Login(nil, address, password)
if err != nil {
return err
}
conn.SetSession(user)
return nil
})
})
return smtpServer
}

91
internal/bridge/smtp_backend.go
View File

@ -18,69 +18,82 @@
package bridge
import (
"sync"
"fmt"
"io"
"github.com/ProtonMail/proton-bridge/v2/internal/user"
"github.com/emersion/go-smtp"
)
type smtpBackend struct {
users map[string]*user.User
usersLock sync.RWMutex
bridge *Bridge
}
func newSMTPBackend() *smtpBackend {
return &smtpBackend{
users: make(map[string]*user.User),
}
type smtpSession struct {
bridge *Bridge
userID string
authID string
from string
to []string
}
func (backend *smtpBackend) Login(_ *smtp.ConnectionState, email, password string) (smtp.Session, error) {
backend.usersLock.RLock()
defer backend.usersLock.RUnlock()
func (be *smtpBackend) NewSession(_ *smtp.Conn) (smtp.Session, error) {
return &smtpSession{
bridge: be.bridge,
}, nil
}
for _, user := range backend.users {
session, err := user.NewSMTPSession(email, []byte(password))
if err != nil {
continue
func (s *smtpSession) AuthPlain(username, password string) error {
return s.bridge.users.ValuesErr(func(users []*user.User) error {
for _, user := range users {
addrID, err := user.CheckAuth(username, []byte(password))
if err != nil {
continue
}
s.userID = user.ID()
s.authID = addrID
return nil
}
return session, nil
}
return nil, ErrNoSuchUser
return fmt.Errorf("invalid username or password")
})
}
func (backend *smtpBackend) AnonymousLogin(state *smtp.ConnectionState) (smtp.Session, error) {
return nil, ErrNotImplemented
func (s *smtpSession) Reset() {
s.from = ""
s.to = nil
}
// addUser adds the given user to the backend.
// It returns an error if a user with the same ID already exists.
func (backend *smtpBackend) addUser(newUser *user.User) error {
backend.usersLock.Lock()
defer backend.usersLock.Unlock()
func (s *smtpSession) Logout() error {
s.Reset()
return nil
}
if _, ok := backend.users[newUser.ID()]; ok {
return ErrUserAlreadyExists
func (s *smtpSession) Mail(from string, opts *smtp.MailOptions) error {
s.from = from
return nil
}
func (s *smtpSession) Rcpt(to string) error {
if len(to) > 0 {
s.to = append(s.to, to)
}
backend.users[newUser.ID()] = newUser
return nil
}
// removeUser removes the given user from the backend.
// It returns an error if the user doesn't exist.
func (backend *smtpBackend) removeUser(user *user.User) error {
backend.usersLock.Lock()
defer backend.usersLock.Unlock()
if _, ok := backend.users[user.ID()]; !ok {
return ErrNoSuchUser
func (s *smtpSession) Data(r io.Reader) error {
if ok, err := s.bridge.users.GetErr(s.userID, func(user *user.User) error {
return user.SendMail(s.authID, s.from, s.to, r)
}); !ok {
return fmt.Errorf("no such user %q", s.userID)
} else if err != nil {
return fmt.Errorf("failed to send mail: %w", err)
}
delete(backend.users, user.ID())
return nil
}

13
internal/bridge/user.go
View File

@ -401,11 +401,6 @@ func (bridge *Bridge) addUserWithVault(
return fmt.Errorf("failed to add IMAP user: %w", err)
}
// Connect the user's address(es) to the SMTP server.
if err := bridge.smtpBackend.addUser(user); err != nil {
return fmt.Errorf("failed to add user to SMTP backend: %w", err)
}
// Handle events coming from the user before forwarding them to the bridge.
// For example, if the user's addresses change, we need to update them in gluon.
go func() {
@ -497,10 +492,6 @@ func (bridge *Bridge) addIMAPUser(ctx context.Context, user *user.User) error {
// logoutUser logs the given user out from bridge.
func (bridge *Bridge) logoutUser(ctx context.Context, userID string) error {
if ok, err := bridge.users.GetDeleteErr(userID, func(user *user.User) error {
if err := bridge.smtpBackend.removeUser(user); err != nil {
logrus.WithError(err).Error("Failed to remove user from SMTP backend")
}
for _, gluonID := range user.GetGluonIDs() {
if err := bridge.imapServer.RemoveUser(ctx, gluonID, false); err != nil {
logrus.WithError(err).Error("Failed to remove IMAP user")
@ -528,10 +519,6 @@ func (bridge *Bridge) logoutUser(ctx context.Context, userID string) error {
// deleteUser deletes the given user from bridge.
func (bridge *Bridge) deleteUser(ctx context.Context, userID string) {
if ok := bridge.users.GetDelete(userID, func(user *user.User) {
if err := bridge.smtpBackend.removeUser(user); err != nil {
logrus.WithError(err).Error("Failed to remove user from SMTP backend")
}
for _, gluonID := range user.GetGluonIDs() {
if err := bridge.imapServer.RemoveUser(ctx, gluonID, true); err != nil {
logrus.WithError(err).Error("Failed to remove IMAP user")

2
internal/user/imap.go
View File

@ -67,7 +67,7 @@ func newIMAPConnector(user *User, addrID string) *imapConnector {
// Authorize returns whether the given username/password combination are valid for this connector.
func (conn *imapConnector) Authorize(username string, password []byte) bool {
addrID, err := conn.checkAuth(username, password)
addrID, err := conn.CheckAuth(username, password)
if err != nil {
return false
}

27
internal/user/keys.go
View File

@ -56,6 +56,33 @@ func (user *User) withAddrKR(addrID string, fn func(*crypto.KeyRing, *crypto.Key
})
}
func (user *User) withAddrKRByEmail(email string, fn func(*crypto.KeyRing, *crypto.KeyRing) error) error {
return user.apiAddrs.ValuesErr(func(apiAddrs []liteapi.Address) error {
addrID, err := getAddrID(apiAddrs, email)
if err != nil {
return fmt.Errorf("failed to get address ID: %w", err)
}
return user.withUserKR(func(userKR *crypto.KeyRing) error {
if ok, err := user.apiAddrs.GetErr(addrID, func(apiAddr liteapi.Address) error {
addrKR, err := apiAddr.Keys.Unlock(user.vault.KeyPass(), userKR)
if err != nil {
return fmt.Errorf("failed to unlock address keys: %w", err)
}
defer userKR.ClearPrivateParams()
return fn(userKR, addrKR)
}); !ok {
return fmt.Errorf("no such address %q", addrID)
} else if err != nil {
return err
}
return nil
})
})
}
func (user *User) withAddrKRs(fn func(*crypto.KeyRing, map[string]*crypto.KeyRing) error) error {
return user.withUserKR(func(userKR *crypto.KeyRing) error {
return user.apiAddrs.ValuesErr(func(apiAddrs []liteapi.Address) error {

230
internal/user/smtp.go
View File

@ -30,204 +30,83 @@ import (
"github.com/ProtonMail/gluon/rfc822"
"github.com/ProtonMail/go-rfc5322"
"github.com/ProtonMail/gopenpgp/v2/crypto"
"github.com/ProtonMail/proton-bridge/v2/internal/safe"
"github.com/ProtonMail/proton-bridge/v2/internal/vault"
"github.com/ProtonMail/proton-bridge/v2/pkg/message"
"github.com/ProtonMail/proton-bridge/v2/pkg/message/parser"
"github.com/bradenaw/juniper/parallel"
"github.com/bradenaw/juniper/xslices"
"github.com/emersion/go-smtp"
"github.com/sirupsen/logrus"
"gitlab.protontech.ch/go/liteapi"
"golang.org/x/exp/slices"
)
type smtpSession struct {
*User
// authID holds the ID of the address that the SMTP client authenticated with to send the message.
authID string
// from is the current sending address (taken from the return path).
from string
// fromAddrID is the ID of the current sending address (taken from the return path).
fromAddrID string
// to holds all to for the current message.
to []string
}
func newSMTPSession(user *User, email string) (*smtpSession, error) {
return safe.MapValuesRetErr(user.apiAddrs, func(apiAddrs []liteapi.Address) (*smtpSession, error) {
authID, err := getAddrID(apiAddrs, email)
if err != nil {
return nil, fmt.Errorf("failed to get address ID: %w", err)
}
return &smtpSession{
User: user,
authID: authID,
}, nil
})
}
// Reset Discard currently processed message.
func (session *smtpSession) Reset() {
logrus.Info("SMTP session reset")
// Clear the from and to fields.
session.from = ""
session.fromAddrID = ""
session.to = nil
}
// Logout Free all resources associated with session.
func (session *smtpSession) Logout() error {
defer session.Reset()
logrus.Info("SMTP session logout")
return nil
}
// Mail Set return path for currently processed message.
func (session *smtpSession) Mail(from string, opts smtp.MailOptions) error {
logrus.Info("SMTP session mail")
return session.apiAddrs.ValuesErr(func(apiAddrs []liteapi.Address) error {
switch {
case opts.RequireTLS:
return ErrNotImplemented
case opts.UTF8:
return ErrNotImplemented
case opts.Auth != nil:
email, err := getAddrEmail(apiAddrs, session.authID)
if err != nil {
return fmt.Errorf("invalid auth address: %w", err)
}
if *opts.Auth != "" && *opts.Auth != email {
return ErrNotImplemented
}
}
addrID, err := getAddrID(apiAddrs, sanitizeEmail(from))
if err != nil {
return fmt.Errorf("invalid return path: %w", err)
}
session.from = from
session.fromAddrID = addrID
return nil
})
}
// Rcpt Add recipient for currently processed message.
func (session *smtpSession) Rcpt(to string) error {
logrus.Info("SMTP session rcpt")
if to == "" {
return ErrInvalidRecipient
}
if !slices.Contains(session.to, to) {
session.to = append(session.to, to)
}
return nil
}
// Data Set currently processed message contents and send it.
func (session *smtpSession) Data(r io.Reader) error { //nolint:funlen
logrus.Info("SMTP session data")
func (user *User) sendMail(authID string, emails []string, from string, to []string, r io.Reader) error { //nolint:funlen
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
switch {
case session.from == "":
return ErrInvalidReturnPath
case len(session.to) == 0:
return ErrInvalidRecipient
}
// Create a new message parser from the reader.
parser, err := parser.New(r)
if err != nil {
return fmt.Errorf("failed to create parser: %w", err)
}
return session.apiAddrs.ValuesErr(func(apiAddrs []liteapi.Address) error {
return session.withAddrKR(session.fromAddrID, func(userKR, addrKR *crypto.KeyRing) error {
// Use the first key for encrypting the message.
addrKR, err := addrKR.FirstKey()
// If the message contains a sender, use it instead of the one from the return path.
if sender, ok := getMessageSender(parser); ok {
from = sender
}
// Load the user's mail settings.
settings, err := user.client.GetMailSettings(ctx)
if err != nil {
return fmt.Errorf("failed to get mail settings: %w", err)
}
return user.withAddrKRByEmail(from, func(userKR, addrKR *crypto.KeyRing) error {
// Use the first key for encrypting the message.
addrKR, err := addrKR.FirstKey()
if err != nil {
return fmt.Errorf("failed to get first key: %w", err)
}
// If we have to attach the public key, do it now.
if settings.AttachPublicKey == liteapi.AttachPublicKeyEnabled {
key, err := addrKR.GetKey(0)
if err != nil {
return fmt.Errorf("failed to get first key: %w", err)
return fmt.Errorf("failed to get sending key: %w", err)
}
// If the message contains a sender, use it instead of the one from the return path.
if sender, ok := getMessageSender(parser); ok {
session.from = sender
}
// Load the user's mail settings.
settings, err := session.client.GetMailSettings(ctx)
pubKey, err := key.GetArmoredPublicKey()
if err != nil {
return fmt.Errorf("failed to get mail settings: %w", err)
return fmt.Errorf("failed to get public key: %w", err)
}
// If we have to attach the public key, do it now.
if settings.AttachPublicKey == liteapi.AttachPublicKeyEnabled {
key, err := addrKR.GetKey(0)
if err != nil {
return fmt.Errorf("failed to get sending key: %w", err)
}
parser.AttachPublicKey(pubKey, fmt.Sprintf("publickey - %v - %v", addrKR.GetIdentities()[0].Name, key.GetFingerprint()[:8]))
}
pubKey, err := key.GetArmoredPublicKey()
if err != nil {
return fmt.Errorf("failed to get public key: %w", err)
}
// Parse the message we want to send (after we have attached the public key).
message, err := message.ParseWithParser(parser)
if err != nil {
return fmt.Errorf("failed to parse message: %w", err)
}
parser.AttachPublicKey(pubKey, fmt.Sprintf("publickey - %v - %v", addrKR.GetIdentities()[0].Name, key.GetFingerprint()[:8]))
}
// Send the message using the correct key.
sent, err := sendWithKey(
ctx,
user.client,
authID,
user.vault.AddressMode(),
settings,
userKR, addrKR,
emails, from, to,
message,
)
if err != nil {
return fmt.Errorf("failed to send message: %w", err)
}
// Parse the message we want to send (after we have attached the public key).
message, err := message.ParseWithParser(parser)
if err != nil {
return fmt.Errorf("failed to parse message: %w", err)
}
logrus.WithField("messageID", sent.ID).Info("Message sent")
// Collect all the user's emails so we can match them to the outgoing message.
emails := xslices.Map(apiAddrs, func(addr liteapi.Address) string {
return addr.Email
})
sent, err := sendWithKey(
ctx,
session.client,
session.authID,
session.vault.AddressMode(),
settings,
userKR,
addrKR,
emails,
session.from,
session.to,
message,
)
if err != nil {
return fmt.Errorf("failed to send message: %w", err)
}
logrus.WithField("messageID", sent.ID).Info("Message sent")
return nil
})
return nil
})
}
@ -252,20 +131,15 @@ func sendWithKey( //nolint:funlen
var decBody string
switch message.MIMEType {
switch message.MIMEType { //nolint:exhaustive
case rfc822.TextHTML:
decBody = string(message.RichBody)
case rfc822.TextPlain:
decBody = string(message.PlainBody)
case rfc822.MultipartRelated:
fallthrough
case rfc822.MultipartMixed:
fallthrough
case rfc822.MessageRFC822:
fallthrough
default:
break
return liteapi.Message{}, fmt.Errorf("unsupported MIME type: %v", message.MIMEType)
}
encBody, err := addrKR.Encrypt(crypto.NewPlainMessageFromString(decBody), nil)

14
internal/user/types.go
View File

@ -22,6 +22,7 @@ import (
"encoding/hex"
"fmt"
"reflect"
"strings"
"gitlab.protontech.ch/go/liteapi"
)
@ -84,7 +85,7 @@ func hexDecode(b []byte) ([]byte, error) {
// getAddrID returns the address ID for the given email address.
func getAddrID(apiAddrs []liteapi.Address, email string) (string, error) {
for _, addr := range apiAddrs {
if addr.Email == email {
if strings.EqualFold(addr.Email, sanitizeEmail(email)) {
return addr.ID, nil
}
}
@ -92,17 +93,6 @@ func getAddrID(apiAddrs []liteapi.Address, email string) (string, error) {
return "", fmt.Errorf("address %s not found", email)
}
// getAddrEmail returns the email address of the given address ID.
func getAddrEmail(apiAddrs []liteapi.Address, addrID string) (string, error) {
for _, addr := range apiAddrs {
if addr.ID == addrID {
return addr.Email, nil
}
}
return "", fmt.Errorf("address %s not found", addrID)
}
// contextWithStopCh returns a new context that is cancelled when the stop channel is closed or a value is sent to it.
func contextWithStopCh(ctx context.Context, stopCh ...<-chan struct{}) (context.Context, context.CancelFunc) {
ctx, cancel := context.WithCancel(ctx)

81
internal/user/user.go
View File

@ -21,6 +21,7 @@ import (
"context"
"crypto/subtle"
"fmt"
"io"
"strings"
"sync/atomic"
"time"
@ -33,7 +34,6 @@ import (
"github.com/ProtonMail/proton-bridge/v2/internal/try"
"github.com/ProtonMail/proton-bridge/v2/internal/vault"
"github.com/bradenaw/juniper/xslices"
"github.com/emersion/go-smtp"
"github.com/sirupsen/logrus"
"gitlab.protontech.ch/go/liteapi"
)
@ -315,13 +315,46 @@ func (user *User) NewIMAPConnectors() (map[string]connector.Connector, error) {
return imapConn, nil
}
// NewSMTPSession returns an SMTP session for the user.
func (user *User) NewSMTPSession(email string, password []byte) (smtp.Session, error) {
if _, err := user.checkAuth(email, password); err != nil {
return nil, err
// SendMail sends an email from the given address to the given recipients.
func (user *User) SendMail(authID string, from string, to []string, r io.Reader) error {
if len(to) == 0 {
return ErrInvalidRecipient
}
return newSMTPSession(user, email)
return user.apiAddrs.ValuesErr(func(apiAddrs []liteapi.Address) error {
if _, err := getAddrID(apiAddrs, from); err != nil {
return ErrInvalidReturnPath
}
emails := xslices.Map(apiAddrs, func(addr liteapi.Address) string {
return addr.Email
})
return user.sendMail(authID, emails, from, to, r)
})
}
// CheckAuth returns whether the given email and password can be used to authenticate over IMAP or SMTP with this user.
// It returns the address ID of the authenticated address.
func (user *User) CheckAuth(email string, password []byte) (string, error) {
dec, err := hexDecode(password)
if err != nil {
return "", fmt.Errorf("failed to decode password: %w", err)
}
if subtle.ConstantTimeCompare(user.vault.BridgePass(), dec) != 1 {
return "", fmt.Errorf("invalid password")
}
return safe.MapValuesRetErr(user.apiAddrs, func(apiAddrs []liteapi.Address) (string, error) {
for _, addr := range apiAddrs {
if strings.EqualFold(addr.Email, email) {
return addr.ID, nil
}
}
return "", fmt.Errorf("invalid email")
})
}
// OnStatusUp is called when the connection goes up.
@ -347,9 +380,6 @@ func (user *User) Logout(ctx context.Context) error {
// Cancel ongoing syncs.
user.stopSync()
// Wait for ongoing syncs to stop.
user.waitSync()
if err := user.client.AuthDelete(ctx); err != nil {
return fmt.Errorf("failed to delete auth: %w", err)
}
@ -369,9 +399,6 @@ func (user *User) Close() error {
// Cancel ongoing syncs.
user.stopSync()
// Wait for ongoing syncs to stop.
user.waitSync()
// Close the user's API client.
user.client.Close()
@ -395,11 +422,13 @@ func (user *User) Close() error {
func (user *User) SetShowAllMail(show bool) {
var value int32
if show {
value = 1
} else {
value = 0
}
atomic.StoreInt32(&user.showAllMail, value)
}
@ -407,27 +436,6 @@ func (user *User) GetShowAllMail() bool {
return atomic.LoadInt32(&user.showAllMail) == 1
}
func (user *User) checkAuth(email string, password []byte) (string, error) {
dec, err := hexDecode(password)
if err != nil {
return "", fmt.Errorf("failed to decode password: %w", err)
}
if subtle.ConstantTimeCompare(user.vault.BridgePass(), dec) != 1 {
return "", fmt.Errorf("invalid password")
}
return safe.MapValuesRetErr(user.apiAddrs, func(apiAddrs []liteapi.Address) (string, error) {
for _, addr := range apiAddrs {
if strings.EqualFold(addr.Email, email) {
return addr.ID, nil
}
}
return "", fmt.Errorf("invalid email")
})
}
// streamEvents begins streaming API events for the user.
// When we receive an API event, we attempt to handle it.
// If successful, we update the event ID in the vault.
@ -496,6 +504,8 @@ func (user *User) startSync() <-chan error {
// AbortSync aborts any ongoing sync.
// GODT-1947: Should probably be done automatically when one of the user's IMAP connectors is closed.
func (user *User) stopSync() {
defer user.syncLock.Wait()
select {
case user.syncStopCh <- struct{}{}:
logrus.Debug("Sent sync abort signal")
@ -514,8 +524,3 @@ func (user *User) lockSync() {
func (user *User) unlockSync() {
user.syncLock.Unlock()
}
// waitSync waits for any ongoing sync to finish.
func (user *User) waitSync() {
user.syncLock.Wait()
}