fix: don't check tls fingerprints when checking connectivity

Changelog.md

@@ -33,6 +33,11 @@ Changelog [format](http://keepachangelog.com/en/1.0.0/)
 * GODT-827 Do not spam sentry with bad ID by integration test.
 * GODT-700 Fix UTF-7 incompatibility.
 * GODT-837 Fix flaky TestFailUnpauseAndStops.
+* GODT-782 Don't use TLS pinning when checking connectivity status.
+
+### Changed
+* TLS pins conform to official list.
+
 
 ## [Bridge 1.4.5] Forth
 

pkg/pmapi/check_connection.go

@@ -0,0 +1,94 @@
+// 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 <https://www.gnu.org/licenses/>.
+
+package pmapi
+
+import (
+	"errors"
+	"fmt"
+	"net/http"
+	"time"
+)
+
+const protonStatusURL = "http://protonstatus.com/vpn_status"
+
+// ErrNoInternetConnection indicates that both protonstatus and the API are unreachable.
+var ErrNoInternetConnection = errors.New("no internet connection")
+
+// CheckConnection returns an error if there is no internet connection.
+// This should be moved to the ConnectionManager when it is implemented.
+func (cm *ClientManager) CheckConnection() error {
+	// We use a normal dialer here which doesn't check tls fingerprints.
+	client := &http.Client{Timeout: time.Second * 10}
+
+	// Do not cumulate timeouts, use goroutines.
+	retStatus := make(chan error)
+	retAPI := make(chan error)
+
+	// vpn_status endpoint is fast and returns only OK. We check the connection only.
+	go checkConnection(client, protonStatusURL, retStatus)
+
+	// Check of API reachability also uses a fast endpoint.
+	go checkConnection(client, cm.GetRootURL()+"/tests/ping", retAPI)
+
+	errStatus := <-retStatus
+	errAPI := <-retAPI
+
+	switch {
+	case errStatus == nil && errAPI == nil:
+		return nil
+
+	case errStatus == nil && errAPI != nil:
+		cm.log.Error("ProtonStatus is reachable but API is not")
+		return ErrAPINotReachable
+
+	case errStatus != nil && errAPI == nil:
+		cm.log.Warn("API is reachable but protonstatus is not")
+		return nil
+
+	case errStatus != nil && errAPI != nil:
+		cm.log.Error("Both ProtonStatus and API are unreachable")
+		return ErrNoInternetConnection
+	}
+
+	return nil
+}
+
+// CheckConnection returns an error if there is no internet connection.
+func CheckConnection() error {
+	client := &http.Client{Timeout: time.Second * 10}
+	retStatus := make(chan error)
+	go checkConnection(client, protonStatusURL, retStatus)
+	return <-retStatus
+}
+
+func checkConnection(client *http.Client, url string, errorChannel chan error) {
+	resp, err := client.Get(url)
+	if err != nil {
+		errorChannel <- err
+		return
+	}
+
+	_ = resp.Body.Close()
+
+	if resp.StatusCode != 200 {
+		errorChannel <- fmt.Errorf("HTTP status code %d", resp.StatusCode)
+		return
+	}
+
+	errorChannel <- nil
+}

pkg/pmapi/clientmanager.go

@@ -305,72 +305,6 @@ func (cm *ClientManager) GetAuthUpdateChannel() chan ClientAuth {
 	return cm.authUpdates
 }
 
-// ErrNoInternetConnection indicates that both protonstatus and the API are unreachable.
-var ErrNoInternetConnection = errors.New("no internet connection")
-
-// CheckConnection returns an error if there is no internet connection.
-// This should be moved to the ConnectionManager when it is implemented.
-func (cm *ClientManager) CheckConnection() error {
-	client := getHTTPClient(cm.config, cm.roundTripper, cm.cookieJar)
-
-	// Do not cumulate timeouts, use goroutines.
-	retStatus := make(chan error)
-	retAPI := make(chan error)
-
-	// vpn_status endpoint is fast and returns only OK. We check the connection only.
-	go checkConnection(client, "https://protonstatus.com/vpn_status", retStatus)
-
-	// Check of API reachability also uses a fast endpoint.
-	go checkConnection(client, cm.GetRootURL()+"/tests/ping", retAPI)
-
-	errStatus := <-retStatus
-	errAPI := <-retAPI
-
-	switch {
-	case errStatus == nil && errAPI == nil:
-		return nil
-
-	case errStatus == nil && errAPI != nil:
-		cm.log.Error("ProtonStatus is reachable but API is not")
-		return ErrAPINotReachable
-
-	case errStatus != nil && errAPI == nil:
-		cm.log.Warn("API is reachable but protonstatus is not")
-		return nil
-
-	case errStatus != nil && errAPI != nil:
-		cm.log.Error("Both ProtonStatus and API are unreachable")
-		return ErrNoInternetConnection
-	}
-
-	return nil
-}
-
-// CheckConnection returns an error if there is no internet connection.
-func CheckConnection() error {
-	client := &http.Client{Timeout: time.Second * 10}
-	retStatus := make(chan error)
-	go checkConnection(client, "https://protonstatus.com/vpn_status", retStatus)
-	return <-retStatus
-}
-
-func checkConnection(client *http.Client, url string, errorChannel chan error) {
-	resp, err := client.Get(url)
-	if err != nil {
-		errorChannel <- err
-		return
-	}
-
-	_ = resp.Body.Close()
-
-	if resp.StatusCode != 200 {
-		errorChannel <- fmt.Errorf("HTTP status code %d", resp.StatusCode)
-		return
-	}
-
-	errorChannel <- nil
-}
-
 // setTokenIfUnset sets the token for the given userID if it wasn't already set.
 // The set token does not expire.
 func (cm *ClientManager) setTokenIfUnset(userID, token string) {

pkg/pmapi/proxy_test.go

@@ -382,7 +382,7 @@ func TestProxyProvider_UseProxy_RevertIfProxyStopsWorkingAndOriginalAPIIsReachab
 	// The error should be ErrAPINotReachable because the connection dropped intermittently but
 	// the original API is now reachable (see Alternative-Routing-v2 spec for details).
 	url, err = cm.switchToReachableServer()
-	require.EqualError(t, err, ErrAPINotReachable.Error())
+	require.Error(t, err)
 	require.Equal(t, rootURL, url)
 	require.Equal(t, rootURL, cm.getHost())
 }

pkg/pmapi/tlsreport.go

@@ -35,13 +35,21 @@ var ErrTLSMismatch = errors.New("no TLS fingerprint match found")
 // TrustedAPIPins contains trusted public keys of the protonmail API and proxies.
 // NOTE: the proxy pins are the same for all proxy servers, guaranteed by infra team ;)
 var TrustedAPIPins = []string{ // nolint[gochecknoglobals]
+	// api.protonmail.ch
 	`pin-sha256="drtmcR2kFkM8qJClsuWgUzxgBkePfRCkRpqUesyDmeE="`, // current
-	`pin-sha256="YRGlaY0jyJ4Jw2/4M8FIftwbDIQfh8Sdro96CeEel54="`, // hot
+	`pin-sha256="YRGlaY0jyJ4Jw2/4M8FIftwbDIQfh8Sdro96CeEel54="`, // hot backup
-	`pin-sha256="AfMENBVvOS8MnISprtvyPsjKlPooqh8nMB/pvCrpJpw="`, // cold
+	`pin-sha256="AfMENBVvOS8MnISprtvyPsjKlPooqh8nMB/pvCrpJpw="`, // cold backup
-	`pin-sha256="EU6TS9MO0L/GsDHvVc9D5fChYLNy5JdGYpJw0ccgetM="`, // proxy main
+
-	`pin-sha256="iKPIHPnDNqdkvOnTClQ8zQAIKG0XavaPkcEo0LBAABA="`, // proxy backup 1
+	// protonmail.com
-	`pin-sha256="MSlVrBCdL0hKyczvgYVSRNm88RicyY04Q2y5qrBt0xA="`, // proxy backup 2
+	`pin-sha256="8joiNBdqaYiQpKskgtkJsqRxF7zN0C0aqfi8DacknnI="`, // current
-	`pin-sha256="C2UxW0T1Ckl9s+8cXfjXxlEqwAfPM4HiW2y3UdtBeCw="`, // proxy backup 3
+	`pin-sha256="JMI8yrbc6jB1FYGyyWRLFTmDNgIszrNEMGlgy972e7w="`, // hot backup
+	`pin-sha256="Iu44zU84EOCZ9vx/vz67/MRVrxF1IO4i4NIa8ETwiIY="`, // cold backup
+
+	// proxies
+	`pin-sha256="EU6TS9MO0L/GsDHvVc9D5fChYLNy5JdGYpJw0ccgetM="`, // main
+	`pin-sha256="iKPIHPnDNqdkvOnTClQ8zQAIKG0XavaPkcEo0LBAABA="`, // backup 1
+	`pin-sha256="MSlVrBCdL0hKyczvgYVSRNm88RicyY04Q2y5qrBt0xA="`, // backup 2
+	`pin-sha256="C2UxW0T1Ckl9s+8cXfjXxlEqwAfPM4HiW2y3UdtBeCw="`, // backup 3
 }
 
 // TLSReportURI is the address where TLS reports should be sent.