From 36ef9f20ae5aaaf1de902a8e74b7a718373ace3e Mon Sep 17 00:00:00 2001 From: James Houlahan Date: Thu, 23 Jul 2020 13:44:22 +0200 Subject: [PATCH] feat: use confirmer in smtp --- Changelog.md | 2 ++ internal/bridge/credits.go | 4 ++-- internal/smtp/backend.go | 30 ++++++++++++++++-------------- internal/smtp/user.go | 30 ++++++++++-------------------- pkg/confirmer/confirmer.go | 19 +++++++++++++++++++ pkg/confirmer/confirmer_test.go | 17 +++++++++++++++++ pkg/confirmer/request.go | 17 +++++++++++++++++ 7 files changed, 83 insertions(+), 36 deletions(-) diff --git a/Changelog.md b/Changelog.md index 4b13ff22..435a34aa 100644 --- a/Changelog.md +++ b/Changelog.md @@ -7,6 +7,8 @@ Changelog [format](http://keepachangelog.com/en/1.0.0/) ### Changed * GODT-409 Set flags have to replace all flags. * GODT-531 Better way to add trusted certificate in macOS. +### Fixed +* GODT-454 Fix send on closed channel when receiving unencrypted send confirmation from GUI. ## [v1.3.x] Emma (beta 2020-07-XXX) diff --git a/internal/bridge/credits.go b/internal/bridge/credits.go index 304c0af1..195301a7 100644 --- a/internal/bridge/credits.go +++ b/internal/bridge/credits.go @@ -15,8 +15,8 @@ // You should have received a copy of the GNU General Public License // along with ProtonMail Bridge. If not, see . -// Code generated by ./credits.sh at Wed Jul 15 13:18:44 CEST 2020. DO NOT EDIT. +// Code generated by ./credits.sh at Thu Jul 23 13:43:24 CEST 2020. DO NOT EDIT. package bridge -const Credits = "github.com/0xAX/notificator;github.com/ProtonMail/bcrypt;github.com/ProtonMail/crypto;github.com/ProtonMail/docker-credential-helpers;github.com/ProtonMail/go-appdir;github.com/ProtonMail/go-apple-mobileconfig;github.com/ProtonMail/go-autostart;github.com/ProtonMail/go-imap-id;github.com/ProtonMail/go-smtp;github.com/ProtonMail/go-vcard;github.com/ProtonMail/gopenpgp/v2;github.com/abiosoft/ishell;github.com/abiosoft/readline;github.com/allan-simon/go-singleinstance;github.com/andybalholm/cascadia;github.com/certifi/gocertifi;github.com/chzyer/logex;github.com/chzyer/test;github.com/cucumber/godog;github.com/docker/docker-credential-helpers;github.com/emersion/go-imap;github.com/emersion/go-imap-appendlimit;github.com/emersion/go-imap-idle;github.com/emersion/go-imap-move;github.com/emersion/go-imap-quota;github.com/emersion/go-imap-specialuse;github.com/emersion/go-imap-unselect;github.com/emersion/go-sasl;github.com/emersion/go-smtp;github.com/emersion/go-textwrapper;github.com/emersion/go-vcard;github.com/fatih/color;github.com/flynn-archive/go-shlex;github.com/getsentry/raven-go;github.com/go-resty/resty/v2;github.com/golang/mock;github.com/google/go-cmp;github.com/gopherjs/gopherjs;github.com/hashicorp/go-multierror;github.com/jameskeane/bcrypt;github.com/jaytaylor/html2text;github.com/jhillyerd/enmime;github.com/kardianos/osext;github.com/keybase/go-keychain;github.com/logrusorgru/aurora;github.com/miekg/dns;github.com/myesui/uuid;github.com/nsf/jsondiff;github.com/pkg/errors;github.com/sirupsen/logrus;github.com/skratchdot/open-golang;github.com/stretchr/testify;github.com/therecipe/qt;github.com/twinj/uuid;github.com/urfave/cli;go.etcd.io/bbolt;golang.org/x/crypto;golang.org/x/net;golang.org/x/text;gopkg.in/stretchr/testify.v1;;Font Awesome 4.7.0;;Qt 5.13 by Qt group;" +const Credits = "github.com/0xAX/notificator;github.com/ProtonMail/bcrypt;github.com/ProtonMail/crypto;github.com/ProtonMail/docker-credential-helpers;github.com/ProtonMail/go-appdir;github.com/ProtonMail/go-apple-mobileconfig;github.com/ProtonMail/go-autostart;github.com/ProtonMail/go-imap-id;github.com/ProtonMail/go-smtp;github.com/ProtonMail/go-vcard;github.com/ProtonMail/gopenpgp/v2;github.com/abiosoft/ishell;github.com/abiosoft/readline;github.com/allan-simon/go-singleinstance;github.com/andybalholm/cascadia;github.com/certifi/gocertifi;github.com/chzyer/logex;github.com/chzyer/test;github.com/cucumber/godog;github.com/docker/docker-credential-helpers;github.com/emersion/go-imap;github.com/emersion/go-imap-appendlimit;github.com/emersion/go-imap-idle;github.com/emersion/go-imap-move;github.com/emersion/go-imap-quota;github.com/emersion/go-imap-specialuse;github.com/emersion/go-imap-unselect;github.com/emersion/go-sasl;github.com/emersion/go-smtp;github.com/emersion/go-textwrapper;github.com/emersion/go-vcard;github.com/fatih/color;github.com/flynn-archive/go-shlex;github.com/getsentry/raven-go;github.com/go-resty/resty/v2;github.com/golang/mock;github.com/google/go-cmp;github.com/google/uuid;github.com/gopherjs/gopherjs;github.com/hashicorp/go-multierror;github.com/jameskeane/bcrypt;github.com/jaytaylor/html2text;github.com/jhillyerd/enmime;github.com/kardianos/osext;github.com/keybase/go-keychain;github.com/logrusorgru/aurora;github.com/miekg/dns;github.com/myesui/uuid;github.com/nsf/jsondiff;github.com/pkg/errors;github.com/sirupsen/logrus;github.com/skratchdot/open-golang;github.com/stretchr/testify;github.com/therecipe/qt;github.com/twinj/uuid;github.com/urfave/cli;go.etcd.io/bbolt;golang.org/x/crypto;golang.org/x/net;golang.org/x/text;gopkg.in/stretchr/testify.v1;;Font Awesome 4.7.0;;Qt 5.13 by Qt group;" diff --git a/internal/smtp/backend.go b/internal/smtp/backend.go index abf06004..fee55818 100644 --- a/internal/smtp/backend.go +++ b/internal/smtp/backend.go @@ -24,8 +24,10 @@ import ( "github.com/ProtonMail/proton-bridge/internal/bridge" "github.com/ProtonMail/proton-bridge/internal/preferences" "github.com/ProtonMail/proton-bridge/pkg/config" + "github.com/ProtonMail/proton-bridge/pkg/confirmer" "github.com/ProtonMail/proton-bridge/pkg/listener" goSMTPBackend "github.com/emersion/go-smtp" + "github.com/sirupsen/logrus" ) type panicHandler interface { @@ -33,12 +35,12 @@ type panicHandler interface { } type smtpBackend struct { - panicHandler panicHandler - eventListener listener.Listener - preferences *config.Preferences - bridge bridger - shouldSendNoEncChannels map[string]chan bool - sendRecorder *sendRecorder + panicHandler panicHandler + eventListener listener.Listener + preferences *config.Preferences + bridge bridger + confirmer *confirmer.Confirmer + sendRecorder *sendRecorder } // NewSMTPBackend returns struct implementing go-smtp/backend interface. @@ -58,12 +60,12 @@ func newSMTPBackend( bridge bridger, ) *smtpBackend { return &smtpBackend{ - panicHandler: panicHandler, - eventListener: eventListener, - preferences: preferences, - bridge: bridge, - shouldSendNoEncChannels: make(map[string]chan bool), - sendRecorder: newSendRecorder(), + panicHandler: panicHandler, + eventListener: eventListener, + preferences: preferences, + bridge: bridge, + confirmer: confirmer.New(), + sendRecorder: newSendRecorder(), } } @@ -103,7 +105,7 @@ func (sb *smtpBackend) shouldReportOutgoingNoEnc() bool { } func (sb *smtpBackend) ConfirmNoEncryption(messageID string, shouldSend bool) { - if ch, ok := sb.shouldSendNoEncChannels[messageID]; ok { - ch <- shouldSend + if err := sb.confirmer.SetResponse(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 70d9119c..b91e8f0b 100644 --- a/internal/smtp/user.go +++ b/internal/smtp/user.go @@ -23,11 +23,9 @@ import ( "encoding/base64" "fmt" "io" - "math/rand" "mime" "net/mail" "regexp" - "strconv" "strings" "time" @@ -38,6 +36,7 @@ import ( "github.com/ProtonMail/proton-bridge/pkg/pmapi" goSMTPBackend "github.com/emersion/go-smtp" "github.com/pkg/errors" + "github.com/sirupsen/logrus" ) type smtpUser struct { @@ -497,28 +496,19 @@ func (su *smtpUser) continueSendingUnencryptedMail(subject string) bool { return true } - messageID := strconv.Itoa(rand.Int()) //nolint[gosec] - ch := make(chan bool) - su.backend.shouldSendNoEncChannels[messageID] = ch - su.eventListener.Emit(events.OutgoingNoEncEvent, messageID+":"+subject) + // 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) - log.Debug("Waiting for sendingUnencrypted confirmation for ", messageID) + su.eventListener.Emit(events.OutgoingNoEncEvent, req.ID()+":"+subject) - var res bool - select { - case res = <-ch: - // 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. - log.Debug("Got sendingUnencrypted for ", messageID, ": ", res) - case <-time.After(15 * time.Second): - log.Debug("sendingUnencrypted timeout, not sending ", messageID) - res = false + res, err := req.Result() + if err != nil { + logrus.WithError(err).Error("Failed to determine whether to send unencrypted, assuming no") + return false } - delete(su.backend.shouldSendNoEncChannels, messageID) - close(ch) - return res } diff --git a/pkg/confirmer/confirmer.go b/pkg/confirmer/confirmer.go index 7b121247..15445d64 100644 --- a/pkg/confirmer/confirmer.go +++ b/pkg/confirmer/confirmer.go @@ -1,3 +1,20 @@ +// Copyright (c) 2020 Proton Technologies AG +// +// This file is part of ProtonMail Bridge. +// +// ProtonMail 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. +// +// ProtonMail 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 ProtonMail Bridge. If not, see . + package confirmer import ( @@ -6,6 +23,8 @@ import ( "time" ) +// NOTE: For now, Confirmer only supports bool values but it could easily be made generic. + type Confirmer struct { requests map[string]*Request locker sync.Locker diff --git a/pkg/confirmer/confirmer_test.go b/pkg/confirmer/confirmer_test.go index 5e0200cf..e2ce6e06 100644 --- a/pkg/confirmer/confirmer_test.go +++ b/pkg/confirmer/confirmer_test.go @@ -1,3 +1,20 @@ +// Copyright (c) 2020 Proton Technologies AG +// +// This file is part of ProtonMail Bridge. +// +// ProtonMail 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. +// +// ProtonMail 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 ProtonMail Bridge. If not, see . + package confirmer import ( diff --git a/pkg/confirmer/request.go b/pkg/confirmer/request.go index 5875e259..199886da 100644 --- a/pkg/confirmer/request.go +++ b/pkg/confirmer/request.go @@ -1,3 +1,20 @@ +// Copyright (c) 2020 Proton Technologies AG +// +// This file is part of ProtonMail Bridge. +// +// ProtonMail 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. +// +// ProtonMail 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 ProtonMail Bridge. If not, see . + package confirmer import (