rfq: auth/transport harmonization pass

docs/release-notes/release-notes-0.8.0.md

@@ -309,6 +309,13 @@
   authentication. A macaroon path can be specified via the
   `experimental.rfq.priceoraclemacaroonpath` config option.
 
+- [PR#2147](https://github.com/lightninglabs/taproot-assets/pull/2147)
+  extends TLS and macaroon auth to the external portfolio pilot
+  client, and unifies the TLS configuration across both RFQ gRPC
+  clients (price oracle, portfolio pilot) under a single shared
+  `experimental.rfq.tls.*` namespace. The portfolio pilot macaroon
+  path is configured via `experimental.rfq.portfoliopilotmacaroonpath`.
+
 ## RPC Updates
 
 - [PR#2005](https://github.com/lightninglabs/taproot-assets/pull/2005)

itest/custom_channels/decode_invoice_test.go

@@ -42,7 +42,7 @@ func testCustomChannelsDecodeAssetInvoice(_ context.Context,
 			"rfqrpc://%s", oracleAddr,
 	))
 	tapdArgs = append(
-		tapdArgs, "--experimental.rfq.priceoracletlsinsecure",
+		tapdArgs, "--experimental.rfq.tls.insecure",
 	)
 
 	// We'll just make a single node here, as this doesn't actually rely on

itest/custom_channels/invoice_expiry_test.go

@@ -50,7 +50,7 @@ func testCustomChannelsInvoiceQuoteExpiryMismatch(
 	))
 	tapdArgs = append(
 		tapdArgs,
-		"--experimental.rfq.priceoracletlsinsecure",
+		"--experimental.rfq.tls.insecure",
 	)
 
 	// We use Charlie as the proof courier.

itest/custom_channels/limit_constraints_test.go

@@ -67,7 +67,7 @@ func testCustomChannelsLimitConstraints(_ context.Context,
 	))
 	tapdArgs = append(
 		tapdArgs,
-		"--experimental.rfq.priceoracletlsinsecure",
+		"--experimental.rfq.tls.insecure",
 	)
 
 	charliePort := port.NextAvailablePort()

itest/custom_channels/oracle_pricing_test.go

@@ -61,7 +61,7 @@ func testCustomChannelsOraclePricing(_ context.Context,
 		oracleAddr,
 	))
 	tapdArgs = append(
-		tapdArgs, "--experimental.rfq.priceoracletlsinsecure",
+		tapdArgs, "--experimental.rfq.tls.insecure",
 	)
 
 	// We use Charlie as the proof courier. But in order for Charlie to also

itest/tapd_harness.go

@@ -449,7 +449,7 @@ func newTapdHarness(t *testing.T, ht *harnessTest, cfg tapdConfig,
 			opts.oracleServerAddress,
 		))
 		args = append(args,
-			"--experimental.rfq.priceoracletlsinsecure",
+			"--experimental.rfq.tls.insecure",
 		)
 	default:
 		args = append(args, fmt.Sprintf(

rfq/cli.go

@@ -22,18 +22,16 @@ const (
 type CliConfig struct {
 	PriceOracleAddress string `long:"priceoracleaddress" description:"Price oracle gRPC server address (rfqrpc://<hostname>:<port>). To use the integrated mock, use the following value: use_mock_price_oracle_service_promise_to_not_use_on_mainnet"`
 
+	PriceOracleMacaroonPath string `long:"priceoraclemacaroonpath" description:"Path to the macaroon to use when connecting to the price oracle gRPC server."`
+
 	// PortfolioPilotAddress is the portfolio pilot gRPC server address.
 	PortfolioPilotAddress string `long:"portfoliopilotaddress" description:"Portfolio pilot gRPC server address (portfoliopilotrpc://<hostname>:<port>)"`
 
-	PriceOracleTLSDisable bool `long:"priceoracletlsdisable" description:"Disable TLS for price oracle communication."`
-
-	PriceOracleTLSInsecure bool `long:"priceoracletlsinsecure" description:"Disable price oracle certificate verification."`
-
-	PriceOracleTLSNoSystemCAs bool `long:"priceoracletlsnosystemcas" description:"Disable use of the operating system's list of root CA's when verifying price oracle certificates."`
+	PortfolioPilotMacaroonPath string `long:"portfoliopilotmacaroonpath" description:"Path to the macaroon to use when connecting to the portfolio pilot gRPC server."`
 
-	PriceOracleTLSCertPath string `long:"priceoracletlscertpath" description:"Path to a PEM-encoded x509 certificate to use when constructing a TLS connection with a price oracle."`
-
-	PriceOracleMacaroonPath string `long:"priceoraclemacaroonpath" description:"Path to the macaroon to use when connecting to the price oracle gRPC server."`
+	// TLS holds TLS-related options shared by all RFQ gRPC client
+	// connections.
+	TLS TLSCliConfig `group:"tls" namespace:"tls"`
 
 	SendPriceHint bool `long:"sendpricehint" description:"Send a price hint from the local price oracle to the RFQ peer when requesting a quote. For privacy reasons, this should only be turned on for self-hosted or trusted price oracles."`
 
@@ -111,14 +109,6 @@ func (c *CliConfig) Validate() error {
 		}
 	}
 
-	// A macaroon requires transport security. If a macaroon path is set
-	// but TLS is disabled, the gRPC dial will fail. Catch this early with
-	// a clear error.
-	if c.PriceOracleMacaroonPath != "" && c.PriceOracleTLSDisable {
-		return fmt.Errorf("priceoraclemacaroonpath requires " +
-			"price oracle TLS to be enabled")
-	}
-
 	// Ensure that if the portfolio pilot address is set, it is valid.
 	if c.PortfolioPilotAddress != "" {
 		_, err := ParsePortfolioPilotAddress(c.PortfolioPilotAddress)
@@ -128,5 +118,36 @@ func (c *CliConfig) Validate() error {
 		}
 	}
 
+	// A macaroon path is only meaningful if the corresponding service is
+	// actually configured. The mock price oracle doesn't accept macaroons
+	// either, so treat that the same as no address.
+	if c.PriceOracleMacaroonPath != "" &&
+		(c.PriceOracleAddress == "" ||
+			c.PriceOracleAddress == MockPriceOracleServiceAddress) {
+
+		return fmt.Errorf("priceoraclemacaroonpath requires a real " +
+			"price oracle address")
+	}
+
+	if c.PortfolioPilotMacaroonPath != "" &&
+		c.PortfolioPilotAddress == "" {
+
+		return fmt.Errorf("portfoliopilotmacaroonpath requires a " +
+			"portfolio pilot address")
+	}
+
+	// A macaroon is a bearer token, so it requires full transport
+	// security: TLS must be enabled and the server certificate must be
+	// verified. Otherwise the macaroon can be intercepted by an attacker
+	// who MITMs the connection (with TLS disabled) or who presents any
+	// cert (with verification disabled).
+	macaroonSet := c.PriceOracleMacaroonPath != "" ||
+		c.PortfolioPilotMacaroonPath != ""
+	if macaroonSet && (c.TLS.Disable || c.TLS.Insecure) {
+		return fmt.Errorf("macaroon paths require RFQ TLS to be " +
+			"enabled and the server certificate to be verified " +
+			"(tls.disable=false, tls.insecure=false)")
+	}
+
 	return nil
 }

rfq/oracle.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"math"
+	"net"
 	"net/url"
 	"time"
 
@@ -251,9 +252,9 @@ func NewRpcPriceOracle(addrStr string, tlsConfig *TLSConfig,
 	})
 
 	// Formulate the server address dial string.
-	serverAddr := fmt.Sprintf("%s:%s", addr.Hostname(), addr.Port())
+	serverAddr := net.JoinHostPort(addr.Hostname(), addr.Port())
 
-	conn, err := grpc.Dial(serverAddr, dialOpts...)
+	conn, err := grpc.NewClient(serverAddr, dialOpts...)
 	if err != nil {
 		return nil, err
 	}

rfq/portfolio_pilot_rpc.go

@@ -2,8 +2,8 @@ package rfq
 
 import (
 	"context"
-	"crypto/tls"
 	"fmt"
+	"net"
 	"net/url"
 	"time"
 
@@ -16,8 +16,6 @@ import (
 	"github.com/lightningnetwork/lnd/lnwire"
 	"github.com/lightningnetwork/lnd/routing/route"
 	"google.golang.org/grpc"
-	"google.golang.org/grpc/credentials"
-	"google.golang.org/grpc/credentials/insecure"
 	"google.golang.org/grpc/keepalive"
 )
 
@@ -67,42 +65,42 @@ type RpcPortfolioPilot struct {
 	rawConn *grpc.ClientConn
 }
 
-// portfolioPilotDialOpts returns gRPC dial options for the portfolio pilot.
-func portfolioPilotDialOpts(insecureDial bool) []grpc.DialOption {
-	var creds credentials.TransportCredentials
-	if insecureDial {
-		creds = insecure.NewCredentials()
-	} else {
-		creds = credentials.NewTLS(&tls.Config{
-			InsecureSkipVerify: true,
-		})
+// NewRpcPortfolioPilot creates a new RPC portfolio pilot handle given the
+// address of the portfolio pilot RPC server. An optional macaroon dial option
+// can be provided for authentication with the pilot server.
+func NewRpcPortfolioPilot(addrStr string, tlsConfig *TLSConfig,
+	macaroonOpt fn.Option[grpc.DialOption]) (*RpcPortfolioPilot, error) {
+
+	addr, err := ParsePortfolioPilotAddress(addrStr)
+	if err != nil {
+		return nil, fmt.Errorf("parse portfolio pilot address: %w",
+			err)
 	}
 
-	return []grpc.DialOption{
-		grpc.WithTransportCredentials(creds),
+	// Create transport credentials and dial options from the supplied TLS
+	// config.
+	transportCredentials, err := configureTransportCredentials(tlsConfig)
+	if err != nil {
+		return nil, err
+	}
+
+	dialOpts := []grpc.DialOption{
+		grpc.WithTransportCredentials(transportCredentials),
 		grpc.WithKeepaliveParams(keepalive.ClientParameters{
 			Time:                30 * time.Second,
 			Timeout:             20 * time.Second,
 			PermitWithoutStream: true,
 		}),
 	}
-}
 
-// NewRpcPortfolioPilot creates a new RPC portfolio pilot handle given the
-// address of the portfolio pilot RPC server.
-func NewRpcPortfolioPilot(addrStr string, dialInsecure bool) (
-	*RpcPortfolioPilot, error) {
-
-	addr, err := ParsePortfolioPilotAddress(addrStr)
-	if err != nil {
-		return nil, fmt.Errorf("parse portfolio pilot address: %w",
-			err)
-	}
-
-	dialOpts := portfolioPilotDialOpts(dialInsecure)
+	// If a macaroon dial option is provided, append it to the dial
+	// options so that the macaroon is sent with every RPC call.
+	macaroonOpt.WhenSome(func(opt grpc.DialOption) {
+		dialOpts = append(dialOpts, opt)
+	})
 
 	// Formulate the server address dial string.
-	serverAddr := fmt.Sprintf("%s:%s", addr.Hostname(), addr.Port())
+	serverAddr := net.JoinHostPort(addr.Hostname(), addr.Port())
 
 	conn, err := grpc.NewClient(serverAddr, dialOpts...)
 	if err != nil {

rfq/tls.go

@@ -9,6 +9,20 @@ import (
 	"google.golang.org/grpc/credentials/insecure"
 )
 
+// TLSCliConfig holds TLS-related CLI configuration options shared by all RFQ
+// gRPC client connections (price oracle, portfolio pilot).
+//
+// nolint: lll
+type TLSCliConfig struct {
+	Disable bool `long:"disable" description:"Disable TLS for RFQ gRPC client communication (price oracle, portfolio pilot)."`
+
+	Insecure bool `long:"insecure" description:"Disable certificate verification for RFQ gRPC client connections. Should only be used for testing."`
+
+	NoSystemCAs bool `long:"nosystemcas" description:"Disable use of the operating system's list of root CA's when verifying RFQ gRPC server certificates."`
+
+	CertPath string `long:"certpath" description:"Path to a PEM-encoded x509 certificate to use when constructing TLS connections for RFQ gRPC clients."`
+}
+
 // TLSConfig represents TLS configuration options for oracle connections.
 type TLSConfig struct {
 	// Disabled indicates that we should not use TLS.

sample-tapd.conf

@@ -464,25 +464,31 @@
 ; use_mock_price_oracle_service_promise_to_not_use_on_mainnet
 ; experimental.rfq.priceoracleaddress=
 
+; Path to the macaroon to use when connecting to the price oracle gRPC server.
+; experimental.rfq.priceoraclemacaroonpath=
+
 ; Portfolio pilot gRPC server address (portfoliopilotrpc://<hostname>:<port>)
 ; experimental.rfq.portfoliopilotaddress=
 
-; Disable TLS for price oracle communication.
-; experimental.rfq.priceoracletlsdisable=false
+; Path to the macaroon to use when connecting to the portfolio pilot gRPC
+; server.
+; experimental.rfq.portfoliopilotmacaroonpath=
 
-; Skip price oracle certificate verification. Should only be used for testing.
-; experimental.rfq.priceoracletlsinsecure=false
+; Disable TLS for RFQ gRPC client communication (price oracle, portfolio
+; pilot).
+; experimental.rfq.tls.disable=false
 
-; Disable use of the operating system's root CA list when verifying a
-; price oracle's certificate.
-; experimental.rfq.priceoracletlsnosystemcas=false
+; Skip RFQ gRPC client certificate verification. Should only be used for
+; testing.
+; experimental.rfq.tls.insecure=false
 
-; Path to a custom certificate (root CA or self-signed) to be used to
-; secure communication with a price oracle.
-; experimental.rfq.priceoracletlscertpath=
+; Disable use of the operating system's root CA list when verifying RFQ gRPC
+; server certificates.
+; experimental.rfq.tls.nosystemcas=false
 
-; Path to the macaroon to use when connecting to the price oracle gRPC server.
-; experimental.rfq.priceoraclemacaroonpath=
+; Path to a custom certificate (root CA or self-signed) to be used to secure
+; RFQ gRPC client connections.
+; experimental.rfq.tls.certpath=
 
 ; Disable sending the local node's public key to the price oracle when
 ; requesting a price rate. By default, the node's identity is sent to allow