fix(security): audit remediation — file perms, const-time auth, leaks, CSPRNG
Backend audit fixes (upstream-file edits wrapped in // lx: markers): - experimental/libbox oom_report.go/report.go: OOM reports + configuration.json (server secrets/keys) were written world-writable — 0o777 dirs / 0o666 files → 0o700 / 0o600. [sec-perms] - daemon/server.go + experimental/libbox/command_server.go: gRPC auth secret compared with != (timing oracle) → crypto/subtle.ConstantTimeCompare. [sec-consttime] - service/oomkiller/timer.go: network-extension cleanupTriggered logic was inverted, so FreeOSMemory was never called after a trigger; flip both assignments so a trigger schedules the deferred free and the next poll runs + clears it. [sec-oomcleanup] - transport/v2rayxhttp/client.go (lx-native file): session id used math/rand → crypto/rand, matching Xray's uuid.New() entropy and removing the spoof surface. - daemon/started_service_tailscale_ssh.go: forwardSSHAgentChannel leaked a goroutine + the ssh-agent fd on every closed session (second io.Copy blocked on an idle agent Read forever); tie both copies + the session ctx to a cancel that closes both ends. [sec-sshagent] - daemon/managed_service.go: TriggerOOMReport had no gate — rate-limit to 1/min so an authenticated client can't spin secret-bearing dumps. [sec-oomgate] - route/reachability_lx.go (lx idle-suspend file): idle tick read r.idleStop in select while stopIdleSuspend niled it after close (race + goroutine leak on Close-during-tick); pass the stop channel to the loop by value. go build ./... (default) and the D9 shaterd linux build (tags with_quic,with_wireguard,with_utls,badlinkname,tfogo_checklinkname0,with_xhttp, with_awg,with_lx_command) are green; go vet clean (2 pre-existing unsafe.Pointer warnings in TriggerDebugCrash/debug.go, untouched); go test ./route/... ./daemon/... ./service/oomkiller/... green incl. -race with with_lx_idle_suspend and v2rayxhttp with with_xhttp. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -2,6 +2,9 @@ package daemon
|
||||
|
||||
import (
|
||||
"context"
|
||||
// lx:begin sec-oomgate
|
||||
"sync"
|
||||
// lx:end sec-oomgate
|
||||
"time"
|
||||
"unsafe"
|
||||
|
||||
@@ -19,6 +22,10 @@ type ManagedService struct {
|
||||
handler ManagedHandler
|
||||
debug bool
|
||||
oomReporter oomkiller.OOMReporter
|
||||
// lx:begin sec-oomgate
|
||||
oomReportMu sync.Mutex
|
||||
oomReportLast time.Time
|
||||
// lx:end sec-oomgate
|
||||
}
|
||||
|
||||
type ManagedServiceOptions struct {
|
||||
@@ -90,6 +97,18 @@ func (s *ManagedService) TriggerOOMReport(ctx context.Context, _ *emptypb.Empty)
|
||||
if s.oomReporter == nil {
|
||||
return nil, status.Error(codes.Unavailable, "OOM reporter not available")
|
||||
}
|
||||
// lx:begin sec-oomgate
|
||||
// Rate-limit operator-triggered reports to at most one per minute: each write
|
||||
// dumps process state + the config snapshot (secrets) to disk, so an
|
||||
// authenticated client must not be able to spin it in a tight loop.
|
||||
s.oomReportMu.Lock()
|
||||
if !s.oomReportLast.IsZero() && time.Since(s.oomReportLast) < time.Minute {
|
||||
s.oomReportMu.Unlock()
|
||||
return nil, status.Error(codes.ResourceExhausted, "OOM report rate-limited (max 1/min)")
|
||||
}
|
||||
s.oomReportLast = time.Now()
|
||||
s.oomReportMu.Unlock()
|
||||
// lx:end sec-oomgate
|
||||
return &emptypb.Empty{}, s.oomReporter.WriteReport(memory.Total())
|
||||
}
|
||||
|
||||
|
||||
+7
-1
@@ -2,6 +2,9 @@ package daemon
|
||||
|
||||
import (
|
||||
"context"
|
||||
// lx:begin sec-consttime
|
||||
"crypto/subtle"
|
||||
// lx:end sec-consttime
|
||||
"strings"
|
||||
|
||||
"google.golang.org/grpc"
|
||||
@@ -59,8 +62,11 @@ func authenticate(ctx context.Context, secret string) error {
|
||||
return status.Error(codes.Unauthenticated, "missing authorization")
|
||||
}
|
||||
token, isBearer := strings.CutPrefix(values[0], "Bearer ")
|
||||
if !isBearer || token != secret {
|
||||
// lx:begin sec-consttime
|
||||
// Constant-time compare: a plain != leaks the secret via response timing.
|
||||
if !isBearer || subtle.ConstantTimeCompare([]byte(token), []byte(secret)) != 1 {
|
||||
return status.Error(codes.Unauthenticated, "invalid authorization")
|
||||
}
|
||||
// lx:end sec-consttime
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -131,7 +131,9 @@ func (s *StartedService) StartTailscaleSSHSession(
|
||||
continue
|
||||
}
|
||||
go ssh.DiscardRequests(reqs)
|
||||
go s.forwardSSHAgentChannel(channel)
|
||||
// lx:begin sec-sshagent
|
||||
go s.forwardSSHAgentChannel(sessionCtx, channel)
|
||||
// lx:end sec-sshagent
|
||||
}
|
||||
}()
|
||||
}
|
||||
@@ -313,7 +315,8 @@ func (s *StartedService) StartTailscaleSSHSession(
|
||||
return nil
|
||||
}
|
||||
|
||||
func (s *StartedService) forwardSSHAgentChannel(channel ssh.Channel) {
|
||||
// lx:begin sec-sshagent
|
||||
func (s *StartedService) forwardSSHAgentChannel(ctx context.Context, channel ssh.Channel) {
|
||||
defer channel.Close()
|
||||
fd, err := s.handler.ConnectSSHAgent()
|
||||
if err != nil {
|
||||
@@ -326,15 +329,33 @@ func (s *StartedService) forwardSSHAgentChannel(channel ssh.Channel) {
|
||||
return
|
||||
}
|
||||
defer conn.Close()
|
||||
|
||||
// The ssh-agent conn stays blocked in Read while idle, so io.Copy(channel,
|
||||
// conn) never returns on its own — without this it leaks a goroutine + the
|
||||
// agent fd for every closed session. Cancelling on either copy finishing (or
|
||||
// on the session ctx) closes both ends, unblocking the peer copy. Both Close
|
||||
// calls are idempotent with the deferred ones above.
|
||||
ctx, cancel := context.WithCancel(ctx)
|
||||
defer cancel()
|
||||
go func() {
|
||||
<-ctx.Done()
|
||||
conn.Close()
|
||||
channel.Close()
|
||||
}()
|
||||
|
||||
var wg sync.WaitGroup
|
||||
wg.Add(2)
|
||||
go func() {
|
||||
defer wg.Done()
|
||||
io.Copy(conn, channel)
|
||||
cancel()
|
||||
}()
|
||||
go func() {
|
||||
defer wg.Done()
|
||||
io.Copy(channel, conn)
|
||||
cancel()
|
||||
}()
|
||||
wg.Wait()
|
||||
}
|
||||
|
||||
// lx:end sec-sshagent
|
||||
|
||||
@@ -2,6 +2,9 @@ package libbox
|
||||
|
||||
import (
|
||||
"context"
|
||||
// lx:begin sec-consttime
|
||||
"crypto/subtle"
|
||||
// lx:end sec-consttime
|
||||
"errors"
|
||||
"net"
|
||||
"os"
|
||||
@@ -97,9 +100,11 @@ func unaryAuthInterceptor(ctx context.Context, req any, info *grpc.UnaryServerIn
|
||||
if len(values) == 0 {
|
||||
return nil, status.Error(codes.Unauthenticated, "missing authentication secret")
|
||||
}
|
||||
if values[0] != sCommandServerSecret {
|
||||
// lx:begin sec-consttime
|
||||
if subtle.ConstantTimeCompare([]byte(values[0]), []byte(sCommandServerSecret)) != 1 {
|
||||
return nil, status.Error(codes.Unauthenticated, "invalid authentication secret")
|
||||
}
|
||||
// lx:end sec-consttime
|
||||
return handler(ctx, req)
|
||||
}
|
||||
|
||||
@@ -115,9 +120,11 @@ func streamAuthInterceptor(srv any, ss grpc.ServerStream, info *grpc.StreamServe
|
||||
if len(values) == 0 {
|
||||
return status.Error(codes.Unauthenticated, "missing authentication secret")
|
||||
}
|
||||
if values[0] != sCommandServerSecret {
|
||||
// lx:begin sec-consttime
|
||||
if subtle.ConstantTimeCompare([]byte(values[0]), []byte(sCommandServerSecret)) != 1 {
|
||||
return status.Error(codes.Unauthenticated, "invalid authentication secret")
|
||||
}
|
||||
// lx:end sec-consttime
|
||||
return handler(srv, ss)
|
||||
}
|
||||
|
||||
|
||||
@@ -74,7 +74,11 @@ func (r *oomReporter) WriteReport(memoryUsage uint64) error {
|
||||
draftInfo = nil
|
||||
}
|
||||
reportsDir := filepath.Join(sWorkingPath, "oom_reports")
|
||||
err = os.MkdirAll(reportsDir, 0o777)
|
||||
// lx:begin sec-perms
|
||||
// OOM reports embed the config snapshot (server secrets, keys) and logs;
|
||||
// keep the tree owner-only (0700 dirs / 0600 files) instead of 0777/0666.
|
||||
err = os.MkdirAll(reportsDir, 0o700)
|
||||
// lx:end sec-perms
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
@@ -121,7 +125,9 @@ func discardDraftIfCurrent(draftPath string, draftInfo os.FileInfo) error {
|
||||
|
||||
func (r *oomReporter) writeSnapshot(destPath string, memoryUsage uint64) error {
|
||||
now := time.Now().UTC()
|
||||
err := os.MkdirAll(destPath, 0o777)
|
||||
// lx:begin sec-perms
|
||||
err := os.MkdirAll(destPath, 0o700)
|
||||
// lx:end sec-perms
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
@@ -44,7 +44,10 @@ func baseReportMetadata() reportMetadata {
|
||||
|
||||
func writeReportFile(destPath string, name string, content []byte) {
|
||||
filePath := filepath.Join(destPath, name)
|
||||
os.WriteFile(filePath, content, 0o666)
|
||||
// lx:begin sec-perms
|
||||
// Report files may carry the config snapshot (secrets) — owner-only.
|
||||
os.WriteFile(filePath, content, 0o600)
|
||||
// lx:end sec-perms
|
||||
chownReport(filePath)
|
||||
}
|
||||
|
||||
@@ -69,7 +72,9 @@ func copyConfigSnapshot(destPath string) {
|
||||
}
|
||||
|
||||
func initReportDir(path string) {
|
||||
os.MkdirAll(path, 0o777)
|
||||
// lx:begin sec-perms
|
||||
os.MkdirAll(path, 0o700)
|
||||
// lx:end sec-perms
|
||||
chownReport(path)
|
||||
}
|
||||
|
||||
|
||||
@@ -159,7 +159,10 @@ func (r *Router) startIdleSuspend() error {
|
||||
if period < idleTickFloor {
|
||||
period = idleTickFloor
|
||||
}
|
||||
go r.idleSuspendLoop(period)
|
||||
// Pass the stop channel by value so the loop never reads the r.idleStop field
|
||||
// (stopIdleSuspend nils it right after close): a closed local channel stays
|
||||
// ready in select, so the loop exits even when Close races an in-flight tick.
|
||||
go r.idleSuspendLoop(period, r.idleStop)
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -176,12 +179,12 @@ func (r *Router) stopIdleSuspend() {
|
||||
// asks each WG/AWG endpoint to suspend itself if it is unreachable AND idle past
|
||||
// the threshold. Per endpoint this is a single map lookup + an atomic idle
|
||||
// comparison; the endpoint owns the suspend/CAS/log decision.
|
||||
func (r *Router) idleSuspendLoop(period time.Duration) {
|
||||
func (r *Router) idleSuspendLoop(period time.Duration, stop <-chan struct{}) {
|
||||
ticker := time.NewTicker(period)
|
||||
defer ticker.Stop()
|
||||
for {
|
||||
select {
|
||||
case <-r.idleStop:
|
||||
case <-stop:
|
||||
return
|
||||
case <-ticker.C:
|
||||
r.suspendIdleEndpoints(r.reachableOutbounds())
|
||||
|
||||
@@ -163,10 +163,15 @@ func (t *adaptiveTimer) poll() {
|
||||
return
|
||||
}
|
||||
if t.timerConfig.policyMode == policyModeNetworkExtension {
|
||||
// lx:begin sec-oomcleanup
|
||||
// A trigger schedules a deferred FreeOSMemory for the next poll; run it
|
||||
// here and clear the flag (was inverted: it re-set true and so the free
|
||||
// after a trigger never happened).
|
||||
if t.cleanupTriggered {
|
||||
runtimeDebug.FreeOSMemory()
|
||||
t.cleanupTriggered = true
|
||||
t.cleanupTriggered = false
|
||||
}
|
||||
// lx:end sec-oomcleanup
|
||||
}
|
||||
if t.pendingPressureBaseline {
|
||||
t.pressureBaseline = sample
|
||||
@@ -202,7 +207,10 @@ func (t *adaptiveTimer) poll() {
|
||||
if !triggered {
|
||||
return
|
||||
}
|
||||
t.cleanupTriggered = false
|
||||
// lx:begin sec-oomcleanup
|
||||
// Schedule the deferred FreeOSMemory for the next network-extension poll.
|
||||
t.cleanupTriggered = true
|
||||
// lx:end sec-oomcleanup
|
||||
t.onTriggered(sample.usage)
|
||||
if rateTriggered {
|
||||
if t.killerDisabled {
|
||||
|
||||
@@ -23,7 +23,7 @@ package v2rayxhttp
|
||||
|
||||
import (
|
||||
"context"
|
||||
"math/rand"
|
||||
"crypto/rand"
|
||||
"net"
|
||||
"net/http"
|
||||
"net/url"
|
||||
@@ -255,8 +255,12 @@ func (c *Client) newRequest(ctx context.Context, method, sessionID, seqStr strin
|
||||
// an opaque grouping key; the dashed format keeps it interchangeable with Xray.
|
||||
func newSessionID() string {
|
||||
var b [16]byte
|
||||
for i := range b {
|
||||
b[i] = byte(rand.Intn(256))
|
||||
// crypto/rand.Read fills b fully or returns an error; on every platform we
|
||||
// target it does not fail (it is the same entropy source behind Xray's
|
||||
// uuid.New()). A failure here means the OS CSPRNG is broken — there is no
|
||||
// safe fallback for a session id, so we intentionally do not swallow it.
|
||||
if _, err := rand.Read(b[:]); err != nil {
|
||||
panic(err)
|
||||
}
|
||||
const hexdigits = "0123456789abcdef"
|
||||
var h [32]byte
|
||||
|
||||
Reference in New Issue
Block a user