mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-08-19 21:56:54 +00:00
Chris LuandGitHub
e2c8791441
* feat(nfs): add NFSv3-only RPC version filter The upstream willscott/go-nfs library dispatches RPC calls by (program, procedure) only — it does not validate the program version. A client sending NFSv4 (prog 100003 vers 4 proc 1 COMPOUND) lands on the same handler map as NFSv3 and gets routed to v3 SETATTR, which parses the COMPOUND args as SETATTR3args and writes a malformed reply. The kernel then returns EPROTONOSUPPORT and mount.nfs prints "requested NFS version or transport protocol is not supported" without retrying v3. This commit adds a listener wrapper that peeks the first RPC frame on each new TCP connection. If the program is NFS or MOUNT and the version is not 3, it writes a protocol-correct PROG_MISMATCH reply (supported range 3..3, per RFC 5531) directly to the socket and closes the connection. v3 frames are replayed unchanged via a bufio reader so go-nfs sees the original bytes. Unknown programs pass through so go-nfs's own PROG_UNAVAIL handling stays in charge. The filter is not yet wired into the server; the next commit activates it. Tests cover NFSv4 reject, MOUNTv4 reject, NFSv3 pass-through, and unknown-program pass-through. * fix(nfs): wire NFSv3 version filter into the listener chain Place the version filter after the optional client allowlist so that unauthorized peers are still rejected first by IP/CIDR before we look at RPC content. With the filter active, a Linux client doing the default v4-first probe gets a clean PROG_MISMATCH reply pointing at v3, which lets mount.nfs (and the in-kernel client) skip v4 and reuse the same v3 mountOptions that already work for rclone serve nfs against this deployment. * test(nfs): exercise MOUNT v4 in the v4-rejection test, not v1 TestVersionFilterRejectsMOUNTv4WithProgMismatch was sending mountProgramID with version 1, so the test never actually covered the "reject MOUNT v4" path it claims to exercise. The filter does reject any non-v3 version uniformly, so the test still passed, but a future change that tightened the version check (for example, only rejecting v4) would let this test silently lie about coverage. Bump the call to version 4 so the name matches what is actually exercised. * refactor(nfs): reuse package RPC constants and io.ReadFull in version filter The RPC numeric constants (msg_type=CALL/REPLY, MSG_ACCEPTED, PROG_MISMATCH, AUTH_NONE, the NFS/MOUNT program numbers) are already named in portmap.go alongside the portmap responder. Reuse them here instead of defining a parallel set in rpc_version_filter.go: keeping one source of truth per package means a future correction in one spot can't drift away from the other. The filter-only constants (peek timeout, peek length, supportedNFSVer) stay local because they have no portmap analog. In the test, drop the bespoke readFull loop in favor of io.ReadFull. The custom version was a near-identical reimplementation that did not return io.ErrUnexpectedEOF on short reads, so the standard library is both shorter and more diagnostic-friendly. * fix(nfs): move RPC peek off the Accept path The previous wrapper called filterFirstRPCFrame inline inside versionFilterListener.Accept, which meant a single slow or idle TCP connect could hold rpcVersionFilterPeekTimeout (10s) of head-of-line blocking against every other accept: gonfs.Serve calls Accept serially, so each in-flight peek stalled the next legitimate client until the deadline expired. An attacker who simply opens a TCP connection without sending any RPC payload could trivially throttle accept throughput. Restructure the wrapper so a background goroutine drives the inner Accept loop and hands each raw conn to its own short-lived goroutine that runs the peek. Validated conns are sent on a buffered-once channel, which the wrapper's Accept reads from; rejected conns finish their PROG_MISMATCH reply and disappear without ever reaching the channel. This means N concurrent slow clients only block themselves, not the N+1th fast client that connects after them. Add Close coordination — sync.WaitGroup for the accept loop and per-conn peek goroutines, plus a closed channel so Accept unblocks immediately on shutdown — so the wrapper now satisfies the full net.Listener contract instead of relying on the embedded listener. Add a regression test that opens a slow conn (TCP only, never writes) and a fast conn (sends a v3 frame) and asserts the fast conn reaches the inner accept handler well below the peek timeout. * test(nfs): assert io.EOF (not just any error) after PROG_MISMATCH close The post-rejection check was only failing when conn.Read succeeded; any error — including a deadline timeout because the server kept the socket open — let the test pass. That defeats the point of the assertion: a regression where the filter replies but forgets to close would slip through silently. Match against io.EOF explicitly. The TCP semantics are deterministic here: the server writes PROG_MISMATCH, calls conn.Close(), the client reads what's left in flight and then sees a clean FIN, which surfaces as io.EOF on the next zero-byte read. * fix(nfs): reject short first fragments before parsing RPC header fields bufio.Reader.Peek(28) is willing to read across record boundaries to satisfy the requested length, so a final fragment whose body is shorter than the 24-byte fixed RPC CALL header (xid + msg_type + rpcvers + prog + vers + proc) leaves the trailing peek bytes pointing at the next RPC's framing or whatever bytes happen to follow on the wire. Indexing hdr[16:24] for prog/vers in that state can spuriously reject (or pass through) traffic based on data that doesn't belong to the request being classified. Drop those frames out of the filter early: if the first fragment can't possibly hold a full CALL header, pass the connection straight to go-nfs, which has its own framing-error handling for malformed input. Add a regression test that crafts a 12-byte first fragment whose trailing peek bytes are deliberately shaped like an NFSv4 CALL — without the length check the filter sends a PROG_MISMATCH; with it, the conn passes through silently. Verified by stashing the production-code change and running the test in isolation: it fails as expected without the fix. * fix(nfs): retry transient Accept() errors instead of treating any error as terminal acceptLoop previously exited on the first error returned by the inner listener's Accept(). That conflates two very different failure modes: permanent shutdown (the listener was Close()d, OS-level fatal failure) and transient resource pressure (EMFILE, EAGAIN, ECONNABORTED on accept). The transient case should not take the entire NFS server down — a single fd-table-full event would leave the deployment offline until restart. Classify the error: errors.Is(err, net.ErrClosed) is the permanent signal we already wanted to surface to Accept(); everything else is transient. Log at V(1) and back off rpcVersionFilterAcceptBackoff (50ms, mirroring portmap.go's portmapRetryBackoff) before retrying. The backoff sleep is interruptible via the closed channel so Close() still shuts the loop down promptly. Add a regression test that wraps a real listener with one that injects 3 fake transient errors before delegating, and asserts Accept() still delivers the next real connection. Verified the test fails on the old "any error is terminal" loop and passes with this change. * fix(nfs): only synthesize PROG_MISMATCH for ONC RPC v2 traffic The filter was rejecting any CALL-shaped record with prog=100003 or 100005 and vers!=3, regardless of the rpcvers field. If the caller is speaking some other protocol that happens to share the port — or just sending garbled bytes — pretending to be an NFSv3 server replying PROG_MISMATCH is misleading at best, and at worst fabricates a coherent RPC reply for traffic we don't actually understand. Add an rpcvers==2 check between the msg_type and prog/vers parses. Any non-v2 record now passes through to go-nfs, whose RFC 5531 §9 RPC_MISMATCH handling is the correct place to reject mis-versioned RPC. Regression test takes a normal v3 NFS CALL frame, overwrites the rpcvers field with 99, and asserts no PROG_MISMATCH-shaped reply lands on the client and that the conn is delivered to the inner accept handler. Verified the test fails on the previous code (filter still rejected on prog/vers alone) and passes with the guard in place. * fix(nfs): bound Close() latency by evicting in-flight prefilter conns Close() does wg.Wait() to drain handleConn goroutines, but each of those goroutines can be parked inside filterFirstRPCFrame's bufio.Peek for up to rpcVersionFilterPeekTimeout (10s) waiting for the very first RPC header. A client that completes the TCP handshake but never sends a byte therefore stretched shutdown by 10s per such conn — a real regression for stop/restart paths and for tests that just want to tear the listener down. Track raw (pre-peek) conns in versionFilterListener.inFlight as handleConn enters, untrack on exit, and have Close() forcibly close every tracked conn before wg.Wait. Closing the underlying conn breaks its Peek immediately, so handleConn returns within a single scheduler hop. trackInFlight also short-circuits if shutdown has already started, so a conn accepted after signalClose can't slip past the eviction. Black-box regression test opens 4 idle TCP-handshake-only conns, lets their handleConn goroutines settle into Peek, and asserts Close() returns under 2s. Verified: same test fails on the previous code with Close taking ~9.9s; passes here at ~100ms.