From 310c6a1ccf71706c736efe94f7e9f0749da7ed56 Mon Sep 17 00:00:00 2001 From: James Houlahan Date: Fri, 19 Aug 2022 16:15:51 +0200 Subject: [PATCH] Other(refactor): Remove unencrypted recipient confirmation --- internal/app/bridge/bridge.go | 4 - internal/bridge/useragent.go | 17 ++++ internal/config/settings/settings.go | 2 - internal/events/events.go | 1 - internal/frontend/frontend.go | 2 - internal/frontend/grpc/service.go | 2 - internal/frontend/types/types.go | 4 - internal/smtp/backend.go | 15 ---- internal/smtp/user.go | 42 ---------- pkg/confirmer/confirmer.go | 70 ---------------- pkg/confirmer/confirmer_test.go | 120 --------------------------- pkg/confirmer/request.go | 70 ---------------- 12 files changed, 17 insertions(+), 332 deletions(-) delete mode 100644 pkg/confirmer/confirmer.go delete mode 100644 pkg/confirmer/confirmer_test.go delete mode 100644 pkg/confirmer/request.go diff --git a/internal/app/bridge/bridge.go b/internal/app/bridge/bridge.go index ecc26e21..9e6435cb 100644 --- a/internal/app/bridge/bridge.go +++ b/internal/app/bridge/bridge.go @@ -71,9 +71,6 @@ func New(base *base.Base) *cli.App { } func main(b *base.Base, c *cli.Context) error { //nolint:funlen - // GODT-1481: Always turn off reporting of unencrypted recipient in v2. - b.Settings.SetBool(settings.ReportOutgoingNoEncKey, false) - cache, cacheErr := loadMessageCache(b) if cacheErr != nil { logrus.WithError(cacheErr).Error("Could not load local cache.") @@ -163,7 +160,6 @@ func main(b *base.Base, c *cli.Context) error { //nolint:funlen b.Listener, b.Updater, bridge, - smtpBackend, b, ) diff --git a/internal/bridge/useragent.go b/internal/bridge/useragent.go index fbfadc84..f9368b2f 100644 --- a/internal/bridge/useragent.go +++ b/internal/bridge/useragent.go @@ -1,3 +1,20 @@ +// Copyright (c) 2022 Proton AG +// +// This file is part of Proton Mail Bridge. +// +// Proton Mail Bridge is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Proton Mail Bridge is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Proton Mail Bridge. If not, see . + package bridge func (b *Bridge) GetCurrentUserAgent() string { diff --git a/internal/config/settings/settings.go b/internal/config/settings/settings.go index e92e0ddf..d0b5bb25 100644 --- a/internal/config/settings/settings.go +++ b/internal/config/settings/settings.go @@ -38,7 +38,6 @@ const ( AutostartKey = "autostart" AutoUpdateKey = "autoupdate" CookiesKey = "cookies" - ReportOutgoingNoEncKey = "report_outgoing_email_without_encryption" LastVersionKey = "last_used_version" UpdateChannelKey = "update_channel" RolloutKey = "rollout" @@ -88,7 +87,6 @@ func (s *Settings) setDefaultValues() { s.setDefault(AllowProxyKey, "true") s.setDefault(AutostartKey, "true") s.setDefault(AutoUpdateKey, "true") - s.setDefault(ReportOutgoingNoEncKey, "false") s.setDefault(LastVersionKey, "") s.setDefault(UpdateChannelKey, "") s.setDefault(RolloutKey, fmt.Sprintf("%v", rand.Float64())) //nolint:gosec // G404 It is OK to use weak random number generator here diff --git a/internal/events/events.go b/internal/events/events.go index 05a00169..f2a73a3a 100644 --- a/internal/events/events.go +++ b/internal/events/events.go @@ -38,7 +38,6 @@ const ( InternetOff = "internetOff" InternetOn = "internetOn" SecondInstanceEvent = "secondInstance" - OutgoingNoEncEvent = "outgoingNoEncryption" NoActiveKeyForRecipientEvent = "noActiveKeyForRecipient" UpgradeApplicationEvent = "upgradeApplication" TLSCertIssue = "tlsCertPinningIssue" diff --git a/internal/frontend/frontend.go b/internal/frontend/frontend.go index fcef9f62..af72b6d8 100644 --- a/internal/frontend/frontend.go +++ b/internal/frontend/frontend.go @@ -46,7 +46,6 @@ func New( eventListener listener.Listener, updater types.Updater, bridge *bridge.Bridge, - noEncConfirmator types.NoEncConfirmator, restarter types.Restarter, ) Frontend { bridgeWrap := types.NewBridgeWrap(bridge) @@ -59,7 +58,6 @@ func New( eventListener, updater, bridgeWrap, - noEncConfirmator, restarter, ) diff --git a/internal/frontend/grpc/service.go b/internal/frontend/grpc/service.go index a25bc271..c6d2b68b 100644 --- a/internal/frontend/grpc/service.go +++ b/internal/frontend/grpc/service.go @@ -75,9 +75,7 @@ func NewService( eventListener listener.Listener, updater types.Updater, bridge types.Bridger, - _ types.NoEncConfirmator, restarter types.Restarter, - ) *Service { s := Service{ UnimplementedBridgeServer: UnimplementedBridgeServer{}, diff --git a/internal/frontend/types/types.go b/internal/frontend/types/types.go index ae1fd5fa..35baf3a0 100644 --- a/internal/frontend/types/types.go +++ b/internal/frontend/types/types.go @@ -37,10 +37,6 @@ type Restarter interface { ForceLauncher(string) } -type NoEncConfirmator interface { - ConfirmNoEncryption(string, bool) -} - type Updater interface { Check() (updater.VersionInfo, error) InstallUpdate(updater.VersionInfo) error diff --git a/internal/smtp/backend.go b/internal/smtp/backend.go index 056378df..77b8697c 100644 --- a/internal/smtp/backend.go +++ b/internal/smtp/backend.go @@ -22,13 +22,10 @@ import ( "time" "github.com/ProtonMail/proton-bridge/v2/internal/bridge" - "github.com/ProtonMail/proton-bridge/v2/internal/config/settings" "github.com/ProtonMail/proton-bridge/v2/internal/users" - "github.com/ProtonMail/proton-bridge/v2/pkg/confirmer" "github.com/ProtonMail/proton-bridge/v2/pkg/listener" goSMTPBackend "github.com/emersion/go-smtp" "github.com/pkg/errors" - "github.com/sirupsen/logrus" ) type panicHandler interface { @@ -44,7 +41,6 @@ type smtpBackend struct { eventListener listener.Listener settings settingsProvider bridge bridger - confirmer *confirmer.Confirmer sendRecorder *sendRecorder } @@ -69,7 +65,6 @@ func newSMTPBackend( eventListener: eventListener, settings: settings, bridge: bridge, - confirmer: confirmer.New(), sendRecorder: newSendRecorder(), } } @@ -116,13 +111,3 @@ func (sb *smtpBackend) AnonymousLogin(_ *goSMTPBackend.ConnectionState) (goSMTPB return nil, errors.New("anonymous login not supported") } - -func (sb *smtpBackend) shouldReportOutgoingNoEnc() bool { - return sb.settings.GetBool(settings.ReportOutgoingNoEncKey) -} - -func (sb *smtpBackend) ConfirmNoEncryption(messageID string, shouldSend bool) { - if err := sb.confirmer.SetResult(messageID, shouldSend); err != nil { - logrus.WithError(err).Error("Failed to set confirmation value") - } -} diff --git a/internal/smtp/user.go b/internal/smtp/user.go index 9636e9a5..c66ab766 100644 --- a/internal/smtp/user.go +++ b/internal/smtp/user.go @@ -26,20 +26,17 @@ import ( "encoding/base64" "fmt" "io" - "mime" "net/mail" "strings" "time" "github.com/ProtonMail/gopenpgp/v2/crypto" - "github.com/ProtonMail/proton-bridge/v2/internal/events" "github.com/ProtonMail/proton-bridge/v2/pkg/listener" pkgMsg "github.com/ProtonMail/proton-bridge/v2/pkg/message" "github.com/ProtonMail/proton-bridge/v2/pkg/message/parser" "github.com/ProtonMail/proton-bridge/v2/pkg/pmapi" goSMTPBackend "github.com/emersion/go-smtp" "github.com/pkg/errors" - "github.com/sirupsen/logrus" ) type smtpUser struct { @@ -361,7 +358,6 @@ func (su *smtpUser) Send(returnPath string, to []string, messageReader io.Reader } req := pmapi.NewSendMessageReq(kr, mimeBody, plainBody, richBody, attkeys) - containsUnencryptedRecipients := false for _, recipient := range message.Recipients() { email := recipient.Address @@ -370,9 +366,6 @@ func (su *smtpUser) Send(returnPath string, to []string, messageReader io.Reader } sendPreferences, err := su.getSendPreferences(email, message.MIMEType, mailSettings) - if !sendPreferences.Encrypt { - containsUnencryptedRecipients = true - } if err != nil { return err } @@ -389,20 +382,6 @@ func (su *smtpUser) Send(returnPath string, to []string, messageReader io.Reader } } - if containsUnencryptedRecipients { - dec := new(mime.WordDecoder) - subject, err := dec.DecodeHeader(message.Header.Get("Subject")) - if err != nil { - return errors.New("error decoding subject message " + message.Header.Get("Subject")) - } - if !su.continueSendingUnencryptedMail(subject) { - if err := su.client().DeleteMessages(context.TODO(), []string{message.ID}); err != nil { - log.WithError(err).Warn("Failed to delete canceled messages") - } - return errors.New("sending was canceled by user") - } - } - req.PreparePackages() dumpMessageData(b.Bytes(), message.Subject) @@ -511,27 +490,6 @@ func (su *smtpUser) handleSenderAndRecipients(m *pmapi.Message, returnPathAddr * return nil } -func (su *smtpUser) continueSendingUnencryptedMail(subject string) bool { - if !su.backend.shouldReportOutgoingNoEnc() { - return true - } - - // GUI should always respond in 10 seconds, but let's have safety timeout - // in case GUI will not respond properly. If GUI didn't respond, we cannot - // be sure if user even saw the notice: better to not send the e-mail. - req := su.backend.confirmer.NewRequest(15 * time.Second) - - su.eventListener.Emit(events.OutgoingNoEncEvent, req.ID()+":"+subject) - - res, err := req.Result() - if err != nil { - logrus.WithError(err).Error("Failed to determine whether to send unencrypted, assuming no") - return false - } - - return res -} - // Logout is called when this User will no longer be used. func (su *smtpUser) Logout() error { log.Debug("SMTP client logged out user ", su.addressID) diff --git a/pkg/confirmer/confirmer.go b/pkg/confirmer/confirmer.go deleted file mode 100644 index 31b1d0e4..00000000 --- a/pkg/confirmer/confirmer.go +++ /dev/null @@ -1,70 +0,0 @@ -// Copyright (c) 2022 Proton AG -// -// This file is part of Proton Mail Bridge. -// -// Proton Mail Bridge is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// Proton Mail Bridge is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with Proton Mail Bridge. If not, see . - -package confirmer - -import ( - "errors" - "sync" - "time" -) - -// NOTE: For now, Confirmer only supports bool values but it could easily be made generic. - -// Confirmer is used to ask for some value (e.g. a confirmation from a GUI element) -// in a threadsafe manner and retrieve that value later. -type Confirmer struct { - requests map[string]*Request - locker sync.Locker -} - -func New() *Confirmer { - return &Confirmer{ - requests: make(map[string]*Request), - locker: &sync.Mutex{}, - } -} - -// NewRequest creates a new request object that waits up to the given amount of time for the result. -func (c *Confirmer) NewRequest(timeout time.Duration) *Request { - c.locker.Lock() - defer c.locker.Unlock() - - req := newRequest(timeout) - - c.requests[req.ID()] = req - - return req -} - -// SetResult sets the result value of the request with the given ID. -func (c *Confirmer) SetResult(id string, value bool) error { - c.locker.Lock() - defer c.locker.Unlock() - - req, ok := c.requests[id] - if !ok { - return errors.New("no such request") - } - - req.ch <- value - - close(req.ch) - delete(c.requests, id) - - return nil -} diff --git a/pkg/confirmer/confirmer_test.go b/pkg/confirmer/confirmer_test.go deleted file mode 100644 index 8e13b70b..00000000 --- a/pkg/confirmer/confirmer_test.go +++ /dev/null @@ -1,120 +0,0 @@ -// Copyright (c) 2022 Proton AG -// -// This file is part of Proton Mail Bridge. -// -// Proton Mail Bridge is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// Proton Mail Bridge is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with Proton Mail Bridge. If not, see . - -package confirmer - -import ( - "testing" - "time" - - "github.com/stretchr/testify/assert" -) - -func TestConfirmerYes(t *testing.T) { - c := New() - - req := c.NewRequest(1 * time.Second) - - go func() { - assert.NoError(t, c.SetResult(req.ID(), true)) - }() - - res, err := req.Result() - assert.NoError(t, err) - assert.True(t, res) -} - -func TestConfirmerNo(t *testing.T) { - c := New() - - req := c.NewRequest(1 * time.Second) - - go func() { - assert.NoError(t, c.SetResult(req.ID(), false)) - }() - - res, err := req.Result() - assert.NoError(t, err) - assert.False(t, res) -} - -func TestConfirmerTimeout(t *testing.T) { - c := New() - - req := c.NewRequest(1 * time.Second) - - go func() { - time.Sleep(2 * time.Second) - assert.NoError(t, c.SetResult(req.ID(), true)) - }() - - _, err := req.Result() - assert.Error(t, err) -} - -func TestConfirmerMultipleResultCalls(t *testing.T) { - c := New() - - req := c.NewRequest(1 * time.Second) - - go func() { - assert.NoError(t, c.SetResult(req.ID(), true)) - }() - - res, err := req.Result() - assert.NoError(t, err) - assert.True(t, res) - - _, errAgain := req.Result() - assert.Error(t, errAgain) -} - -func TestConfirmerMultipleSimultaneousResultCalls(t *testing.T) { - c := New() - - req := c.NewRequest(1 * time.Second) - - go func() { - time.Sleep(1 * time.Second) - assert.NoError(t, c.SetResult(req.ID(), true)) - }() - - // We just check that nothing panics. We can't know which Result() will get the result though. - - go func() { _, _ = req.Result() }() - go func() { _, _ = req.Result() }() - go func() { _, _ = req.Result() }() - - _, _ = req.Result() -} - -func TestConfirmerMultipleSetResultCalls(t *testing.T) { - c := New() - - req := c.NewRequest(1 * time.Second) - - go func() { - assert.NoError(t, c.SetResult(req.ID(), true)) - assert.Error(t, c.SetResult(req.ID(), true)) - assert.Error(t, c.SetResult(req.ID(), true)) - assert.Error(t, c.SetResult(req.ID(), true)) - }() - - res, err := req.Result() - assert.NoError(t, err) - assert.True(t, res) -} diff --git a/pkg/confirmer/request.go b/pkg/confirmer/request.go deleted file mode 100644 index dc3e4af1..00000000 --- a/pkg/confirmer/request.go +++ /dev/null @@ -1,70 +0,0 @@ -// Copyright (c) 2022 Proton AG -// -// This file is part of Proton Mail Bridge. -// -// Proton Mail Bridge is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// Proton Mail Bridge is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with Proton Mail Bridge. If not, see . - -package confirmer - -import ( - "errors" - "sync" - "time" - - "github.com/google/uuid" -) - -// Request provides a result when it becomes available. -type Request struct { - uuid string - ch chan bool - timeout time.Duration - - expired bool - locker sync.Locker -} - -func newRequest(timeout time.Duration) *Request { - return &Request{ - uuid: uuid.New().String(), - ch: make(chan bool), - timeout: timeout, - locker: &sync.Mutex{}, - } -} - -// ID returns the request's ID, used to set the request's value. -func (r *Request) ID() string { - return r.uuid -} - -// Result returns the result or an error if it is not available within the request timeout. -func (r *Request) Result() (bool, error) { - r.locker.Lock() - defer r.locker.Unlock() - - if r.expired { - return false, errors.New("this result has expired") - } - - defer func() { r.expired = true }() - - select { - case res := <-r.ch: - return res, nil - - case <-time.After(r.timeout): - return false, errors.New("timed out waiting for result") - } -}