Refuse cross-site mirror jobs and framing of the UI

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 \

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)

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)

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.

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)
+	})
+}

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"
+}

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)
+	}
+}

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)

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)
+		}
+	}
+}