diff --git a/docs/configuration.md b/docs/configuration.md index fa77ec69..d5946276 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1399,7 +1399,7 @@ Or via environment variable: `PROXY_MIRROR_API=true`. When disabled, the endpoints are not registered and return 404. -Start a mirror job with either PURLs or an inline CycloneDX or SPDX JSON document: +Start a mirror job with either PURLs or an inline CycloneDX or SPDX JSON document. Requests without `Content-Type: application/json` get 415, so other websites cannot start jobs through a visitor's browser: ```bash curl -X POST http://localhost:8080/api/mirror \ diff --git a/internal/server/browse.go b/internal/server/browse.go index d172aceb..1184f2e7 100644 --- a/internal/server/browse.go +++ b/internal/server/browse.go @@ -424,7 +424,7 @@ func (s *Server) writeBrowseFile(w http.ResponseWriter, file io.Reader, filePath contentType = detectContentTypeFromPrefix(head[:min(len(head), browseSniffSize)]) } w.Header().Set("Content-Type", contentType) - w.Header().Set("Content-Security-Policy", "sandbox") + w.Header().Set("Content-Security-Policy", "sandbox; "+frameAncestorsNone) w.Header().Set("X-Content-Type-Options", "nosniff") _, filename := path.Split(filePath) diff --git a/internal/server/browse_test.go b/internal/server/browse_test.go index c3352a42..34c864cc 100644 --- a/internal/server/browse_test.go +++ b/internal/server/browse_test.go @@ -163,8 +163,11 @@ func TestHandleBrowseFile(t *testing.T) { if got := w.Header().Get("Content-Type"); got != file.contentType { t.Errorf("Content-Type = %q, want %q", got, file.contentType) } - if got := w.Header().Get("Content-Security-Policy"); got != "sandbox" { - t.Errorf("Content-Security-Policy = %q, want sandbox", got) + if got, want := w.Header().Get("Content-Security-Policy"), "sandbox; frame-ancestors 'none'"; got != want { + t.Errorf("Content-Security-Policy = %q, want %q", got, want) + } + if got := w.Header().Get("X-Frame-Options"); got != "DENY" { + t.Errorf("X-Frame-Options = %q, want DENY", got) } if got := w.Header().Get("X-Content-Type-Options"); got != "nosniff" { t.Errorf("X-Content-Type-Options = %q, want nosniff", got) diff --git a/internal/server/errors.go b/internal/server/errors.go index c856c0e4..88fa325e 100644 --- a/internal/server/errors.go +++ b/internal/server/errors.go @@ -15,6 +15,8 @@ const ( ErrCodeUpstream = "UPSTREAM_ERROR" ErrCodeInternal = "INTERNAL_ERROR" ErrCodeBusy = "BUSY" + + ErrCodeUnsupportedMediaType = "UNSUPPORTED_MEDIA_TYPE" ) // ErrorResponse is the JSON body returned for API errors. diff --git a/internal/server/middleware.go b/internal/server/middleware.go index ad0fff5e..5470d76c 100644 --- a/internal/server/middleware.go +++ b/internal/server/middleware.go @@ -174,3 +174,15 @@ func sandboxUpstreamContent(next http.Handler) http.Handler { next.ServeHTTP(w, r) }) } + +// frameAncestorsNone stops other sites from framing the web UI, where a +// framed page could trick a signed-in user into clicking Delete. +const frameAncestorsNone = "frame-ancestors 'none'" + +func denyFraming(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("X-Frame-Options", "DENY") + w.Header().Set("Content-Security-Policy", frameAncestorsNone) + next.ServeHTTP(w, r) + }) +} diff --git a/internal/server/mirror_api.go b/internal/server/mirror_api.go index 028d4e01..89de3d56 100644 --- a/internal/server/mirror_api.go +++ b/internal/server/mirror_api.go @@ -2,6 +2,7 @@ package server import ( "encoding/json" + "mime" "net/http" "github.com/git-pkgs/proxy/internal/mirror" @@ -20,6 +21,14 @@ func NewMirrorAPIHandler(jobs *mirror.JobStore) *MirrorAPIHandler { // HandleCreate starts a new mirror job. func (h *MirrorAPIHandler) HandleCreate(w http.ResponseWriter, r *http.Request) { + // A browser can only send a JSON content type cross-origin after a CORS + // preflight, which the proxy never answers, so requiring it stops other + // sites from starting mirror jobs through a visitor's browser. + if !isJSONRequest(r) { + writeError(w, http.StatusUnsupportedMediaType, ErrCodeUnsupportedMediaType, + "Content-Type must be application/json") + return + } r.Body = http.MaxBytesReader(w, r.Body, maxBodySize) var req mirror.JobRequest if err := json.NewDecoder(r.Body).Decode(&req); err != nil { @@ -59,3 +68,8 @@ func (h *MirrorAPIHandler) HandleCancel(w http.ResponseWriter, r *http.Request) notFound(w, "job not found or not running") } } + +func isJSONRequest(r *http.Request) bool { + mediaType, _, err := mime.ParseMediaType(r.Header.Get("Content-Type")) + return err == nil && mediaType == "application/json" +} diff --git a/internal/server/mirror_api_test.go b/internal/server/mirror_api_test.go index c9603487..4567ae1e 100644 --- a/internal/server/mirror_api_test.go +++ b/internal/server/mirror_api_test.go @@ -8,6 +8,7 @@ import ( "net/http" "net/http/httptest" "os" + "strings" "testing" "github.com/git-pkgs/proxy/internal/database" @@ -55,6 +56,7 @@ func TestMirrorAPICreateJob(t *testing.T) { }) req := httptest.NewRequest("POST", "/api/mirror", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") w := httptest.NewRecorder() h.HandleCreate(w, req) @@ -91,6 +93,7 @@ func TestMirrorAPICreateJobFromInlineSBOM(t *testing.T) { } req := httptest.NewRequest(http.MethodPost, "/api/mirror", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") w := httptest.NewRecorder() h.HandleCreate(w, req) @@ -119,6 +122,7 @@ func TestMirrorAPICreateJobRejectsPURLsAndSBOM(t *testing.T) { } req := httptest.NewRequest(http.MethodPost, "/api/mirror", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") w := httptest.NewRecorder() h.HandleCreate(w, req) @@ -132,6 +136,7 @@ func TestMirrorAPICreateOversizedBody(t *testing.T) { body := bytes.Repeat([]byte("x"), int(maxBodySize)+1) req := httptest.NewRequest("POST", "/api/mirror", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") w := httptest.NewRecorder() h.HandleCreate(w, req) @@ -144,6 +149,7 @@ func TestMirrorAPICreateInvalidBody(t *testing.T) { h := setupMirrorAPI(t) req := httptest.NewRequest("POST", "/api/mirror", bytes.NewReader([]byte("not json"))) + req.Header.Set("Content-Type", "application/json") w := httptest.NewRecorder() h.HandleCreate(w, req) @@ -157,6 +163,7 @@ func TestMirrorAPICreateEmptyRequest(t *testing.T) { body, _ := json.Marshal(mirror.JobRequest{}) req := httptest.NewRequest("POST", "/api/mirror", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") w := httptest.NewRecorder() h.HandleCreate(w, req) @@ -203,6 +210,7 @@ func TestMirrorAPICreateAndGetJob(t *testing.T) { PURLs: []string{"pkg:npm/lodash@4.17.21"}, }) createReq := httptest.NewRequest("POST", "/api/mirror", bytes.NewReader(body)) + createReq.Header.Set("Content-Type", "application/json") createW := httptest.NewRecorder() h.HandleCreate(createW, createReq) @@ -230,3 +238,31 @@ func TestMirrorAPICreateAndGetJob(t *testing.T) { t.Errorf("job ID = %q, want %q", job.ID, jobID) } } + +// TestMirrorAPICreateRequiresJSON checks that only JSON requests start jobs. +// A page on another site can make a browser POST text/plain or form data +// without a CORS preflight, but not application/json. +func TestMirrorAPICreateRequiresJSON(t *testing.T) { + h := setupMirrorAPI(t) + body := `{"purls":["pkg:npm/lodash@4.17.21"],"x":"="}` + + for _, contentType := range []string{"", "text/plain", "application/x-www-form-urlencoded", "multipart/form-data; boundary=x"} { + req := httptest.NewRequest(http.MethodPost, "/api/mirror", strings.NewReader(body)) + if contentType != "" { + req.Header.Set("Content-Type", contentType) + } + w := httptest.NewRecorder() + h.HandleCreate(w, req) + if w.Code != http.StatusUnsupportedMediaType { + t.Errorf("Content-Type %q: status = %d, want %d", contentType, w.Code, http.StatusUnsupportedMediaType) + } + } + + req := httptest.NewRequest(http.MethodPost, "/api/mirror", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json; charset=utf-8") + w := httptest.NewRecorder() + h.HandleCreate(w, req) + if w.Code != http.StatusAccepted { + t.Errorf("application/json with charset: status = %d, want %d", w.Code, http.StatusAccepted) + } +} diff --git a/internal/server/server.go b/internal/server/server.go index 5b0ebf72..c19d3031 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -343,6 +343,7 @@ func (s *Server) serve(listener net.Listener) error { // Web UI. Mounted under /ui so a reverse proxy can apply different // access rules to it than to the package endpoints above (#123). r.Route("/ui", func(ui chi.Router) { + ui.Use(denyFraming) ui.Mount("/static", http.StripPrefix("/ui/static/", staticHandler())) ui.Get("/", s.handleRoot) ui.Get("/analytics", s.handleAnalytics) diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 6d937dcd..afd0ca90 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -121,6 +121,7 @@ func newTestServer(t *testing.T) *testServer { r.Get("/stats", s.handleStats) r.Get("/openapi.json", s.handleOpenAPIJSON) r.Route("/ui", func(ui chi.Router) { + ui.Use(denyFraming) ui.Mount("/static", http.StripPrefix("/ui/static/", staticHandler())) ui.Get("/", s.handleRoot) ui.Get("/analytics", s.handleAnalytics) @@ -1695,3 +1696,21 @@ func TestStatsEndpoint_StorageURL(t *testing.T) { t.Errorf("unexpected JSON key storage_path in response (should be storage_url)") } } + +// TestUIRefusesFraming checks that other sites cannot frame UI pages, where +// a framed Delete button could be clicked by a signed-in user. +func TestUIRefusesFraming(t *testing.T) { + ts := newTestServer(t) + defer ts.close() + + for _, target := range []string{"/ui/", "/ui/install", "/ui/static/style.css"} { + w := httptest.NewRecorder() + ts.handler.ServeHTTP(w, httptest.NewRequest(http.MethodGet, target, nil)) + if got := w.Header().Get("X-Frame-Options"); got != "DENY" { + t.Errorf("%s (status %d): X-Frame-Options = %q, want DENY", target, w.Code, got) + } + if got := w.Header().Get("Content-Security-Policy"); got != frameAncestorsNone { + t.Errorf("%s (status %d): Content-Security-Policy = %q, want %q", target, w.Code, got, frameAncestorsNone) + } + } +}