diff --git a/docs/release-notes/release-notes-0.7.0.md b/docs/release-notes/release-notes-0.7.0.md index c9f91d57d..0766c88d5 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 @@ -101,6 +101,18 @@ controls whether the peer's identity public key is sent to the local price oracle when querying asset price rates. +- [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`). + ## RPC Additions - The [price oracle RPC calls now have an intent, optional peer ID and metadata @@ -123,14 +135,14 @@ - [Rename](https://github.com/lightninglabs/taproot-assets/pull/1682) the `MintAsset` RPC message field from `universe_commitments` to `enable_supply_commitments`. -- 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. diff --git a/itest/tapd_harness.go b/itest/tapd_harness.go index 1c4e4d620..e882e27af 100644 --- a/itest/tapd_harness.go +++ b/itest/tapd_harness.go @@ -238,6 +238,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. diff --git a/rfq/cli.go b/rfq/cli.go index 8559504c2..156715861 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"` + 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 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/oracle.go b/rfq/oracle.go index e7ee28694..7c68d8502 100644 --- a/rfq/oracle.go +++ b/rfq/oracle.go @@ -2,7 +2,6 @@ package rfq import ( "context" - "crypto/tls" "fmt" "math" "net/url" @@ -16,8 +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" ) // PriceQueryIntent is an enum that represents the intent of a price rate @@ -186,35 +183,9 @@ type RpcPriceOracle struct { rawConn *grpc.ClientConn } -// 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 - - // Skip TLS certificate verification. - tlsConfig := tls.Config{InsecureSkipVerify: true} - transportCredentials := credentials.NewTLS(&tlsConfig) - opts = append(opts, grpc.WithTransportCredentials(transportCredentials)) - - 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(), - )) - - return opts, nil -} - // 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) @@ -222,24 +193,21 @@ func NewRpcPriceOracle(addrStr string, dialInsecure bool) (*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 } - // 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/oracle_test.go b/rfq/oracle_test.go index c120ffe72..531e8a33c 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, true) + 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, true) + client, err := NewRpcPriceOracle(serviceAddr, insecureTLS()) require.NoError(t, err) // Query for an ask price. diff --git a/rfq/tls.go b/rfq/tls.go new file mode 100644 index 000000000..a9735f9c6 --- /dev/null +++ b/rfq/tls.go @@ -0,0 +1,68 @@ +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 +} + +// 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 000000000..4ab578407 --- /dev/null +++ b/rfq/tls_test.go @@ -0,0 +1,130 @@ +package rfq + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +// validCertificate is a valid certificate. +const validCertificate = `-----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-----` + +// invalidCertificate is an invalid certificate. +const invalidCertificate = `-----BEGIN CERTIFICATE----- +This is not a valid certificate +-----END CERTIFICATE-----` + +// testCaseConfigureTransportCredentials is a test case for the +// configureTransportCredentials function. +type testCaseConfigureTransportCredentials struct { + name string + + expectInsecure bool + + tlsConfig *TLSConfig +} + +// runConfigureTransportCredentialsTest tests that we get the expected +// security protocol from the provided test case. +func runConfigureTransportCredentialsTest(t *testing.T, + tc *testCaseConfigureTransportCredentials) { + + creds, err := configureTransportCredentials(tc.tlsConfig) + + // We should never see an error here. + require.Nil(t, err) + + protocol := creds.Info().SecurityProtocol + + if tc.expectInsecure { + require.Equal(t, "insecure", protocol) + return + } + + require.Equal(t, "tls", protocol) +} + +// defaultTLSConfig is the default TLS config. +func DefaultTLSConfig() *TLSConfig { + return &TLSConfig{ + Enabled: true, + InsecureSkipVerify: false, + TrustSystemRootCAs: true, + } +} + +// 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), + }, + }, + } + + for _, tc := range testCases { + runConfigureTransportCredentialsTest(t, tc) + } +} diff --git a/sample-tapd.conf b/sample-tapd.conf index 896cc98e2..b6576a76a 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. @@ -435,6 +435,21 @@ ; use_mock_price_oracle_service_promise_to_not_use_on_mainnet ; experimental.rfq.priceoracleaddress= +; 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. +; 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. @@ -445,7 +460,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 1674bd698..55bcf0099 100644 --- a/tapcfg/config.go +++ b/tapcfg/config.go @@ -143,6 +143,24 @@ const ( // defaultMailboxAuthTimeout is the default timeout we'll use for // mailbox message retrieval client authentication. defaultMailboxAuthTimeout = 10 * time.Second + + // 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 = "" ) var ( @@ -317,8 +335,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() } @@ -478,7 +499,11 @@ func DefaultConfig() Config { }, Experimental: &ExperimentalConfig{ Rfq: rfq.CliConfig{ - AcceptPriceDeviationPpm: rfq.DefaultAcceptPriceDeviationPpm, + AcceptPriceDeviationPpm: rfq.DefaultAcceptPriceDeviationPpm, + PriceOracleTLSDisable: defaultPriceOracleTLSDisable, + PriceOracleTLSInsecure: defaultPriceOracleTLSInsecure, + PriceOracleTLSNoSystemCAs: defaultPriceOracleTLSNoSystemCAs, + PriceOracleTLSCertPath: defaultPriceOracleTLSCertPath, }, }, } @@ -914,8 +939,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) @@ -1150,6 +1175,37 @@ 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{ + // 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. + 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 f21444de6..0b7aedd05 100644 --- a/tapcfg/server.go +++ b/tapcfg/server.go @@ -461,8 +461,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, false, + rfqCfg.PriceOracleAddress, tlsConfig, ) if err != nil { return nil, fmt.Errorf("unable to create price "+