| index 3c091e561..4158ee387 100644 |
| --- a/internal/agentsock/agentsock.go |
| +++ b/internal/agentsock/agentsock.go |
| @@ -131,6 +131,13 @@ func (s *Server) guard(fn func(http.ResponseWriter, *http.Request, string)) http |
| // replaced with fresh random bytes, and the comparison then simply cannot |
| // succeed. Refusal is a property of the compare, not of a branch above it. |
| // |
| +// A caller that names no host at all is refused here, not left to WorkerToken. |
| +// Asking a collaborator what "" means is the same mistake in a different |
| +// place: this package would be safe only for as long as the activator's map |
| +// happens to hold no "" key, which is not a property this package can see or |
| +// keep. An unnamed caller has nothing to authorize, so it never reaches the |
| +// lookup — and gets the same 401 as every other refusal. |
| +// |
| // The compare itself is constant-time, and both failure paths — no live token, |
| // wrong live token — run the same code, so the structure carries no timing |
| // signal beyond the length leak inherent to any comparison. |
| @@ -138,7 +145,10 @@ func (s *Server) authorize(r *http.Request) (string, bool) { |
| host := r.Header.Get(hostHeader) |
| presented := bearer(r.Header.Get("Authorization")) |
| |
| - want := s.w.WorkerToken(host) |
| + want := "" |
| + if host != "" { |
| + want = s.w.WorkerToken(host) |
| + } |
| if want == "" { |
| want = unmatchable() |
| } |
| @@ -264,7 +274,8 @@ func displayable(s string) bool { |
| // use" — so a stale file is removed before binding. A file somebody is still |
| // LISTENING on is a different thing entirely: unlinking that would orphan a |
| // live agent, whose children would go on talking to a process nothing else can |
| -// reach. So we probe first, and refuse rather than steal. |
| +// reach. So we probe first, and refuse rather than steal — and, for the same |
| +// reason, this function unlinks nothing it did not create (see below). |
| func (s *Server) Serve(ctx context.Context, path string) error { |
| if listening(path) { |
| return fmt.Errorf("agentsock: %s is already served by a live listener", path) |
| @@ -276,12 +287,25 @@ func (s *Server) Serve(ctx context.Context, path string) error { |
| if err != nil { |
| return fmt.Errorf("agentsock: listen on %s: %w", path, err) |
| } |
| - // Go unlinks a unix socket it created when the listener closes; this is |
| - // the belt to that braces, so no exit path can leave the file behind. |
| - defer func() { _ = os.Remove(path) }() |
| + // Cleanup is the LISTENER's job and nothing else's. Go unlinks a unix |
| + // socket it created when the listener closes, and that unlink happens |
| + // inside Close — indivisibly, from the point of view of a successor |
| + // trying to bind the same path. |
| + // |
| + // A deferred os.Remove(path) here would not be. Between the listener's |
| + // own unlink and the deferred call, a successor agent can have bound the |
| + // path, and we would then unlink ITS socket: the successor serves on an |
| + // inode nothing can reach, its children never arrive, and the box has the |
| + // same split-brain the probe above exists to refuse to cause. Guarding |
| + // one door and leaving the other open is worse than guarding neither, |
| + // because it reads as guarded. So there is no unconditional remove, and |
| + // every return below closes the listener instead. |
| + if ul, ok := ln.(*net.UnixListener); ok { |
| + ul.SetUnlinkOnClose(true) // the default; pinned because it is load-bearing |
| + } |
| |
| if err := os.Chmod(path, socketMode); err != nil { |
| - ln.Close() |
| + ln.Close() // unlinks the socket we just created, and only that one |
| return fmt.Errorf("agentsock: chmod %s: %w", path, err) |
| } |
| |
amadan