From 13bc1049a5f724ce416bf6be136e90288c0385e1 Mon Sep 17 00:00:00 2001 From: Jared Tobin Date: Thu, 28 Aug 2025 06:37:00 -0230 Subject: [PATCH 1/7] rfq: introduce, use tls module Introduces rfq/tls.go, which contains a basic TLSConfig type and default value of such. The default value, which for now only indicates that certificate verification should be skipped, is used in place of the 'dialInsecure' bool when setting up the price oracle RPC. --- rfq/oracle.go | 5 ++++- rfq/oracle_test.go | 4 ++-- rfq/tls.go | 14 ++++++++++++++ tapcfg/server.go | 2 +- 4 files changed, 21 insertions(+), 4 deletions(-) create mode 100644 rfq/tls.go diff --git a/rfq/oracle.go b/rfq/oracle.go index 96b3c565fa..b190ddc19d 100644 --- a/rfq/oracle.go +++ b/rfq/oracle.go @@ -239,7 +239,7 @@ func insecureServerDialOpts() ([]grpc.DialOption, error) { // NewRpcPriceOracle creates a new RPC price oracle handle given the address // of the price oracle RPC server. -func NewRpcPriceOracle(addrStr string, dialInsecure bool) (*RpcPriceOracle, +func NewRpcPriceOracle(addrStr string, tlsConfig *TLSConfig) (*RpcPriceOracle, error) { addr, err := ParsePriceOracleAddress(addrStr) @@ -253,6 +253,9 @@ func NewRpcPriceOracle(addrStr string, dialInsecure bool) (*RpcPriceOracle, return nil, err } + // Determine whether we should skip certificate verification. + dialInsecure := tlsConfig.InsecureSkipVerify + // Allow connecting to a non-TLS (h2c, http over cleartext) gRPC server, // should be used for testing only. if dialInsecure { diff --git a/rfq/oracle_test.go b/rfq/oracle_test.go index c120ffe72e..46963bcc95 100644 --- a/rfq/oracle_test.go +++ b/rfq/oracle_test.go @@ -141,7 +141,7 @@ func runQuerySalePriceTest(t *testing.T, tc *testCaseQuerySalePrice) { // Create a new RPC price oracle client and connect to the mock service. serviceAddr := fmt.Sprintf("rfqrpc://%s", testServiceAddress) - client, err := NewRpcPriceOracle(serviceAddr, true) + client, err := NewRpcPriceOracle(serviceAddr, DefaultTLSConfig()) require.NoError(t, err) // Query for an ask price. @@ -251,7 +251,7 @@ func runQueryPurchasePriceTest(t *testing.T, tc *testCaseQueryPurchasePrice) { // Create a new RPC price oracle client and connect to the mock service. serviceAddr := fmt.Sprintf("rfqrpc://%s", testServiceAddress) - client, err := NewRpcPriceOracle(serviceAddr, true) + client, err := NewRpcPriceOracle(serviceAddr, DefaultTLSConfig()) require.NoError(t, err) // Query for an ask price. diff --git a/rfq/tls.go b/rfq/tls.go new file mode 100644 index 0000000000..85494942ac --- /dev/null +++ b/rfq/tls.go @@ -0,0 +1,14 @@ +package rfq + +// TLSConfig represents TLS configuration options for oracle connections. +type TLSConfig struct { + // InsecureSkipVerify disables certificate verification. + InsecureSkipVerify bool +} + +// DefaultTLSConfig returns a default TLS configuration. +func DefaultTLSConfig() *TLSConfig { + return &TLSConfig{ + InsecureSkipVerify: true, + } +} diff --git a/tapcfg/server.go b/tapcfg/server.go index b90f561dbd..5a486fdf28 100644 --- a/tapcfg/server.go +++ b/tapcfg/server.go @@ -483,7 +483,7 @@ func genServerConfig(cfg *Config, cfgLogger btclog.Logger, default: priceOracle, err = rfq.NewRpcPriceOracle( - rfqCfg.PriceOracleAddress, false, + rfqCfg.PriceOracleAddress, rfq.DefaultTLSConfig(), ) if err != nil { return nil, fmt.Errorf("unable to create price "+ From 2557d9d9a0b2746decbeaa71d18fa4b26752b894 Mon Sep 17 00:00:00 2001 From: Jared Tobin Date: Thu, 28 Aug 2025 08:16:43 -0230 Subject: [PATCH 2/7] rfq: allow os, custom certificates Adds both 'TrustSystemRootCAs' and 'CustomCertificates' to the rfq TLSConfig. The former indicates whether or not to trust the operating system's root CA list; the latter allows additional certificates (CA or self-signed) to be trusted. Also adds a basic unit test skeleton. --- rfq/oracle.go | 72 ++++------------------------------------- rfq/tls.go | 61 +++++++++++++++++++++++++++++++++++ rfq/tls_test.go | 85 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 152 insertions(+), 66 deletions(-) create mode 100644 rfq/tls_test.go diff --git a/rfq/oracle.go b/rfq/oracle.go index b190ddc19d..7c68d8502b 100644 --- a/rfq/oracle.go +++ b/rfq/oracle.go @@ -2,7 +2,6 @@ package rfq import ( "context" - "crypto/tls" "fmt" "math" "net/url" @@ -16,9 +15,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" ) // PriceQueryIntent is an enum that represents the intent of a price rate @@ -187,56 +183,6 @@ type RpcPriceOracle struct { rawConn *grpc.ClientConn } -// clientKeepaliveDialOption configures bidirectional health probing to prevent -// idle RFQ connections from being silently terminated by network intermediaries -// (NATs, load balancers) or aggressive server timeouts. Without active -// keepalive, the first price query after an idle period would fail with -// "connection reset by peer" and require a retry. -var clientKeepaliveDialOption = grpc.WithKeepaliveParams( - keepalive.ClientParameters{ - // Ping server after 30 seconds of inactivity. - Time: 30 * time.Second, - - // Wait 20 seconds for ping response. - Timeout: 20 * time.Second, - - // Permit keepalive pings even when there are no active - // streams. This is critical for long-lived connections with - // infrequent RFQ requests. - PermitWithoutStream: true, - }, -) - -// serverDialOpts returns the set of server options needed to connect to the -// price oracle RPC server using a TLS connection. -func serverDialOpts() ([]grpc.DialOption, error) { - var opts []grpc.DialOption - - tlsConfig := tls.Config{InsecureSkipVerify: true} - transportCredentials := credentials.NewTLS(&tlsConfig) - - opts = append(opts, grpc.WithTransportCredentials(transportCredentials)) - - opts = append(opts, clientKeepaliveDialOption) - - return opts, nil -} - -// insecureServerDialOpts returns the set of server options needed to connect to -// the price oracle RPC server using a TLS connection. -func insecureServerDialOpts() ([]grpc.DialOption, error) { - var opts []grpc.DialOption - - // Skip TLS certificate verification. - opts = append(opts, grpc.WithTransportCredentials( - insecure.NewCredentials(), - )) - - opts = append(opts, clientKeepaliveDialOption) - - return opts, nil -} - // NewRpcPriceOracle creates a new RPC price oracle handle given the address // of the price oracle RPC server. func NewRpcPriceOracle(addrStr string, tlsConfig *TLSConfig) (*RpcPriceOracle, @@ -247,27 +193,21 @@ func NewRpcPriceOracle(addrStr string, tlsConfig *TLSConfig) (*RpcPriceOracle, return nil, err } - // Connect to the RPC server. - dialOpts, err := serverDialOpts() + // Create transport credentials and dial options from the supplied TLS + // config. + transportCredentials, err := configureTransportCredentials(tlsConfig) if err != nil { return nil, err } - // Determine whether we should skip certificate verification. - dialInsecure := tlsConfig.InsecureSkipVerify - - // Allow connecting to a non-TLS (h2c, http over cleartext) gRPC server, - // should be used for testing only. - if dialInsecure { - dialOpts, err = insecureServerDialOpts() - if err != nil { - return nil, err - } + dialOpts := []grpc.DialOption{ + grpc.WithTransportCredentials(transportCredentials), } // Formulate the server address dial string. serverAddr := fmt.Sprintf("%s:%s", addr.Hostname(), addr.Port()) + // Connect to the RPC server. conn, err := grpc.Dial(serverAddr, dialOpts...) if err != nil { return nil, err diff --git a/rfq/tls.go b/rfq/tls.go index 85494942ac..679d3448d7 100644 --- a/rfq/tls.go +++ b/rfq/tls.go @@ -1,9 +1,28 @@ package rfq +import ( + "crypto/tls" + "crypto/x509" + + "google.golang.org/grpc/credentials" + "google.golang.org/grpc/credentials/insecure" +) + // TLSConfig represents TLS configuration options for oracle connections. type TLSConfig struct { + // Enabled indicates that we should use TLS. + Enabled bool + // InsecureSkipVerify disables certificate verification. InsecureSkipVerify bool + + // TrustSystemRootCAs indicates whether or not to use the operating + // system's root certificate authority list. + TrustSystemRootCAs bool + + // CustomCertificates contains PEM data for additional root CA and + // self-signed certificates to trust. + CustomCertificates []byte } // DefaultTLSConfig returns a default TLS configuration. @@ -12,3 +31,45 @@ func DefaultTLSConfig() *TLSConfig { InsecureSkipVerify: true, } } + +// configureTransportCredentials configures the TLS transport credentials to +// be used for RPC connections. +func configureTransportCredentials( + config *TLSConfig) (credentials.TransportCredentials, error) { + + // If TLS is disabled, return insecure credentials. + if !config.Enabled { + return insecure.NewCredentials(), nil + } + + // If we're to skip certificate verification, then return TLS + // credentials with certificate verification disabled. + if config.InsecureSkipVerify { + creds := credentials.NewTLS(&tls.Config{ + InsecureSkipVerify: true, + }) + return creds, nil + } + + // Initialize the certificate pool. + certPool, err := constructCertPool(config.TrustSystemRootCAs) + if err != nil { + return nil, err + } + + // If we have any custom certificates, add them to the certificate + // pool. + certPool.AppendCertsFromPEM(config.CustomCertificates) + + // Return the constructed transport credentials. + return credentials.NewClientTLSFromCert(certPool, ""), nil +} + +// constructCertPool is a helper for constructing an initial certificate pool, +// depending on whether or not we should trust the system root CA list. +func constructCertPool(trustSystem bool) (*x509.CertPool, error) { + if trustSystem { + return x509.SystemCertPool() + } + return x509.NewCertPool(), nil +} diff --git a/rfq/tls_test.go b/rfq/tls_test.go new file mode 100644 index 0000000000..b7364108fe --- /dev/null +++ b/rfq/tls_test.go @@ -0,0 +1,85 @@ +package rfq + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +// Test certificate data - a valid self-signed certificate for testing +const validTestCertPEM = `-----BEGIN CERTIFICATE----- +MIICmjCCAYICCQCuu1gzY+BBKjANBgkqhkiG9w0BAQsFADAPMQ0wCwYDVQQDDAR0 +ZXN0MB4XDTI1MDgyODEwNDA1NVoXDTI1MDgyOTEwNDA1NVowDzENMAsGA1UEAwwE +dGVzdDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBALTWCm8l3d9nE2QK +TK8HJ36ftO8pK3//nb8Nj/p97FrPFSgzdgL1ZNJs4gP5/ZsU+iE6VeKhalHoSf6/ +IMLe3ATTL0rWA1M6z7cw6ll8VS8NQMaMSFWNomncsxyoJAQde++SC5f1RwQJBD/0 +gGB4bJIIqUHtT12m23GLX48d6JGEEi5kEQtk91S/QGnHtglzZ8CQOogDBzDhSHu2 +jj4mKYDgkXcyAqN7DoDzoEcrpeAaeAwem8k1sFBeTtrqT1ot7Ey5KG+RUyJbdKGt +5adJiwH782NgsSnISQ2X7Sct6Uu0JzHKx9JzyABsA05tf3cNJkLhh1Is9edYI2e9 +m0dqedECAwEAATANBgkqhkiG9w0BAQsFAAOCAQEAQOCs/7xZVPjabbhdv30mUJMG +lddi2A+R/5IRXW1MKnpemwiv4ZWYQ9PMTmuR7kqaF7AGLkvx+5sp2evUJN4x7vHP +ao6wihbdh+vBkrobE+Y9dE7nbkvMQSNi1sXzDnfZB9LqY9Huun2soUwBQNCMPVMa +Wo7g6udwyA48doEVJMjThFLPcW7xmsy6Ldew682m1kD8/ag+9qihX1IJyiqiEjha +3uT4CT+zEg0RJorEJKbR38fE4Uhx1wZO4zvjEg6qZeW/I4lw+UzSY5xV7lJ1EQvf +BcoNuBHB65RxQM5fpA7hkEFm1bxBoowGX2hx6VCCeBBwREISRfgvkUxZahUXNg== +-----END CERTIFICATE-----` + +// Invalid PEM data for testing failure cases +const invalidTestCertPEM = `-----BEGIN CERTIFICATE----- +This is not a valid certificate +-----END CERTIFICATE-----` + +// DefaultTLSConfig returns a default TLS configuration for testing. +func DefaultTLSConfig() *TLSConfig { + return &TLSConfig{ + InsecureSkipVerify: true, + } +} + +// TestConfigureTransportCredentials_InsecureSkipVerify tests the function +// when InsecureSkipVerify is true. +func TestConfigureTransportCredentials_InsecureSkipVerify(t *testing.T) { + config := &TLSConfig{ + InsecureSkipVerify: true, + } + + creds, err := configureTransportCredentials(config) + + require.NoError(t, err) + require.NotNil(t, creds) + + // Verify that we got insecure credentials by checking the type + require.Equal(t, "insecure", creds.Info().SecurityProtocol) +} + +// TestConfigureTransportCredentials_ValidCustomCertificates tests the +// function when valid custom certificates are provided. +func TestConfigureTransportCredentials_ValidCustomCertificates(t *testing.T) { + config := &TLSConfig{ + InsecureSkipVerify: false, + CustomCertificates: []byte(validTestCertPEM), + } + + creds, err := configureTransportCredentials(config) + + require.NoError(t, err) + require.NotNil(t, creds) + + // Verify that we got TLS credentials (not insecure) + require.Equal(t, "tls", creds.Info().SecurityProtocol) +} + +// TestConfigureTransportCredentials_NoCredentialsConfigured tests the +// function when no credentials are configured. +func TestConfigureTransportCredentials_NoCredentialsConfigured(t *testing.T) { + config := &TLSConfig{ + InsecureSkipVerify: false, + CustomCertificates: nil, + } + + creds, err := configureTransportCredentials(config) + + require.NoError(t, err) + require.NotNil(t, creds) + require.Equal(t, "tls", creds.Info().SecurityProtocol) +} From f554957e950a9afc95d7a8fca3a7603f35fcc0f6 Mon Sep 17 00:00:00 2001 From: Jared Tobin Date: Fri, 29 Aug 2025 08:52:50 -0230 Subject: [PATCH 3/7] tapcfg: add price oracle TLS config We don't skip certificate verification by default, and also default to trusting the operating system's root CA list. --- rfq/cli.go | 8 ++++++ rfq/tls.go | 7 ------ sample-tapd.conf | 21 +++++++++++++--- tapcfg/config.go | 64 ++++++++++++++++++++++++++++++++++++++++++++---- tapcfg/server.go | 8 +++++- 5 files changed, 92 insertions(+), 16 deletions(-) diff --git a/rfq/cli.go b/rfq/cli.go index 8559504c2e..3f41fabd88 100644 --- a/rfq/cli.go +++ b/rfq/cli.go @@ -22,6 +22,14 @@ 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"` + PriceOracleTLS bool `long:"priceoracletls" description:"Enable TLS for communication with a price oracle."` + + PriceOracleTLSInsecure bool `long:"priceoracletlsinsecure" description:"Disable verification of price oracle certificates."` + + PriceOracleTLSNoSystemCAs bool `long:"priceoracletlsnosystemcas" description:"Disable use of the operating system's list of root CA's when verifiying price oracle certificates."` + + PriceOracleTLSCertPath string `long:"priceoracletlscertpath" description:"Path to a PEM-encoded x509 certificate to use when constructing a TLS connection with a price oracle."` + 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."` PriceOracleSendPeerId bool `long:"priceoraclesendpeerid" description:"Send the peer ID (public key of the peer) to the price oracle when requesting a price rate. For privacy reasons, this should only be turned on for self-hosted or trusted price oracles."` diff --git a/rfq/tls.go b/rfq/tls.go index 679d3448d7..a9735f9c6b 100644 --- a/rfq/tls.go +++ b/rfq/tls.go @@ -25,13 +25,6 @@ type TLSConfig struct { CustomCertificates []byte } -// DefaultTLSConfig returns a default TLS configuration. -func DefaultTLSConfig() *TLSConfig { - return &TLSConfig{ - InsecureSkipVerify: true, - } -} - // configureTransportCredentials configures the TLS transport credentials to // be used for RPC connections. func configureTransportCredentials( diff --git a/sample-tapd.conf b/sample-tapd.conf index 8621843961..82a064f614 100644 --- a/sample-tapd.conf +++ b/sample-tapd.conf @@ -11,8 +11,8 @@ ; always set to false. ; If only one value is specified for an option, then this is also the -; default value used by tapd. In case of multiple (example) values, the default -; is explicitly mentioned. +; default value used by tapd. In case of multiple (example) values, the default +; is explicitly mentioned. ; If the part after the equal sign is empty then tapd has no default for this ; option. @@ -446,6 +446,21 @@ ; use_mock_price_oracle_service_promise_to_not_use_on_mainnet ; experimental.rfq.priceoracleaddress= +; Enable TLS for price oracle communication. +; experimental.rfq.priceoracletls=true + +; Skip price oracle certificate verification, yielding an insecure (cleartext) +; channel with the price oracle. Should only be used for testing. +; experimental.rfq.priceoracletlsinsecure=false + +; Disable use of the operating system's root CA list when verifying a +; price oracle's certificate. +; experimental.rfq.priceoracletlsnosystemcas=false + +; Path to a custom certificate (root CA or self-signed) to be used to +; secure communication with a price oracle. +; experimental.rfq.priceoracletlscertpath= + ; 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. @@ -456,7 +471,7 @@ ; self-hosted or trusted price oracles. ; experimental.rfq.priceoraclesendpeerid=false -; The default price deviation inparts per million that is accepted by +; The default price deviation inparts per million that is accepted by ; the RFQ negotiator. ; Example: 50,000 ppm => price deviation is set to 5% . ; experimental.rfq.acceptpricedeviationppm=50000 diff --git a/tapcfg/config.go b/tapcfg/config.go index 76f5adb00b..a068e6ba1d 100644 --- a/tapcfg/config.go +++ b/tapcfg/config.go @@ -148,6 +148,24 @@ const ( // DefaultPsbtMaxFeeRatio is the default maximum for fees to total // output amount ratio to use when funding PSBTs. DefaultPsbtMaxFeeRatio = lndservices.DefaultPsbtMaxFeeRatio + + // defaultPriceOracleTLS is the default TLS setting to use when + // communicating with price oracles. + defaultPriceOracleTLS = true + + // 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 = "" ) var ( @@ -335,8 +353,11 @@ type ExperimentalConfig struct { Rfq rfq.CliConfig `group:"rfq" namespace:"rfq"` } -// Validate returns an error if the configuration is invalid. -func (c *ExperimentalConfig) Validate() error { +// 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) return c.Rfq.Validate() } @@ -502,7 +523,11 @@ func DefaultConfig() Config { }, Experimental: &ExperimentalConfig{ Rfq: rfq.CliConfig{ - AcceptPriceDeviationPpm: rfq.DefaultAcceptPriceDeviationPpm, + AcceptPriceDeviationPpm: rfq.DefaultAcceptPriceDeviationPpm, + PriceOracleTLS: defaultPriceOracleTLS, + PriceOracleTLSInsecure: defaultPriceOracleTLSInsecure, + PriceOracleTLSNoSystemCAs: defaultPriceOracleTLSNoSystemCAs, + PriceOracleTLSCertPath: defaultPriceOracleTLSCertPath, }, }, } @@ -938,8 +963,8 @@ func ValidateConfig(cfg Config, cfgLogger btclog.Logger) (*Config, error) { } } - // Validate the experimental command line config. - err = cfg.Experimental.Validate() + // Clean and validate the experimental command line config. + err = cfg.Experimental.CleanAndValidate() if err != nil { return nil, fmt.Errorf("error in experimental command line "+ "config: %w", err) @@ -1184,6 +1209,35 @@ 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) { + var certBytes []byte + + // Read any specified certificate data. + if rfqCfg.PriceOracleTLSCertPath != "" { + var err error + certBytes, err = os.ReadFile( + rfqCfg.PriceOracleTLSCertPath) + if err != nil { + return nil, fmt.Errorf("unable to read "+ + "price oracle certificate: %w", err) + } + } + + // Construct the oracle's TLS configuration. + tlsConfig := &rfq.TLSConfig{ + Enabled: rfqCfg.PriceOracleTLS, + InsecureSkipVerify: rfqCfg.PriceOracleTLSInsecure, + // Note the subtle flip on the flag, since the user has + // configured whether to *not* trust the system CA's. + TrustSystemRootCAs: !rfqCfg.PriceOracleTLSNoSystemCAs, + CustomCertificates: certBytes, + } + + return tlsConfig, 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 5a486fdf28..cd7f7e0527 100644 --- a/tapcfg/server.go +++ b/tapcfg/server.go @@ -482,8 +482,14 @@ 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) + } + priceOracle, err = rfq.NewRpcPriceOracle( - rfqCfg.PriceOracleAddress, rfq.DefaultTLSConfig(), + rfqCfg.PriceOracleAddress, tlsConfig, ) if err != nil { return nil, fmt.Errorf("unable to create price "+ From ad205e2fec0930fecb8eceba57bcc98306b57ca7 Mon Sep 17 00:00:00 2001 From: Jared Tobin Date: Sat, 30 Aug 2025 09:03:32 -0230 Subject: [PATCH 4/7] rfq: add tls test cases Adds some basic test cases for configuring transport credentials. --- rfq/oracle_test.go | 12 ++++- rfq/tls_test.go | 131 ++++++++++++++++++++++++++++++--------------- 2 files changed, 98 insertions(+), 45 deletions(-) diff --git a/rfq/oracle_test.go b/rfq/oracle_test.go index 46963bcc95..531e8a33c5 100644 --- a/rfq/oracle_test.go +++ b/rfq/oracle_test.go @@ -141,7 +141,8 @@ func runQuerySalePriceTest(t *testing.T, tc *testCaseQuerySalePrice) { // Create a new RPC price oracle client and connect to the mock service. serviceAddr := fmt.Sprintf("rfqrpc://%s", testServiceAddress) - client, err := NewRpcPriceOracle(serviceAddr, DefaultTLSConfig()) + insecureTLS := &TLSConfig{Enabled: false} + client, err := NewRpcPriceOracle(serviceAddr, insecureTLS) require.NoError(t, err) // Query for an ask price. @@ -239,6 +240,13 @@ type testCaseQueryPurchasePrice struct { assetGroupKey *btcec.PublicKey } +// insecureTLS returns a TLSConfig with TLS disabled. +func insecureTLS() *TLSConfig { + return &TLSConfig{ + Enabled: false, + } +} + // runQueryPurchasePriceTest runs the RPC price oracle client QueryBuyPrice // test. func runQueryPurchasePriceTest(t *testing.T, tc *testCaseQueryPurchasePrice) { @@ -251,7 +259,7 @@ func runQueryPurchasePriceTest(t *testing.T, tc *testCaseQueryPurchasePrice) { // Create a new RPC price oracle client and connect to the mock service. serviceAddr := fmt.Sprintf("rfqrpc://%s", testServiceAddress) - client, err := NewRpcPriceOracle(serviceAddr, DefaultTLSConfig()) + client, err := NewRpcPriceOracle(serviceAddr, insecureTLS()) require.NoError(t, err) // Query for an ask price. diff --git a/rfq/tls_test.go b/rfq/tls_test.go index b7364108fe..4ab578407e 100644 --- a/rfq/tls_test.go +++ b/rfq/tls_test.go @@ -6,8 +6,8 @@ import ( "github.com/stretchr/testify/require" ) -// Test certificate data - a valid self-signed certificate for testing -const validTestCertPEM = `-----BEGIN CERTIFICATE----- +// validCertificate is a valid certificate. +const validCertificate = `-----BEGIN CERTIFICATE----- MIICmjCCAYICCQCuu1gzY+BBKjANBgkqhkiG9w0BAQsFADAPMQ0wCwYDVQQDDAR0 ZXN0MB4XDTI1MDgyODEwNDA1NVoXDTI1MDgyOTEwNDA1NVowDzENMAsGA1UEAwwE dGVzdDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBALTWCm8l3d9nE2QK @@ -24,62 +24,107 @@ Wo7g6udwyA48doEVJMjThFLPcW7xmsy6Ldew682m1kD8/ag+9qihX1IJyiqiEjha BcoNuBHB65RxQM5fpA7hkEFm1bxBoowGX2hx6VCCeBBwREISRfgvkUxZahUXNg== -----END CERTIFICATE-----` -// Invalid PEM data for testing failure cases -const invalidTestCertPEM = `-----BEGIN CERTIFICATE----- +// invalidCertificate is an invalid certificate. +const invalidCertificate = `-----BEGIN CERTIFICATE----- This is not a valid certificate -----END CERTIFICATE-----` -// DefaultTLSConfig returns a default TLS configuration for testing. -func DefaultTLSConfig() *TLSConfig { - return &TLSConfig{ - InsecureSkipVerify: true, - } -} +// testCaseConfigureTransportCredentials is a test case for the +// configureTransportCredentials function. +type testCaseConfigureTransportCredentials struct { + name string -// TestConfigureTransportCredentials_InsecureSkipVerify tests the function -// when InsecureSkipVerify is true. -func TestConfigureTransportCredentials_InsecureSkipVerify(t *testing.T) { - config := &TLSConfig{ - InsecureSkipVerify: true, - } + expectInsecure bool - creds, err := configureTransportCredentials(config) + tlsConfig *TLSConfig +} - require.NoError(t, err) - require.NotNil(t, creds) +// runConfigureTransportCredentialsTest tests that we get the expected +// security protocol from the provided test case. +func runConfigureTransportCredentialsTest(t *testing.T, + tc *testCaseConfigureTransportCredentials) { - // Verify that we got insecure credentials by checking the type - require.Equal(t, "insecure", creds.Info().SecurityProtocol) -} + creds, err := configureTransportCredentials(tc.tlsConfig) -// TestConfigureTransportCredentials_ValidCustomCertificates tests the -// function when valid custom certificates are provided. -func TestConfigureTransportCredentials_ValidCustomCertificates(t *testing.T) { - config := &TLSConfig{ - InsecureSkipVerify: false, - CustomCertificates: []byte(validTestCertPEM), - } + // We should never see an error here. + require.Nil(t, err) - creds, err := configureTransportCredentials(config) + protocol := creds.Info().SecurityProtocol - require.NoError(t, err) - require.NotNil(t, creds) + if tc.expectInsecure { + require.Equal(t, "insecure", protocol) + return + } - // Verify that we got TLS credentials (not insecure) - require.Equal(t, "tls", creds.Info().SecurityProtocol) + require.Equal(t, "tls", protocol) } -// TestConfigureTransportCredentials_NoCredentialsConfigured tests the -// function when no credentials are configured. -func TestConfigureTransportCredentials_NoCredentialsConfigured(t *testing.T) { - config := &TLSConfig{ +// defaultTLSConfig is the default TLS config. +func DefaultTLSConfig() *TLSConfig { + return &TLSConfig{ + Enabled: true, InsecureSkipVerify: false, - CustomCertificates: nil, + TrustSystemRootCAs: true, } +} - creds, err := configureTransportCredentials(config) +// TestConfigureTransportCredentials tests the configureTransportCredentials +// function. +func TestConfigureTransportCredentials(t *testing.T) { + testCases := []*testCaseConfigureTransportCredentials{ + { + name: "default configuration", + expectInsecure: false, + tlsConfig: DefaultTLSConfig(), + }, + { + name: "tls disabled", + expectInsecure: true, + tlsConfig: &TLSConfig{ + Enabled: false, + }, + }, + { + name: "trust os root CAs", + expectInsecure: false, + tlsConfig: &TLSConfig{ + Enabled: true, + InsecureSkipVerify: false, + TrustSystemRootCAs: true, + }, + }, + { + name: "no trust os root CAs", + expectInsecure: false, + tlsConfig: &TLSConfig{ + Enabled: true, + InsecureSkipVerify: false, + TrustSystemRootCAs: false, + }, + }, + { + name: "valid custom certificate", + expectInsecure: false, + tlsConfig: &TLSConfig{ + Enabled: true, + InsecureSkipVerify: false, + TrustSystemRootCAs: false, + CustomCertificates: []byte(validCertificate), + }, + }, + { + name: "invalid custom certificate", + expectInsecure: false, + tlsConfig: &TLSConfig{ + Enabled: true, + InsecureSkipVerify: false, + TrustSystemRootCAs: false, + CustomCertificates: []byte(invalidCertificate), + }, + }, + } - require.NoError(t, err) - require.NotNil(t, creds) - require.Equal(t, "tls", creds.Info().SecurityProtocol) + for _, tc := range testCases { + runConfigureTransportCredentialsTest(t, tc) + } } From 29845468497f5420141e7b879ed4839eeb683244 Mon Sep 17 00:00:00 2001 From: Jared Tobin Date: Sun, 31 Aug 2025 08:30:26 -0230 Subject: [PATCH 5/7] itest: use insecure connection with oracle harness Ensures that certificate verification is skipped when constructing a communication channel with the itest oracle harness. --- itest/tapd_harness.go | 1 + 1 file changed, 1 insertion(+) diff --git a/itest/tapd_harness.go b/itest/tapd_harness.go index 7dd5372e8d..3d8a26b81c 100644 --- a/itest/tapd_harness.go +++ b/itest/tapd_harness.go @@ -260,6 +260,7 @@ func newTapdHarness(t *testing.T, ht *harnessTest, cfg tapdConfig, case len(opts.oracleServerAddress) > 0: tapCfg.Experimental.Rfq.PriceOracleAddress = opts.oracleServerAddress + tapCfg.Experimental.Rfq.PriceOracleTLSInsecure = true default: // Set the experimental config for the RFQ service. From fe806e81f8791af6c800d2916d5612c15746d773 Mon Sep 17 00:00:00 2001 From: Jared Tobin Date: Sun, 31 Aug 2025 06:00:04 -0230 Subject: [PATCH 6/7] docs: add release notes --- docs/release-notes/release-notes-0.7.0.md | 19 ++++++++++--------- docs/release-notes/release-notes-0.8.0.md | 12 ++++++++++++ 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/docs/release-notes/release-notes-0.7.0.md b/docs/release-notes/release-notes-0.7.0.md index 46d5615f93..3cc89f23e3 100644 --- a/docs/release-notes/release-notes-0.7.0.md +++ b/docs/release-notes/release-notes-0.7.0.md @@ -26,8 +26,8 @@ - [An integration test flake was fixed](https://github.com/lightninglabs/taproot-assets/pull/1651). -- Fixed two send related bugs that would lead to either a `invalid transfer - asset witness` or `unable to fund address send: error funding packet: unable +- Fixed two send related bugs that would lead to either a `invalid transfer + asset witness` or `unable to fund address send: error funding packet: unable to list eligible coins: unable to query commitments: mismatch of managed utxo and constructed tap commitment root` error when sending assets. The [PR that fixed the two @@ -36,7 +36,7 @@ tombstone outputs on a full-value send (by using interactive transfers for V2 addresses). -- [Updated](https://github.com/lightninglabs/taproot-assets/pull/1774) +- [Updated](https://github.com/lightninglabs/taproot-assets/pull/1774) `BuyOrderRequest` and `SellOrderRequest` proto docs to mark `peer_pub_key` as required. Previously, the field was incorrectly documented as optional. This change corrects the documentation to match the current implementation. @@ -179,20 +179,21 @@ - [Rename](https://github.com/lightninglabs/taproot-assets/pull/1682) the `MintAsset` RPC message field from `universe_commitments` to `enable_supply_commitments`. + - [Enhanced RFQ accepted quote messages with asset identification fields](https://github.com/lightninglabs/taproot-assets/pull/1805): The `PeerAcceptedBuyQuote` and `PeerAcceptedSellQuote` proto messages now include asset ID and asset group pub key fields (via the `AssetSpecBytes` message), allowing clients to directly associate quotes with their corresponding assets without manual tracking. -- The `SubscribeSendEvents` RPC now supports [historical event replay of +- The `SubscribeSendEvents` RPC now supports [historical event replay of completed sends with efficient database-level filtering](https://github.com/lightninglabs/taproot-assets/pull/1685). - [Add universe RPC endpoint FetchSupplyLeaves](https://github.com/lightninglabs/taproot-assets/pull/1693) that allows users to fetch the supply leaves of a universe supply commitment. This is useful for verification. -- A [new field `unconfirmed_transfers` was added to the response of the +- A [new field `unconfirmed_transfers` was added to the response of the `ListBalances` RPC method](https://github.com/lightninglabs/taproot-assets/pull/1691) to indicate that unconfirmed asset-related transactions don't count toward the balance. @@ -204,9 +205,9 @@ - The `AddrReceives` RPC now supports timestamp filtering with [new `StartTimestamp` and `EndTimestamp` fields](https://github.com/lightninglabs/taproot-assets/pull/1794). -- The [FetchSupplyLeaves RPC endpoint](https://github.com/lightninglabs/taproot-assets/pull/1829) - is now accessible without authentication when the universe server is - configured with public read access. This matches the behavior of the +- The [FetchSupplyLeaves RPC endpoint](https://github.com/lightninglabs/taproot-assets/pull/1829) + is now accessible without authentication when the universe server is + configured with public read access. This matches the behavior of the existing FetchSupplyCommit RPC endpoint. - [PR#1839](https://github.com/lightninglabs/taproot-assets/pull/1839) The @@ -239,7 +240,7 @@ includes unset and zero-valued proto fields (e.g. transaction output indexes). This ensures consistent output shape across all proto messages. -- The `tapcli addrs receives` command now supports +- The `tapcli addrs receives` command now supports [new `--start_timestamp` and `--end_timestamp` flags](https://github.com/lightninglabs/taproot-assets/pull/1794). - The `tapcli addrs receives` command now has new flags `--limit`, `--offset` and diff --git a/docs/release-notes/release-notes-0.8.0.md b/docs/release-notes/release-notes-0.8.0.md index ff74028436..a83aee90f3 100644 --- a/docs/release-notes/release-notes-0.8.0.md +++ b/docs/release-notes/release-notes-0.8.0.md @@ -34,6 +34,18 @@ ## RPC Updates +- [TLS connections with price oracles will now be constructed by + default](https://github.com/lightninglabs/taproot-assets/pull/1775), using + the operating system's root CA list for certificate verification. + `experimental.rfq.priceoracletls` (default `true`) can be set to `false` + to disable TLS entirely. For certificate-level configuration, + `experimental.rfq.priceoracletlsnosystemcas` (default `false`) can be set + to `true` to disable use of the OS's root CA list, and + `experimental.rfq.priceoracletlscertpath` allows a custom (root CA or + self-signed) certificate to be used. + `experimental.rfq.priceoracletlsinsecure` can be used to skip certificate + verification (default `false`). + - [PR#1841](https://github.com/lightninglabs/taproot-assets/pull/1841): Remove the defaultMacaroonWhitelist map and inline its entries directly into the conditional logic within MacaroonWhitelist. This ensures that From 0e69a19bafc2c2b5fcddcebb5ca48b69a5122394 Mon Sep 17 00:00:00 2001 From: Jared Tobin Date: Sun, 31 Aug 2025 16:09:11 -0230 Subject: [PATCH 7/7] rfq+tapcfg: disable tls when flag present Ensures the price oracle TLS toggle fits the existing pattern of flags defaulting to false. --- rfq/cli.go | 4 ++-- sample-tapd.conf | 4 ++-- tapcfg/config.go | 12 +++++++----- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/rfq/cli.go b/rfq/cli.go index 3f41fabd88..1567158614 100644 --- a/rfq/cli.go +++ b/rfq/cli.go @@ -22,9 +22,9 @@ 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"` - PriceOracleTLS bool `long:"priceoracletls" description:"Enable TLS for communication with a price oracle."` + PriceOracleTLSDisable bool `long:"priceoracletlsdisable" description:"Disable TLS for price oracle communication."` - PriceOracleTLSInsecure bool `long:"priceoracletlsinsecure" description:"Disable verification of price oracle certificates."` + 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 verifiying price oracle certificates."` diff --git a/sample-tapd.conf b/sample-tapd.conf index 82a064f614..502e71b755 100644 --- a/sample-tapd.conf +++ b/sample-tapd.conf @@ -446,8 +446,8 @@ ; use_mock_price_oracle_service_promise_to_not_use_on_mainnet ; experimental.rfq.priceoracleaddress= -; Enable TLS for price oracle communication. -; experimental.rfq.priceoracletls=true +; Disable TLS for price oracle communication. +; experimental.rfq.priceoracletlsdisable=false ; Skip price oracle certificate verification, yielding an insecure (cleartext) ; channel with the price oracle. Should only be used for testing. diff --git a/tapcfg/config.go b/tapcfg/config.go index a068e6ba1d..842d03305a 100644 --- a/tapcfg/config.go +++ b/tapcfg/config.go @@ -149,9 +149,9 @@ const ( // output amount ratio to use when funding PSBTs. DefaultPsbtMaxFeeRatio = lndservices.DefaultPsbtMaxFeeRatio - // defaultPriceOracleTLS is the default TLS setting to use when - // communicating with price oracles. - defaultPriceOracleTLS = true + // 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 @@ -524,7 +524,7 @@ func DefaultConfig() Config { Experimental: &ExperimentalConfig{ Rfq: rfq.CliConfig{ AcceptPriceDeviationPpm: rfq.DefaultAcceptPriceDeviationPpm, - PriceOracleTLS: defaultPriceOracleTLS, + PriceOracleTLSDisable: defaultPriceOracleTLSDisable, PriceOracleTLSInsecure: defaultPriceOracleTLSInsecure, PriceOracleTLSNoSystemCAs: defaultPriceOracleTLSNoSystemCAs, PriceOracleTLSCertPath: defaultPriceOracleTLSCertPath, @@ -1227,7 +1227,9 @@ func getPriceOracleTLSConfig(rfqCfg rfq.CliConfig) (*rfq.TLSConfig, error) { // Construct the oracle's TLS configuration. tlsConfig := &rfq.TLSConfig{ - Enabled: rfqCfg.PriceOracleTLS, + // Note the subtle flip on the flag, since the user has + // configured whether to *disable* TLS. + Enabled: !rfqCfg.PriceOracleTLSDisable, InsecureSkipVerify: rfqCfg.PriceOracleTLSInsecure, // Note the subtle flip on the flag, since the user has // configured whether to *not* trust the system CA's.