diff --git a/backend/app/cmd/server.go b/backend/app/cmd/server.go index c34e9374..ef7ee611 100644 --- a/backend/app/cmd/server.go +++ b/backend/app/cmd/server.go @@ -610,6 +610,7 @@ func (s *ServerCommand) newServerApp(ctx context.Context) (*serverApp, error) { SubscribersOnly: s.SubscribersOnly, DisableSignature: s.DisableSignature, DisableFancyTextFormatting: s.DisableFancyTextFormatting, + ExternalImageProxy: s.ImageProxy.CacheExternal, } srv.ScoreThresholds.Low, srv.ScoreThresholds.Critical = s.LowScore, s.CriticalScore diff --git a/backend/app/rest/api/rest.go b/backend/app/rest/api/rest.go index 8c875008..9f38e45b 100644 --- a/backend/app/rest/api/rest.go +++ b/backend/app/rest/api/rest.go @@ -70,6 +70,7 @@ type Rest struct { SubscribersOnly bool DisableSignature bool // prevent signature from being added to headers DisableFancyTextFormatting bool // disables SmartyPants in the comment text rendering of the posted comments + ExternalImageProxy bool SSLConfig SSLConfig httpsServer *http.Server @@ -205,6 +206,7 @@ func (s *Rest) routes() chi.Router { } router := chi.NewRouter() router.Use(middleware.Throttle(1000), middleware.RealIP, R.Recoverer(log.Default())) + router.Use(securityHeadersMiddleware(s.ExternalImageProxy, s.AllowedAncestors)) if !s.DisableSignature { router.Use(R.AppInfo("remark42", "umputun", s.Version)) } @@ -226,11 +228,6 @@ func (s *Rest) routes() chi.Router { router.Use(corsMiddleware.Handler) } - if len(s.AllowedAncestors) > 0 { - log.Printf("[INFO] allowed from %+v only", s.AllowedAncestors) - router.Use(frameAncestors(s.AllowedAncestors)) - } - ipFn := func(ip string) string { return store.HashValue(ip, s.SharedSecret)[:12] } // logger uses it for anonymization logInfoWithBody := logger.New(logger.Log(log.Default()), logger.WithBody, logger.IPfn(ipFn), logger.Prefix("[INFO]")).Handler @@ -623,19 +620,23 @@ func cacheControl(expiration time.Duration, version string) func(http.Handler) h } } -// frameAncestors is a middleware setting Content-Security-Policy "frame-ancestors host1 host2 ..." -// prevents loading of comments widgets from any other origins. In case if the list of allowed empty, ignored. -func frameAncestors(hosts []string) func(http.Handler) http.Handler { - return func(h http.Handler) http.Handler { - fn := func(w http.ResponseWriter, r *http.Request) { - if len(hosts) == 0 { - h.ServeHTTP(w, r) - return +// securityHeadersMiddleware sets security-related headers: Content-Security-Policy and Permissions-Policy +func securityHeadersMiddleware(imageProxyEnabled bool, allowedAncestors []string) func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + imgSrc := "'self'" + if imageProxyEnabled { + imgSrc = "*" } - w.Header().Set("Content-Security-Policy", "frame-ancestors "+strings.Join(hosts, " ")+";") - h.ServeHTTP(w, r) - } - return http.HandlerFunc(fn) + frameAncestors := "*" + if len(allowedAncestors) > 0 { + log.Printf("[INFO] frame embedding allowed from %+v only", allowedAncestors) + frameAncestors = strings.Join(allowedAncestors, " ") + } + w.Header().Set("Content-Security-Policy", fmt.Sprintf("default-src 'none'; base-uri 'none'; form-action 'none'; connect-src 'self'; frame-src 'self'; img-src %s; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline'; font-src data:; object-src 'none'; frame-ancestors %s;", imgSrc, frameAncestors)) + w.Header().Set("Permissions-Policy", "accelerometer=(), autoplay=(), camera=(), cross-origin-isolated=(), display-capture=(), encrypted-media=(), fullscreen=(), geolocation=(), gyroscope=(), keyboard-map=(), magnetometer=(), microphone=(), midi=(), payment=(), picture-in-picture=(), publickey-credentials-get=(), screen-wake-lock=(), sync-xhr=(), usb=(), xr-spatial-tracking=(), clipboard-read=(), clipboard-write=(), gamepad=(), hid=(), idle-detection=(), interest-cohort=(), serial=(), unload=(), window-management=()") + next.ServeHTTP(w, r) + }) } } diff --git a/backend/app/rest/api/rest_test.go b/backend/app/rest/api/rest_test.go index 22e8126e..63f19e34 100644 --- a/backend/app/rest/api/rest_test.go +++ b/backend/app/rest/api/rest_test.go @@ -320,30 +320,29 @@ func TestRest_cacheControl(t *testing.T) { } func TestRest_frameAncestors(t *testing.T) { - tbl := []struct { - hosts []string - header string - }{ - {[]string{"http://example.com"}, "frame-ancestors http://example.com;"}, - {[]string{}, ""}, - {[]string{"http://example.com", "http://example2.com"}, "frame-ancestors http://example.com http://example2.com;"}, - } + ts, _, teardown := startupT(t, func(o *Rest) { + o.AllowedAncestors = []string{"'self'", "https://example.com"} + }) - for i, tt := range tbl { - tt := tt - t.Run(strconv.Itoa(i), func(t *testing.T) { - req := httptest.NewRequest("GET", "http://example.com", http.NoBody) - w := httptest.NewRecorder() + // Test case with frame-ancestors + client := http.Client{} + resp, err := client.Get(ts.URL + "/web/index.html") + require.NoError(t, err) + defer resp.Body.Close() + assert.Equal(t, http.StatusOK, resp.StatusCode) + assert.Contains(t, resp.Header.Get("Content-Security-Policy"), "frame-ancestors 'self' https://example.com;") + teardown() - h := frameAncestors(tt.hosts)(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {})) - h.ServeHTTP(w, req) - resp := w.Result() - assert.Equal(t, http.StatusOK, resp.StatusCode) - assert.NoError(t, resp.Body.Close()) - t.Logf("%+v", resp.Header) - assert.Equal(t, tt.header, resp.Header.Get("Content-Security-Policy")) - }) - } + // Test case without frame-ancestors + ts, _, teardown = startupT(t, func(srv *Rest) { + srv.AllowedAncestors = []string{} + }) + defer teardown() + resp, err = client.Get(ts.URL + "/web/index.html") + require.NoError(t, err) + defer resp.Body.Close() + assert.Equal(t, http.StatusOK, resp.StatusCode) + assert.Contains(t, resp.Header.Get("Content-Security-Policy"), "frame-ancestors *;") } func TestRest_subscribersOnly(t *testing.T) {