diff --git a/cmd/msa-agent/main.go b/cmd/msa-agent/main.go index 68a6672..15e3d60 100644 --- a/cmd/msa-agent/main.go +++ b/cmd/msa-agent/main.go @@ -98,8 +98,9 @@ func run(configPath string, checkOnly, once bool) error { defer audit.Close() exec, err := runtime.NewExecutor(pol, runtime.Config{ - Port: cfg.LocalPort, - Audit: audit, + Port: cfg.LocalPort, + Audit: audit, + NodePassword: cfg.NodePassword, }) if err != nil { return err @@ -151,6 +152,10 @@ func loop(ctx context.Context, node *agent.Agent, client *transport.Client, pol // a tight loop against the relay. slog.Warn("relay rejected the agent session, re-authenticating", "error", err) backoff = minBackoff * 5 + case errors.Is(err, transport.ErrRateLimited): + // The relay said how long to wait; doubling its own backoff on top of that would be + // the agent overriding an explicit instruction. + backoff = client.RetryAfter() default: slog.Warn("poll cycle failed", "error", err, "retryIn", backoff) backoff = next(backoff) diff --git a/internal/agent/agent.go b/internal/agent/agent.go index a7ceb22..d42de73 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -67,6 +67,10 @@ const ( OutcomeRateLimited = "rate_limited" OutcomeTooLarge = "payload_too_large" OutcomeUpstream = "upstream_error" + // OutcomeNodeAuth means the agent's own login to the local SimAdmin failed. The phone shows it + // as "this node needs its agent reconfigured", not "your command failed", because nothing the + // user does on the phone can fix a wrong node password. + OutcomeNodeAuth = "node_auth_failed" ) // Agent owns the loop over one node. @@ -208,6 +212,8 @@ func statusFor(class string) int { return 429 case runtime.ClassTooLarge: return 413 + case runtime.ClassNodeAuth: + return 503 case runtime.ClassUpstream: return 502 default: diff --git a/internal/config/config.go b/internal/config/config.go index dcc8b85..c2e765d 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -40,6 +40,10 @@ type File struct { SignKey string `json:"signKey"` // KxKey is the agent's X25519 private key, base64. KxKey string `json:"kxKey"` + // NodePassword is the local SimAdmin password the agent logs in with so the phone never + // handles or transmits it. Empty declares the node unprotected; a protected node then refuses + // commands with node_auth_failed rather than looping against a lockout. + NodePassword string `json:"nodePassword,omitempty"` // AuditPath is where the audit log is appended. AuditPath string `json:"auditPath"` Devices []Device `json:"devices"` diff --git a/internal/runtime/auth.go b/internal/runtime/auth.go new file mode 100644 index 0000000..a5c86c7 --- /dev/null +++ b/internal/runtime/auth.go @@ -0,0 +1,280 @@ +package runtime + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "regexp" + "strings" + "sync" + "time" + + "fun.chick/msa-agent/internal/egress" +) + +// NodeAuth holds the agent's own session against the local SimAdmin. +// +// This exists because the phone must never hold a node password: the agent logs in itself and the +// credential never traverses the relay in either direction. The login endpoints are therefore +// reachable by this code and permanently unreachable through the policy (authPaths.proxied=false +// plus the auth-pattern rejection in policy.Decision), so the only way to trigger a login attempt +// against the node is for the agent itself to need one. +// +// The semantics mirror server/core.js ensureAuthenticated in the console, including its failure +// modes: a stale password must not turn every command into a fresh login attempt, because some +// devices punish repeated failures with lockouts. +type NodeAuth struct { + client *http.Client + origin string + password string + cooldown time.Duration + clock func() time.Time + + mu sync.Mutex + state authState + cookie string + // retryAt is the wall time after which a failed login may be attempted again. + retryAt time.Time +} + +type authState int + +const ( + // authUnknown is the startup state: nothing has been probed yet. + authUnknown authState = iota + // authOpen means the node answers without a session (protection disabled, or the auth status + // endpoint does not exist on an old build). Commands forward without a cookie. + authOpen + // authReady means cookie holds a live session. + authReady + // authFailed means a login was attempted and rejected; no command may trigger another until + // retryAt passes. This is the cooldown that keeps a stale password from becoming a lockout. + authFailed + // authNoPassword means the node demands authentication but the agent was configured without a + // password. Retrying cannot help; the operator has to fix the config. + authNoPassword +) + +var ( + // ErrAuthNoPassword is returned when a protected node was given an agent with no password. + ErrAuthNoPassword = errors.New("runtime: node requires authentication but the agent has no password configured") + // ErrAuthRejected is returned while a failed login is inside its cooldown. + ErrAuthRejected = errors.New("runtime: node rejected the agent credential (in login cooldown)") + // ErrAuthUnavailable is returned when the local SimAdmin could not be reached during a probe. + ErrAuthUnavailable = errors.New("runtime: local SimAdmin unreachable during authentication") + + sessionCookieRe = regexp.MustCompile(`simadmin_session=([^;\s,]+)`) +) + +// DefaultLoginCooldown matches the console's 60s auto-login cooldown. Two knobs in two languages +// would drift; this one lives next to the code that enforces it. +const DefaultLoginCooldown = 60 * time.Second + +// AuthConfig configures NodeAuth. Password may be empty: an agent pointed at an unprotected node +// never logs in, and an agent pointed at a protected one fails with ErrAuthNoPassword rather than +// looping. +type AuthConfig struct { + Password string + Cooldown time.Duration + Clock func() time.Time +} + +// NewNodeAuth builds the auth holder over the same loopback client the executor forwards with, so +// the login path cannot dial anywhere a command path could not. +func NewNodeAuth(client *http.Client, target egress.Target, cfg AuthConfig) (*NodeAuth, error) { + if client == nil { + return nil, errors.New("runtime: auth client is nil") + } + clock := cfg.Clock + if clock == nil { + clock = time.Now + } + cooldown := cfg.Cooldown + if cooldown <= 0 { + cooldown = DefaultLoginCooldown + } + return &NodeAuth{ + client: client, + origin: target.Origin(), + password: cfg.Password, + cooldown: cooldown, + clock: clock, + }, nil +} + +// Cookie returns the current session cookie value, or "" when the node needs none. Callers on the +// forwarding path call Ensure first, which is what performs the probe and login. +func (a *NodeAuth) Cookie() string { + a.mu.Lock() + defer a.mu.Unlock() + return a.cookie +} + +// Ensure brings the agent to a usable authentication state, at most one probe/login per call. +// +// It is called before forwarding. A ready or open state returns without touching the network, so +// steady-state commands pay nothing; only an unknown or expired state triggers traffic. +func (a *NodeAuth) Ensure(ctx context.Context) error { + a.mu.Lock() + switch a.state { + case authOpen, authReady: + a.mu.Unlock() + return nil + case authNoPassword: + a.mu.Unlock() + return ErrAuthNoPassword + case authFailed: + if a.clock().Before(a.retryAt) { + a.mu.Unlock() + return ErrAuthRejected + } + // Cooldown elapsed: fall through to a fresh probe. A node whose password was fixed, or + // whose lockout expired, recovers without a restart. + } + a.mu.Unlock() + + authenticated, loginRequired, err := a.probeStatus(ctx) + if err != nil { + return err + } + if authenticated || !loginRequired { + a.mu.Lock() + a.state = authOpen + a.cookie = "" + a.mu.Unlock() + return nil + } + return a.login(ctx) +} + +// Invalidate marks the session unusable after a forwarded request came back unauthenticated, so +// the next Ensure re-probes and (if needed) re-logs in. An agent that never invalidates turns every +// SimAdmin restart into permanent 401s until the agent restarts too. +func (a *NodeAuth) Invalidate() { + a.mu.Lock() + defer a.mu.Unlock() + if a.state == authReady { + a.state = authUnknown + a.cookie = "" + } +} + +// probeStatus answers "does this request need a session, and does ours already work". +// +// The parse mirrors core.js: the payload nests under data on some builds and not others, the +// protection flag is snake_case on some and camelCase on others. The default when the shape is not +// recognised is *unprotected*, and that is deliberate in both directions. A legacy or odd build +// that answers 200 with something this parser cannot read is better served without a cookie, since +// the alternative is the agent demanding a password the node never asked for. Login is triggered +// only when the node explicitly says it wants authentication. +func (a *NodeAuth) probeStatus(ctx context.Context) (authenticated, loginRequired bool, err error) { + request, err := http.NewRequestWithContext(ctx, http.MethodGet, a.origin+"/api/auth/status", nil) + if err != nil { + return false, false, err + } + if a.Cookie() != "" { + request.Header.Set("cookie", a.Cookie()) + } + response, err := a.client.Do(request) + if err != nil { + return false, false, fmt.Errorf("%w: %v", ErrAuthUnavailable, err) + } + defer drain(response) + // 401/403 means the node is protected and our cookie (if any) did not authenticate it. + if response.StatusCode == http.StatusUnauthorized || response.StatusCode == http.StatusForbidden { + return false, true, nil + } + if response.StatusCode < 200 || response.StatusCode >= 300 { + // 404 and friends: an old SimAdmin with no auth endpoints at all is an open node. + return false, false, nil + } + payload, err := io.ReadAll(io.LimitReader(response.Body, 1<<20)) + if err != nil { + return false, false, fmt.Errorf("%w: reading auth status: %v", ErrAuthUnavailable, err) + } + var doc map[string]any + if err := json.Unmarshal(payload, &doc); err != nil { + return false, false, nil + } + auth, _ := doc["data"].(map[string]any) + if auth == nil { + auth = doc + } + if v, ok := auth["authenticated"].(bool); ok { + return v, !v, nil + } + if v, ok := auth["configured"].(bool); ok && !v { + return false, false, nil + } + settings, _ := auth["settings"].(map[string]any) + if settings != nil { + if v, ok := settings["password_protection_enabled"].(bool); ok { + return false, v, nil + } + if v, ok := settings["passwordProtectionEnabled"].(bool); ok { + return false, v, nil + } + } + return false, false, nil +} + +// login performs POST /api/auth/login and stores the session cookie. Failures enter the cooldown; +// the cooldown is the reason a wrong password can be fixed in place rather than requiring a +// reinstall. +func (a *NodeAuth) login(ctx context.Context) error { + if a.password == "" { + a.mu.Lock() + a.state = authNoPassword + a.mu.Unlock() + return ErrAuthNoPassword + } + body := strings.NewReader(`{"password":` + jsonQuote(a.password) + `}`) + request, err := http.NewRequestWithContext(ctx, http.MethodPost, a.origin+"/api/auth/login", body) + if err != nil { + return err + } + request.Header.Set("content-type", "application/json") + request.Header.Set("accept", "application/json") + response, err := a.client.Do(request) + if err != nil { + return fmt.Errorf("%w: %v", ErrAuthUnavailable, err) + } + defer drain(response) + if response.StatusCode < 200 || response.StatusCode >= 300 { + a.mu.Lock() + a.state = authFailed + a.cookie = "" + a.retryAt = a.clock().Add(a.cooldown) + a.mu.Unlock() + return fmt.Errorf("runtime: local SimAdmin rejected the agent password (status %d)", response.StatusCode) + } + match := sessionCookieRe.FindStringSubmatch(response.Header.Get("Set-Cookie")) + if match == nil { + // A 2xx without the session cookie is its own failure mode (a proxy that answered on the + // node's behalf, say). Treat it as a rejection rather than forwarding cookie-less. + a.mu.Lock() + a.state = authFailed + a.retryAt = a.clock().Add(a.cooldown) + a.mu.Unlock() + return errors.New("runtime: login succeeded but set no session cookie") + } + a.mu.Lock() + a.state = authReady + a.cookie = match[0] + a.mu.Unlock() + return nil +} + +func drain(response *http.Response) { + io.Copy(io.Discard, response.Body) + response.Body.Close() +} + +// jsonQuote encodes a string as a JSON string literal without pulling in an encoder for one field. +func jsonQuote(s string) string { + b, _ := json.Marshal(s) + return string(b) +} diff --git a/internal/runtime/auth_test.go b/internal/runtime/auth_test.go new file mode 100644 index 0000000..ba88d22 --- /dev/null +++ b/internal/runtime/auth_test.go @@ -0,0 +1,260 @@ +package runtime + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "strconv" + "sync" + "testing" + "time" + + "fun.chick/msa-agent/internal/egress" +) + +// NodeAuth is tested against a real local server because the property under test is the dialogue +// with a real SimAdmin: status probe, login, cookie attach, 401 invalidation, and the cooldown +// that keeps a wrong password from becoming a lockout. + +type authServer struct { + mu sync.Mutex + requests []string + protected bool + password string + logins int +} + +func (s *authServer) handler() http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + s.mu.Lock() + s.requests = append(s.requests, r.Method+" "+r.URL.Path) + protected := s.protected + password := s.password + loggedIn := r.Header.Get("Cookie") == "simadmin_session=agent-session" + if r.URL.Path == "/api/auth/login" { + s.logins++ + } + s.mu.Unlock() + switch { + case r.URL.Path == "/api/auth/status": + w.Header().Set("Content-Type", "application/json") + if !protected { + w.Write([]byte(`{"data":{"authenticated":true}}`)) + } else if loggedIn { + w.Write([]byte(`{"data":{"authenticated":true}}`)) + } else { + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"data":{"authenticated":false,"settings":{"password_protection_enabled":true}}}`)) + } + case r.URL.Path == "/api/auth/login": + var body struct { + Password string `json:"password"` + } + json.NewDecoder(r.Body).Decode(&body) + if protected && body.Password == password { + w.Header().Set("Set-Cookie", "simadmin_session=agent-session; Path=/; HttpOnly") + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"ok":true}`)) + } else { + w.WriteHeader(http.StatusUnauthorized) + } + case r.URL.Path == "/api/health": + if protected && !loggedIn { + w.WriteHeader(http.StatusUnauthorized) + return + } + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"ok":true}`)) + default: + w.WriteHeader(http.StatusNotFound) + } + }) +} + +func (s *authServer) count(path string) int { + s.mu.Lock() + defer s.mu.Unlock() + n := 0 + for _, r := range s.requests { + if r == "GET "+path || r == "POST "+path { + n++ + } + } + return n +} + +func (s *authServer) loginCount() int { + s.mu.Lock() + defer s.mu.Unlock() + return s.logins +} + +func startAuthServer(t *testing.T, protected bool, password string) (*authServer, int) { + t.Helper() + srv := &authServer{protected: protected, password: password} + server := httptest.NewServer(srv.handler()) + t.Cleanup(server.Close) + parsed, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + port, err := strconv.Atoi(parsed.Port()) + if err != nil { + t.Fatal(err) + } + return srv, port +} + +func testAuth(t *testing.T, port int, password string, clock func() time.Time) (*NodeAuth, *http.Client) { + t.Helper() + target, err := egress.NewTarget(port) + if err != nil { + t.Fatal(err) + } + client, err := egress.NewClient(target, 2*time.Second) + if err != nil { + t.Fatal(err) + } + auth, err := NewNodeAuth(client, target, AuthConfig{Password: password, Clock: clock}) + if err != nil { + t.Fatal(err) + } + return auth, client +} + +func TestUnprotectedNodeNeedsNoLogin(t *testing.T) { + srv, port := startAuthServer(t, false, "") + auth, _ := testAuth(t, port, "", nil) + if err := auth.Ensure(context.Background()); err != nil { + t.Fatalf("Ensure: %v", err) + } + if auth.Cookie() != "" { + t.Fatalf("cookie %q on an open node", auth.Cookie()) + } + if srv.loginCount() != 0 { + t.Fatal("an unprotected node was logged into") + } +} + +func TestProtectedNodeLogsInAndPresentsTheCookie(t *testing.T) { + srv, port := startAuthServer(t, true, "hunter2") + auth, client := testAuth(t, port, "hunter2", nil) + if err := auth.Ensure(context.Background()); err != nil { + t.Fatalf("Ensure: %v", err) + } + if auth.Cookie() == "" { + t.Fatal("no cookie after a successful login") + } + request, _ := http.NewRequest(http.MethodGet, "http://127.0.0.1:"+strconv.Itoa(port)+"/api/health", nil) + request.Header.Set("Cookie", auth.Cookie()) + response, err := client.Do(request) + if err != nil { + t.Fatal(err) + } + defer response.Body.Close() + if response.StatusCode != 200 { + t.Fatalf("health with the agent cookie: %d", response.StatusCode) + } + if srv.count("/api/auth/status") != 1 { + t.Fatalf("status probed %d times, want 1 (steady state is free)", srv.count("/api/auth/status")) + } + // Ensure again must not re-probe or re-login. + if err := auth.Ensure(context.Background()); err != nil { + t.Fatal(err) + } + if srv.count("/api/auth/status") != 1 { + t.Fatal("a ready auth re-probed the status endpoint") + } +} + +func TestAProtectedNodeWithoutAPasswordFailsClosed(t *testing.T) { + srv, port := startAuthServer(t, true, "hunter2") + auth, _ := testAuth(t, port, "", nil) + if err := auth.Ensure(context.Background()); err != ErrAuthNoPassword { + t.Fatalf("got %v, want %v", err, ErrAuthNoPassword) + } + if srv.loginCount() != 0 { + t.Fatal("a login was attempted without a configured password") + } +} + +// The cooldown is the property: a wrong password must not turn every command into a fresh login +// attempt, because some devices lock accounts after repeated failures. +func TestAWrongPasswordEntersTheCooldown(t *testing.T) { + now := time.Unix(1_800_000_000, 0).UTC() + clock := func() time.Time { return now } + srv, port := startAuthServer(t, true, "hunter2") + auth, _ := testAuth(t, port, "wrong", clock) + + if err := auth.Ensure(context.Background()); err == nil { + t.Fatal("Ensure accepted a rejected login") + } + // Repeated Ensures inside the window must not re-attempt the login. + for i := 0; i < 5; i++ { + if err := auth.Ensure(context.Background()); err != ErrAuthRejected { + t.Fatalf("attempt %d: got %v, want %v", i, err, ErrAuthRejected) + } + } + if srv.loginCount() != 1 { + t.Fatalf("login attempted %d times inside the cooldown, want 1", srv.loginCount()) + } + // Past the window it tries once more, so a fixed password recovers without a restart. + now = now.Add(DefaultLoginCooldown + time.Second) + if err := auth.Ensure(context.Background()); err == nil { + t.Fatal("Ensure accepted a rejected login after the cooldown") + } + if srv.loginCount() != 2 { + t.Fatalf("login attempted %d times after the window, want 2", srv.loginCount()) + } +} + +// A SimAdmin restart drops its sessions. Without invalidation every later command 401s until the +// agent restarts too, which is indistinguishable to the user from a broken pairing. +func TestA401FromTheNodeInvalidatesTheSession(t *testing.T) { + srv, port := startAuthServer(t, true, "hunter2") + auth, _ := testAuth(t, port, "hunter2", nil) + if err := auth.Ensure(context.Background()); err != nil { + t.Fatal(err) + } + // Simulate the node losing its session store: logins still work, old cookies do not. + srv.mu.Lock() + srv.password = "hunter2" // unchanged; the cookie check is what fails now + srv.mu.Unlock() + auth.Invalidate() + if err := auth.Ensure(context.Background()); err != nil { + t.Fatalf("Ensure after invalidation: %v", err) + } + if auth.Cookie() == "" { + t.Fatal("the session was not re-established") + } + if srv.loginCount() != 2 { + t.Fatalf("logins: %d, want 2", srv.loginCount()) + } +} + +// Executor-level: a command to a protected node succeeds end to end, and the phone never sees the +// login traffic — it is not on the allowlist and policy rejects it before any dial. +func TestExecutorLogsInBeforeTheFirstCommand(t *testing.T) { + srv, port := startAuthServer(t, true, "hunter2") + pol := testPolicy(t) + executor, err := NewExecutor(pol, Config{ + Port: port, Timeout: 2 * time.Second, Audit: NullAudit(), NodePassword: "hunter2", + }) + if err != nil { + t.Fatal(err) + } + result, refusal := executor.Execute(context.Background(), + envelopeAt("nd_test0001", "f1f1f1f1-f1f1-41f1-81f1-f1f1f1f1f1f1", uint64(time.Now().Unix())), + request("GET", "/api/health", nil)) + if refusal != nil { + t.Fatalf("refused: %v", refusal) + } + if result.Status != 200 { + t.Fatalf("status %d", result.Status) + } + if srv.loginCount() != 1 { + t.Fatalf("logins: %d", srv.loginCount()) + } +} diff --git a/internal/runtime/executor.go b/internal/runtime/executor.go index b5c9a44..3861148 100644 --- a/internal/runtime/executor.go +++ b/internal/runtime/executor.go @@ -40,6 +40,11 @@ const ( ClassTooLarge = "payload_too_large" ClassUpstream = "upstream_error" ClassLoopback = "egress_violation" + // ClassNodeAuth is reported when the agent cannot establish its own session against the local + // SimAdmin. It is distinct from ClassUpstream because the phone's remedy differs: "the node is + // misconfigured" calls for fixing the agent's password, while "the node errored" calls for + // looking at SimAdmin itself. + ClassNodeAuth = "node_auth_failed" ) // Executor turns a decrypted InnerRequest into one loopback HTTP call. @@ -54,6 +59,7 @@ type Executor struct { audit *Audit clock func() time.Time limiter *rateLimiter + auth *NodeAuth } // Config is what the agent needs to build an Executor. Port is a number and nothing else: no host, @@ -64,6 +70,10 @@ type Config struct { Audit *Audit Clock func() time.Time DangerousPerMin int + // NodePassword is the credential the agent uses to log into the local SimAdmin itself. Empty + // means the node is expected to be unprotected; a protected node then surfaces as + // ClassNodeAuth rather than forwarding anonymous requests that will fail. + NodePassword string } // NewExecutor wires the policy, the loopback client and the replay cache together. @@ -101,6 +111,10 @@ func NewExecutor(pol *policy.Policy, cfg Config) (*Executor, error) { if perMinute <= 0 { perMinute = pol.Limits.DangerousWritesPerMinute } + auth, err := NewNodeAuth(client, target, AuthConfig{Password: cfg.NodePassword, Clock: clock}) + if err != nil { + return nil, err + } return &Executor{ client: client, target: target, @@ -109,6 +123,7 @@ func NewExecutor(pol *policy.Policy, cfg Config) (*Executor, error) { audit: cfg.Audit, clock: clock, limiter: newRateLimiter(perMinute, time.Minute), + auth: auth, }, nil } @@ -165,8 +180,11 @@ func (e *Executor) Execute(ctx context.Context, envelope wire.Envelope, inner wi result, err := e.forward(ctx, method, inner.Path, inner.Body) if err != nil { class := ClassUpstream - if errors.Is(err, ErrResponseTooLarge) { + switch { + case errors.Is(err, ErrResponseTooLarge): class = ClassTooLarge + case errors.Is(err, ErrAuthNoPassword), errors.Is(err, ErrAuthRejected), errors.Is(err, ErrAuthUnavailable): + class = ClassNodeAuth } return Result{}, e.refuse(envelope, inner, class, err) } @@ -177,10 +195,17 @@ func (e *Executor) Execute(ctx context.Context, envelope wire.Envelope, inner wi // forward performs the loopback call. The origin comes from egress.Target, which has no host field, // and the path has already survived policy.Decision, so nothing here is assembled from parts that // a caller could steer off the machine. +// +// Authentication is established here, after every phone-facing gate: probing /api/auth/status on a +// request the policy will reject anyway would let a relay-fed caller use rejected commands as a +// login-attempt amplifier against a local lockout. func (e *Executor) forward(ctx context.Context, method, path string, body []byte) (Result, error) { if !strings.HasPrefix(path, "/api/") { return Result{}, ClassLoopbackErr } + if err := e.auth.Ensure(ctx); err != nil { + return Result{}, err + } var reader io.Reader if len(body) > 0 { reader = strings.NewReader(string(body)) @@ -192,13 +217,23 @@ func (e *Executor) forward(ctx context.Context, method, path string, body []byte if len(body) > 0 { request.Header.Set("Content-Type", "application/json") } - // No credential is attached here. The agent logs into the local SimAdmin itself on startup and - // holds the session; a phone cannot use this path to present a node password it does not have. + // The agent's own session, held from a login the phone cannot trigger or see. A node password + // never traverses the relay in either direction. + if cookie := e.auth.Cookie(); cookie != "" { + request.Header.Set("Cookie", cookie) + } response, err := e.client.Do(request) if err != nil { return Result{}, fmt.Errorf("local SimAdmin unreachable: %w", err) } defer response.Body.Close() + if response.StatusCode == http.StatusUnauthorized || response.StatusCode == http.StatusForbidden { + // The session died between Ensure and this call (SimAdmin restarted, cookie expired). + // Drop it so the next command re-probes and re-logs in rather than 401ing until restart. + // This command still answers with the node's refusal; retrying it here would turn every + // genuine 401 from a node endpoint into a login attempt on the phone's behalf. + e.auth.Invalidate() + } limited := io.LimitReader(response.Body, int64(e.pol.Limits.MaxResponseBytes)+1) payload, err := io.ReadAll(limited) if err != nil { diff --git a/internal/runtime/runtime_test.go b/internal/runtime/runtime_test.go index c30f9ad..706a77b 100644 --- a/internal/runtime/runtime_test.go +++ b/internal/runtime/runtime_test.go @@ -172,7 +172,12 @@ func TestAClockedRequestIsClassifiedAsExpiredNotReplay(t *testing.T) { func TestAuthPathsAreUnreachable(t *testing.T) { f := newFixture(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - t.Error("the login endpoint must never be dialed on behalf of a phone") + // The agent's own status probe is allowed here (and answers "unprotected", so no login + // happens); what must never be dialed is a login on behalf of a phone. + if r.URL.Path != "/api/auth/status" { + t.Error("the login endpoint must never be dialed on behalf of a phone") + } + w.WriteHeader(200) })) for index, path := range []string{"/api/auth/login", "/api/login", "/api/logout"} { _, refusal := f.executor.Execute(context.Background(), @@ -189,7 +194,10 @@ func TestAuthPathsAreUnreachable(t *testing.T) { func TestADangerousWriteNeedsAMatchingIntent(t *testing.T) { calls := 0 f := newFixture(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - calls++ + // The auth probe reaches the local service too; only the intended path is counted. + if r.URL.Path == "/api/sms/send" { + calls++ + } w.WriteHeader(200) w.Write([]byte(`{"sent":true}`)) })) @@ -328,10 +336,12 @@ func TestAResponseExactlyAtTheLimitIsForwarded(t *testing.T) { } } -func TestAnUnreachableLocalServiceIsAnUpstreamError(t *testing.T) { +func TestAnUnreachableLocalServiceIsANodeAuthError(t *testing.T) { pol := testPolicy(t) - // A port with nothing on it: the loopback dial fails, which is a node problem rather than a - // policy problem, and the refusal class must say so. + // A port with nothing on it: the authentication probe fails first, which is a node problem + // rather than a policy problem. It is classified as node_auth_failed rather than + // upstream_error because nothing of the phone's command was ever attempted, and the phone's + // remedy ("check that SimAdmin is running on this node") is the operator's, not the user's. executor, err := NewExecutor(pol, Config{Port: 1, Timeout: 500 * time.Millisecond, Audit: NullAudit()}) if err != nil { t.Fatal(err) @@ -339,8 +349,8 @@ func TestAnUnreachableLocalServiceIsAnUpstreamError(t *testing.T) { _, refusal := executor.Execute(context.Background(), envelopeAt("nd_test0001", "eeeeeeee-eeee-4eee-8eee-eeeeeeeeeeee", uint64(time.Now().Unix())), request("GET", "/api/health", nil)) - if refusal == nil || refusal.Class != ClassUpstream { - t.Fatalf("got %v, want %s", refusal, ClassUpstream) + if refusal == nil || refusal.Class != ClassNodeAuth { + t.Fatalf("got %v, want %s", refusal, ClassNodeAuth) } } diff --git a/internal/transport/transport.go b/internal/transport/transport.go index 7e165e3..9c3ad6c 100644 --- a/internal/transport/transport.go +++ b/internal/transport/transport.go @@ -20,6 +20,7 @@ import ( "io" "net/http" "net/url" + "strconv" "strings" "time" ) @@ -236,6 +237,8 @@ func (c *Client) post(ctx context.Context, path string, body any, out any, sessi // 204 and 408 are how the relay says "nothing yet", so the agent loops straight back. return ErrNoWork case http.StatusTooManyRequests: + retry := parseRetryAfter(response.Header.Get("Retry-After")) + c.retryAfter = retry return fmt.Errorf("%w: retry after %s", ErrRateLimited, response.Header.Get("Retry-After")) default: return fmt.Errorf("transport: %s: relay returned %d", path, response.StatusCode) @@ -263,6 +266,16 @@ func (c *Client) post(ctx context.Context, path string, body any, out any, sessi // ErrRateLimited is returned when the relay asks the agent to slow down. var ErrRateLimited = errors.New("transport: rate limited by the relay") +// parseRetryAfter reads an RFC 9110 delay-seconds value. Dates are not supported and fall back to +// the default: a clock-skewed node should not be waiting on an arithmetic date subtraction. +func parseRetryAfter(value string) time.Duration { + seconds, err := strconv.Atoi(strings.TrimSpace(value)) + if err != nil || seconds < 0 || seconds > 3600 { + return 2 * time.Second + } + return time.Duration(seconds) * time.Second +} + // RetryAfter reports how long the agent should wait before its next poll, honouring the relay's // Retry-After when it gave one. func (c *Client) RetryAfter() time.Duration { diff --git a/internal/transport/transport_test.go b/internal/transport/transport_test.go index 98b33ad..0453b32 100644 --- a/internal/transport/transport_test.go +++ b/internal/transport/transport_test.go @@ -213,3 +213,23 @@ func TestRetryAfterHasASaneDefault(t *testing.T) { t.Fatalf("RetryAfter() = %v", got) } } + +// The relay's Retry-After is an instruction, and the loop honours it; a value the agent cannot +// parse falls back to the default rather than becoming an unbounded wait or an immediate retry. +func TestRetryAfterHonoursTheRelayHeader(t *testing.T) { + client, _ := newClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Retry-After", "7") + w.WriteHeader(http.StatusTooManyRequests) + }), "t") + if _, err := client.Poll(context.Background()); !errors.Is(err, ErrRateLimited) { + t.Fatalf("got %v, want ErrRateLimited", err) + } + if got := client.RetryAfter(); got != 7*time.Second { + t.Fatalf("RetryAfter() = %v, want 7s", got) + } + for _, junk := range []string{"", "Wed, 21 Oct 2015 07:28:00 GMT", "-3", "999999"} { + if got := parseRetryAfter(junk); got != 2*time.Second { + t.Errorf("parseRetryAfter(%q) = %v, want the default", junk, got) + } + } +}