fix: fingerprint and proxy

Use tls.Config.VerifyPeerCertificate, not the dialer, to verify the
Uphold TLS certificate fingerprint. This ensures that the fingerprint
checks are done even if a proxy is used.

Author: ibukanov

libs/pindialer/dialer.go

@@ -5,17 +5,17 @@ import (
 	"context"
 	"crypto/sha256"
 	"crypto/tls"
+	"crypto/x509"
 	"encoding/base64"
 	"errors"
-	"fmt"
 	"net"
 )
 
 // ContextDialer is a function connecting to the address on the named network
 type ContextDialer func(ctx context.Context, network, addr string) (net.Conn, error)
 
-func validateChain(fingerprint string, connstate tls.ConnectionState) error {
-	for _, chain := range connstate.VerifiedChains {
+func validateChain(fingerprint string, verifiedChains [][]*x509.Certificate) error {
+	for _, chain := range verifiedChains {
 		for _, cert := range chain {
 			hash := sha256.Sum256(cert.RawSubjectPublicKeyInfo)
 			digest := base64.StdEncoding.EncodeToString(hash[:])
@@ -27,22 +27,13 @@ func validateChain(fingerprint string, connstate tls.ConnectionState) error {
 	return errors.New("the server certificate was not valid")
 }
 
-// MakeContextDialer returns a ContextDialer that only succeeds on connection to a TLS secured address with the pinned fingerprint
-func MakeContextDialer(fingerprint string) ContextDialer {
-	return func(ctx context.Context, network, addr string) (net.Conn, error) {
-		c, err := tls.Dial(network, addr, nil)
-		if err != nil {
-			return c, err
-		}
-		select {
-		case <-ctx.Done():
-			return nil, fmt.Errorf("context completed")
-		default:
-			if err := validateChain(fingerprint, c.ConnectionState()); err != nil {
-				return nil, fmt.Errorf("failed to validate certificate chain: %w", err)
-			}
-		}
-		return c, nil
+// Get tls.Config that validates the connection certificate chain against the
+// given fingerprint.
+func GetTLSConfig(fingerprint string) *tls.Config {
+	return &tls.Config{
+		VerifyPeerCertificate: func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error {
+			return validateChain(fingerprint, verifiedChains)
+		},
 	}
 }
 

libs/wallet/provider/uphold/uphold.go

@@ -138,9 +138,9 @@ func init() {
 		Timeout: httpTimeout,
 		Transport: middleware.InstrumentRoundTripper(
 			&http.Transport{
-				DialTLSContext: fingerprintDialer,
-				Proxy:          proxy,
-				TLSNextProto:   disableHTTP2,
+				Proxy:           proxy,
+				TLSClientConfig: pindialer.GetTLSConfig(upholdCertFingerprint),
+				TLSNextProto:    disableHTTP2,
 			}, "uphold"),
 	}
 	httpClientNoFP = &http.Client{

libs/wallet/provider/uphold/uphold_test.go

@@ -2,6 +2,7 @@ package uphold
 
 import (
 	"context"
+	"crypto/tls"
 	"encoding/hex"
 	"errors"
 	"net/http"
@@ -307,28 +308,53 @@ func TestFingerprintCheck(t *testing.T) {
 	var proxy func(*http.Request) (*url.URL, error)
 	wrongFingerprint := "IYSLsapSKlkofKfi6M2hmS4gzXbQKGIX/DHBWIgstw3="
 
+	w := requireDonorWallet(t)
+
+	req, err := w.signRegistration("randomlabel")
+	if err != nil {
+		t.Error(err)
+	}
+
+	// Check fingerprint error case
 	client := &http.Client{
 		Timeout: time.Second * 60,
 		// remove middleware calls
 		Transport: &http.Transport{
-			Proxy:          proxy,
-			DialTLSContext: pindialer.MakeContextDialer(wrongFingerprint),
+			Proxy:           proxy,
+			TLSClientConfig: pindialer.GetTLSConfig(wrongFingerprint),
 		},
 	}
 
-	w := requireDonorWallet(t)
+	_, err = client.Do(req)
+	assert.ErrorContains(t, err, "the server certificate was not valid")
 
-	req, err := w.signRegistration("randomlabel")
-	if err != nil {
-		t.Error(err)
+	// Check the fingerprint success case.
+	tlsConfig := pindialer.GetTLSConfig(upholdCertFingerprint)
+
+	// VerifyConnection callback is only called after
+	// tlsConfig.VerifyPeerCertificate returns success.
+	verifyConnectionCalled := false
+	if tlsConfig.VerifyConnection != nil {
+		t.Fatalf("tlsConfig.VerifyConnection must be unset")
+	}
+	tlsConfig.VerifyConnection = func(tls.ConnectionState) error {
+		if verifyConnectionCalled {
+			t.Fatalf("Unexpected extra call to VerifyConnection")
+		}
+		verifyConnectionCalled = true
+		return nil
 	}
 
-	_, err = client.Do(req)
-	// should fail here
-	if err == nil {
-		t.Error("unable to fail with bad cert")
+	client = &http.Client{
+		Timeout: time.Second * 60,
+		Transport: &http.Transport{
+			Proxy:           proxy,
+			TLSClientConfig: tlsConfig,
+		},
 	}
-	assert.Equal(t, errors.Unwrap(err).Error(), "failed to validate certificate chain: the server certificate was not valid")
+
+	_, _ = client.Do(req)
+	assert.Equal(t, true, verifyConnectionCalled)
 }
 
 func requireDonorWallet(t *testing.T) *Wallet {