diff --git a/CHANGELOG.md b/CHANGELOG.md index 6883990a6..249149e52 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,28 @@ - Better error handling for access denied errors (#1249) - Add --no-wait/--finalize/--non-interactive flags for agent-driven releases (#1360) +### Behavior Changes + +- **Light proxy WebSocket origin policy is now zero-trust by default.** The + `light` RPC proxy previously accepted WebSocket connections from any web + origin. It now accepts only same-host origins and Origin-less (non-browser) + clients, matching the main RPC server behavior. To permit specific browser + origins, set `rpc.cors-allowed-origins` (same semantics as the main RPC + `cors-allowed-origins`); a `*` entry restores the old allow-all behavior but + is discouraged. (#1368) +- **Light client no longer fails on a single dissenting witness.** A single + witness reporting a header that conflicts with the (corroborated) primary is + now treated as a faulty witness and removed, instead of failing verification + — preventing one bad witness from denying service to the light client. + Verification still fails (fork evidence) when two or more witnesses conflict, + or when a conflicting witness has no corroborating peer. (#1369) +- **`voting_power_threshold` override below the 2/3+1 BFT floor now logs an + explicit `OVERRIDE ENABLED - NOT SAFE FOR PRODUCTION` error at startup.** The + on-chain `voting_power_threshold` validator param can lower the commit gate + below the strict BFT floor (a dev/single-node escape hatch, #1052). Operators + must never set this on production quorums: doing so reduces fault tolerance + below BFT assumptions and can enable safety violations or halting. (#1370) + ### Miscellaneous Tasks - Add AI agent instructions and streamline style guide (#1250) diff --git a/cmd/tenderdash/commands/light.go b/cmd/tenderdash/commands/light.go index ff307401c..718bab4b2 100644 --- a/cmd/tenderdash/commands/light.go +++ b/cmd/tenderdash/commands/light.go @@ -180,6 +180,9 @@ for applications built w/ Cosmos SDK). if err != nil { return err } + // Zero-trust WebSocket origin policy by default; operators opt in to + // specific browser origins via rpc.cors-allowed-origins. + p.AllowedOrigins = conf.RPC.CORSAllowedOrigins ctx, cancel := signal.NotifyContext(cmd.Context(), os.Interrupt, syscall.SIGTERM) defer cancel() diff --git a/docs/nodes/configuration.md b/docs/nodes/configuration.md index 3c88f7503..c7cf2cfdc 100644 --- a/docs/nodes/configuration.md +++ b/docs/nodes/configuration.md @@ -123,6 +123,12 @@ laddr = "tcp://127.0.0.1:26657" # Use '["*"]' to allow any origin cors-allowed-origins = [] +# This list also governs the WebSocket origin policy of the `light` RPC proxy +# (tenderdash light --proxy). By default (empty) the proxy accepts only +# same-host origins and Origin-less (non-browser) clients — a zero-trust policy. +# Add specific origins here to permit browser clients; a "*" entry allows any +# origin and is discouraged. + # A list of methods the client is allowed to use with cross-domain requests cors-allowed-methods = ["HEAD", "GET", "POST", ] diff --git a/internal/consensus/state_data.go b/internal/consensus/state_data.go index d48b98539..ede6b0825 100644 --- a/internal/consensus/state_data.go +++ b/internal/consensus/state_data.go @@ -272,7 +272,7 @@ func (s *StateData) updateToState(state sm.State, commit *types.Commit, blockSto s.logger.Info("Updating validators", "from", s.Validators.BasicInfoString(), "to", validators.BasicInfoString()) if validators.BelowStrictThreshold() { - s.logger.Warn("validator set voting threshold is below the 2/3+1 BFT safety bound; fault tolerance is reduced", + s.logger.Error("OVERRIDE ENABLED - NOT SAFE FOR PRODUCTION: validator set voting threshold is below the 2/3+1 BFT safety bound; fault tolerance is reduced and the chain is exposed to safety violations", "quorum_type", validators.QuorumType.Name(), "threshold", validators.QuorumVotingThresholdPower(), "total_power", validators.TotalVotingPower(), diff --git a/light/client.go b/light/client.go index b5369b577..6de95a79b 100644 --- a/light/client.go +++ b/light/client.go @@ -833,8 +833,13 @@ func (c *Client) compareFirstHeaderWithWitnesses(ctx context.Context, h *types.S go c.compareNewHeaderWithWitness(compareCtx, errc, h, witness, i) } + // Separate counters for the two outcomes we care about: + // - conflictingWitnesses: witnesses whose header disagrees with the primary's. + // - corroborating: witnesses whose header matches the primary's (a healthy + // primary is expected to be corroborated by at least one other party). witnessesToRemove := make([]int, 0, len(c.witnesses)) conflictingWitnesses := make([]int, 0, len(c.witnesses)) + corroborating := 0 // handle errors from the header comparisons as they come in for i := 0; i < cap(errc); i++ { @@ -842,13 +847,12 @@ func (c *Client) compareFirstHeaderWithWitnesses(ctx context.Context, h *types.S switch e := err.(type) { case nil: + corroborating++ continue case errConflictingHeaders: c.logger.Error(fmt.Sprintf("witness #%d has a different header. Please check primary is correct and"+ " remove witness. Otherwise, use the different primary", e.WitnessIndex), "witness", c.witnesses[e.WitnessIndex]) - // The primary and a witness disagree; keep the dissenting witness, as - // the primary may be the faulty party, and fail the comparison below. conflictingWitnesses = append(conflictingWitnesses, e.WitnessIndex) case errBadWitness: // If witness sent us an invalid header, then remove it @@ -881,12 +885,32 @@ func (c *Client) compareFirstHeaderWithWitnesses(ctx context.Context, h *types.S return err } - // A witness presenting a header that conflicts with the primary's means the - // primary's header cannot be trusted: fail the comparison. - if len(conflictingWitnesses) > 0 { + // A single conflicting witness must not be allowed to break verification: a + // lone dissenter is far more likely to be a faulty/compromised witness than a + // sign that the primary is malicious, and failing here would let one bad + // witness denial-of-service an otherwise-healthy light client. We therefore + // treat a single conflicting witness as faulty and remove it, exactly like + // errBadWitness. We still FAIL (preserving the safety-over-availability + // guarantee) only when the conflict is corroborated: + // - two or more witnesses disagree with the primary (genuine fork / primary + // likely faulty), OR + // - at least one witness conflicts and NO witness corroborates the primary + // (we cannot establish trust in either party, so we refuse to proceed). + if len(conflictingWitnesses) > 1 || (len(conflictingWitnesses) == 1 && corroborating == 0) { return fmt.Errorf("%w: witnesses %v", ErrConflictingWitnessHeader, conflictingWitnesses) } + // Exactly one conflicting witness with at least one corroborating witness: + // drop the dissenter and continue. + if len(conflictingWitnesses) == 1 { + c.logger.Warn("single witness conflicts with the primary but others corroborate it; "+ + "removing the dissenting witness rather than failing verification", + "witness", c.witnesses[conflictingWitnesses[0]]) + if err := c.removeWitnesses(conflictingWitnesses); err != nil { + return err + } + } + return nil } diff --git a/light/client_test.go b/light/client_test.go index d0e41229b..f67e22d08 100644 --- a/light/client_test.go +++ b/light/client_test.go @@ -515,13 +515,12 @@ func TestClient(t *testing.T) { require.NoError(t, err) assert.Equal(t, 2, len(c.Witnesses())) - // A witness reporting a header that conflicts with the primary's fails the - // verification; the dissenting witness is kept, since the primary may be - // the faulty party. + // A single witness reporting a header that conflicts with the primary's is + // removed (treated as faulty), NOT fatal: the good witness corroborates the + // primary, so one dissenter must not break verification. _, err = c.VerifyLightBlockAtHeight(ctx, 2, bTime.Add(2*time.Hour).Add(1*time.Second)) - require.Error(t, err) - assert.ErrorIs(t, err, light.ErrConflictingWitnessHeader) - assert.Equal(t, 2, len(c.Witnesses())) + require.NoError(t, err) + assert.Equal(t, 1, len(c.Witnesses())) mockFullNode.AssertExpectations(t) }) t.Run("PrunesHeadersAndValidatorSets", func(t *testing.T) { diff --git a/light/detector_internal_test.go b/light/detector_internal_test.go index 24dd14beb..09df49f3e 100644 --- a/light/detector_internal_test.go +++ b/light/detector_internal_test.go @@ -91,8 +91,9 @@ func TestCompareNewHeaderWithWitness_Match(t *testing.T) { } // TestCompareFirstHeaderWithWitnesses covers the witness cross-check flow: a -// conflicting witness fails verification while being kept, a bad witness is -// removed, and full agreement succeeds. +// single conflicting witness is removed when corroborated, multiple conflicting +// witnesses fail as fork evidence, a bad witness is removed, and full agreement +// succeeds. func TestCompareFirstHeaderWithWitnesses(t *testing.T) { primary := testLightBlock(5, []byte("primary-validators-hash")) matching := func() *types.LightBlock { return testLightBlock(5, []byte("primary-validators-hash")) } @@ -117,16 +118,39 @@ func TestCompareFirstHeaderWithWitnesses(t *testing.T) { wantWitnessCount: 2, }, { - name: "one conflicting witness fails verification but is kept", + name: "one conflicting witness with a corroborating witness is removed, not fatal", witnesses: func() []provider.Provider { return []provider.Provider{ mockWitness(matching(), "w0"), mockWitness(conflicting(), "w1"), } }, + wantErr: false, + wantWitnessCount: 1, + }, + { + name: "two conflicting witnesses fail verification (fork evidence)", + witnesses: func() []provider.Provider { + return []provider.Provider{ + mockWitness(matching(), "w0"), + mockWitness(conflicting(), "w1"), + mockWitness(conflicting(), "w2"), + } + }, wantErr: true, wantConflict: true, - wantWitnessCount: 2, + wantWitnessCount: 3, + }, + { + name: "single conflicting witness with no corroborating witness fails", + witnesses: func() []provider.Provider { + return []provider.Provider{ + mockWitness(conflicting(), "w1"), + } + }, + wantErr: true, + wantConflict: true, + wantWitnessCount: 1, }, { name: "an erroring witness is removed when others agree", diff --git a/light/proxy/proxy.go b/light/proxy/proxy.go index b31c705a8..d9bc9b8a0 100644 --- a/light/proxy/proxy.go +++ b/light/proxy/proxy.go @@ -22,6 +22,14 @@ type Proxy struct { Client *lrpc.Client Logger log.Logger Listener net.Listener + + // AllowedOrigins restricts which web origins may open a WebSocket to the + // proxy. It mirrors rpc.Config.CORSAllowedOrigins semantics. An empty/nil + // slice enforces the zero-trust default: only same-host origins and + // Origin-less (non-browser) clients are accepted. Operators that must permit + // specific browser origins set this explicitly; a "*" entry allows any + // origin (discouraged). + AllowedOrigins []string } // NewProxy creates the struct used to run an HTTP server for serving light @@ -39,10 +47,11 @@ func NewProxy( } return &Proxy{ - Addr: listenAddr, - Config: config, - Client: lrpc.NewClient(logger, rpcClient, lightClient, opts...), - Logger: logger, + Addr: listenAddr, + Config: config, + Client: lrpc.NewClient(logger, rpcClient, lightClient, opts...), + Logger: logger, + AllowedOrigins: nil, }, nil } @@ -105,13 +114,13 @@ func (p *Proxy) listen(ctx context.Context) (net.Listener, *http.ServeMux, error }), rpcserver.ReadLimit(p.Config.MaxBodyBytes), ) - // Preserve the light proxy's historic permissive websocket origin policy: - // allow every origin. The proxy is configured via rpcserver.Config, which - // exposes no CORS allow-list knob, so the WebsocketManager default - // (same-host/Origin-less only) would silently reject browser clients that - // worked before. Operators that need origin restrictions are expected to - // enforce AuthN/AuthZ in front of the proxy. - wm.CheckOrigin = func(*http.Request) bool { return true } + // WebSocket origin policy is zero-trust by default: OriginChecker with no + // allow-list accepts only same-host origins and Origin-less (non-browser) + // clients, rejecting cross-origin browser requests. Operators that need to + // permit specific browser origins set Proxy.AllowedOrigins (mirrors + // rpc.Config.CORSAllowedOrigins); a "*" entry allows any origin but is + // discouraged. This replaces the previous blanket allow-all policy. + wm.CheckOrigin = rpcserver.OriginChecker(p.Logger, p.AllowedOrigins) mux.HandleFunc("/websocket", wm.WebsocketHandler)