From da36645aafdc9f4c9cac7973b55d995f09e69a65 Mon Sep 17 00:00:00 2001 From: Thayol Date: Sat, 12 Sep 2026 23:48:03 +0200 Subject: [PATCH] Add --port --- README.md | 29 +++ internal/config/config.go | 21 +++ internal/config/config_test.go | 40 +++++ internal/config/flags.go | 4 +- internal/server/cookie.go | 100 +++++++++++ internal/server/delete.go | 56 ++++-- internal/server/pages.go | 25 ++- internal/server/server.go | 9 +- internal/server/server_test.go | 310 +++++++++++++++++++++++++++++++++ internal/server/upload.go | 40 ++++- main.go | 12 +- web/static/app.js | 24 ++- web/static/style.css | 24 +++ web/templates/index.html | 23 ++- 14 files changed, 677 insertions(+), 40 deletions(-) create mode 100644 internal/server/cookie.go diff --git a/README.md b/README.md index 62d30f8..7af4e33 100644 --- a/README.md +++ b/README.md @@ -36,9 +36,14 @@ Options take **one hyphen with a single letter** and **two with a full word**: Every option can also be set from the environment as `SEND_MAX_SIZE` and so on. `./send --help` lists them all. +`--port` is a convenience over `--listen`: it replaces only the port, so +`./send -p 9000` listens on `127.0.0.1:9000` and `--listen 0.0.0.0 -p 9000` +listens on all interfaces. + | Option | Default | Meaning | |---|---|---| | `-l`, `--listen` | `127.0.0.1:8080` | address to listen on | +| `-p`, `--port` | — | port to listen on, replacing the one in `--listen` | | `-d`, `--data` | `./data` | data directory | | `-b`, `--base-url` | `/` | path prefix when mounted under a subdirectory | | `-u`, `--public-url` | — | absolute base URL used in generated links | @@ -76,6 +81,23 @@ as `Authorization: Bearer `, or paste it into the form's token field. and on `SIGHUP`. It must stay mode `0600` — the server refuses to start otherwise, since it holds credential material. +### Remembering a token + +Tick **Remember this token on this device** and the server sets a cookie, so the +token only has to be pasted once. The upload page then says who you are and +shows your real limits; **Forget** clears it, as does unticking the box on your +next upload. It works with JavaScript disabled, because the browser sends the +cookie either way. + +The cookie is `HttpOnly`, which means the page's own script cannot read it — the +server resolves the identity and renders it instead. That is deliberately +unlike `localStorage`, where any script injected into the origin could read the +token straight out and walk away with it. It is also `SameSite=Strict`, so no +other site can make your browser upload or delete anything with it attached. + +Callers sending `Authorization: Bearer` are never given a cookie; an API client +keeps its own credentials. + ## Uploading From the browser, just use the page. It works with JavaScript disabled; with it @@ -119,6 +141,7 @@ file is accepted. | `GET /d/{id}` | the file, as an attachment; supports resuming | | `GET /i/{id}` | a page showing name, size, expiry and digest | | `POST /api/d/{id}/delete` | delete, with `token=` in the form or `Authorization: Bearer` | +| `POST /api/forget` | clear a remembered token | Deleting accepts the object's delete token, the token that uploaded it, or any admin token. @@ -172,6 +195,12 @@ Worth knowing if you are going to run this somewhere real. protection — so the upload handler maintains a per-read deadline instead. - **`X-Forwarded-For` is ignored** unless the peer is a configured `--trusted-proxy`, and then only to skip further trusted hops. +- **A remembered token lives in an `HttpOnly`, `SameSite=Strict` cookie**, not + in `localStorage`, so neither an injected script nor another website can get + at it. `Secure` is set whenever the service knows it is being served over + HTTPS — from `--public-url`, from a TLS connection, or from a trusted proxy's + `X-Forwarded-Proto`. On browsers old enough to ignore `SameSite` entirely + (pre-2017) the cookie would be CSRF-exposed; nothing here defends that case. - Rate limiting is per client address, with a separate bound on uploads in flight. Both are in memory and reset on restart. diff --git a/internal/config/config.go b/internal/config/config.go index 81d4e2b..d14b8e5 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -6,6 +6,7 @@ import ( "fmt" "net" "net/url" + "strconv" "strings" "time" ) @@ -13,6 +14,7 @@ import ( // Config is the fully-resolved server configuration. type Config struct { Listen string + Port int // overrides the port in Listen when set DataDir string BasePath string // normalised: always "/" or "/prefix/" PublicURL string // absolute origin+path for generated links; "" => relative @@ -40,6 +42,8 @@ const EnvPrefix = "SEND_" func (c *Config) Register(s *Set) { s.String(&c.Listen, "listen", "l", "127.0.0.1:8080", "ADDR", "address to listen on; keep it on loopback behind a reverse proxy") + s.Int(&c.Port, "port", "p", 0, + "port to listen on, replacing the one in --listen") s.String(&c.DataDir, "data", "d", "./data", "DIR", "directory holding uploaded objects and their metadata") s.String(&c.BasePath, "base-url", "b", "/", "PATH", @@ -78,6 +82,23 @@ func (c *Config) Register(s *Set) { func (c *Config) Normalise() error { c.BasePath = NormalisePath(c.BasePath) + // --port is a convenience over --listen: it replaces only the port, so the + // host stays wherever --listen (or its default) put it. + if c.Port != 0 { + if c.Port < 1 || c.Port > 65535 { + return fmt.Errorf("--port: %d is not a port number", c.Port) + } + host, _, err := net.SplitHostPort(c.Listen) + if err != nil { + // --listen held a bare host, which is fine once a port is supplied. + host = strings.TrimSpace(c.Listen) + } + c.Listen = net.JoinHostPort(host, strconv.Itoa(c.Port)) + } + if _, _, err := net.SplitHostPort(c.Listen); err != nil { + return fmt.Errorf("--listen: %q is not an address:port (use --port to set just the port)", c.Listen) + } + if c.PublicURL != "" { u, err := url.Parse(c.PublicURL) if err != nil { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index a195f7c..6918511 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -165,3 +165,43 @@ func TestDefaultExpiryMustFitWithinMax(t *testing.T) { t.Error("an unlimited default under a finite maximum was accepted") } } + +func TestPortOverridesListen(t *testing.T) { + cases := []struct { + listen string + port int + want string + }{ + {"127.0.0.1:8080", 0, "127.0.0.1:8080"}, + {"127.0.0.1:8080", 9000, "127.0.0.1:9000"}, + {"0.0.0.0:8080", 3000, "0.0.0.0:3000"}, + {"localhost", 3000, "localhost:3000"}, // a bare host is fine with --port + {"[::1]:8080", 3000, "[::1]:3000"}, + } + for _, c := range cases { + cfg := &Config{ + Listen: c.listen, Port: c.port, BasePath: "/", DataDir: "d", + SweepInterval: time.Minute, MaxConcurrent: 1, + } + if err := cfg.Normalise(); err != nil { + t.Errorf("listen %q port %d: %v", c.listen, c.port, err) + continue + } + if cfg.Listen != c.want { + t.Errorf("listen %q port %d => %q, want %q", c.listen, c.port, cfg.Listen, c.want) + } + } + + for _, c := range []struct { + listen string + port int + }{{"127.0.0.1:8080", 70000}, {"127.0.0.1:8080", -1}, {"localhost", 0}} { + cfg := &Config{ + Listen: c.listen, Port: c.port, BasePath: "/", DataDir: "d", + SweepInterval: time.Minute, MaxConcurrent: 1, + } + if err := cfg.Normalise(); err == nil { + t.Errorf("listen %q port %d was accepted", c.listen, c.port) + } + } +} diff --git a/internal/config/flags.go b/internal/config/flags.go index 5443aad..186b42d 100644 --- a/internal/config/flags.go +++ b/internal/config/flags.go @@ -222,7 +222,9 @@ func (s *Set) PrintUsage(w io.Writer, header string) { } fmt.Fprintf(w, "%s\n", name) fmt.Fprintf(w, " %s", sp.usage) - if sp.def != "" && sp.def != "false" { + // A zero default means "unset" for the options that have one, and + // printing it would read as a real value. + if sp.def != "" && sp.def != "false" && sp.def != "0" { fmt.Fprintf(w, " (default %s)", sp.def) } fmt.Fprintf(w, "\n [%s]\n", s.envName(sp.long)) diff --git a/internal/server/cookie.go b/internal/server/cookie.go new file mode 100644 index 0000000..3987fb9 --- /dev/null +++ b/internal/server/cookie.go @@ -0,0 +1,100 @@ +package server + +import ( + "net" + "net/http" + "strings" + "time" +) + +// tokenCookie remembers a caller's token so it does not have to be pasted for +// every upload. +// +// It is HttpOnly, so a script on this origin cannot read it back — which is the +// reason to prefer it over localStorage, where any injected script could +// exfiltrate the credential. The page never needs to see the value: the server +// resolves it and renders who the caller is. +const tokenCookie = "send_token" + +// rememberFor is how long a remembered token survives. Tokens are revoked by +// deleting them from the token file, so a long window costs nothing. +const rememberFor = 365 * 24 * time.Hour + +// cookieCredential returns the remembered token, if any. +func cookieCredential(r *http.Request) string { + c, err := r.Cookie(tokenCookie) + if err != nil { + return "" + } + return strings.TrimSpace(c.Value) +} + +// credential resolves the caller's token from an explicit header first, then +// from the remembered cookie. Upload additionally accepts a form field, which +// takes precedence over both. +func credential(r *http.Request) string { + if t := bearer(r); t != "" { + return t + } + return cookieCredential(r) +} + +// remember stores the token in a cookie. +// +// SameSite=Strict is what makes accepting a cookie as a credential safe here: +// without it, any site could make the browser post an upload or a deletion with +// the cookie attached. Scoping the path to the mount point keeps the credential +// out of requests to the rest of the host when running under a subdirectory. +func (s *Server) remember(w http.ResponseWriter, r *http.Request, token string) { + http.SetCookie(w, &http.Cookie{ + Name: tokenCookie, + Value: token, + Path: s.cfg.BasePath, + MaxAge: int(rememberFor.Seconds()), + HttpOnly: true, + Secure: s.isHTTPS(r), + SameSite: http.SameSiteStrictMode, + }) +} + +// forget clears a remembered token. +func (s *Server) forget(w http.ResponseWriter, r *http.Request) { + http.SetCookie(w, &http.Cookie{ + Name: tokenCookie, + Value: "", + Path: s.cfg.BasePath, + MaxAge: -1, + HttpOnly: true, + Secure: s.isHTTPS(r), + SameSite: http.SameSiteStrictMode, + }) +} + +// isHTTPS decides whether the cookie may carry the Secure attribute. Setting it +// on a plain-HTTP development server would stop the browser storing the cookie +// at all, so it is only set when the connection is genuinely secure. +func (s *Server) isHTTPS(r *http.Request) bool { + if strings.HasPrefix(s.cfg.PublicURL, "https://") { + return true // the operator said so, and they are behind the proxy + } + if r.TLS != nil { + return true + } + // Believed only from a proxy that is trusted for forwarded headers at all. + if host, _, err := net.SplitHostPort(r.RemoteAddr); err == nil { + if ip := net.ParseIP(host); ip != nil && s.cfg.TrustsProxy(ip) { + return r.Header.Get("X-Forwarded-Proto") == "https" + } + } + return false +} + +// handleForget drops the remembered token and returns to the upload page. +func (s *Server) handleForget(w http.ResponseWriter, r *http.Request) { + s.forget(w, r) + if wantsJSON(r) { + writeJSON(w, http.StatusOK, map[string]string{"status": "forgotten"}) + return + } + http.Redirect(w, r, s.cfg.BasePath, http.StatusSeeOther) +} diff --git a/internal/server/delete.go b/internal/server/delete.go index fa084cd..4f68307 100644 --- a/internal/server/delete.go +++ b/internal/server/delete.go @@ -26,24 +26,16 @@ func (s *Server) handleDelete(w http.ResponseWriter, r *http.Request) { return } - secret := bearer(r) - if secret == "" { - // A small form post; the 4 KiB cap keeps this from being a way to - // stream a body into memory. - r.Body = http.MaxBytesReader(w, r.Body, maxFieldBytes) - if err := r.ParseForm(); err == nil { - secret = strings.TrimSpace(r.PostFormValue("token")) - } - } - if secret == "" { + presented := s.deleteCredentials(w, r) + if len(presented) == 0 { s.fail(w, r, http.StatusUnauthorized, "A delete token or an owning token is required.") return } - - if !s.mayDelete(m, secret) { + if !s.authorised(m, presented) { s.fail(w, r, http.StatusForbidden, "That token cannot delete this file.") return } + if err := s.store.Delete(id); err != nil { s.log.Error("deleting object", "id", id, "err", err) s.fail(w, r, http.StatusInternalServerError, "Could not delete the file.") @@ -62,8 +54,44 @@ func (s *Server) handleDelete(w http.ResponseWriter, r *http.Request) { }) } -// mayDelete checks the presented secret against the object's delete token -// first, then against the token file. +// deleteCredentials collects every secret the request carries. +// +// Three can legitimately arrive at once — the object's delete token in the +// form, a token in the header, and a remembered token in the cookie — and any +// one of them may be the sufficient one. They are all collected so that the +// first one present cannot shadow the others. +func (s *Server) deleteCredentials(w http.ResponseWriter, r *http.Request) []string { + var out []string + add := func(secret string) { + if secret = strings.TrimSpace(secret); secret != "" { + out = append(out, secret) + } + } + + add(bearer(r)) + add(cookieCredential(r)) + + // A small form post; the cap keeps this from being a way to stream a body + // into memory. A non-form body simply fails to parse and is ignored. + r.Body = http.MaxBytesReader(w, r.Body, maxFieldBytes) + if err := r.ParseForm(); err == nil { + add(r.PostFormValue("token")) + } + return out +} + +// authorised reports whether any of the presented secrets may delete m. +func (s *Server) authorised(m *store.Meta, presented []string) bool { + for _, secret := range presented { + if s.mayDelete(m, secret) { + return true + } + } + return false +} + +// mayDelete checks one secret against the object's delete token first, then +// against the token file. func (s *Server) mayDelete(m *store.Meta, secret string) bool { if auth.EqualHash(m.DeleteHash, auth.HashSecret(secret)) { return true diff --git a/internal/server/pages.go b/internal/server/pages.go index a34b8f1..e553afb 100644 --- a/internal/server/pages.go +++ b/internal/server/pages.go @@ -49,23 +49,39 @@ type indexPage struct { MaxExpiry string DefaultExpiry string AbsBase string + TokenName string // the remembered token's name, if there is one + AllowVanity bool + Stale bool // a remembered token that no longer exists } func (s *Server) handleIndex(w http.ResponseWriter, r *http.Request) { - // The page always renders the anonymous tier; the script refreshes it from - // /api/limits once a token is entered. - lim := auth.Anonymous(s.cfg) + // A remembered token is resolved server-side, so the page can show the real + // limits without the cookie ever being readable by a script. + remembered := cookieCredential(r) + lim, err := s.limitsFor(remembered) + stale := false + if err != nil { + // The token was revoked or the file was edited; drop the cookie rather + // than leave the caller wondering why uploads fail. + s.forget(w, r) + lim, stale = auth.Anonymous(s.cfg), true + } + s.render(w, http.StatusOK, "index.html", indexPage{ page: s.page("Upload", true), MaxSize: config.FormatSize(lim.MaxSize), MaxExpiry: config.FormatDuration(lim.MaxExpiry), DefaultExpiry: config.FormatDuration(lim.DefaultExpiry), AbsBase: s.absBase(r), + TokenName: lim.Name, + AllowVanity: lim.AllowVanity, + Stale: stale, }) } type limitsJSON struct { Name string `json:"name"` + Remembered bool `json:"remembered"` MaxSize *int64 `json:"max_size"` // null means unlimited MaxExpiry string `json:"max_expiry"` DefaultExpiry string `json:"default_expiry"` @@ -81,13 +97,14 @@ func (s *Server) handleLimits(w http.ResponseWriter, r *http.Request) { s.fail(w, r, http.StatusTooManyRequests, "Too many requests; try again shortly.") return } - lim, err := s.limitsFor(bearer(r)) + lim, err := s.limitsFor(credential(r)) if err != nil { s.fail(w, r, http.StatusUnauthorized, "Unrecognised token.") return } out := limitsJSON{ Name: lim.Name, + Remembered: lim.Name != "" && bearer(r) == "", MaxExpiry: config.FormatDuration(lim.MaxExpiry), DefaultExpiry: config.FormatDuration(lim.DefaultExpiry), AllowVanity: lim.AllowVanity, diff --git a/internal/server/server.go b/internal/server/server.go index 9fb04e6..d8ba937 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -62,6 +62,7 @@ func (s *Server) routes() http.Handler { mux.HandleFunc("GET /d/{id}", s.handleDownload) mux.HandleFunc("GET /i/{id}", s.handleInfo) mux.HandleFunc("POST /api/d/{id}/delete", s.handleDelete) + mux.HandleFunc("POST /api/forget", s.handleForget) mux.Handle("GET /static/", http.StripPrefix("/static/", s.staticHandler())) mux.HandleFunc("/", s.handleNotFound) @@ -94,8 +95,14 @@ func (s *Server) staticHandler() http.Handler { // appCSP locks the application pages down to their own origin. The frontend has // no inline script and no third-party anything, so this can be strict. +// +// connect-src is not optional here: the upload page talks to /api/upload and +// /api/limits over XMLHttpRequest, and every fetch-directive left unlisted +// falls back to default-src, so omitting it makes the browser block every +// upload before it reaches the network. See TestAppCSPAllowsWhatThePageDoes. const appCSP = "default-src 'none'; script-src 'self'; style-src 'self'; " + - "img-src 'self' data:; form-action 'self'; base-uri 'none'; frame-ancestors 'none'" + "img-src 'self' data:; connect-src 'self'; form-action 'self'; " + + "base-uri 'none'; frame-ancestors 'none'" func (s *Server) securityHeaders(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/internal/server/server_test.go b/internal/server/server_test.go index d00a80c..1923fbf 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -647,3 +647,313 @@ func assertNoDebris(t *testing.T, dir string) { t.Errorf("leftover object directory: %s", e.Name()) } } + +// --- remembered tokens --------------------------------------------------- + +// A browser form post that carries a token and the remember box gets a cookie +// back, and that cookie then authenticates later uploads on its own. +func TestTokenIsRememberedInACookie(t *testing.T) { + h := newHarness(t, nil) + + resp := h.formUpload(t, map[string]string{"token": h.token, "remember": "1"}, "a.bin", "one") + if resp.StatusCode != http.StatusCreated { + t.Fatalf("status = %s", resp.Status) + } + resp.Body.Close() + + cookie := findCookie(resp, tokenCookie) + if cookie == nil { + t.Fatal("no token cookie was set") + } + if cookie.Value != h.token { + t.Error("the cookie does not hold the token") + } + if !cookie.HttpOnly { + t.Error("the token cookie is readable by scripts") + } + if cookie.SameSite != http.SameSiteStrictMode { + t.Error("the token cookie is not SameSite=Strict, so it is CSRF-exposed") + } + + // The cookie alone is now enough to claim a vanity name, which anonymous + // callers cannot do. + req, _ := http.NewRequest("POST", h.ts.URL+"/api/upload", strings.NewReader("two")) + req.Header.Set("Accept", "application/json") + req.Header.Set("Vanity", "remembered") + req.AddCookie(cookie) + resp, err := h.ts.Client().Do(req) + if err != nil { + t.Fatal(err) + } + if resp.StatusCode != http.StatusCreated { + t.Fatalf("upload with only the cookie: status = %s", resp.Status) + } + if res := decode[uploadResult](t, resp); res.ID != "remembered" { + t.Errorf("id = %q, want remembered", res.ID) + } +} + +// A typed token wins over whatever the browser remembered. +func TestExplicitTokenBeatsTheCookie(t *testing.T) { + h := newHarness(t, nil) + resp := h.formUpload(t, map[string]string{"token": h.token, "remember": "1"}, "a.bin", "x") + cookie := findCookie(resp, tokenCookie) + resp.Body.Close() + + resp = h.formUploadWith(t, cookie, map[string]string{"token": h.admin, "remember": "1"}, "b.bin", "y") + defer resp.Body.Close() + res := decode[uploadResult](t, resp) + + m, err := h.store.Get(res.ID, h.now) + if err != nil { + t.Fatal(err) + } + if m.Owner != "boss" { + t.Errorf("owner = %q, want boss: the cookie shadowed the typed token", m.Owner) + } +} + +// Leaving the box unchecked clears a token the browser had remembered. +func TestUncheckingRememberForgetsTheCookie(t *testing.T) { + h := newHarness(t, nil) + resp := h.formUpload(t, map[string]string{"token": h.token, "remember": "1"}, "a.bin", "x") + cookie := findCookie(resp, tokenCookie) + resp.Body.Close() + + resp = h.formUploadWith(t, cookie, map[string]string{}, "b.bin", "y") + defer resp.Body.Close() + cleared := findCookie(resp, tokenCookie) + if cleared == nil || cleared.MaxAge >= 0 { + t.Fatalf("the cookie was not cleared: %v", cleared) + } +} + +func TestForgetEndpointClearsTheCookie(t *testing.T) { + h := newHarness(t, nil) + req, _ := http.NewRequest("POST", h.ts.URL+"/api/forget", nil) + req.Header.Set("Accept", "application/json") + req.AddCookie(&http.Cookie{Name: tokenCookie, Value: h.token}) + resp, err := h.ts.Client().Do(req) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + c := findCookie(resp, tokenCookie) + if c == nil || c.MaxAge >= 0 || c.Value != "" { + t.Fatalf("the cookie was not cleared: %v", c) + } +} + +// A revoked token left in a cookie must not wedge the page. +func TestStaleCookieIsDropped(t *testing.T) { + h := newHarness(t, nil) + req, _ := http.NewRequest("GET", h.ts.URL+"/", nil) + req.AddCookie(&http.Cookie{Name: tokenCookie, Value: "a-token-that-was-revoked"}) + resp, err := h.ts.Client().Do(req) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Fatalf("status = %s, want the page to still render", resp.Status) + } + if c := findCookie(resp, tokenCookie); c == nil || c.MaxAge >= 0 { + t.Error("a stale cookie was not dropped") + } + page, _ := io.ReadAll(resp.Body) + if strings.Contains(string(page), "Uploading as") { + t.Error("the page claims an identity it could not resolve") + } +} + +// The index page resolves a remembered token server-side, so the limits shown +// are the caller's real ones even though the cookie is unreadable by script. +func TestIndexShowsTheRememberedIdentity(t *testing.T) { + h := newHarness(t, nil) + req, _ := http.NewRequest("GET", h.ts.URL+"/", nil) + req.AddCookie(&http.Cookie{Name: tokenCookie, Value: h.token}) + resp, err := h.ts.Client().Do(req) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + page, _ := io.ReadAll(resp.Body) + if !strings.Contains(string(page), "Uploading as friend") { + t.Error("the page does not show the remembered identity") + } + if strings.Contains(string(page), h.token) { + t.Error("the page echoes the token back into the HTML") + } +} + +// The per-object delete token must still work when a cookie is also present. +func TestCookieDoesNotShadowTheDeleteToken(t *testing.T) { + h := newHarness(t, nil) + // Uploaded anonymously, so the remembered token owns nothing here. + res := decode[uploadResult](t, h.upload(t, []byte("x"), nil)) + + form := strings.NewReader("token=" + res.DeleteToken) + req, _ := http.NewRequest("POST", h.ts.URL+"/api/d/"+res.ID+"/delete", form) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.Header.Set("Accept", "application/json") + req.AddCookie(&http.Cookie{Name: tokenCookie, Value: h.token}) + resp, err := h.ts.Client().Do(req) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Fatalf("status = %s, want 200: the cookie shadowed the delete token", resp.Status) + } +} + +// An API caller sending a bearer token manages its own credentials and should +// not be handed a cookie it never asked for. +func TestBearerCallersAreNotGivenACookie(t *testing.T) { + h := newHarness(t, nil) + resp := h.upload(t, []byte("x"), map[string]string{"Authorization": "Bearer " + h.token}) + defer resp.Body.Close() + if c := findCookie(resp, tokenCookie); c != nil { + t.Errorf("a cookie was set for a bearer-token upload: %v", c) + } +} + +func findCookie(resp *http.Response, name string) *http.Cookie { + for _, c := range resp.Cookies() { + if c.Name == name { + return c + } + } + return nil +} + +// formUpload posts the multipart form the browser would, with fields ordered +// ahead of the file part. +func (h *harness) formUpload(t *testing.T, fields map[string]string, filename, content string) *http.Response { + t.Helper() + return h.formUploadWith(t, nil, fields, filename, content) +} + +func (h *harness) formUploadWith(t *testing.T, cookie *http.Cookie, fields map[string]string, filename, content string) *http.Response { + t.Helper() + var body bytes.Buffer + mw := multipart.NewWriter(&body) + for k, v := range fields { + mw.WriteField(k, v) + } + fw, err := mw.CreateFormFile("file", filename) + if err != nil { + t.Fatal(err) + } + fw.Write([]byte(content)) + mw.Close() + + req, _ := http.NewRequest("POST", h.ts.URL+"/api/upload", &body) + req.Header.Set("Content-Type", mw.FormDataContentType()) + req.Header.Set("Accept", "application/json") + if cookie != nil { + req.AddCookie(cookie) + } + resp, err := h.ts.Client().Do(req) + if err != nil { + t.Fatal(err) + } + return resp +} + +// --- content security policy --------------------------------------------- + +// The page's own behaviour and its CSP have to agree, and nothing in a Go test +// or a curl invocation enforces CSP — only a browser does. This reads the +// script that is actually shipped, works out which fetch directives the page +// needs, and checks the policy grants them. +// +// It exists because omitting connect-src once made the browser block every +// upload while every server-side test still passed. +func TestAppCSPAllowsWhatThePageDoes(t *testing.T) { + h := newHarness(t, nil) + + resp, err := h.ts.Client().Get(h.ts.URL + "/static/app.js") + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + script, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatal(err) + } + + // Which directive each capability the script might use depends on. Every + // fetch directive falls back to default-src when unlisted, and default-src + // here is 'none', so anything the script does must be granted explicitly. + needs := []struct { + directive string + used bool + because string + }{ + {"connect-src", bytes.Contains(script, []byte("XMLHttpRequest")) || + bytes.Contains(script, []byte("fetch(")), "the page makes XHR or fetch calls"}, + {"script-src", true, "the page loads an external script"}, + {"style-src", true, "the page loads an external stylesheet"}, + {"form-action", true, "the page posts a form"}, + } + + page, err := h.ts.Client().Get(h.ts.URL + "/") + if err != nil { + t.Fatal(err) + } + page.Body.Close() + csp := page.Header.Get("Content-Security-Policy") + if csp == "" { + t.Fatal("the upload page carries no Content-Security-Policy") + } + + directives := map[string]string{} + for _, d := range strings.Split(csp, ";") { + name, value, _ := strings.Cut(strings.TrimSpace(d), " ") + directives[strings.ToLower(name)] = strings.TrimSpace(value) + } + if directives["default-src"] != "'none'" { + t.Errorf("default-src = %q, want 'none': the checks below assume it denies by default", + directives["default-src"]) + } + + for _, n := range needs { + if !n.used { + continue + } + value, ok := directives[n.directive] + if !ok { + t.Errorf("CSP has no %s, but %s; the browser will fall back to default-src and block it", + n.directive, n.because) + continue + } + if !strings.Contains(value, "'self'") { + t.Errorf("CSP %s = %q, which does not allow this origin, but %s", + n.directive, value, n.because) + } + } +} + +// The download policy is the opposite case: it must stay maximally restrictive, +// since it governs bytes a stranger uploaded. +func TestDownloadCSPStaysInert(t *testing.T) { + h := newHarness(t, nil) + res := decode[uploadResult](t, h.upload(t, []byte(""), nil)) + + get, err := h.ts.Client().Get(h.ts.URL + "/d/" + res.ID) + if err != nil { + t.Fatal(err) + } + get.Body.Close() + + csp := get.Header.Get("Content-Security-Policy") + if !strings.Contains(csp, "default-src 'none'") || !strings.Contains(csp, "sandbox") { + t.Errorf("download CSP = %q, want default-src 'none' and sandbox", csp) + } + for _, forbidden := range []string{"connect-src", "script-src 'self'", "'unsafe-inline'"} { + if strings.Contains(csp, forbidden) { + t.Errorf("download CSP contains %q; uploaded bytes must be granted nothing", forbidden) + } + } +} diff --git a/internal/server/upload.go b/internal/server/upload.go index b931a7f..125737c 100644 --- a/internal/server/upload.go +++ b/internal/server/upload.go @@ -38,6 +38,12 @@ type uploadRequest struct { vanity string expiry string filename string + + // remember is set by the form's checkbox. It decides whether a token used + // here is stored in a cookie for next time, and unchecking it is how a + // remembered token is cleared from the upload page itself. + remember bool + explicit bool // the token was typed or sent, not read back from the cookie } func (s *Server) handleUpload(w http.ResponseWriter, r *http.Request) { @@ -75,6 +81,10 @@ func (s *Server) uploadRaw(w http.ResponseWriter, r *http.Request, ip string) { expiry: strings.TrimSpace(r.Header.Get("Expiry")), filename: filenameFromDisposition(r.Header.Get("Content-Disposition")), } + req.explicit = req.token != "" + if req.token == "" { + req.token = cookieCredential(r) + } s.storeUpload(w, r, req, r.Body, ip) } @@ -92,6 +102,7 @@ func (s *Server) uploadMultipart(w http.ResponseWriter, r *http.Request, boundar } mr := multipart.NewReader(r.Body, boundary) req := uploadRequest{token: bearer(r)} + req.explicit = req.token != "" for n := 0; ; n++ { if n > maxFieldCount { @@ -112,6 +123,12 @@ func (s *Server) uploadMultipart(w http.ResponseWriter, r *http.Request, boundar if req.filename == "" { req.filename = part.FileName() } + // Nothing explicit was supplied, so fall back to what the browser + // remembered. This happens after the fields precisely so a typed + // token still wins. + if req.token == "" { + req.token = cookieCredential(r) + } s.storeUpload(w, r, req, part, ip) return } @@ -124,9 +141,11 @@ func (s *Server) uploadMultipart(w http.ResponseWriter, r *http.Request, boundar } switch part.FormName() { case "token": - if value != "" { - req.token = value + if v := strings.TrimSpace(value); v != "" { + req.token, req.explicit = v, true } + case "remember": + req.remember = true case "vanity": req.vanity = strings.TrimSpace(value) case "expiry": @@ -250,11 +269,28 @@ func (s *Server) storeUpload(w http.ResponseWriter, r *http.Request, req uploadR } committed = true + s.updateRemembered(w, r, req, lim) + s.log.Info("stored", "id", m.ID, "bytes", m.Size, "owner", orAnonymous(lim.Name), "ip", ip, "expires", m.Expires) s.respondUploaded(w, r, m, secret) } +// updateRemembered applies the form's "remember" checkbox to the cookie. It +// only ever acts on a browser form post: an API caller sending a bearer token +// has its own way of keeping credentials and should not be handed a cookie. +func (s *Server) updateRemembered(w http.ResponseWriter, r *http.Request, req uploadRequest, lim auth.Limits) { + if bearer(r) != "" { + return + } + switch { + case req.remember && req.explicit && lim.Name != "": + s.remember(w, r, req.token) + case !req.remember && cookieCredential(r) != "": + s.forget(w, r) + } +} + func orAnonymous(name string) string { if name == "" { return "(anonymous)" diff --git a/main.go b/main.go index 69c8a95..121a291 100644 --- a/main.go +++ b/main.go @@ -8,6 +8,7 @@ import ( "flag" "fmt" "log/slog" + "net" "net/http" "os" "os/signal" @@ -99,6 +100,13 @@ func serve(args []string) error { ErrorLog: slog.NewLogLogger(log.Handler(), slog.LevelWarn), } + // Bind before announcing anything: logging "listening" and only then + // failing to bind reads as a server that started and then died. + ln, err := net.Listen("tcp", cfg.Listen) + if err != nil { + return err + } + ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) defer stop() @@ -106,14 +114,14 @@ func serve(args []string) error { go reloadOnHUP(ctx, tokens, log) log.Info("listening", - "addr", cfg.Listen, "data", cfg.DataDir, "base", cfg.BasePath, + "addr", ln.Addr().String(), "data", cfg.DataDir, "base", cfg.BasePath, "objects", st.Count(), "stored", config.FormatBytes(st.Total()), "max_size", config.FormatSize(cfg.MaxSize), "max_expiry", config.FormatDuration(cfg.MaxExpiry)) errc := make(chan error, 1) go func() { - err := httpSrv.ListenAndServe() + err := httpSrv.Serve(ln) if errors.Is(err, http.ErrServerClosed) { err = nil } diff --git a/web/static/app.js b/web/static/app.js index efca6d0..c440311 100644 --- a/web/static/app.js +++ b/web/static/app.js @@ -32,27 +32,25 @@ errorBox.hidden = !msg; } - // --- token memory ------------------------------------------------------- - // Kept in localStorage only so the field survives a reload; it is never sent - // anywhere but this origin's upload endpoint. - try { - var saved = localStorage.getItem('send.token'); - if (saved && tokenInput) tokenInput.value = saved; - } catch (e) { /* private mode; not important */ } - + // --- credentials -------------------------------------------------------- + // A token is remembered in an HttpOnly cookie the server sets, not here: + // this script cannot read it back, so an injected script cannot steal it + // either. The page is told who it is by the server when it renders, and the + // limits panel is refreshed from /api/limits, which reads the same cookie. function refreshLimits() { var token = tokenInput ? tokenInput.value.trim() : ''; - try { - if (token) localStorage.setItem('send.token', token); - else localStorage.removeItem('send.token'); - } catch (e) { /* ignore */ } var xhr = new XMLHttpRequest(); xhr.open('GET', base + 'api/limits'); xhr.setRequestHeader('Accept', 'application/json'); + // Sent only when the field holds something; otherwise the cookie answers. if (token) xhr.setRequestHeader('Authorization', 'Bearer ' + token); xhr.onload = function () { - if (xhr.status !== 200) return; + if (xhr.status !== 200) { + if (xhr.status === 401 && token) showError('That token is not recognised.'); + return; + } + showError(''); var l; try { l = JSON.parse(xhr.responseText); } catch (e) { return; } limits = l; diff --git a/web/static/style.css b/web/static/style.css index 3038b96..7f78e98 100644 --- a/web/static/style.css +++ b/web/static/style.css @@ -142,3 +142,27 @@ pre { .warn p { margin: .25rem 0 .75rem; font-size: .875rem; } .result .field { margin-top: 1rem; } + +.check { display: flex; align-items: center; gap: .5rem; margin-bottom: 1rem; font-size: .875rem; } +.check input { margin: 0; } + +.notice { + margin: 0 0 1rem; + padding: .625rem .875rem; + background: var(--card); + border: 1px solid var(--line); + border-left: 3px solid var(--accent); + border-radius: 6px; + font-size: .875rem; +} +.notice .forget-form, .notice form { display: inline; } + +button.link { + padding: 0; + background: none; + border: 0; + color: var(--accent); + font: inherit; + text-decoration: underline; + cursor: pointer; +} diff --git a/web/templates/index.html b/web/templates/index.html index b6c45a5..d63a2ad 100644 --- a/web/templates/index.html +++ b/web/templates/index.html @@ -1,10 +1,27 @@ {{define "content"}} +{{if .Stale}} +

The token remembered on this device no longer exists. It has been forgotten.

+{{end}} + +{{if .TokenName}} +
+ Uploading as {{.TokenName}}. +
+
+{{end}} +
+ +
@@ -15,7 +32,7 @@
@@ -39,7 +56,7 @@
Maximum size
{{.MaxSize}}
Longest lifetime
{{.MaxExpiry}}
Default lifetime
{{.DefaultExpiry}}
-
Vanity names
requires a token
+
Vanity names
{{if .AllowVanity}}allowed{{else}}requires a token{{end}}