diff --git a/config.go b/config.go index 32b18060..f8d49ee6 100644 --- a/config.go +++ b/config.go @@ -438,6 +438,12 @@ func loadAndValidateConfig(interceptor signal.Interceptor) (*Config, error) { cfg.Lnd.RPCMiddleware.Enable = true } + if !cfg.Autopilot.Disable && cfg.Firewall.RequestLogger.Disable { + return nil, fmt.Errorf("firewall.request-logger.disable " + + "cannot be set to true, without also setting " + + "autopilot.disable to true") + } + // We want to make sure the users don't shoot themselves in the foot by // using a too low value for the lnd RPC timeout. if cfg.LndRPCTimeout < minimumRPCTimeout { diff --git a/firewall/config.go b/firewall/config.go index 9b05315f..1aae66fc 100644 --- a/firewall/config.go +++ b/firewall/config.go @@ -11,6 +11,13 @@ type Config struct { // //nolint:ll type RequestLoggerConfig struct { + // Disable completely disables request logging. This option exists as a + // separate flag rather than a log level because there are scenarios + // where logging should be entirely skipped (no interceptor, no + // database writes) rather than just filtered. Disabling improves + // performance by avoiding all logging overhead, whereas a log level + // would still process and filter each request. + Disable bool `long:"disable" description:"Disable request logging completely. If set, autopilot.disable must also be set"` RequestLoggerLevel RequestLoggerLevel `long:"level" description:"Set the request logger level. Options include 'all', 'full' and 'interceptor''"` } @@ -18,6 +25,7 @@ type RequestLoggerConfig struct { func DefaultConfig() *Config { return &Config{ RequestLogger: &RequestLoggerConfig{ + Disable: false, RequestLoggerLevel: RequestLoggerLevelInterceptor, }, } diff --git a/itest/litd_firewall_test.go b/itest/litd_firewall_test.go index 878559bb..33d948a1 100644 --- a/itest/litd_firewall_test.go +++ b/itest/litd_firewall_test.go @@ -243,6 +243,146 @@ func testFirewallRules(ctx context.Context, net *NetworkHarness, }) } +// testRequestLoggerDisable verifies that disabling the request logger only +// works when the autopilot client is also disabled, since autopilot depends on +// action logs for enforcement and auditing. +func testRequestLoggerDisable(ctx context.Context, net *NetworkHarness, + t *harnessTest) { + + ctx, cancel := context.WithTimeout(ctx, defaultTimeout) + defer cancel() + + // Run 1: Request logging disabled + autopilot disabled. We make an + // account-caveated request and assert no actions are persisted, proving + // the logger is fully bypassed. + node, err := net.NewNode( + t.t, "reqlog-off", nil, false, true, + "--firewall.request-logger.disable", + "--autopilot.disable", + ) + require.NoError(t.t, err) + + rawConn, err := connectLitRPC( + ctx, node.Cfg.LitAddr(), node.Cfg.LitTLSCertPath, + node.Cfg.LitMacPath, + ) + require.NoError(t.t, err) + defer rawConn.Close() + + acctClient := litrpc.NewAccountsClient(rawConn) + acctResp, err := acctClient.CreateAccount( + ctx, &litrpc.CreateAccountRequest{ + AccountBalance: 1_000, + Label: "reqlog-off", + }, + ) + require.NoError(t.t, err) + + lndConn, err := connectRPC( + ctx, node.Cfg.RPCAddr(), node.Cfg.TLSCertPath, + ) + require.NoError(t.t, err) + defer lndConn.Close() + + ctxa := macaroonContext(ctx, acctResp.Macaroon) + _, err = lnrpc.NewLightningClient(lndConn).GetInfo( + ctxa, &lnrpc.GetInfoRequest{}, + ) + require.NoError(t.t, err) + + litFWClient := litrpc.NewFirewallClient(rawConn) + actions, err := litFWClient.ListActions( + ctx, &litrpc.ListActionsRequest{CountTotal: true}, + ) + require.NoError(t.t, err) + require.Empty(t.t, actions.Actions) + require.Zero(t.t, actions.TotalCount) + + require.NoError(t.t, net.ShutdownNode(node)) + + // Run 2: Request logging enabled (default) + autopilot disabled. We + // repeat the exact same account-caveated request and assert actions are + // persisted, proving the logger is active. + node, err = net.NewNode( + t.t, "reqlog-on", nil, false, true, + "--autopilot.disable", + ) + require.NoError(t.t, err) + defer func() { + _ = net.ShutdownNode(node) + }() + + rawConn, err = connectLitRPC( + ctx, node.Cfg.LitAddr(), node.Cfg.LitTLSCertPath, + node.Cfg.LitMacPath, + ) + require.NoError(t.t, err) + defer rawConn.Close() + + acctClient = litrpc.NewAccountsClient(rawConn) + acctResp, err = acctClient.CreateAccount( + ctx, &litrpc.CreateAccountRequest{ + AccountBalance: 1_000, + Label: "reqlog-on", + }, + ) + require.NoError(t.t, err) + + lndConn, err = connectRPC( + ctx, node.Cfg.RPCAddr(), node.Cfg.TLSCertPath, + ) + require.NoError(t.t, err) + defer lndConn.Close() + + litFWClient = litrpc.NewFirewallClient(rawConn) + before, err := litFWClient.ListActions( + ctx, &litrpc.ListActionsRequest{CountTotal: true}, + ) + require.NoError(t.t, err) + + ctxa = macaroonContext(ctx, acctResp.Macaroon) + _, err = lnrpc.NewLightningClient(lndConn).GetInfo( + ctxa, &lnrpc.GetInfoRequest{}, + ) + require.NoError(t.t, err) + + after, err := litFWClient.ListActions( + ctx, &litrpc.ListActionsRequest{CountTotal: true}, + ) + require.NoError(t.t, err) + require.Greater(t.t, after.TotalCount, before.TotalCount) + + // Run 3: Request logging disabled + autopilot enabled. We assert + // startup fails during config validation with the expected error. + node, err = net.NewNode( + t.t, "reqlog-off-autopilot", nil, false, false, + "--firewall.request-logger.disable", + ) + require.NoError(t.t, err) + + select { + case err := <-net.ProcessErrors(): + require.Error(t.t, err) + require.Contains( + t.t, err.Error(), + "firewall.request-logger.disable cannot be set to "+ + "true, without also setting autopilot.disable "+ + "to true", + ) + + case <-ctx.Done(): + _ = net.ShutdownNode(node) + t.Fatalf("timed out waiting for expected start error") + } + + select { + case <-node.processExit: + case <-ctx.Done(): + _ = net.ShutdownNode(node) + t.Fatalf("timed out waiting for process exit") + } +} + // testPrivacyFlags tests that the privacy flags are enforced correctly. // We want to test the three privacy related interactions: // 1. the privacy mapper for the interception of messages diff --git a/itest/litd_test_list_on_test.go b/itest/litd_test_list_on_test.go index fda3f165..a1870d04 100644 --- a/itest/litd_test_list_on_test.go +++ b/itest/litd_test_list_on_test.go @@ -19,6 +19,10 @@ var allTestCases = []*testCase{ name: "terminal firewall rules", test: testFirewallRules, }, + { + name: "terminal request logger disable", + test: testRequestLoggerDisable, + }, { name: "terminal large http header", test: testLargeHttpHeader, diff --git a/terminal.go b/terminal.go index 498be469..387ec3b1 100644 --- a/terminal.go +++ b/terminal.go @@ -1151,13 +1151,6 @@ func (g *LightningTerminal) startInternalSubServers(ctx context.Context, closeAccountService() } - requestLogger, err := firewall.NewRequestLogger( - g.cfg.Firewall.RequestLogger, g.stores.firewall, - ) - if err != nil { - return fmt.Errorf("error creating new request logger") - } - privacyMapper := firewall.NewPrivacyMapper( g.stores.firewall, firewall.CryptoRandIntn, g.stores.sessions, @@ -1166,7 +1159,35 @@ func (g *LightningTerminal) startInternalSubServers(ctx context.Context, mw := []mid.RequestInterceptor{ privacyMapper, g.accountService, - requestLogger, + } + + var ( + requestLogger *firewall.RequestLogger + markActionErrored = func(context.Context, uint64, + string) error { + + return nil + } + ) + + if !g.cfg.Firewall.RequestLogger.Disable { + requestLogger, err = firewall.NewRequestLogger( + g.cfg.Firewall.RequestLogger, g.stores.firewall, + ) + if err != nil { + return fmt.Errorf("error creating new request "+ + "logger: %w", err) + } + + markActionErrored = func(ctx context.Context, reqID uint64, + reason string) error { + + return requestLogger.MarkAction( + ctx, reqID, firewalldb.ActionStateError, reason, + ) + } + + mw = append(mw, requestLogger) } if !g.cfg.Autopilot.Disable { @@ -1177,14 +1198,7 @@ func (g *LightningTerminal) startInternalSubServers(ctx context.Context, g.permsMgr, g.lndClient.NodePubkey, g.lndClient.Router, g.lndClient.Client, g.lndConnID, g.ruleMgrs, - func(ctx context.Context, reqID uint64, - reason string) error { - - return requestLogger.MarkAction( - ctx, reqID, firewalldb.ActionStateError, - reason, - ) - }, g.stores.firewall, + markActionErrored, g.stores.firewall, ) mw = append(mw, ruleEnforcer) @@ -1533,7 +1547,7 @@ func (g *LightningTerminal) shutdownSubServers() error { if g.stores != nil { if g.stores.firewall != nil { if err := g.stores.firewall.Stop(); err != nil { - log.Errorf("Error stoppint firewall DB: %v", + log.Errorf("Error stopping firewall DB: %v", err) returnErr = err