diff --git a/docs/configuration.md b/docs/configuration.md index 4a173d5..118f7cc 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -92,6 +92,20 @@ Any error of the cluster shows as the cluster wrote it, without the response code around it: `job not found`, not `Unexpected response code: 404 (job not found)`. +The token goes with every request. Over plain HTTP, anyone on the network +between you and the cluster can read it. So when a token is set and the +address starts with `http://` and is not this machine (`localhost`, +`127.0.0.0/8` or `::1`), the status line warns once the cluster first answers, +and again after each switch of cluster: + +``` +! the token goes to http://nomad.example:4646 without TLS +``` + +The warning does not replace an error, another warning or a command that is +still running: it shows with a later answer. To stop it, use an `https://` +address. + ## Newer releases When a newer release of urga is out, the header says so next to the version: diff --git a/internal/nomad/client.go b/internal/nomad/client.go index 30fa297..131c46e 100644 --- a/internal/nomad/client.go +++ b/internal/nomad/client.go @@ -4,7 +4,9 @@ package nomad import ( "context" + "net" "net/http" + "net/url" "slices" "time" @@ -61,6 +63,8 @@ type TLS struct { type Client struct { api *api.Client region string + + tokenInClear bool } // New builds a client for the cluster the config points at. @@ -93,14 +97,33 @@ func New(cfg Config) (*Client, error) { return nil, err } - return &Client{api: client, region: c.Region}, nil + return &Client{api: client, region: c.Region, tokenInClear: inClear(c.Address, c.SecretID)}, nil +} + +// inClear is SendsTokenInClear for a token sent to address. +func inClear(address, token string) bool { + u, err := url.Parse(address) + if err != nil || token == "" || u.Scheme != "http" { + return false + } + + if u.Hostname() == "localhost" { + return false + } + + ip := net.ParseIP(u.Hostname()) + + return ip == nil || !ip.IsLoopback() } // InRegion is a client for the same cluster in another region. The original // client does not change: a request in flight keeps the region it was // sent to. func (c *Client) InRegion(region string) *Client { - return &Client{api: c.api, region: region} + in := *c + in.region = region + + return &in } // Address is the cluster the client talks to. @@ -108,6 +131,12 @@ func (c *Client) Address() string { return c.api.Address() } +// SendsTokenInClear says anyone on the way to the cluster can read the token: +// it goes over plain HTTP to another machine. +func (c *Client) SendsTokenInClear() bool { + return c.tokenInClear +} + // Region is the region the client asks in, empty for the one of the agent. func (c *Client) Region() string { return c.region diff --git a/internal/nomad/client_test.go b/internal/nomad/client_test.go index 0031d1c..2fb6d85 100644 --- a/internal/nomad/client_test.go +++ b/internal/nomad/client_test.go @@ -320,6 +320,36 @@ func TestNew_TheCertificatesOfANamedCluster(t *testing.T) { require.ErrorContains(t, err, "missing-ca.pem") } +func TestNew_SendsTheTokenInClearOverHTTPToAnotherMachine(t *testing.T) { + for _, c := range []struct { + address, token string + want bool + }{ + {"http://nomad.example:4646", "secret", true}, + {"HTTP://10.0.0.5:4646", "secret", true}, + {"https://nomad.example:4646", "secret", false}, + {"http://nomad.example:4646", "", false}, + {"http://127.0.0.1:4646", "secret", false}, + {"http://127.0.0.2:4646", "secret", false}, + {"http://localhost:4646", "secret", false}, + {"http://[::1]:4646", "secret", false}, + {"unix:///var/run/nomad.sock", "secret", false}, + } { + client, err := nomad.New(nomad.Config{Named: true, Address: c.address, Token: c.token}) + require.NoError(t, err) + require.Equal(t, c.want, client.SendsTokenInClear(), "%s with token %q", c.address, c.token) + require.Equal(t, c.want, client.InRegion("eu").SendsTokenInClear(), "%s in another region", c.address) + } +} + +func TestNew_TheTokenOfTheEnvironmentIsSentInClearToo(t *testing.T) { + t.Setenv("NOMAD_TOKEN", "the-token-of-dev") + + client, err := nomad.New(nomad.Config{Address: "http://nomad.example:4646"}) + require.NoError(t, err) + require.True(t, client.SendsTokenInClear()) +} + func TestEveryNamespace(t *testing.T) { for namespace, want := range map[string]bool{"": true, "*": true, "default": false, "web": false} { require.Equal(t, want, nomad.EveryNamespace(namespace), "namespace %q", namespace) diff --git a/internal/ui/app.go b/internal/ui/app.go index bfb5674..6ae6faf 100644 --- a/internal/ui/app.go +++ b/internal/ui/app.go @@ -249,6 +249,10 @@ type Model struct { // token the session sends. refused error + // inClearDue says the warning that the token goes over plain HTTP is + // still to show for this connection. + inClearDue bool + // now is the time of the message being handled. What a screen shows of // time counts from it, so a render gives the same string for the same // state. @@ -266,6 +270,8 @@ func New(client Client, opts Options) Model { opts: opts, namespace: opts.Namespace, list: newList(jobTitles), + + inClearDue: client.SendsTokenInClear(), } m.screen = jobsView.opened() m = m.restore() @@ -336,7 +342,7 @@ func (m Model) takeError(err error) (Model, tea.Cmd) { // markAnswered marks what is open as answered, and schedules the next poll. func (m Model) markAnswered() (Model, tea.Cmd) { m.answered = true - m = m.forget() + m = m.forget().warnInClear() m.layout() return m.schedulePoll() diff --git a/internal/ui/app_test.go b/internal/ui/app_test.go index 3ee8d59..1388af8 100644 --- a/internal/ui/app_test.go +++ b/internal/ui/app_test.go @@ -1,6 +1,7 @@ package ui import ( + "cmp" "context" "encoding/json" "errors" @@ -138,6 +139,11 @@ type fakeClient struct { err error raftErr error + // address is where the cluster is; inClear says the token goes to it + // over plain HTTP. + address string + inClear bool + // nodesErr and allocsErr fail the client list or a list of allocations // alone, ahead of err, as a token that may not read one of them does. nodesErr error @@ -383,7 +389,9 @@ type fakeClient struct { // wrote keeps a call that changes the cluster. func (f *fakeClient) wrote(method string) { f.writes = append(f.writes, method) } -func (f *fakeClient) Address() string { return "https://nomad.example.com" } +func (f *fakeClient) Address() string { return cmp.Or(f.address, "https://nomad.example.com") } + +func (f *fakeClient) SendsTokenInClear() bool { return f.inClear } func (f *fakeClient) Region() string { return f.region } diff --git a/internal/ui/clusters.go b/internal/ui/clusters.go index 19d927e..afd0244 100644 --- a/internal/ui/clusters.go +++ b/internal/ui/clusters.go @@ -128,7 +128,7 @@ func (m Model) connected(conn Connection) (Model, tea.Cmd) { m.note = m.note.restart() m.connection++ - m.client = conn.Client + m.client, m.inClearDue = conn.Client, conn.Client.SendsTokenInClear() m.opts.Cluster, m.opts.Color, m.opts.ReadOnly = conn.Name, conn.Color, conn.ReadOnly m.opts.InRegion, m.opts.Shell = conn.InRegion, conn.Shell m.opts.NamespaceGiven = false diff --git a/internal/ui/clusters_test.go b/internal/ui/clusters_test.go index 9b1a577..2b34998 100644 --- a/internal/ui/clusters_test.go +++ b/internal/ui/clusters_test.go @@ -98,6 +98,23 @@ func TestClusters_SwitchByName(t *testing.T) { r.Contains(plain(m.render()), "read-only: Edit is off") } +func TestClusters_AWarningWhenTheTokenGoesInClear(t *testing.T) { + r := require.New(t) + + m, clusters := onDev(t) + clusters.prod.address, clusters.prod.inClear = "http://prod.example:4646", true + + m = typeCommand(m, "ctx prod") + r.Contains(plain(statusLine(m)), "! the token goes to http://prod.example:4646 without TLS") + + m = typeCommand(m, "ctx dev") + r.NotContains(plain(statusLine(m)), "without TLS") + + // Again for each connection. + m = typeCommand(m, "ctx prod") + r.Contains(plain(statusLine(m)), "without TLS") +} + func TestClusters_EachKeepsItsNamespace(t *testing.T) { r := require.New(t) diff --git a/internal/ui/roles.go b/internal/ui/roles.go index dd322ff..b0037d8 100644 --- a/internal/ui/roles.go +++ b/internal/ui/roles.go @@ -33,9 +33,10 @@ type Client interface { // The roles the cluster plays for the screens, one per kind of resource. type ( // clusterClient is the cluster as a whole: where it is, what it runs, - // whose token it is sent, and how busy it is. + // whose token it is sent and how, and how busy it is. clusterClient interface { Address() string + SendsTokenInClear() bool Region() string Agent(ctx context.Context) (nomad.Agent, error) AgentLogs(ctx context.Context, target nomad.AgentTarget, level string) *nomad.LogStream diff --git a/internal/ui/token.go b/internal/ui/token.go index 12fca20..3c0827b 100644 --- a/internal/ui/token.go +++ b/internal/ui/token.go @@ -186,3 +186,17 @@ func (m Model) refusedBecause(err error) (string, bool) { return fmt.Sprintf("Permission denied: %s may not do this", tokenName(token)), true } + +// warnInClear says, once for the connection, that the token goes over plain +// HTTP. An error, a warning or a command still running on the status line +// stays: the warning waits for the next answer. +func (m Model) warnInClear() Model { + f := m.flash + if !m.inClearDue || f.fresh(m.now) && (f.level != flashInfo || f.held) { + return m + } + + m.inClearDue = false + + return m.warn(sprintf("the token goes to %s without TLS", m.client.Address())) +} diff --git a/internal/ui/token_test.go b/internal/ui/token_test.go index 207ce3e..3f49b95 100644 --- a/internal/ui/token_test.go +++ b/internal/ui/token_test.go @@ -1,6 +1,7 @@ package ui import ( + "errors" "strings" "testing" "time" @@ -167,3 +168,78 @@ func TestToken_ARefusalOfTheClusterLeftIsNotExplainedByTheNext(t *testing.T) { r.NotContains(plain(statusLine(m)), "no token is set") } + +// onHTTP is a session on a cluster the token goes to over plain HTTP, before +// its first answer. +func onHTTP(client *fakeClient) Model { + client.address, client.inClear, client.changes = "http://nomad.example:4646", true, newChanges() + + m := New(client, Options{Version: "v-test"}) + m, _ = m.update(sizeMsg()) + + return m +} + +const inClearWarning = "! the token goes to http://nomad.example:4646 without TLS" + +func TestToken_AWarningWhenItGoesInClear(t *testing.T) { + r := require.New(t) + + m := onHTTP(&fakeClient{jobs: twoJobs()}) + r.NotContains(plain(statusLine(m)), "without TLS") + + m = drain(m, m.fetch()) + r.Contains(plain(statusLine(m)), inClearWarning) + + // Once for the connection: the next answer does not show it again. + m = m.quiet() + m = drain(m, m.fetch()) + r.NotContains(plain(statusLine(m)), "without TLS") +} + +func TestToken_NoWarningWhenItDoesNotGoInClear(t *testing.T) { + m := New(&fakeClient{jobs: twoJobs(), changes: newChanges()}, Options{Version: "v-test"}) + m, _ = m.update(sizeMsg()) + m = drain(m, m.fetch()) + + require.NotContains(t, plain(statusLine(m)), "without TLS") +} + +func TestToken_TheWarningInClearDoesNotReplaceAnError(t *testing.T) { + r := require.New(t) + + client := &fakeClient{err: errors.New("no route to host")} + m := onHTTP(client) + + // The first read fails: its error stays. + m = drain(m, m.fetch()) + r.Contains(plain(statusLine(m)), "no route to host") + r.NotContains(plain(statusLine(m)), "without TLS") + + // The warning comes with the first answer. + client.err, client.jobs = nil, twoJobs() + m = drain(m, m.fetch()) + r.Contains(plain(statusLine(m)), inClearWarning) +} + +func TestToken_TheWarningInClearWaitsForTheLineToClear(t *testing.T) { + for name, shown := range map[string]func(Model) Model{ + "another warning": func(m Model) Model { return m.warn("not following the cluster") }, + "an action error": func(m Model) Model { return m.failedLate("job not found") }, + "a running command": func(m Model) Model { return m.busy("Stopping web...") }, + } { + t.Run(name, func(t *testing.T) { + r := require.New(t) + + m := shown(onHTTP(&fakeClient{jobs: twoJobs()})) + text := m.flash.text + + m = drain(m, m.fetch()) + r.Contains(plain(statusLine(m)), text) + + m = m.quiet() + m = drain(m, m.fetch()) + r.Contains(plain(statusLine(m)), inClearWarning) + }) + } +} diff --git a/internal/ui/writes_test.go b/internal/ui/writes_test.go index 1f27c2c..a02a1d2 100644 --- a/internal/ui/writes_test.go +++ b/internal/ui/writes_test.go @@ -32,7 +32,7 @@ var ( } clientReads = []string{ - "Address", "Region", "Agent", "Regions", "Datacenters", "AllocationChecks", "Files", "File", "Deployment", "JobDeployments", "DeploymentAllocations", "Token", "ServiceInstances", + "Address", "SendsTokenInClear", "Region", "Agent", "Regions", "Datacenters", "AllocationChecks", "Files", "File", "Deployment", "JobDeployments", "DeploymentAllocations", "Token", "ServiceInstances", "Jobs", "NextLaunch", "JobSpec", "JobMeta", "JobVersions", "JobVersionDiff", "TaskGroups", "PlanJob", "PlanRevert", "JobTarget", "JobCounts", "DescribeJob", "DescribeAllocation", "DescribeDeployment", "DescribeService", "Allocations", "NodeAllocations", "Allocation", "Logs", "LogBefore", "AgentLogs",