diff --git a/examples/route-proxy/main.go b/examples/route-proxy/main.go index 2962a85..9b59643 100644 --- a/examples/route-proxy/main.go +++ b/examples/route-proxy/main.go @@ -10,10 +10,31 @@ // program. What lives here is that contract, written as something that runs so it can be read // rather than described. // +// **A route also carries what a request arriving at it may do** (novox/hq ADR 0108). The grant used +// to say only where to send traffic, so this proxy applied nothing; the four things the ingress it +// replaces actually relies on are now part of the contribution. The set is closed at four, because +// an open middleware surface recreates the thing being replaced and is far harder to narrow later +// than a closed one is to widen. +// // What it is given, written by the host from an ordinary declaration: // // $ROUTES every consumer, the name it asked for, and where the mesh says that machine is // +// Each contribution's values carry the name and port as before, and optionally: +// +// path the path prefix this rule is scoped to; absent means every path +// priority which rule wins where two match; higher first, and the order is total +// deny refuse the request outright — the shape an incident mitigation needs +// redirect answer with a permanent redirect to this name, keeping the path and query +// auth the *path of a secret* holding `user:hash` lines, never the credential itself +// +// A host may appear more than once, which is what path scoping means: one rule refusing a path +// while another serves everything else on the same name. +// +// **`auth` names a secret and never holds one.** A declaration carrying a credential is refused +// outright rather than served unprotected, and a secret that cannot be read makes the route refuse +// rather than open — a gate that cannot check is not a gate that opens. +// // It re-reads on change rather than being restarted, for the same reason the provisioner does: // a route arriving or leaving is an ordinary event and must not drop the connections of every // other workload. @@ -23,6 +44,7 @@ import ( "bytes" "context" "crypto/sha256" + "crypto/subtle" "crypto/tls" "crypto/x509" "encoding/hex" @@ -42,6 +64,7 @@ import ( "golang.org/x/crypto/acme" "golang.org/x/crypto/acme/autocert" + "golang.org/x/crypto/bcrypt" ) // Where public certificates come from when nothing says otherwise. @@ -74,7 +97,7 @@ func issuer() string { // to what it may serve. func onlyWhatTheMeshSaid(held *table) autocert.HostPolicy { return func(_ context.Context, host string) error { - if _, known := held.find(host); known { + if held.routed(host) { return nil } return fmt.Errorf("no route for %q in this mesh, so no certificate is asked for", host) @@ -95,48 +118,145 @@ type contribution struct { Values map[string]any `json:"values"` } +// policy is what a rule does with a request that matched it. +// +// **Decided by the mesh, not here** (novox/hq ADR 0108). A route grant used to hand back a name and +// say nothing about what the name admitted, so this proxy admitted everything. The set is closed at +// four — authentication, refusal, path scoping, redirect — because an open middleware surface +// recreates the thing being replaced and is far harder to narrow later than a closed one is to widen. +type policy struct { + // deny refuses the request outright, whatever it is. + deny bool + // redirectTo answers with a permanent redirect instead of proxying. The request's own path and + // query are carried across, which is what canonicalising one public name onto another means. + redirectTo string + // users is what a request must present, read at load time from the secret the declaration + // *named*. A declaration never carries the credential itself. + users map[string]string + // sealed is set when authentication was declared and the secret could not be read. The rule then + // refuses everything and says why. + // + // **Fail closed.** The alternative — serve the route unauthenticated because the gate is + // missing — turns an unreadable file into a silently public admin surface, which is the exact + // outcome ADR 0108 exists to prevent. A gate that cannot check is not a gate that opens. + sealed string +} + +// rule is one way a host may be routed. A host may have several, which is what path scoping means. +type rule struct { + path string // "" matches every path + priority int + policy policy + to *httputil.ReverseProxy + target string +} + // table is what the proxy is currently serving, replaced whole whenever the file changes. // // Replaced rather than merged: the file is the whole truth about who has a route, so merging // would keep serving a name whose module was unassigned — which is the stale-route fault // 08-connectivity lists as open, reintroduced one level down. +// +// Keyed by host to an *ordered* list rather than to one target, because two of the four policies +// need a single host routed more than one way: a refusal on a path the ordinary route also matches, +// and a certificate-challenge path on a host that otherwise serves a workload. type table struct { - mu sync.RWMutex - to map[string]*httputil.ReverseProxy - targets map[string]string + mu sync.RWMutex + to map[string][]rule } -func (t *table) set(routes map[string]string) { - made := map[string]*httputil.ReverseProxy{} - for name, target := range routes { - where, err := url.Parse(target) - if err != nil { - log.Printf("route %s points at %q, which is not a URL: %v", name, target, err) +func (t *table) set(routes map[string][]rule) { + made := map[string][]rule{} + for host, rules := range routes { + kept := make([]rule, 0, len(rules)) + for _, r := range rules { + // A rule that only refuses or only redirects has nowhere to send anything, and needs + // nowhere: it answers by itself. + if r.policy.deny || r.policy.redirectTo != "" { + kept = append(kept, r) + continue + } + where, err := url.Parse(r.target) + if err != nil { + log.Printf("route %s points at %q, which is not a URL: %v", host, r.target, err) + continue + } + r.to = httputil.NewSingleHostReverseProxy(where) + kept = append(kept, r) + } + if len(kept) == 0 { continue } - made[name] = httputil.NewSingleHostReverseProxy(where) + inOrder(kept) + made[host] = kept } t.mu.Lock() - t.to, t.targets = made, routes + t.to = made t.mu.Unlock() } -func (t *table) find(host string) (*httputil.ReverseProxy, bool) { - // The port is not part of the name. A request to app.example:8080 is for app.example. +// inOrder puts the rules for one host into the order they are matched in, and does so totally. +// +// **Equal priorities must resolve identically every time** (ADR 0108). Sorting only by priority +// leaves rules that share one in whatever order the map produced, so the same declaration would +// serve differently between restarts — a proxy that is not reproducible. Longest path first within a +// priority is also the intuitive reading: the more specific rule wins. The last two keys exist only +// to make the order total. +func inOrder(rules []rule) { + sort.SliceStable(rules, func(i, j int) bool { + a, b := rules[i], rules[j] + if a.priority != b.priority { + return a.priority > b.priority + } + if len(a.path) != len(b.path) { + return len(a.path) > len(b.path) + } + if a.path != b.path { + return a.path < b.path + } + return a.target < b.target + }) +} + +// find is the rule that answers this request, or nothing if the host is not routed here at all. +func (t *table) find(host, path string) (rule, bool) { + t.mu.RLock() + defer t.mu.RUnlock() + for _, r := range t.to[bareHost(host)] { + if r.path == "" || strings.HasPrefix(path, r.path) { + return r, true + } + } + return rule{}, false +} + +// routed says whether this proxy serves the name at all, whatever the path. +// +// Separate from find because certificate issuance is a question about the *name*: a host whose only +// rules are path-scoped is still a name this proxy answers to, and still needs a certificate. +func (t *table) routed(host string) bool { + t.mu.RLock() + defer t.mu.RUnlock() + return len(t.to[bareHost(host)]) > 0 +} + +// bareHost is the name without the port, lower-cased. +// +// The port is not part of the name: a request to app.example:8080 is for app.example. Lower-cased +// because a Host header is not case-sensitive, and a route that only answers the spelling in the +// manifest answers half the requests made to it. +func bareHost(host string) string { if h, _, err := net.SplitHostPort(host); err == nil { host = h } - t.mu.RLock() - defer t.mu.RUnlock() - p, ok := t.to[strings.ToLower(host)] - return p, ok + return strings.ToLower(host) } func (t *table) names() []string { t.mu.RLock() defer t.mu.RUnlock() - out := make([]string, 0, len(t.targets)) - for name := range t.targets { + out := make([]string, 0, len(t.to)) + for name := range t.to { out = append(out, name) } sort.Strings(out) @@ -285,29 +405,115 @@ func forThisAuthority(cache, directory string, root []byte) string { // newTable is an empty routing table. func newTable() *table { - return &table{to: map[string]*httputil.ReverseProxy{}, targets: map[string]string{}} + return &table{to: map[string][]rule{}} } // handler is the proxy itself, separated so it can be driven by a test without a listener. func handler(held *table) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - proxy, known := held.find(r.Host) + matched, known := held.find(r.Host, r.URL.Path) if !known { // **Named, not a bare 404.** A route that was withdrawn and a name that never existed // are different things, and a proxy that says only "not found" makes an operator go // and read the mesh to tell them apart. What it is serving is the answer to both. + // + // And since a host may now be routed only on some paths, those are a third thing: + // saying "no route for this name" while listing that very name as served is a + // contradiction an operator would have to disbelieve the proxy to get past. w.Header().Set("Content-Type", "text/plain; charset=utf-8") w.WriteHeader(http.StatusNotFound) + if held.routed(r.Host) { + fmt.Fprintf(w, "%s is served here, but no route covers %q.\n", + bareHost(r.Host), r.URL.Path) + return + } fmt.Fprintf(w, "no route for %q in this mesh.\nserving: %s\n", r.Host, strings.Join(held.names(), ", ")) return } - proxy.ServeHTTP(w, r) + + switch { + case matched.policy.sealed != "": + // Declared a gate, cannot check it. Refused, and says why — an operator reading this + // learns the secret is missing, rather than wondering why a protected name is 503. + w.Header().Set("Content-Type", "text/plain; charset=utf-8") + w.WriteHeader(http.StatusServiceUnavailable) + fmt.Fprintf(w, "this route requires authentication and its credentials cannot be read: %s\n", + matched.policy.sealed) + return + + case matched.policy.deny: + http.Error(w, "this path is not served to you", http.StatusForbidden) + return + + case matched.policy.redirectTo != "": + http.Redirect(w, r, canonical(matched.policy.redirectTo, r.URL), http.StatusMovedPermanently) + return + + case len(matched.policy.users) > 0 && !allowed(matched.policy.users, r): + // The realm is the name asked for, so a browser's prompt says which route it is for. + w.Header().Set("WWW-Authenticate", fmt.Sprintf("Basic realm=%q, charset=\"UTF-8\"", bareHost(r.Host))) + http.Error(w, "unauthorized", http.StatusUnauthorized) + return + } + + matched.to.ServeHTTP(w, r) }) } -// routesFrom reads what the mesh wrote and turns it into name → target. -func routesFrom(path string) (map[string]string, error) { +// canonical is where a redirect sends this request. +// +// The declaration names the destination *name*; the request keeps its own path and query. That is +// what canonicalising one public name onto another means — a link to a page under the old name has +// to arrive at the same page under the new one, or the redirect silently loses every deep link. +func canonical(to string, from *url.URL) string { + where, err := url.Parse(to) + if err != nil { + return to + } + if where.Path == "" || where.Path == "/" { + where.Path = from.Path + } + if where.RawQuery == "" { + where.RawQuery = from.RawQuery + } + return where.String() +} + +// allowed says whether the request presented credentials this route accepts. +// +// **Every path costs one bcrypt comparison**, including an unknown user, which is why the miss +// compares against a fixed hash rather than returning early. Returning early would make an unknown +// user measurably faster than a known one with a wrong password, and that difference is a way to +// enumerate the users of a route from outside it. +func allowed(users map[string]string, r *http.Request) bool { + // A hash of nothing anybody knows. Its only job is to cost what a real comparison costs. + const absent = "$2a$10$N9qo8uLOickgx2ZMRZoMyeIjZAgcfl7p92ldGxad68LJZdL17lhWy" + user, password, ok := r.BasicAuth() + if !ok { + return false + } + want, known := users[user] + if !known { + want = absent + } + if err := bcrypt.CompareHashAndPassword([]byte(want), []byte(password)); err != nil { + return false + } + // `known` is checked after the comparison, not instead of it, so the timing is the same either + // way. subtle.ConstantTimeByteEq keeps the branch from being the thing that differs. + return subtle.ConstantTimeByteEq(boolByte(known), 1) == 1 +} + +func boolByte(b bool) byte { + if b { + return 1 + } + return 0 +} + +// routesFrom reads what the mesh wrote and turns it into host → the rules for that host. +func routesFrom(path string) (map[string][]rule, error) { raw, err := os.ReadFile(path) if err != nil { return nil, err @@ -317,31 +523,134 @@ func routesFrom(path string) (map[string]string, error) { return nil, err } - out := map[string]string{} + out := map[string][]rule{} for _, c := range said.Given { name, _ := c.Values["name"].(string) if name == "" { log.Printf("%s on %s asked for a route and named nothing; skipped", c.From, c.Node) continue } - port, ok := asPort(c.Values["port"]) - if !ok { - log.Printf("%s on %s asked for route %q and gave no usable port; skipped", - c.From, c.Node, name) - continue + host := strings.ToLower(name) + + made := rule{path: asPath(c.Values["path"])} + if p, ok := asWhole(c.Values["priority"]); ok { + made.priority = p } - // Where the mesh says that machine is. Empty means it is this one — a workload beside the - // proxy is ordinary, and reaching it over loopback is both correct and the only thing - // that works when there is no private network. - at := c.At - if at == "" { - at = "127.0.0.1" + made.policy.deny, _ = c.Values["deny"].(bool) + made.policy.redirectTo, _ = c.Values["redirect"].(string) + + if named, carried := c.Values["auth"].(string); carried && strings.TrimSpace(named) != "" { + // **A declaration names a secret; it never holds one** (ADR 0108). Refused rather than + // tolerated, and the whole rule is dropped rather than served unprotected — the + // rejected option cannot come back by accident, which is the failure this check exists + // to make impossible. + if looksLikeACredential(named) { + log.Printf("%s on %s declared route %q with a credential in the declaration rather "+ + "than the name of a secret; the whole route is refused (novox/hq ADR 0108)", + c.From, c.Node, name) + continue + } + users, err := usersFrom(named) + if err != nil { + // Fail closed: the rule is kept so the name stays routed and answers, and it + // answers by refusing. Dropping it instead would make the name 404 and read as a + // withdrawn route rather than an unreadable secret. + made.policy.sealed = err.Error() + } + made.policy.users = users } - out[strings.ToLower(name)] = fmt.Sprintf("http://%s:%d", at, port) + + // Only a rule that actually proxies needs somewhere to send the request. + if !made.policy.deny && made.policy.redirectTo == "" { + port, ok := asPort(c.Values["port"]) + if !ok { + log.Printf("%s on %s asked for route %q and gave no usable port; skipped", + c.From, c.Node, name) + continue + } + // Where the mesh says that machine is. Empty means it is this one — a workload beside + // the proxy is ordinary, and reaching it over loopback is both correct and the only + // thing that works when there is no private network. + at := c.At + if at == "" { + at = "127.0.0.1" + } + made.target = fmt.Sprintf("http://%s:%d", at, port) + } + + out[host] = append(out[host], made) } return out, nil } +// asWhole is any whole number the mesh wrote, whatever its magnitude. +// +// **Not asPort.** Priority was read with the port reader first, which caps at 65535 — so a rule +// declared at a priority above that silently became priority 0 and stopped shadowing the route it +// exists to shadow. The one real rule this has to reproduce is declared at 100000, so the bug was +// exactly load-bearing. A priority is an ordering, not a port: it has no range. +func asWhole(v any) (int, bool) { + switch n := v.(type) { + case float64: + // JSON makes a float of every number, so a non-integral one was not meant as a priority. + if n != float64(int(n)) { + return 0, false + } + return int(n), true + case int: + return n, true + } + return 0, false +} + +// asPath is the path prefix a rule is scoped to, or "" for every path. +func asPath(v any) string { + p, _ := v.(string) + p = strings.TrimSpace(p) + if p == "" { + return "" + } + if !strings.HasPrefix(p, "/") { + p = "/" + p + } + return p +} + +// looksLikeACredential is the check that keeps a secret out of a declaration. +// +// It errs towards refusing: a value holding a `:` (the htpasswd separator) or opening with a bcrypt +// identifier is a credential, not a path, and no filesystem path the mesh writes needs either. A +// false refusal is a loud log and a route that does not serve; a false accept is a credential +// committed to a declaration, which is the thing being prevented. +func looksLikeACredential(v string) bool { + v = strings.TrimSpace(v) + return strings.Contains(v, ":") || strings.HasPrefix(v, "$2") +} + +// usersFrom reads the credentials the mesh mounted, in the one format every htpasswd already is. +func usersFrom(path string) (map[string]string, error) { + raw, err := os.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("cannot read the secret named for this route: %w", err) + } + users := map[string]string{} + for _, line := range strings.Split(string(raw), "\n") { + line = strings.TrimSpace(line) + if line == "" || strings.HasPrefix(line, "#") { + continue + } + user, hash, ok := strings.Cut(line, ":") + if !ok || user == "" || hash == "" { + continue + } + users[user] = hash + } + if len(users) == 0 { + return nil, fmt.Errorf("the secret named for this route holds no usable credentials") + } + return users, nil +} + // asPort accepts what JSON makes of a number, which is a float even when it was written 8080. func asPort(v any) (int, bool) { switch n := v.(type) { diff --git a/examples/route-proxy/policy_test.go b/examples/route-proxy/policy_test.go new file mode 100644 index 0000000..db1dc0b --- /dev/null +++ b/examples/route-proxy/policy_test.go @@ -0,0 +1,273 @@ +package main + +import ( + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strconv" + "strings" + "testing" + + "golang.org/x/crypto/bcrypt" +) + +// What a route carries about the requests arriving at it — novox/hq ADR 0108. +// +// Each test here is one of the four capabilities that record closed the set at, plus the negative +// case it promised would be refused. The negative case is the one that rots quietly: nothing fails +// if it stops working, so nothing tells you it has. + +// served starts a workload and gives back the host and port the mesh would have recorded for it. +func served(t *testing.T, body string) (string, int) { + t.Helper() + workload := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(body)) + })) + t.Cleanup(workload.Close) + host, port, _ := strings.Cut(strings.TrimPrefix(workload.URL, "http://"), ":") + n, err := strconv.Atoi(port) + if err != nil { + t.Fatal(err) + } + return host, n +} + +// ask makes one request through the proxy for a given name and path, without following redirects. +func ask(t *testing.T, proxy, name, path string, auth [2]string) *http.Response { + t.Helper() + req, err := http.NewRequest(http.MethodGet, proxy+path, nil) + if err != nil { + t.Fatal(err) + } + req.Host = name + if auth[0] != "" { + req.SetBasicAuth(auth[0], auth[1]) + } + client := &http.Client{CheckRedirect: func(*http.Request, []*http.Request) error { + return http.ErrUseLastResponse + }} + answer, err := client.Do(req) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = answer.Body.Close() }) + return answer +} + +func proxyFor(t *testing.T, routesJSON string) string { + t.Helper() + path := filepath.Join(t.TempDir(), "routes.json") + if err := os.WriteFile(path, []byte(routesJSON), 0o644); err != nil { + t.Fatal(err) + } + routes, err := routesFrom(path) + if err != nil { + t.Fatal(err) + } + held := newTable() + held.set(routes) + server := httptest.NewServer(handler(held)) + t.Cleanup(server.Close) + return server.URL +} + +// A refusal on a path shadows the ordinary route for that path and leaves every other path alone. +// +// **This is why path scoping is a prerequisite and not a sibling capability.** The rule being +// reproduced matches a path on a host that is already routed to a workload, so a table mapping a +// host to one target cannot express it at all — no amount of authentication or source filtering +// would have helped. +func TestARefusedPathShadowsTheRouteAndLeavesTheRestServed(t *testing.T) { + at, port := served(t, "the workload") + proxy := proxyFor(t, `{"given":[ + {"from":"forge","node":"anchor","at":"`+at+`","values":{"name":"forge.example","port":`+strconv.Itoa(port)+`}}, + {"from":"forge","node":"anchor","values":{"name":"forge.example","path":"/api/internal","priority":100,"deny":true}} + ]}`) + + if got := ask(t, proxy, "forge.example", "/api/internal/hook", [2]string{}).StatusCode; got != http.StatusForbidden { + t.Fatalf("the refused path answered %d, so the block that was put in front of it during an "+ + "incident is not in front of it any more", got) + } + if got := ask(t, proxy, "forge.example", "/", [2]string{}).StatusCode; got != http.StatusOK { + t.Fatalf("refusing one path took the whole route with it: %d", got) + } +} + +// A redirect answers with the redirect, and the request keeps its own path and query. +// +// Losing the path would turn canonicalising one name onto another into "every deep link now lands +// on the front page", which is the kind of breakage that produces no error anywhere. +func TestARedirectKeepsThePathAndQuery(t *testing.T) { + proxy := proxyFor(t, `{"given":[ + {"from":"site","node":"anchor","values":{"name":"www.example","redirect":"https://example/"}} + ]}`) + + answer := ask(t, proxy, "www.example", "/deep/page?ref=1", [2]string{}) + if answer.StatusCode != http.StatusMovedPermanently { + t.Fatalf("a declared redirect answered %d", answer.StatusCode) + } + where := answer.Header.Get("Location") + if !strings.Contains(where, "/deep/page") || !strings.Contains(where, "ref=1") { + t.Fatalf("the redirect dropped the path or the query: %q", where) + } +} + +// Authentication refuses a request with no credentials, admits one with the right ones, and refuses +// the wrong ones — with the credentials read from the secret the declaration *named*. +func TestAuthenticationAdmitsOnlyWhatTheSecretSays(t *testing.T) { + at, port := served(t, "the console") + hash, err := bcrypt.GenerateFromPassword([]byte("correct horse"), bcrypt.MinCost) + if err != nil { + t.Fatal(err) + } + secret := filepath.Join(t.TempDir(), "console-auth") + if err := os.WriteFile(secret, []byte("# a comment\nadmin:"+string(hash)+"\n"), 0o600); err != nil { + t.Fatal(err) + } + + proxy := proxyFor(t, `{"given":[ + {"from":"console","node":"anchor","at":"`+at+`","values":{"name":"console.example","port":`+strconv.Itoa(port)+`,"auth":"`+secret+`"}} + ]}`) + + if got := ask(t, proxy, "console.example", "/", [2]string{}).StatusCode; got != http.StatusUnauthorized { + t.Fatalf("an admin surface with no login of its own answered %d without credentials", got) + } + if got := ask(t, proxy, "console.example", "/", [2]string{"admin", "wrong"}).StatusCode; got != http.StatusUnauthorized { + t.Fatalf("the wrong password answered %d", got) + } + if got := ask(t, proxy, "console.example", "/", [2]string{"admin", "correct horse"}).StatusCode; got != http.StatusOK { + t.Fatalf("the right password answered %d", got) + } +} + +// The negative case ADR 0108 promised would be refused: a credential in the declaration. +// +// **Refused whole, not tolerated and not served unprotected.** A hash carried in a declaration was +// the rejected option; nothing in the running system should quietly accept it later, because the +// precedent is far easier to set than to withdraw. If this test is deleted the option returns and +// nothing else notices. +func TestACredentialInTheDeclarationIsRefusedRatherThanServed(t *testing.T) { + inline := []string{ + `{"given":[{"from":"c","node":"n","at":"127.0.0.1","values":{"name":"c.example","port":8080,"auth":"admin:$2a$10$abcdefghijklmnopqrstuv"}}]}`, + `{"given":[{"from":"c","node":"n","at":"127.0.0.1","values":{"name":"c.example","port":8080,"auth":"$2a$10$abcdefghijklmnopqrstuv"}}]}`, + } + for _, body := range inline { + path := filepath.Join(t.TempDir(), "routes.json") + if err := os.WriteFile(path, []byte(body), 0o644); err != nil { + t.Fatal(err) + } + routes, err := routesFrom(path) + if err != nil { + t.Fatal(err) + } + if len(routes) != 0 { + t.Fatalf("a declaration carrying a credential was served anyway: %v", routes) + } + } +} + +// Authentication declared, secret unreadable: the route refuses. It does not serve unprotected. +// +// **Fail closed.** The alternative turns a missing file into a silently public admin surface, which +// is the outcome the whole record exists to prevent. It answers rather than 404s, so an operator +// sees "cannot read the credentials" instead of concluding the route was withdrawn. +func TestAnUnreadableSecretFailsClosed(t *testing.T) { + at, port := served(t, "the console") + missing := filepath.Join(t.TempDir(), "not-mounted") + + proxy := proxyFor(t, `{"given":[ + {"from":"console","node":"anchor","at":"`+at+`","values":{"name":"console.example","port":`+strconv.Itoa(port)+`,"auth":"`+missing+`"}} + ]}`) + + answer := ask(t, proxy, "console.example", "/", [2]string{}) + if answer.StatusCode == http.StatusOK { + t.Fatal("a route whose credentials could not be read served the workload unprotected") + } + if answer.StatusCode != http.StatusServiceUnavailable { + t.Fatalf("expected the route to say it cannot check, got %d", answer.StatusCode) + } +} + +// Equal priorities resolve the same way every time, so the same declaration serves the same way +// after a restart. +// +// Sorting only by priority leaves rules that share one in whatever order the map produced. The +// proxy would still work, and would work differently between restarts — which is the hardest kind +// of fault to believe when it is reported. +func TestRulesThatShareAPriorityAreStillTotallyOrdered(t *testing.T) { + first := []rule{ + {path: "/a", priority: 10, target: "http://x:1"}, + {path: "/bb", priority: 10, target: "http://y:2"}, + {path: "", priority: 10, target: "http://z:3"}, + } + second := []rule{ + {path: "", priority: 10, target: "http://z:3"}, + {path: "/bb", priority: 10, target: "http://y:2"}, + {path: "/a", priority: 10, target: "http://x:1"}, + } + inOrder(first) + inOrder(second) + for i := range first { + if first[i].path != second[i].path || first[i].target != second[i].target { + t.Fatalf("two orderings of the same rules disagree at %d: %q vs %q", + i, first[i].path, second[i].path) + } + } + // And the more specific rule is matched first, which is the intuitive reading. + if first[0].path != "/bb" { + t.Fatalf("the longest path is not matched first: %q", first[0].path) + } +} + +// Priority decides before path length does, so a rule can be made to win regardless of specificity. +func TestPriorityOutranksPathLength(t *testing.T) { + rules := []rule{ + {path: "/very/long/path", priority: 1, target: "http://x:1"}, + {path: "", priority: 100, target: "http://y:2"}, + } + inOrder(rules) + if rules[0].priority != 100 { + t.Fatalf("a higher priority did not win: %+v", rules[0]) + } +} + +// A priority above a port number survives, because a priority is an ordering and not a port. +// +// **Found by review, and it was load-bearing.** Priority was first read with the port reader, which +// caps at 65535 — so a rule declared above that silently became priority 0 and stopped shadowing the +// route it exists to shadow. The one real rule this has to reproduce is declared at 100000, so the +// capability would have shipped looking complete and doing nothing. +func TestAPriorityAboveAPortNumberSurvives(t *testing.T) { + at, port := served(t, "the workload") + proxy := proxyFor(t, `{"given":[ + {"from":"forge","node":"anchor","at":"`+at+`","values":{"name":"forge.example","port":`+strconv.Itoa(port)+`}}, + {"from":"forge","node":"anchor","values":{"name":"forge.example","path":"/api/internal","priority":100000,"deny":true}} + ]}`) + + if got := ask(t, proxy, "forge.example", "/api/internal/hook", [2]string{}).StatusCode; got != http.StatusForbidden { + t.Fatalf("a rule declared at priority 100000 answered %d instead of refusing", got) + } +} + +// A host routed only on some paths says so, rather than claiming the name is not served here. +// +// Saying "no route for this name" while listing that very name as served is a contradiction an +// operator has to disbelieve the proxy to get past — and path scoping makes it reachable, because a +// host can now have rules that none of this request's paths match. +func TestAHostRoutedOnlyOnSomePathsSaysSo(t *testing.T) { + proxy := proxyFor(t, `{"given":[ + {"from":"forge","node":"anchor","values":{"name":"forge.example","path":"/api/internal","deny":true}} + ]}`) + + answer := ask(t, proxy, "forge.example", "/elsewhere", [2]string{}) + if answer.StatusCode != http.StatusNotFound { + t.Fatalf("an uncovered path answered %d", answer.StatusCode) + } + body := make([]byte, 256) + n, _ := answer.Body.Read(body) + said := string(body[:n]) + if !strings.Contains(said, "is served here") || !strings.Contains(said, "/elsewhere") { + t.Fatalf("the refusal does not distinguish an uncovered path from an unserved name: %q", said) + } +} diff --git a/examples/route-proxy/routes_test.go b/examples/route-proxy/routes_test.go index 6740569..c4d6ef2 100644 --- a/examples/route-proxy/routes_test.go +++ b/examples/route-proxy/routes_test.go @@ -19,6 +19,23 @@ func write(t *testing.T, body string) string { return path } +// plain is the table an ordinary set of routes makes: one host, one target, no policy. +func plain(routes map[string]string) map[string][]rule { + out := map[string][]rule{} + for host, target := range routes { + out[host] = []rule{{target: target}} + } + return out +} + +// targetOf is where a host's first matching rule sends a request. +func targetOf(routes map[string][]rule, host string) string { + if rules := routes[host]; len(rules) > 0 { + return rules[0].target + } + return "" +} + // A route is a grant: the consumer supplies a target, and where that machine is comes from the // mesh rather than from a naming convention the proxy has to know. func TestARouteGoesToWhereTheMeshSaysTheConsumerIs(t *testing.T) { @@ -30,7 +47,7 @@ func TestARouteGoesToWhereTheMeshSaysTheConsumerIs(t *testing.T) { } // Lower-cased, because a Host header is not case-sensitive and a route that only answers the // spelling in the manifest answers half the requests made to it. - if routes["app.example"] != "http://laptop.internal:8080" { + if targetOf(routes, "app.example") != "http://laptop.internal:8080" { t.Fatalf("the route does not point at the consumer: %v", routes) } } @@ -44,7 +61,7 @@ func TestAConsumerOnTheProxysOwnMachineIsReachedOverLoopback(t *testing.T) { if err != nil { t.Fatal(err) } - if routes["app.example"] != "http://127.0.0.1:9000" { + if targetOf(routes, "app.example") != "http://127.0.0.1:9000" { t.Fatalf("a workload on this machine was not reachable: %v", routes) } } @@ -59,7 +76,7 @@ func TestAContributionMissingWhatARouteNeedsIsSkipped(t *testing.T) { if err != nil { t.Fatal(err) } - if len(routes) != 1 || routes["fine.example"] == "" { + if len(routes) != 1 || targetOf(routes, "fine.example") == "" { t.Fatalf("an unusable contribution was served: %v", routes) } } @@ -75,7 +92,7 @@ func TestTheProxyReachesTheWorkloadAndNamesWhatItServes(t *testing.T) { host, port, _ := strings.Cut(target, ":") held := newTable() - held.set(map[string]string{"app.example": "http://" + host + ":" + port}) + held.set(plain(map[string]string{"app.example": "http://" + host + ":" + port})) proxy := httptest.NewServer(handler(held)) defer proxy.Close() @@ -120,16 +137,16 @@ func TestTheProxyReachesTheWorkloadAndNamesWhatItServes(t *testing.T) { // nothing fails more visibly than a stale grant, which is exactly why it must not survive. func TestWithdrawingARouteStopsServingIt(t *testing.T) { held := newTable() - held.set(map[string]string{ + held.set(plain(map[string]string{ "going.example": "http://a.internal:80", "staying.example": "http://b.internal:80", - }) - held.set(map[string]string{"staying.example": "http://b.internal:80"}) + })) + held.set(plain(map[string]string{"staying.example": "http://b.internal:80"})) - if _, still := held.find("going.example"); still { + if _, still := held.find("going.example", "/"); still { t.Fatal("a route whose module was unassigned is still served") } - if _, kept := held.find("staying.example"); !kept { + if _, kept := held.find("staying.example", "/"); !kept { t.Fatal("withdrawing one route took another with it") } } @@ -137,8 +154,8 @@ func TestWithdrawingARouteStopsServingIt(t *testing.T) { // A Host header carries a port and the name does not. func TestARequestNamingAPortStillFindsItsRoute(t *testing.T) { held := newTable() - held.set(map[string]string{"app.example": "http://a.internal:8080"}) - if _, found := held.find("app.example:8080"); !found { + held.set(plain(map[string]string{"app.example": "http://a.internal:8080"})) + if _, found := held.find("app.example:8080", "/"); !found { t.Fatal("a request to app.example:8080 did not find the route for app.example") } } @@ -168,7 +185,7 @@ func TestTheIssuerIsStagingUnlessNamed(t *testing.T) { // rate limit — and the proxy would look healthy throughout. func TestNoCertificateIsAskedForOnAnUnroutedName(t *testing.T) { held := newTable() - held.set(map[string]string{"photos.example": "http://127.0.0.1:8080"}) + held.set(plain(map[string]string{"photos.example": "http://127.0.0.1:8080"})) policy := onlyWhatTheMeshSaid(held) if err := policy(context.Background(), "photos.example"); err != nil { @@ -184,7 +201,7 @@ func TestNoCertificateIsAskedForOnAnUnroutedName(t *testing.T) { // A route withdrawn stops being certifiable, without the proxy restarting. func TestWithdrawingARouteWithdrawsItsCertificate(t *testing.T) { held := newTable() - held.set(map[string]string{"photos.example": "http://127.0.0.1:8080"}) + held.set(plain(map[string]string{"photos.example": "http://127.0.0.1:8080"})) policy := onlyWhatTheMeshSaid(held) if err := policy(context.Background(), "photos.example"); err != nil { t.Fatal(err)