diff --git a/docs/release-notes/release-notes-0.8.0.md b/docs/release-notes/release-notes-0.8.0.md index f34ee0bc20..c4ecc95adf 100644 --- a/docs/release-notes/release-notes-0.8.0.md +++ b/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) diff --git a/itest/custom_channels/decode_invoice_test.go b/itest/custom_channels/decode_invoice_test.go index d67d4a01ca..5ebc191891 100644 --- a/itest/custom_channels/decode_invoice_test.go +++ b/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 diff --git a/itest/custom_channels/invoice_expiry_test.go b/itest/custom_channels/invoice_expiry_test.go index c738ff2448..76d73eaea4 100644 --- a/itest/custom_channels/invoice_expiry_test.go +++ b/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. diff --git a/itest/custom_channels/limit_constraints_test.go b/itest/custom_channels/limit_constraints_test.go index 38df67f763..99ebd41aee 100644 --- a/itest/custom_channels/limit_constraints_test.go +++ b/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() diff --git a/itest/custom_channels/oracle_pricing_test.go b/itest/custom_channels/oracle_pricing_test.go index 3446c74284..4992ab4815 100644 --- a/itest/custom_channels/oracle_pricing_test.go +++ b/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 diff --git a/itest/tapd_harness.go b/itest/tapd_harness.go index c6108037a3..82dd810742 100644 --- a/itest/tapd_harness.go +++ b/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( diff --git a/rfq/cli.go b/rfq/cli.go index 80aeeb722e..8068033095 100644 --- a/rfq/cli.go +++ b/rfq/cli.go @@ -22,18 +22,16 @@ const ( type CliConfig struct { PriceOracleAddress string `long:"priceoracleaddress" description:"Price oracle gRPC server address (rfqrpc://:). 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://:)"` - 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 } diff --git a/rfq/oracle.go b/rfq/oracle.go index fac74f337e..4b8ca1e7fd 100644 --- a/rfq/oracle.go +++ b/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 } diff --git a/rfq/portfolio_pilot_rpc.go b/rfq/portfolio_pilot_rpc.go index ca5da9b7b8..c77e7ed7cf 100644 --- a/rfq/portfolio_pilot_rpc.go +++ b/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 { diff --git a/rfq/tls.go b/rfq/tls.go index 76227c59fc..45281d4df3 100644 --- a/rfq/tls.go +++ b/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. diff --git a/sample-tapd.conf b/sample-tapd.conf index 70551b6299..bb3abef038 100644 --- a/sample-tapd.conf +++ b/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://:) ; 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 diff --git a/tapcfg/config.go b/tapcfg/config.go index fc19a37656..47a688ea73 100644 --- a/tapcfg/config.go +++ b/tapcfg/config.go @@ -155,24 +155,6 @@ const ( // output amount ratio to use when funding PSBTs. DefaultPsbtMaxFeeRatio = lndservices.DefaultPsbtMaxFeeRatio - // defaultPriceOracleTLSDisable disables TLS for price oracle - // communication. - defaultPriceOracleTLSDisable = false - - // defaultPriceOracleTLSInsecure is the default value we'll use for - // deciding to verify certificates in TLS connections with price - // oracles. - defaultPriceOracleTLSInsecure = false - - // defaultPriceOracleNoSystemCAs is the default value we'll use - // regarding trust of the operating system's root CA list. We'll use - // 'false', i.e. we will trust the OS root CA list by default. - defaultPriceOracleTLSNoSystemCAs = false - - // defaultPriceOracleTLSCertPath is the default (empty) path to a - // certificate to use for securing price oracle communication. - defaultPriceOracleTLSCertPath = "" - // Set defaults for a health check which ensures that the TLS certificate // is not expired. Although this check is off by default (not all setups // require it), we still set the other default values so that the health @@ -380,10 +362,11 @@ type ExperimentalConfig struct { // CleanAndValidate performs final processing on the ExperimentalConfig, // returning an error if the configuration is invalid. func (c *ExperimentalConfig) CleanAndValidate() error { - c.Rfq.PriceOracleTLSCertPath = CleanAndExpandPath( - c.Rfq.PriceOracleTLSCertPath) + c.Rfq.TLS.CertPath = CleanAndExpandPath(c.Rfq.TLS.CertPath) c.Rfq.PriceOracleMacaroonPath = CleanAndExpandPath( c.Rfq.PriceOracleMacaroonPath) + c.Rfq.PortfolioPilotMacaroonPath = CleanAndExpandPath( + c.Rfq.PortfolioPilotMacaroonPath) return c.Rfq.Validate() } @@ -553,11 +536,7 @@ func DefaultConfig() Config { }, Experimental: &ExperimentalConfig{ Rfq: rfq.CliConfig{ - AcceptPriceDeviationPpm: rfq.DefaultAcceptPriceDeviationPpm, - PriceOracleTLSDisable: defaultPriceOracleTLSDisable, - PriceOracleTLSInsecure: defaultPriceOracleTLSInsecure, - PriceOracleTLSNoSystemCAs: defaultPriceOracleTLSNoSystemCAs, - PriceOracleTLSCertPath: defaultPriceOracleTLSCertPath, + AcceptPriceDeviationPpm: rfq.DefaultAcceptPriceDeviationPpm, }, }, HealthChecks: &HealthCheckConfig{ @@ -1272,34 +1251,30 @@ func getCertificateConfig(cfg *Config, cfgLogger btclog.Logger) (*tls.Config, return tlsCfg, restCreds, nil } -// getPriceOracleTLSConfig returns a TLS configuration for a price oracle, -// given a valid RFQ CLI configuration. -func getPriceOracleTLSConfig(rfqCfg rfq.CliConfig) (*rfq.TLSConfig, error) { +// getRfqTLSConfig returns a TLS configuration shared by all RFQ gRPC client +// connections (price oracle, portfolio pilot), given a valid RFQ CLI +// configuration. +func getRfqTLSConfig(rfqCfg rfq.CliConfig) (*rfq.TLSConfig, error) { var certBytes []byte // Read any specified certificate data. - if rfqCfg.PriceOracleTLSCertPath != "" { + if rfqCfg.TLS.CertPath != "" { var err error - certBytes, err = os.ReadFile( - rfqCfg.PriceOracleTLSCertPath, - ) + certBytes, err = os.ReadFile(rfqCfg.TLS.CertPath) if err != nil { - return nil, fmt.Errorf("unable to read "+ - "price oracle certificate: %w", err) + return nil, fmt.Errorf("unable to read RFQ TLS "+ + "certificate: %w", err) } } - // Construct the oracle's TLS configuration. - tlsConfig := &rfq.TLSConfig{ - Disabled: rfqCfg.PriceOracleTLSDisable, - InsecureSkipVerify: rfqCfg.PriceOracleTLSInsecure, + return &rfq.TLSConfig{ + Disabled: rfqCfg.TLS.Disable, + InsecureSkipVerify: rfqCfg.TLS.Insecure, // Note the subtle flip on the flag, since the user has // configured whether to *not* trust the system CA's. - TrustSystemRootCAs: !rfqCfg.PriceOracleTLSNoSystemCAs, + TrustSystemRootCAs: !rfqCfg.TLS.NoSystemCAs, CustomCertificates: certBytes, - } - - return tlsConfig, nil + }, nil } // getPriceOracleMacaroonOpt reads the price oracle macaroon file @@ -1322,6 +1297,26 @@ func getPriceOracleMacaroonOpt( return fn.Some(opt), nil } +// getPortfolioPilotMacaroonOpt reads the portfolio pilot macaroon file +// from disk (if configured) and returns it as an optional gRPC dial +// option. +func getPortfolioPilotMacaroonOpt( + rfqCfg rfq.CliConfig) (fn.Option[grpc.DialOption], error) { + + if rfqCfg.PortfolioPilotMacaroonPath == "" { + return fn.None[grpc.DialOption](), nil + } + + opt, err := rfq.NewMacaroonDialOption( + rfqCfg.PortfolioPilotMacaroonPath, + ) + if err != nil { + return fn.None[grpc.DialOption](), err + } + + return fn.Some(opt), nil +} + // fileExists reports whether the named file or directory exists. // This function is taken from https://github.com/btcsuite/btcd func fileExists(name string) bool { diff --git a/tapcfg/server.go b/tapcfg/server.go index 444a7176cb..2b0aed1932 100644 --- a/tapcfg/server.go +++ b/tapcfg/server.go @@ -500,6 +500,15 @@ func genServerConfig(cfg *Config, cfgLogger btclog.Logger, var portfolioPilot rfq.PortfolioPilot rfqCfg := cfg.Experimental.Rfq + + // Build the TLS configuration shared by all RFQ gRPC client + // connections (price oracle, portfolio pilot). + rfqTLSConfig, err := getRfqTLSConfig(rfqCfg) + if err != nil { + return nil, fmt.Errorf("couldn't construct RFQ TLS "+ + "configuration: %w", err) + } + switch rfqCfg.PriceOracleAddress { case rfq.MockPriceOracleServiceAddress: switch { @@ -520,12 +529,6 @@ func genServerConfig(cfg *Config, cfgLogger btclog.Logger, // skip setting suggested prices for outgoing quote requests. default: - tlsConfig, err := getPriceOracleTLSConfig(rfqCfg) - if err != nil { - return nil, fmt.Errorf("couldn't construct price "+ - "oracle configuration: %w", err) - } - macaroonOpt, err := getPriceOracleMacaroonOpt(rfqCfg) if err != nil { return nil, fmt.Errorf("unable to load price "+ @@ -538,7 +541,7 @@ func genServerConfig(cfg *Config, cfgLogger btclog.Logger, } priceOracle, err = rfq.NewRpcPriceOracle( - rfqCfg.PriceOracleAddress, tlsConfig, + rfqCfg.PriceOracleAddress, rfqTLSConfig, macaroonOpt, nodeID, ) if err != nil { @@ -554,8 +557,15 @@ func genServerConfig(cfg *Config, cfgLogger btclog.Logger, // used. default: + macaroonOpt, err := getPortfolioPilotMacaroonOpt(rfqCfg) + if err != nil { + return nil, fmt.Errorf("unable to load portfolio "+ + "pilot macaroon: %w", err) + } + portfolioPilot, err = rfq.NewRpcPortfolioPilot( - rfqCfg.PortfolioPilotAddress, false, + rfqCfg.PortfolioPilotAddress, rfqTLSConfig, + macaroonOpt, ) if err != nil { return nil, fmt.Errorf("unable to create "+