Avoid fingerprint verification when checking IsUserKYC (#2548)

* chore: no fingerprint verification when checking

uphold.IsUserKYC() is used only for online checks but not for payments.
So skip certificate fingerprint checks to prevent outages when Uphold
changes the certificate.

Remove unused uphold.Wallet.Logger. Wallet methods always extract the
logger from the passed Context. Plus remove from tests log level
initialization. It was no-op as LogLevelCTXKey must be zerolog.Level,
not a string.

CLOSE: #2394

* fix: review comments

Author: ibukanov

libs/clients/coingecko/client_test.go

@@ -39,10 +39,8 @@ func (suite *CoingeckoTestSuite) SetupTest() {
 	// setup the context
 	suite.ctx = context.Background()
 
-	// setup debug for client
+	// debug logging generates too much noise, so do not activate it.
 	suite.ctx = context.WithValue(suite.ctx, appctx.DebugLoggingCTXKey, false)
-	// setup debug log level
-	suite.ctx = context.WithValue(suite.ctx, appctx.LogLevelCTXKey, "info")
 
 	// setup a logger and put on context
 	suite.ctx, _ = logutils.SetupLogger(suite.ctx)

libs/clients/stripe/client_test.go

@@ -36,7 +36,6 @@ func (suite *StripeTestSuite) SetupTest() {
 	// setup the context
 	suite.ctx = context.Background()
 	suite.ctx = context.WithValue(suite.ctx, appctx.DebugLoggingCTXKey, false)
-	suite.ctx = context.WithValue(suite.ctx, appctx.LogLevelCTXKey, "info")
 	suite.ctx, _ = logutils.SetupLogger(suite.ctx)
 
 	stripeKey := os.Getenv("STRIPE_ONRAMP_SECRET_KEY")

libs/wallet/provider/uphold/uphold.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"crypto"
+	"crypto/tls"
 	"encoding/base64"
 	"encoding/hex"
 	"encoding/json"
@@ -45,7 +46,6 @@ import (
 // A wallet corresponds to a single Uphold "card"
 type Wallet struct {
 	walletutils.Info
-	Logger  *zerolog.Logger
 	PrivKey crypto.Signer
 	PubKey  httpsignature.Verifier
 }
@@ -54,6 +54,7 @@ const (
 	dateFormat              = "2006-01-02T15:04:05.000Z"
 	batchSize               = 50
 	listTransactionsRetries = 5
+	httpTimeout             = time.Second * 60
 )
 
 const (
@@ -90,7 +91,13 @@ var (
 		"sandbox": sandboxFingerprint,
 		"prod":    prodFingerprint,
 	}[environment]
-	client *http.Client
+
+	// The client to connect to Uphold servers while performing fingerprint
+	// checks on the server certificates.
+	defaultHTTPClient *http.Client
+
+	// The client without fingerprint checks.
+	httpClientNoFP *http.Client
 )
 
 func init() {
@@ -115,12 +122,33 @@ func init() {
 	} else {
 		proxy = nil
 	}
-	client = &http.Client{
-		Timeout: time.Second * 60,
+
+	fingerprintDialer := pindialer.MakeContextDialer(upholdCertFingerprint)
+
+	// Uphold reports HTTP 401 error when connecting with HTTP2, so disable
+	// HTTP/2 via setting TLSNextProto to an empty map. We do not need to set
+	// this field on defaultHTTPClient as we set DialTLSContext without setting
+	// ForceAttemptHTTP2 and that disables HTTP/2 also. But for clarity we
+	// always set TLSNextProto.
+	disableHTTP2 := make(
+		map[string]func(authority string, c *tls.Conn) http.RoundTripper,
+		0,
+	)
+	defaultHTTPClient = &http.Client{
+		Timeout: httpTimeout,
 		Transport: middleware.InstrumentRoundTripper(
 			&http.Transport{
+				DialTLSContext: fingerprintDialer,
 				Proxy:          proxy,
-				DialTLSContext: pindialer.MakeContextDialer(upholdCertFingerprint),
+				TLSNextProto:   disableHTTP2,
+			}, "uphold"),
+	}
+	httpClientNoFP = &http.Client{
+		Timeout: httpTimeout,
+		Transport: middleware.InstrumentRoundTripper(
+			&http.Transport{
+				Proxy:        proxy,
+				TLSNextProto: disableHTTP2,
 			}, "uphold"),
 	}
 }
@@ -141,7 +169,11 @@ func New(ctx context.Context, info walletutils.Info, privKey crypto.Signer, pubK
 	if !info.AltCurrency.IsValid() {
 		return nil, errors.New("a wallet must have a valid altcurrency")
 	}
-	return &Wallet{Info: info, PrivKey: privKey, PubKey: pubKey}, nil
+	return &Wallet{
+		Info:    info,
+		PrivKey: privKey,
+		PubKey:  pubKey,
+	}, nil
 }
 
 // FromWalletInfo returns an uphold wallet matching the provided wallet info
@@ -169,7 +201,11 @@ func newRequest(method, path string, body io.Reader) (*http.Request, error) {
 	return req, err
 }
 
-func submit(logger *zerolog.Logger, req *http.Request) ([]byte, *http.Response, error) {
+func submit(
+	logger *zerolog.Logger,
+	client *http.Client,
+	req *http.Request,
+) ([]byte, *http.Response, error) {
 	req.Header.Add("content-type", "application/json")
 
 	dump, err := httputil.DumpRequestOut(req, true)
@@ -262,8 +298,18 @@ func (w *Wallet) IsUserKYC(ctx context.Context, destination string) (string, boo
 		return "", false, "", fmt.Errorf("failed to prepare transaction: %w", err)
 	}
 
-	// submit the transaction the payload
-	uhResp, err := grantWallet.SubmitTransaction(ctx, transactionB64, false)
+	// Submit the transaction payload.
+	//
+	// As we use the wallet only for validation but not for a payment, skip
+	// fingerprint checks to avoid outages when Uphold change the certificate.
+	// For payments it is not a problem as they are done in batch and can be
+	// repeated.
+	uhResp, err := grantWallet.submitTransaction(
+		ctx,
+		httpClientNoFP,
+		transactionB64,
+		false, /*confirm*/
+	)
 	if err != nil {
 		logger.Error().Err(err).Msg("failed to submit transaction")
 		return "", false, "", fmt.Errorf("failed to submit transaction: %w", err)
@@ -361,7 +407,7 @@ func (w *Wallet) Register(ctx context.Context, label string) error {
 		return err
 	}
 
-	body, _, err := submit(logger, req)
+	body, _, err := submit(logger, defaultHTTPClient, req)
 	if err != nil {
 		return err
 	}
@@ -400,7 +446,7 @@ func (w *Wallet) SubmitRegistration(ctx context.Context, registrationB64 string)
 		return err
 	}
 
-	body, _, err := submit(logger, req)
+	body, _, err := submit(logger, defaultHTTPClient, req)
 	if err != nil {
 		return err
 	}
@@ -456,7 +502,7 @@ func (w *Wallet) GetCardDetails(ctx context.Context) (*CardDetails, error) {
 	if err != nil {
 		return nil, err
 	}
-	body, _, err := submit(logger, req)
+	body, _, err := submit(logger, defaultHTTPClient, req)
 	if err != nil {
 		return nil, err
 	}
@@ -588,7 +634,7 @@ func (w *Wallet) Transfer(ctx context.Context, altcurrency altcurrency.AltCurren
 		return nil, fmt.Errorf("failed to sign the transfer: %w", err)
 	}
 
-	respBody, _, err := submit(logger, req)
+	respBody, _, err := submit(logger, defaultHTTPClient, req)
 	if err != nil {
 		// we need this to be draincoded wrapped error so we get the reason for failure in drains
 		if codedErr, ok := err.(Coded); ok {
@@ -843,10 +889,21 @@ func (resp upholdTransactionResponse) ToTransactionInfo() *walletutils.Transacti
 	return &txInfo
 }
 
-// SubmitTransaction submits the base64 encoded transaction for verification but does not move funds
-//
-//	unless confirm is set to true.
+// SubmitTransaction submits the base64 encoded transaction for verification but
+// does not move funds unless confirm is set to true.
 func (w *Wallet) SubmitTransaction(ctx context.Context, transactionB64 string, confirm bool) (*walletutils.TransactionInfo, error) {
+	return w.submitTransaction(ctx, defaultHTTPClient, transactionB64, confirm)
+}
+
+func (w *Wallet) submitTransaction(
+	ctx context.Context,
+	client *http.Client,
+	transactionB64 string,
+	confirm bool,
+) (*walletutils.TransactionInfo, error) {
+	if confirm && client != defaultHTTPClient {
+		panic("TLS fingerprint checks must be enabled when confirming")
+	}
 	logger := logging.FromContext(ctx)
 
 	_, err := w.VerifyTransaction(ctx, transactionB64)
@@ -879,7 +936,7 @@ func (w *Wallet) SubmitTransaction(ctx context.Context, transactionB64 string, c
 		return nil, err
 	}
 
-	respBody, _, err := submit(logger, req)
+	respBody, _, err := submit(logger, client, req)
 	if err != nil {
 		return nil, err
 	}
@@ -901,7 +958,7 @@ func (w *Wallet) ConfirmTransaction(ctx context.Context, id string) (*walletutil
 	if err != nil {
 		return nil, err
 	}
-	body, _, err := submit(logger, req)
+	body, _, err := submit(logger, defaultHTTPClient, req)
 	if err != nil {
 		return nil, err
 	}
@@ -927,7 +984,7 @@ func (w *Wallet) GetTransaction(ctx context.Context, id string) (*walletutils.Tr
 	if err != nil {
 		return nil, err
 	}
-	body, _, err := submit(logger, req)
+	body, _, err := submit(logger, defaultHTTPClient, req)
 	if err != nil {
 		return nil, err
 	}
@@ -970,7 +1027,7 @@ func (w *Wallet) ListTransactions(ctx context.Context, limit int, startDate time
 		var body []byte
 		var resp *http.Response
 		for i := 0; i < listTransactionsRetries; i++ {
-			body, resp, err = submit(logger, req)
+			body, resp, err = submit(logger, defaultHTTPClient, req)
 			if nerr, ok := err.(net.Error); ok && nerr.Temporary() {
 				logger.Debug().
 					Str("path", "github.com/brave-intl/bat-go/wallet/provider/uphold").
@@ -1070,7 +1127,7 @@ func (w *Wallet) CreateCardAddress(ctx context.Context, network string) (string,
 		return "", err
 	}
 
-	body, _, err := submit(logger, req)
+	body, _, err := submit(logger, defaultHTTPClient, req)
 	if err != nil {
 		return "", err
 	}

libs/wallet/provider/uphold/uphold_test.go

@@ -307,7 +307,7 @@ func TestFingerprintCheck(t *testing.T) {
 	var proxy func(*http.Request) (*url.URL, error)
 	wrongFingerprint := "IYSLsapSKlkofKfi6M2hmS4gzXbQKGIX/DHBWIgstw3="
 
-	client = &http.Client{
+	client := &http.Client{
 		Timeout: time.Second * 60,
 		// remove middleware calls
 		Transport: &http.Transport{

services/skus/controllers_test.go

@@ -653,14 +653,11 @@ func (suite *ControllersTestSuite) TestE2EOrdersUpholdTransactions() {
 	ctx := context.Background()
 	// setup debug for client
 	ctx = context.WithValue(ctx, appctx.DebugLoggingCTXKey, true)
-	// setup debug log level
-	ctx = context.WithValue(ctx, appctx.LogLevelCTXKey, "debug")
 
 	// setup a new logger, add to context as well
-	_, logger := logutils.SetupLogger(ctx)
+	ctx, _ = logutils.SetupLogger(ctx)
 
 	w := uphold.Wallet{
-		Logger:  logger,
 		Info:    info,
 		PrivKey: privKey,
 		PubKey:  publicKey,
@@ -827,7 +824,11 @@ func generateWallet(ctx context.Context, t *testing.T) *uphold.Wallet {
 		t.Fatal(err)
 	}
 	info.PublicKey = hex.EncodeToString(publicKey)
-	newWallet := &uphold.Wallet{Info: info, PrivKey: privateKey, PubKey: publicKey}
+	newWallet := &uphold.Wallet{
+		Info:    info,
+		PrivKey: privateKey,
+		PubKey:  publicKey,
+	}
 	err = newWallet.Register(ctx, "bat-go test card")
 	if err != nil {
 		t.Fatal(err)

tools/vault/cmd/create_wallet.go

@@ -110,7 +110,7 @@ func CreateWallet(command *cobra.Command, args []string) error {
 
 		state.WalletInfo.PublicKey = signer.String()
 
-		wallet := &uphold.Wallet{Logger: logger, Info: state.WalletInfo, PrivKey: signer, PubKey: signer}
+		wallet := &uphold.Wallet{Info: state.WalletInfo, PrivKey: signer, PubKey: signer}
 
 		reg, err := wallet.PrepareRegistration(name)
 		if err != nil {
@@ -138,7 +138,7 @@ func CreateWallet(command *cobra.Command, args []string) error {
 			return err
 		}
 
-		wallet := uphold.Wallet{Logger: logger, Info: state.WalletInfo, PrivKey: ed25519.PrivateKey{}, PubKey: publicKey}
+		wallet := uphold.Wallet{Info: state.WalletInfo, PrivKey: ed25519.PrivateKey{}, PubKey: publicKey}
 
 		err = wallet.SubmitRegistration(ctx, state.Registration)
 		if err != nil {