diff --git a/internalauth/internalauth.go b/internalauth/internalauth.go new file mode 100644 index 0000000000000000000000000000000000000000..1c9b6d9d3b023f7e0dcc0f8dcf983f7bc6257769 --- /dev/null +++ b/internalauth/internalauth.go @@ -0,0 +1,373 @@ +// Package internalauth is the service-to-service authentication of a SourceHut +// instance: the "Authorization: Internal " header one service +// presents to another, and the check the receiving service runs on it. +// +// Both halves live here, and that is the package rather than a convenience. +// core-go implements the check unexported, inside auth.Middleware, so a service +// that wants the guard without the rest of that middleware — an endpoint whose +// subject comes from the request body, with no session to resolve and no cookie +// to fall back to — writes the thirty lines again. dolt.sr.ht did, in +// web/handlers_internal.go. The program that mints the header for it is not even +// in the same package: it is cmd/dolt-git-hook, git.sr.ht's post-update hook, +// which spells the payload out by hand. So the two ends of one protocol sat in +// two files that shared no type, no constant and no test. Change the payload +// shape, the header scheme or the expiry at one end and nothing fails to +// compile, nothing fails a test, and the symptom is that provisioning quietly +// stops happening on the next push. Here the two ends are two functions over one +// struct, and that change breaks the build on both of them. +// +// The other half of the argument is what a copy loses rather than what it drifts +// from. The check is two checks and both must pass: the source address must be +// inside [sr.ht]internal-ipnet, and the header must be a fernet token sealed +// with the shared [sr.ht]network-key and less than Expiry old. A copy that keeps +// only one of them, or that widens the window while someone is debugging, still +// works — it works for everybody — and nothing about it looks wrong until the +// endpoint is reached from outside. That is not a class of change a code review +// of the fourth copy is going to catch. +// +// Receiving side: +// +// guard := internalauth.Guard("git.sr.ht", "dolt-git-hook", nil) +// mux.Handle("/internal/repos", guard(http.HandlerFunc(a.handleInternalCreate))) +// +// Calling side: +// +// header, err := internalauth.Authorization("git.sr.ht", "dolt-git-hook") +// ... +// req.Header.Set("Authorization", header) +// +// Two process-global preconditions, both core-go's and neither checked here at +// request time. crypto.InitCrypto must have run, or there is no network key to +// seal or open a token with. config.LoadConfig must have run, or the internal +// network list is empty and every address on earth is external — which is the +// safe direction to fail in, but it fails as "source address is not internal" +// for callers that are, which is worth knowing before debugging one. +package internalauth + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "net" + "net/http" + "strings" + "time" + + "sourcecraft.dev/bigbes/sr-ht-core/config" + "sourcecraft.dev/bigbes/sr-ht-core/crypto" +) + +// Scheme is the authorization scheme of this protocol. It is matched +// case-insensitively on the way in, as RFC 7235 requires, and spelled this way +// on the way out. +const Scheme = "Internal" + +// Expiry is how old a token may be. It is core-go's 30 seconds, unchanged, and +// this is the one constant in the package worth not touching. +// +// The window has to cover the clock skew between two hosts of one instance plus +// the latency of a single request, and nothing else: the token is minted for one +// call and is never stored, so there is no legitimate reason for it to be +// presented a minute later. What the window costs is replay — fernet has no +// nonce and this package keeps no seen-token set, so anyone who can read a token +// off the wire can present it again until it ages out. Thirty seconds is short +// enough that this is only reachable by something already inside the internal +// network with the traffic in front of it, and long enough that a peer whose +// clock is a few seconds off still gets through. +// +// Fernet widens this at the other end and there is nothing here that can narrow +// it: its verifier also refuses a token dated more than 60 seconds in the +// future, and accepts everything below that. The real acceptance window is +// therefore [now-Expiry, now+60s], and shortening Expiry does not shorten the +// forward half. +const Expiry = 30 * time.Second + +// The refusals. A caller distinguishes them with errors.Is, and the distinction +// that matters most is the first one against the last: ErrSourceIP means the +// request did not come from the instance at all, ErrPeer means it did, with a +// token this instance's own key sealed, but on behalf of a service this endpoint +// does not serve. The first is somebody knocking; the second is a provisioning +// bug, a stale deployment, or a service calling an endpoint it was not meant to, +// and it wants a different log line and probably a different alert. +var ( + // ErrSourceIP is a request from an address outside [sr.ht]internal-ipnet, or + // from a RemoteAddr that does not parse as an address at all. + ErrSourceIP = errors.New("internalauth: source address is not internal") + + // ErrMissing is a request with no Authorization header, or one that does not + // carry the Internal scheme. It is deliberately not distinguished from a + // Bearer or Basic header: to this endpoint they are all "no internal + // authorization was presented". + ErrMissing = errors.New("internalauth: Internal authorization is required") + + // ErrToken is a token that does not open with the network key: corrupt, + // truncated, sealed with a different key, or older than Expiry. Fernet gives + // one answer for all of those and this package does not invent more — a + // forged token and an expired one are the same event from here, and telling + // a caller which it was is telling an attacker whether they have the key. + ErrToken = errors.New("internalauth: token does not open, or has expired") + + // ErrPayload is a token that opened but does not hold an Auth: not JSON, or + // missing the client or node id. Only a holder of the network key can + // produce one, so it means a peer that is minting the wrong shape, not an + // attacker. + ErrPayload = errors.New("internalauth: token payload is not an internal auth") + + // ErrPeer is a valid, unexpired token from the instance, naming a client or + // node other than the one this endpoint accepts. + ErrPeer = errors.New("internalauth: token names a different caller") + + // ErrNetworkKey is this process, not the request: crypto.InitCrypto has not + // run, so there is no key to seal or open anything with. It is the only + // refusal here that is a 500. + ErrNetworkKey = errors.New("internalauth: network key is not initialised") +) + +// Auth is the token payload — core-go's client.InternalAuth, wire-compatible +// field for field, because the peers on the other end of this are core-go +// services and the format is theirs. +// +// Name is the user the call is made on behalf of, empty for a call that has no +// user yet (core-go calls that anonymous internal auth and uses it for account +// registration and SSH key lookup). Nothing in this package resolves it: a +// custom service's user table is its own business, and the guard's job is to +// establish that the *caller* is a sibling service, not who they are acting for. +// +// ClientID names the calling service ("git.sr.ht") and NodeID the instance of it +// ("dolt-git-hook", or a hostname). Both are required — an internal call that +// cannot say who is making it is refused even when the seal is perfect, which is +// upstream's rule and the reason the mint side refuses to produce one. +// +// core-go's auth.InternalAuth carries a fourth field, oauth_client_id, honoured +// only by meta.sr.ht routes that resolve an OAuth client instead of a user. It +// is deliberately absent: no custom service can act on it, and a field that is +// minted but never read is a field that will one day be trusted by accident. +type Auth struct { + Name string `json:"name,omitempty"` + ClientID string `json:"client_id"` + NodeID string `json:"node_id"` +} + +// Guard refuses a request that is not a sibling service calling in, and passes +// one that is to next with the caller's Auth in its context (see FromContext). +// +// clientID and nodeID are the caller this endpoint accepts; an empty one accepts +// any non-empty value, which is core-go's own behaviour — upstream checks that +// the two fields are present and never that they are anybody in particular. +// Pinning them is this package's addition and the better default for a custom +// service: such a service is typically reachable by exactly one sibling for +// exactly one purpose, and "any service on the instance may drive this endpoint" +// is a decision worth writing down as Guard("", "", …) rather than inheriting. +// +// deny handles the refusal and may be nil, which installs Deny. It is given the +// request with the reason in its context, so a service can log what failed while +// still answering with its own error page; see Reason. +func Guard(clientID, nodeID string, deny http.HandlerFunc) func(http.Handler) http.Handler { + if deny == nil { + deny = Deny + } + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + auth, err := Identify(r, clientID, nodeID) + if err != nil { + deny(w, r.WithContext(context.WithValue(r.Context(), reasonKey, err))) + return + } + next.ServeHTTP(w, r.WithContext(context.WithValue(r.Context(), authKey, auth))) + }) + } +} + +// Verify is the check without the middleware, for a service that routes +// internal calls itself. It is Identify with the caller's identity dropped. +func Verify(r *http.Request, clientID, nodeID string) error { + _, err := Identify(r, clientID, nodeID) + return err +} + +// Identify runs the check and returns who the caller says it is. +// +// The order is fixed and both checks are required — this is core-go's rule and +// the reason to state it here is that the two are not redundant and neither is +// sufficient. The token is the credential: only a service that has the shared +// [sr.ht]network-key can mint one, and it is what actually proves the caller is +// part of the instance. The address check is defence in depth for the case that +// makes the difference — an internal route accidentally published through the +// public ingress — and it is much weaker than it looks, because behind a reverse +// proxy the address it sees is the proxy's, which is internal for every request +// the proxy forwards. That is exactly why it is not allowed to stand alone. +// +// The address is RemoteAddr and never X-Forwarded-For. A forwarded header is +// written by whoever is in front, is trivially set by a client, and honouring it +// would turn the weaker of the two checks into one an outsider can pass by +// asking. core-go reads X-Forwarded-For too, but only to record the route it +// came by, never to decide with. +func Identify(r *http.Request, clientID, nodeID string) (Auth, error) { + host, _, err := net.SplitHostPort(r.RemoteAddr) + if err != nil { + // Not host:port; a bare address is what a unix socket or a test writes. + host = r.RemoteAddr + } + ip := net.ParseIP(host) + if ip == nil { + // core-go panics here. A request whose RemoteAddr does not parse is not + // a programmer error on this side, and 401 is the same answer the next + // line would give it anyway. + return Auth{}, fmt.Errorf("%w: %q does not parse", ErrSourceIP, host) + } + if !config.IsInternalIP(ip) { + return Auth{}, fmt.Errorf("%w: %s", ErrSourceIP, ip) + } + + scheme, token, ok := strings.Cut(r.Header.Get("Authorization"), " ") + if !ok || !strings.EqualFold(scheme, Scheme) { + return Auth{}, ErrMissing + } + payload, err := open([]byte(token)) + if err != nil { + return Auth{}, err + } + if payload == nil { + return Auth{}, ErrToken + } + + var auth Auth + if err := json.Unmarshal(payload, &auth); err != nil { + // core-go panics here as well, on the grounds that a payload it could + // decrypt is one a sibling service wrote. True, and still a 500 handed + // to whoever holds the key: a peer minting the wrong shape takes the + // receiver's handler down with it. Refuse it instead. + return Auth{}, fmt.Errorf("%w: %v", ErrPayload, err) + } + if auth.ClientID == "" || auth.NodeID == "" { + return Auth{}, fmt.Errorf("%w: client_id and node_id are both required", ErrPayload) + } + if clientID != "" && auth.ClientID != clientID { + return Auth{}, fmt.Errorf("%w: client_id is %q, want %q", ErrPeer, auth.ClientID, clientID) + } + if nodeID != "" && auth.NodeID != nodeID { + return Auth{}, fmt.Errorf("%w: node_id is %q, want %q", ErrPeer, auth.NodeID, nodeID) + } + return auth, nil +} + +// Authorization mints the header for the calling side: the whole value, +// "Internal ", ready for r.Header.Set("Authorization", …). +// +// It refuses exactly what Identify refuses — an empty client or node id — so +// that a caller finds out at the call site rather than from a 403 out of a +// service that will not say which field was missing. +func Authorization(clientID, nodeID string) (string, error) { + return AuthorizationAs("", clientID, nodeID) +} + +// AuthorizationAs mints the header for a call made on behalf of a user, which is +// what core-go's client.Do does for every GraphQL call it makes: the username +// travels in the token's name field and the receiving core-go service resolves +// its whole auth context from it. +// +// A custom service calling a core-go one needs this; a custom service calling +// another custom one usually does not, because the guard here does not resolve +// anything from the name. Passing a username the receiver has never heard of is +// not this side's error to catch. +func AuthorizationAs(username, clientID, nodeID string) (string, error) { + if clientID == "" || nodeID == "" { + return "", fmt.Errorf("%w: client_id and node_id are both required", ErrPayload) + } + blob, err := json.Marshal(Auth{Name: username, ClientID: clientID, NodeID: nodeID}) + if err != nil { + return "", fmt.Errorf("%w: %v", ErrPayload, err) + } + token, err := seal(blob) + if err != nil { + return "", err + } + return Scheme + " " + string(token), nil +} + +// seal and open wrap the two core-go crypto calls, whose failure mode with no +// key installed is a nil dereference inside fernet rather than an error. +// +// Recovering it is worth the ugliness because of where the mint side runs: the +// caller in production is a git hook, in a process that has just enough of an +// instance to have loaded a config, and an unconfigured network key there should +// cost a companion database, not the push. The hook already guards its own +// InitCrypto call this way (that one log.Fatals, which recover cannot catch); +// this covers the case where InitCrypto was simply never reached. +func seal(payload []byte) (tok []byte, err error) { + defer func() { + if v := recover(); v != nil { + tok, err = nil, fmt.Errorf("%w: %v", ErrNetworkKey, v) + } + }() + return crypto.Encrypt(payload), nil +} + +func open(tok []byte) (payload []byte, err error) { + defer func() { + if v := recover(); v != nil { + payload, err = nil, fmt.Errorf("%w: %v", ErrNetworkKey, v) + } + }() + return crypto.DecryptWithExpiration(tok, Expiry), nil +} + +// Status maps a refusal to the status code core-go and dolt.sr.ht already answer +// with, so adopting this package changes no response a caller is switching on. +// +// 401 for the two failures that mean nothing was presented — a request from +// outside, or one with no Internal header — and 403 for a presented credential +// that was refused. ErrNetworkKey is the receiver's own misconfiguration and is +// the only 500. Anything unrecognised is 403 rather than 200, so a caller that +// hands this an error it did not come from still refuses. +func Status(err error) int { + switch { + case errors.Is(err, ErrSourceIP), errors.Is(err, ErrMissing): + return http.StatusUnauthorized + case errors.Is(err, ErrNetworkKey): + return http.StatusInternalServerError + default: + return http.StatusForbidden + } +} + +// Deny is the refusal Guard installs when it is given none: the mapped status +// and the reason as plain text. +// +// The reason is safe to return. Every string in it comes from this package or +// from a token that opened with the instance's own key, so the only detail it +// discloses to a stranger is which of the two checks they failed — and the one +// they can reach without the key is the address check, whose answer they already +// know. +func Deny(w http.ResponseWriter, r *http.Request) { + err := Reason(r.Context()) + if err == nil { + err = ErrMissing + } + http.Error(w, err.Error(), Status(err)) +} + +type ctxKey int + +const ( + authKey ctxKey = iota + reasonKey +) + +// FromContext returns the verified caller of the request Guard admitted. It is +// how a handler behind the guard finds out which sibling service called and on +// whose behalf, without parsing the header again. +func FromContext(ctx context.Context) (Auth, bool) { + auth, ok := ctx.Value(authKey).(Auth) + return auth, ok +} + +// Reason returns the refusal Guard is calling a deny handler about, or nil if +// this context is not one. It exists so that a service can supply a deny handler +// that renders its own error page and still log which check failed — the thing +// this protocol is most often debugged by. +func Reason(ctx context.Context) error { + err, _ := ctx.Value(reasonKey).(error) + return err +} diff --git a/internalauth/internalauth_test.go b/internalauth/internalauth_test.go new file mode 100644 index 0000000000000000000000000000000000000000..1aa15d391873ad5ff905180116d583cd864d3dfd --- /dev/null +++ b/internalauth/internalauth_test.go @@ -0,0 +1,484 @@ +package internalauth + +import ( + "crypto/hmac" + "crypto/sha256" + "encoding/base64" + "encoding/binary" + "net" + "net/http" + "net/http/httptest" + "os" + "strings" + "testing" + "testing/fstest" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "sourcecraft.dev/bigbes/sr-ht-core/config" + "sourcecraft.dev/bigbes/sr-ht-core/crypto" + + "sourcecraft.dev/bigbes/sr-ht-ecore/ecoretest" +) + +// The peer these tests pin the guard to: git.sr.ht's post-update hook, which is +// the one real caller of this protocol on the instance today. +const ( + callerClientID = "git.sr.ht" + callerNodeID = "dolt-git-hook" +) + +// Two source addresses, both documentation ranges so that nothing here could +// ever resolve to a real host. internalAddr is RFC 1918, which the config below +// makes internal; externalAddr is TEST-NET-3, which nothing does. +const ( + internalAddr = "10.0.0.5:34512" + externalAddr = "203.0.113.9:44321" +) + +// TestMain installs the two pieces of core-go process state this package reads +// and neither creates. +// +// config.LoadConfig is called for one thing only: it is what fills the internal +// network list that config.IsInternalIP answers from, and without it that list +// is empty and every address is external. The synthetic config deliberately +// carries no [sr.ht]internal-ipnet, so the whole suite runs against the built-in +// default — see TestUnsetInternalIPNetKeepsTheLANDefault, which is the test of +// that fallback. +// +// The keys come from ecoretest rather than from this file, so a test here seals +// with the same network key every other service's tests do. +func TestMain(m *testing.M) { + config.FS = fstest.MapFS{ + "config.ini": &fstest.MapFile{Data: []byte("[sr.ht]\nsite-name=srht.example\n")}, + } + config.LoadConfig() + ecoretest.InitCrypto() + os.Exit(m.Run()) +} + +// --------------------------------------------------------------------------- +// Fixtures +// --------------------------------------------------------------------------- + +// result is everything one guarded request produced: what the caller saw, and — +// through the recording deny handler — which refusal produced it. +type result struct { + code int + body string + auth *Auth + reason error +} + +// call runs one request through a guard pinned to clientID/nodeID. Its deny +// handler records the reason and then delegates to Deny, so every refusing test +// also asserts the status and body a service that supplies no deny handler gets. +func call(t *testing.T, clientID, nodeID, remoteAddr, authorization string) result { + t.Helper() + + var res result + deny := func(w http.ResponseWriter, r *http.Request) { + res.reason = Reason(r.Context()) + Deny(w, r) + } + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + auth, ok := FromContext(r.Context()) + require.True(t, ok, "an admitted request must carry its caller") + res.auth = &auth + w.WriteHeader(http.StatusNoContent) + }) + + req := httptest.NewRequest(http.MethodPost, "/internal/repos", nil) + req.RemoteAddr = remoteAddr + if authorization != "" { + req.Header.Set("Authorization", authorization) + } + + rec := httptest.NewRecorder() + Guard(clientID, nodeID, deny)(next).ServeHTTP(rec, req) + + res.code = rec.Code + res.body = strings.TrimSpace(rec.Body.String()) + return res +} + +// mint is Authorization with the error asserted away. +func mint(t *testing.T, clientID, nodeID string) string { + t.Helper() + header, err := Authorization(clientID, nodeID) + require.NoError(t, err) + return header +} + +// backdate rewrites a minted header's fernet timestamp to age ago and reseals +// it, producing the token a slow replay presents: correctly encrypted, signed +// with the instance's real key, and too old. +// +// It has to reach into the wire format because fernet stamps EncryptAndSign with +// time.Now() and offers no way to say otherwise. The layout is the fernet spec's: +// version byte, 8-byte big-endian unix timestamp, 16-byte IV, ciphertext, and a +// trailing HMAC-SHA256 over everything before it keyed with the first half of +// the network key. +func backdate(t *testing.T, header string, age time.Duration) string { + t.Helper() + + scheme, token, ok := strings.Cut(header, " ") + require.True(t, ok) + raw, err := base64.URLEncoding.DecodeString(token) + require.NoError(t, err) + require.Greater(t, len(raw), 9+sha256.Size) + + binary.BigEndian.PutUint64(raw[1:9], uint64(time.Now().Add(-age).Unix())) + + key, err := base64.URLEncoding.DecodeString(ecoretest.NetworkKey) + require.NoError(t, err) + require.Len(t, key, 32) + mac := hmac.New(sha256.New, key[:16]) + mac.Write(raw[:len(raw)-sha256.Size]) + copy(raw[len(raw)-sha256.Size:], mac.Sum(nil)) + + return scheme + " " + base64.URLEncoding.EncodeToString(raw) +} + +// --------------------------------------------------------------------------- +// The round trip +// --------------------------------------------------------------------------- + +// TestGuardAdmitsAMintedHeader is the whole point of the package in one test: +// what one half produces, the other half accepts, and the handler behind the +// guard learns who called. +func TestGuardAdmitsAMintedHeader(t *testing.T) { + res := call(t, callerClientID, callerNodeID, internalAddr, mint(t, callerClientID, callerNodeID)) + + assert.Equal(t, http.StatusNoContent, res.code) + require.NotNil(t, res.auth) + assert.Equal(t, callerClientID, res.auth.ClientID) + assert.Equal(t, callerNodeID, res.auth.NodeID) + assert.Empty(t, res.auth.Name, "Authorization mints an anonymous internal call") +} + +// TestGuardCarriesTheUserAnInternalCallIsMadeFor covers the other mint: the +// name travels through the seal untouched, which is what a core-go service on +// the far end resolves its auth context from. +func TestGuardCarriesTheUserAnInternalCallIsMadeFor(t *testing.T) { + header, err := AuthorizationAs("bigbes", callerClientID, callerNodeID) + require.NoError(t, err) + + res := call(t, callerClientID, callerNodeID, internalAddr, header) + + require.Equal(t, http.StatusNoContent, res.code) + require.NotNil(t, res.auth) + assert.Equal(t, "bigbes", res.auth.Name) +} + +// TestVerifyAgreesWithTheGuard: the routing-free entry point is the same check, +// so a service that does its own dispatch cannot end up with a weaker one. +func TestVerifyAgreesWithTheGuard(t *testing.T) { + req := httptest.NewRequest(http.MethodPost, "/internal/repos", nil) + req.RemoteAddr = internalAddr + req.Header.Set("Authorization", mint(t, callerClientID, callerNodeID)) + assert.NoError(t, Verify(req, callerClientID, callerNodeID)) + + req.RemoteAddr = externalAddr + assert.ErrorIs(t, Verify(req, callerClientID, callerNodeID), ErrSourceIP) +} + +// --------------------------------------------------------------------------- +// Refusals +// --------------------------------------------------------------------------- + +// TestGuardRefusesAnExpiredToken checks the window from both sides, in seconds +// rather than in terms of Expiry: a test that ages a token by Expiry+5s passes +// for every value of Expiry, which makes it a test of arithmetic instead of a +// test of the window. The 25-second case is the control — backdate reseals the +// token, so a refusal of the 31-second one has to be its age and not the +// rewriting. +func TestGuardRefusesAnExpiredToken(t *testing.T) { + assert.Equal(t, 30*time.Second, Expiry, "the window these ages are chosen around") + + header := mint(t, callerClientID, callerNodeID) + + fresh := call(t, callerClientID, callerNodeID, internalAddr, backdate(t, header, 25*time.Second)) + assert.Equal(t, http.StatusNoContent, fresh.code) + + stale := call(t, callerClientID, callerNodeID, internalAddr, backdate(t, header, 31*time.Second)) + assert.Equal(t, http.StatusForbidden, stale.code) + assert.ErrorIs(t, stale.reason, ErrToken) + assert.Nil(t, stale.auth) +} + +// TestGuardRefusesAnotherCaller is the check core-go does not do: a token this +// instance sealed, unexpired, from an internal address, and still refused +// because it was minted for somebody else. Both fields are pinned separately — +// a second node of the right service is as wrong as a different service. +func TestGuardRefusesAnotherCaller(t *testing.T) { + for _, tc := range []struct { + name string + clientID, nodeID string + }{ + {"different service", "meta.sr.ht", callerNodeID}, + {"different node", callerClientID, "us-east-3.git.sr.ht"}, + {"neither", "builds.sr.ht", "runner-7"}, + } { + t.Run(tc.name, func(t *testing.T) { + res := call(t, callerClientID, callerNodeID, internalAddr, mint(t, tc.clientID, tc.nodeID)) + + assert.Equal(t, http.StatusForbidden, res.code) + assert.ErrorIs(t, res.reason, ErrPeer) + assert.Nil(t, res.auth) + }) + } +} + +// TestGuardAcceptsAnyCallerWhenUnpinned: empty means "any", which is upstream's +// behaviour and what an endpoint several siblings drive asks for explicitly. +func TestGuardAcceptsAnyCallerWhenUnpinned(t *testing.T) { + res := call(t, "", "", internalAddr, mint(t, "builds.sr.ht", "runner-7")) + + require.Equal(t, http.StatusNoContent, res.code) + require.NotNil(t, res.auth) + assert.Equal(t, "builds.sr.ht", res.auth.ClientID) +} + +// TestGuardRefusesANonInternalSource: a perfectly good token presented from +// outside is refused, and refused as 401 — the request never got far enough to +// be a credential decision. +func TestGuardRefusesANonInternalSource(t *testing.T) { + res := call(t, callerClientID, callerNodeID, externalAddr, mint(t, callerClientID, callerNodeID)) + + assert.Equal(t, http.StatusUnauthorized, res.code) + assert.ErrorIs(t, res.reason, ErrSourceIP) + assert.Nil(t, res.auth) +} + +// TestGuardRefusesAForwardedSourceAddress: X-Forwarded-For is written by +// whoever is in front and is not allowed to make an outside request internal. +func TestGuardRefusesAForwardedSourceAddress(t *testing.T) { + var res result + deny := func(w http.ResponseWriter, r *http.Request) { + res.reason = Reason(r.Context()) + Deny(w, r) + } + req := httptest.NewRequest(http.MethodPost, "/internal/repos", nil) + req.RemoteAddr = externalAddr + req.Header.Set("X-Forwarded-For", "10.0.0.5") + req.Header.Set("Authorization", mint(t, callerClientID, callerNodeID)) + + rec := httptest.NewRecorder() + Guard(callerClientID, callerNodeID, deny)(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { + t.Fatal("a forwarded address must not admit anyone") + })).ServeHTTP(rec, req) + + assert.Equal(t, http.StatusUnauthorized, rec.Code) + assert.ErrorIs(t, res.reason, ErrSourceIP) +} + +// TestGuardRefusesAMissingHeader: an internal address on its own admits nobody. +// The IP check is defence in depth, never the credential. +func TestGuardRefusesAMissingHeader(t *testing.T) { + res := call(t, callerClientID, callerNodeID, internalAddr, "") + + assert.Equal(t, http.StatusUnauthorized, res.code) + assert.ErrorIs(t, res.reason, ErrMissing) + assert.Nil(t, res.auth) +} + +// TestGuardRefusesAMalformedHeader walks the shapes a broken or hostile caller +// presents, and pins which side of the taxonomy each lands on: nothing was +// presented (401) against something was presented and refused (403). +func TestGuardRefusesAMalformedHeader(t *testing.T) { + valid := mint(t, callerClientID, callerNodeID) + + for _, tc := range []struct { + name string + header string + want error + code int + }{ + {"no scheme", "gAAAAAAAAAAA", ErrMissing, http.StatusUnauthorized}, + {"scheme only", Scheme, ErrMissing, http.StatusUnauthorized}, + {"another scheme", "Bearer " + valid, ErrMissing, http.StatusUnauthorized}, + {"not base64", Scheme + " ~~~not-a-token~~~", ErrToken, http.StatusForbidden}, + {"empty token", Scheme + " ", ErrToken, http.StatusForbidden}, + {"truncated token", Scheme + " " + tokenOf(valid)[:20], ErrToken, http.StatusForbidden}, + {"tampered token", Scheme + " " + tamper(tokenOf(valid)), ErrToken, http.StatusForbidden}, + } { + t.Run(tc.name, func(t *testing.T) { + res := call(t, callerClientID, callerNodeID, internalAddr, tc.header) + + assert.Equal(t, tc.code, res.code) + assert.ErrorIs(t, res.reason, tc.want) + assert.Nil(t, res.auth) + }) + } +} + +// TestGuardAcceptsAnyCaseOfTheScheme: RFC 7235 makes the scheme token +// case-insensitive, and core-go matches it that way. Pinned so that a parser +// tightened later cannot start refusing a caller spelling it as the RFC allows. +func TestGuardAcceptsAnyCaseOfTheScheme(t *testing.T) { + token := tokenOf(mint(t, callerClientID, callerNodeID)) + + for _, scheme := range []string{"internal", "INTERNAL", "InTeRnAl"} { + res := call(t, callerClientID, callerNodeID, internalAddr, scheme+" "+token) + assert.Equal(t, http.StatusNoContent, res.code, "scheme %q", scheme) + } +} + +// TestGuardRefusesAPayloadThatIsNotAnInternalAuth covers the tokens only a +// holder of the network key can produce: sealed correctly, carrying the wrong +// thing. core-go panics on the first of these; here they are refusals. +func TestGuardRefusesAPayloadThatIsNotAnInternalAuth(t *testing.T) { + for _, tc := range []struct { + name string + payload string + }{ + {"not json", "this is not a payload"}, + {"json but not an object", `["git.sr.ht"]`}, + {"no client id", `{"node_id":"dolt-git-hook"}`}, + {"no node id", `{"client_id":"git.sr.ht"}`}, + {"empty ids", `{"client_id":"","node_id":""}`}, + } { + t.Run(tc.name, func(t *testing.T) { + header := Scheme + " " + string(crypto.Encrypt([]byte(tc.payload))) + res := call(t, "", "", internalAddr, header) + + assert.Equal(t, http.StatusForbidden, res.code) + assert.ErrorIs(t, res.reason, ErrPayload) + assert.Nil(t, res.auth) + }) + } +} + +// --------------------------------------------------------------------------- +// The network list +// --------------------------------------------------------------------------- + +// TestUnsetInternalIPNetKeepsTheLANDefault documents what an instance that never +// configured [sr.ht]internal-ipnet gets: core-go substitutes loopback, the three +// RFC 1918 ranges, unique-local and link-local. So an unset key is not an open +// door, and it is not a closed one either — it is "anything on the LAN", which +// is right for a single-host instance and too wide for a service sharing a +// network with something it does not trust. The suite's whole config is the +// unset case (see TestMain), so this asserts the shape of that default rather +// than installing another one. +func TestUnsetInternalIPNetKeepsTheLANDefault(t *testing.T) { + for _, addr := range []string{"127.0.0.1", "10.0.0.5", "172.16.4.1", "192.168.1.9", "::1", "fe80::1"} { + assert.True(t, config.IsInternalIP(net.ParseIP(addr)), "%s is on the default LAN list", addr) + } + for _, addr := range []string{"203.0.113.9", "8.8.8.8", "2001:db8::1"} { + assert.False(t, config.IsInternalIP(net.ParseIP(addr)), "%s is not internal", addr) + } + + admitted := call(t, callerClientID, callerNodeID, "127.0.0.1:9001", mint(t, callerClientID, callerNodeID)) + assert.Equal(t, http.StatusNoContent, admitted.code) + + refused := call(t, callerClientID, callerNodeID, "8.8.8.8:9001", mint(t, callerClientID, callerNodeID)) + assert.Equal(t, http.StatusUnauthorized, refused.code) + assert.ErrorIs(t, refused.reason, ErrSourceIP) +} + +// TestGuardRefusesAnUnparsableRemoteAddr: core-go panics on this one. A request +// with no usable source address is not a programmer error on the receiving side, +// and it is refused with the same answer an outside address gets. +func TestGuardRefusesAnUnparsableRemoteAddr(t *testing.T) { + res := call(t, callerClientID, callerNodeID, "@", mint(t, callerClientID, callerNodeID)) + + assert.Equal(t, http.StatusUnauthorized, res.code) + assert.ErrorIs(t, res.reason, ErrSourceIP) +} + +// --------------------------------------------------------------------------- +// The mint side, the default deny handler, the taxonomy +// --------------------------------------------------------------------------- + +// TestAuthorizationRefusesAnIncompleteIdentity: the mint refuses exactly what +// the guard would, so the caller finds out at the call site instead of from a +// 403 that will not say which field was missing. +func TestAuthorizationRefusesAnIncompleteIdentity(t *testing.T) { + for _, tc := range []struct{ clientID, nodeID string }{ + {"", callerNodeID}, + {callerClientID, ""}, + {"", ""}, + } { + header, err := Authorization(tc.clientID, tc.nodeID) + assert.ErrorIs(t, err, ErrPayload) + assert.Empty(t, header) + } + + header, err := AuthorizationAs("bigbes", "", "") + assert.ErrorIs(t, err, ErrPayload) + assert.Empty(t, header) +} + +// TestAuthorizationMintsTheWholeHeaderValue: the return value goes straight into +// Header.Set, scheme included — the one detail a caller re-implementing this got +// to choose and would otherwise get wrong in each copy. +func TestAuthorizationMintsTheWholeHeaderValue(t *testing.T) { + header := mint(t, callerClientID, callerNodeID) + + assert.True(t, strings.HasPrefix(header, Scheme+" "), "header is %q", header) + assert.NotEmpty(t, tokenOf(header)) + assert.NotEqual(t, header, mint(t, callerClientID, callerNodeID), + "every mint is a fresh seal: fernet's IV and timestamp are per token") +} + +// TestGuardInstallsTheDefaultDenyHandler: a service that passes nil still gets a +// refusal with a status and a body, not a nil-handler panic. +func TestGuardInstallsTheDefaultDenyHandler(t *testing.T) { + req := httptest.NewRequest(http.MethodPost, "/internal/repos", nil) + req.RemoteAddr = externalAddr + + rec := httptest.NewRecorder() + Guard(callerClientID, callerNodeID, nil)(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { + t.Fatal("refused requests must not reach the handler") + })).ServeHTTP(rec, req) + + assert.Equal(t, http.StatusUnauthorized, rec.Code) + assert.Contains(t, rec.Body.String(), ErrSourceIP.Error()) +} + +// TestStatusSplitsNothingPresentedFromCredentialRefused is the taxonomy a +// caller switches on, pinned so that adopting this package changes no status +// dolt.sr.ht and core-go already answer with. +func TestStatusSplitsNothingPresentedFromCredentialRefused(t *testing.T) { + assert.Equal(t, http.StatusUnauthorized, Status(ErrSourceIP)) + assert.Equal(t, http.StatusUnauthorized, Status(ErrMissing)) + assert.Equal(t, http.StatusForbidden, Status(ErrToken)) + assert.Equal(t, http.StatusForbidden, Status(ErrPayload)) + assert.Equal(t, http.StatusForbidden, Status(ErrPeer)) + assert.Equal(t, http.StatusInternalServerError, Status(ErrNetworkKey)) + + assert.Equal(t, http.StatusForbidden, Status(nil), "no reason at all still refuses") + assert.Equal(t, http.StatusForbidden, Status(net.ErrClosed), "an unrecognised error still refuses") +} + +// TestReasonAndFromContextAreEmptyOffThePath: neither accessor invents an answer +// for a context that never went through the guard. +func TestReasonAndFromContextAreEmptyOffThePath(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/", nil) + + assert.NoError(t, Reason(req.Context())) + auth, ok := FromContext(req.Context()) + assert.False(t, ok) + assert.Equal(t, Auth{}, auth) +} + +// tokenOf strips the scheme from a minted header. +func tokenOf(header string) string { + _, token, _ := strings.Cut(header, " ") + return token +} + +// tamper flips the last byte of a token's HMAC, producing a token that decodes +// and does not verify. +func tamper(token string) string { + raw, err := base64.URLEncoding.DecodeString(token) + if err != nil { + return token + } + raw[len(raw)-1] ^= 0xff + return base64.URLEncoding.EncodeToString(raw) +}