Found reviewing my own change before merging it, and it was load-bearing rather than cosmetic. Priority was read with asPort, which caps at 65535. 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 path scoping and refusal would both have shipped looking complete, passing their tests, and doing nothing on the only case that motivated them. A priority is an ordering and has no range. asWhole takes any whole number the mesh wrote and rejects a non-integral one, which was not meant as a priority. Also: a host may now be routed on some paths and not others, which made the 404 dishonest — it said "no route for this name" while listing that very name as served, a contradiction an operator has to disbelieve the proxy to get past. An uncovered path now says so, and a name that is genuinely not served still lists what is. Two regression tests, both through the proxy rather than against the parser, because the parser was where the bug looked fine.
274 lines
11 KiB
Go
274 lines
11 KiB
Go
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)
|
|
}
|
|
}
|