[feature/bugfix] Probe S3 storage for CSP uri, add config flag for extra URIs (#2134)

* [feature/bugfix] Probe S3 storage for CSP uri, add config flag for extra URIs

* env parsing tests, my coy mistress

cmd/gotosocial/action/server/server.go

@@ -204,6 +204,29 @@ var Start action.GTSAction = func(ctx context.Context) error {
 		middleware.ExtraHeaders(),
 	}...)
 
+	// Instantiate Content-Security-Policy
+	// middleware, with extra URIs.
+	cspExtraURIs := make([]string, 0)
+
+	// Probe storage to check if extra URI is needed in CSP.
+	// Error here means something is wrong with storage.
+	storageCSPUri, err := state.Storage.ProbeCSPUri(ctx)
+	if err != nil {
+		return fmt.Errorf("error deriving Content-Security-Policy uri from storage: %w", err)
+	}
+
+	// storageCSPUri may be empty string if
+	// not S3-backed storage; check for this.
+	if storageCSPUri != "" {
+		cspExtraURIs = append(cspExtraURIs, storageCSPUri)
+	}
+
+	// Add any extra CSP URIs from config.
+	cspExtraURIs = append(cspExtraURIs, config.GetAdvancedCSPExtraURIs()...)
+
+	// Add CSP to middlewares.
+	middlewares = append(middlewares, middleware.ContentSecurityPolicy(cspExtraURIs...))
+
 	// attach global middlewares which are used for every request
 	router.AttachGlobalMiddleware(middlewares...)
 

cmd/gotosocial/action/testrig/testrig.go

@@ -70,7 +70,11 @@ var Start action.GTSAction = func(ctx context.Context) error {
 	testrig.StandardDBSetup(state.DB, nil)
 
 	if os.Getenv("GTS_STORAGE_BACKEND") == "s3" {
-		state.Storage, _ = storage.NewS3Storage()
+		var err error
+		state.Storage, err = storage.NewS3Storage()
+		if err != nil {
+			return fmt.Errorf("error initializing storage: %w", err)
+		}
 	} else {
 		state.Storage = testrig.NewInMemoryStorage()
 	}
@@ -136,6 +140,29 @@ var Start action.GTSAction = func(ctx context.Context) error {
 		middleware.ExtraHeaders(),
 	}...)
 
+	// Instantiate Content-Security-Policy
+	// middleware, with extra URIs.
+	cspExtraURIs := make([]string, 0)
+
+	// Probe storage to check if extra URI is needed in CSP.
+	// Error here means something is wrong with storage.
+	storageCSPUri, err := state.Storage.ProbeCSPUri(ctx)
+	if err != nil {
+		return fmt.Errorf("error deriving Content-Security-Policy uri from storage: %w", err)
+	}
+
+	// storageCSPUri may be empty string if
+	// not S3-backed storage; check for this.
+	if storageCSPUri != "" {
+		cspExtraURIs = append(cspExtraURIs, storageCSPUri)
+	}
+
+	// Add any extra CSP URIs from config.
+	cspExtraURIs = append(cspExtraURIs, config.GetAdvancedCSPExtraURIs()...)
+
+	// Add CSP to middlewares.
+	middlewares = append(middlewares, middleware.ContentSecurityPolicy(cspExtraURIs...))
+
 	// attach global middlewares which are used for every request
 	router.AttachGlobalMiddleware(middlewares...)
 
@@ -146,7 +173,6 @@ var Start action.GTSAction = func(ctx context.Context) error {
 
 	// build router modules
 	var idp oidc.IDP
-	var err error
 	if config.GetOIDCEnabled() {
 		idp, err = oidc.NewIDP(ctx)
 		if err != nil {

docs/configuration/advanced.md

@@ -118,4 +118,22 @@ advanced-throttling-retry-after: "30s"
 # 2 cpu = 1 concurrent sender
 # 4 cpu = 1 concurrent sender
 advanced-sender-multiplier: 2
+
+# Array of string. Extra URIs to add to 'img-src' and 'media-src'
+# when building the Content-Security-Policy header for your instance.
+#
+# This can be used to allow the browser to load resources from additional
+# sources like S3 buckets and so on when viewing your instance's pages
+# and profiles in the browser.
+#
+# Since non-proxying S3 storage will be probed on instance launch to
+# generate a correct Content-Security-Policy, you probably won't need
+# to ever touch this setting, but it's included in the 'spirit of more
+# configurable (usually) means more good'.
+# 
+# See: https://developer.mozilla.org/en-US/docs/Web/HTTP/CSP
+#
+# Example: ["s3.example.org", "some-bucket-name.s3.example.org"]
+# Default: []
+advanced-csp-extra-uris: []
 ```

example/config.yaml

@@ -903,3 +903,21 @@ advanced-throttling-retry-after: "30s"
 # 2 cpu = 1 concurrent sender
 # 4 cpu = 1 concurrent sender
 advanced-sender-multiplier: 2
+
+# Array of string. Extra URIs to add to 'img-src' and 'media-src'
+# when building the Content-Security-Policy header for your instance.
+#
+# This can be used to allow the browser to load resources from additional
+# sources like S3 buckets and so on when viewing your instance's pages
+# and profiles in the browser.
+#
+# Since non-proxying S3 storage will be probed on instance launch to
+# generate a correct Content-Security-Policy, you probably won't need
+# to ever touch this setting, but it's included in the 'spirit of more
+# configurable (usually) means more good'.
+# 
+# See: https://developer.mozilla.org/en-US/docs/Web/HTTP/CSP
+#
+# Example: ["s3.example.org", "some-bucket-name.s3.example.org"]
+# Default: []
+advanced-csp-extra-uris: []

internal/config/config.go

@@ -150,6 +150,7 @@ type Configuration struct {
 	AdvancedThrottlingMultiplier int           `name:"advanced-throttling-multiplier" usage:"Multiplier to use per cpu for http request throttling. 0 or less turns throttling off."`
 	AdvancedThrottlingRetryAfter time.Duration `name:"advanced-throttling-retry-after" usage:"Retry-After duration response to send for throttled requests."`
 	AdvancedSenderMultiplier     int           `name:"advanced-sender-multiplier" usage:"Multiplier to use per cpu for batching outgoing fedi messages. 0 or less turns batching off (not recommended)."`
+	AdvancedCSPExtraURIs         []string      `name:"advanced-csp-extra-uris" usage:"Additional URIs to allow when building content-security-policy for media + images."`
 
 	// HTTPClient configuration vars.
 	HTTPClient HTTPClientConfiguration `name:"http-client"`

internal/config/defaults.go

@@ -124,6 +124,7 @@ var Defaults = Configuration{
 	AdvancedThrottlingMultiplier: 8,   // 8 open requests per CPU
 	AdvancedThrottlingRetryAfter: time.Second * 30,
 	AdvancedSenderMultiplier:     2, // 2 senders per CPU
+	AdvancedCSPExtraURIs:         []string{},
 
 	Cache: CacheConfiguration{
 		// Rough memory target that the total

internal/config/flags.go

@@ -151,6 +151,7 @@ func (s *ConfigState) AddServerFlags(cmd *cobra.Command) {
 		cmd.Flags().Int(AdvancedThrottlingMultiplierFlag(), cfg.AdvancedThrottlingMultiplier, fieldtag("AdvancedThrottlingMultiplier", "usage"))
 		cmd.Flags().Duration(AdvancedThrottlingRetryAfterFlag(), cfg.AdvancedThrottlingRetryAfter, fieldtag("AdvancedThrottlingRetryAfter", "usage"))
 		cmd.Flags().Int(AdvancedSenderMultiplierFlag(), cfg.AdvancedSenderMultiplier, fieldtag("AdvancedSenderMultiplier", "usage"))
+		cmd.Flags().StringSlice(AdvancedCSPExtraURIsFlag(), cfg.AdvancedCSPExtraURIs, fieldtag("AdvancedCSPExtraURIs", "usage"))
 
 		cmd.Flags().String(RequestIDHeaderFlag(), cfg.RequestIDHeader, fieldtag("RequestIDHeader", "usage"))
 	})

internal/config/helpers.gen.go

@@ -2324,6 +2324,31 @@ func GetAdvancedSenderMultiplier() int { return global.GetAdvancedSenderMultipli
 // SetAdvancedSenderMultiplier safely sets the value for global configuration 'AdvancedSenderMultiplier' field
 func SetAdvancedSenderMultiplier(v int) { global.SetAdvancedSenderMultiplier(v) }
 
+// GetAdvancedCSPExtraURIs safely fetches the Configuration value for state's 'AdvancedCSPExtraURIs' field
+func (st *ConfigState) GetAdvancedCSPExtraURIs() (v []string) {
+	st.mutex.RLock()
+	v = st.config.AdvancedCSPExtraURIs
+	st.mutex.RUnlock()
+	return
+}
+
+// SetAdvancedCSPExtraURIs safely sets the Configuration value for state's 'AdvancedCSPExtraURIs' field
+func (st *ConfigState) SetAdvancedCSPExtraURIs(v []string) {
+	st.mutex.Lock()
+	defer st.mutex.Unlock()
+	st.config.AdvancedCSPExtraURIs = v
+	st.reloadToViper()
+}
+
+// AdvancedCSPExtraURIsFlag returns the flag name for the 'AdvancedCSPExtraURIs' field
+func AdvancedCSPExtraURIsFlag() string { return "advanced-csp-extra-uris" }
+
+// GetAdvancedCSPExtraURIs safely fetches the value for global configuration 'AdvancedCSPExtraURIs' field
+func GetAdvancedCSPExtraURIs() []string { return global.GetAdvancedCSPExtraURIs() }
+
+// SetAdvancedCSPExtraURIs safely sets the value for global configuration 'AdvancedCSPExtraURIs' field
+func SetAdvancedCSPExtraURIs(v []string) { global.SetAdvancedCSPExtraURIs(v) }
+
 // GetHTTPClientAllowIPs safely fetches the Configuration value for state's 'HTTPClient.AllowIPs' field
 func (st *ConfigState) GetHTTPClientAllowIPs() (v []string) {
 	st.mutex.RLock()

internal/middleware/contentsecuritypolicy.go

@@ -0,0 +1,144 @@
+// GoToSocial
+// Copyright (C) GoToSocial Authors admin@gotosocial.org
+// SPDX-License-Identifier: AGPL-3.0-or-later
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package middleware
+
+import (
+	"strings"
+
+	"codeberg.org/gruf/go-debug"
+	"github.com/gin-gonic/gin"
+)
+
+func ContentSecurityPolicy(extraURIs ...string) gin.HandlerFunc {
+	csp := BuildContentSecurityPolicy(extraURIs...)
+
+	return func(c *gin.Context) {
+		// Inform the browser we only load
+		// CSS/JS/media using the given policy.
+		c.Header("Content-Security-Policy", csp)
+	}
+}
+
+func BuildContentSecurityPolicy(extraURIs ...string) string {
+	const (
+		defaultSrc = "default-src"
+		objectSrc  = "object-src"
+		imgSrc     = "img-src"
+		mediaSrc   = "media-src"
+
+		self = "'self'"
+		none = "'none'"
+		blob = "blob:"
+	)
+
+	// CSP values keyed by directive.
+	values := make(map[string][]string, 4)
+
+	/*
+		default-src
+		https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/default-src
+	*/
+
+	if !debug.DEBUG {
+		// Restrictive 'self' policy
+		values[defaultSrc] = []string{self}
+	} else {
+		// If debug is enabled, allow
+		// serving things from localhost
+		// as well (regardless of port).
+		values[defaultSrc] = []string{
+			self,
+			"localhost:*",
+			"ws://localhost:*",
+		}
+	}
+
+	/*
+		object-src
+		https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/object-src
+	*/
+
+	// Disallow object-src as recommended.
+	values[objectSrc] = []string{none}
+
+	/*
+		img-src
+		https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/img-src
+	*/
+
+	// Restrictive 'self' policy,
+	// include extraURIs, and 'blob:'
+	// for previewing uploaded images
+	// (header, avi, emojis) in settings.
+	values[imgSrc] = append(
+		[]string{self, blob},
+		extraURIs...,
+	)
+
+	/*
+		media-src
+		https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/media-src
+	*/
+
+	// Restrictive 'self' policy,
+	// include extraURIs.
+	values[mediaSrc] = append(
+		[]string{self},
+		extraURIs...,
+	)
+
+	/*
+		Assemble policy directives.
+	*/
+
+	// Iterate through an ordered slice rather than
+	// iterating through the map, since we want these
+	// policyDirectives in a determinate order.
+	policyDirectives := make([]string, 4)
+	for i, directive := range []string{
+		defaultSrc,
+		objectSrc,
+		imgSrc,
+		mediaSrc,
+	} {
+		// Each policy directive should look like:
+		// `[directive] [value1] [value2] [etc]`
+
+		// Get assembled values
+		// for this directive.
+		values := values[directive]
+
+		// Prepend values with
+		// the directive name.
+		directiveValues := append(
+			[]string{directive},
+			values...,
+		)
+
+		// Space-separate them.
+		policyDirective := strings.Join(directiveValues, " ")
+
+		// Done.
+		policyDirectives[i] = policyDirective
+	}
+
+	// Content-security-policy looks like this:
+	// `Content-Security-Policy: <policy-directive>; <policy-directive>`
+	// So join each policy directive appropriately.
+	return strings.Join(policyDirectives, "; ")
+}

internal/middleware/extraheaders.go

@@ -18,15 +18,11 @@
 package middleware
 
 import (
-	"codeberg.org/gruf/go-debug"
 	"github.com/gin-gonic/gin"
-	"github.com/superseriousbusiness/gotosocial/internal/config"
 )
 
 // ExtraHeaders returns a new gin middleware which adds various extra headers to the response.
 func ExtraHeaders() gin.HandlerFunc {
-	csp := BuildContentSecurityPolicy()
-
 	return func(c *gin.Context) {
 		// Inform all callers which server implementation this is.
 		c.Header("Server", "gotosocial")
@@ -39,56 +35,5 @@ func ExtraHeaders() gin.HandlerFunc {
 		//
 		// See: https://github.com/patcg-individual-drafts/topics
 		c.Header("Permissions-Policy", "browsing-topics=()")
-
-		// Inform the browser we only load
-		// CSS/JS/media using the given policy.
-		c.Header("Content-Security-Policy", csp)
 	}
 }
-
-func BuildContentSecurityPolicy() string {
-	// Start with restrictive policy.
-	policy := "default-src 'self'"
-
-	if debug.DEBUG {
-		// Debug is enabled, allow
-		// serving things from localhost
-		// as well (regardless of port).
-		policy += " localhost:* ws://localhost:*"
-	}
-
-	// Disallow object-src as recommended https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/object-src
-	policy += "; object-src 'none'"
-
-	s3Endpoint := config.GetStorageS3Endpoint()
-	if s3Endpoint == "" || config.GetStorageS3Proxy() {
-		// S3 not configured or in proxy mode, just allow images from self and blob:
-		policy += "; img-src 'self' blob:"
-		return policy
-	}
-
-	// S3 is on and in non-proxy mode, so we need to add the S3 host to
-	// the policy to allow images and video to be pulled from there too.
-
-	// If secure is false,
-	// use 'http' scheme.
-	scheme := "https"
-	if !config.GetStorageS3UseSSL() {
-		scheme = "http"
-	}
-
-	// Construct endpoint URL.
-	s3EndpointURLStr := scheme + "://" + s3Endpoint
-
-	// When object storage is in use in non-proxied mode, GtS still serves some
-	// assets itself like the logo, so keep 'self' in there. That should also
-	// handle any redirects from the fileserver to object storage.
-
-	// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/img-src
-	policy += "; img-src 'self' blob: " + s3EndpointURLStr
-
-	// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/media-src
-	policy += "; media-src 'self' " + s3EndpointURLStr
-
-	return policy
-}

internal/middleware/middleware_test.go

@@ -20,80 +20,53 @@ package middleware_test
 import (
 	"testing"
 
-	"github.com/superseriousbusiness/gotosocial/internal/config"
 	"github.com/superseriousbusiness/gotosocial/internal/middleware"
 )
 
 func TestBuildContentSecurityPolicy(t *testing.T) {
 	type cspTest struct {
-		s3Endpoint string
+		extraURLs []string
-		s3Proxy    bool
-		s3Secure   bool
 		expected  string
-		actual     string
 	}
 
 	for _, test := range []cspTest{
 		{
-			s3Endpoint: "",
+			extraURLs: nil,
-			s3Proxy:    false,
+			expected:  "default-src 'self'; object-src 'none'; img-src 'self' blob:; media-src 'self'",
-			s3Secure:   false,
-			expected:   "default-src 'self'; object-src 'none'; img-src 'self' blob:",
 		},
 		{
-			s3Endpoint: "some-bucket-provider.com",
+			extraURLs: []string{
-			s3Proxy:    false,
+				"https://some-bucket-provider.com",
-			s3Secure:   true,
+			},
 			expected: "default-src 'self'; object-src 'none'; img-src 'self' blob: https://some-bucket-provider.com; media-src 'self' https://some-bucket-provider.com",
 		},
 		{
-			s3Endpoint: "some-bucket-provider.com:6969",
+			extraURLs: []string{
-			s3Proxy:    false,
+				"https://some-bucket-provider.com:6969",
-			s3Secure:   true,
+			},
 			expected: "default-src 'self'; object-src 'none'; img-src 'self' blob: https://some-bucket-provider.com:6969; media-src 'self' https://some-bucket-provider.com:6969",
 		},
 		{
-			s3Endpoint: "some-bucket-provider.com:6969",
+			extraURLs: []string{
-			s3Proxy:    false,
+				"http://some-bucket-provider.com:6969",
-			s3Secure:   false,
+			},
 			expected: "default-src 'self'; object-src 'none'; img-src 'self' blob: http://some-bucket-provider.com:6969; media-src 'self' http://some-bucket-provider.com:6969",
 		},
 		{
-			s3Endpoint: "s3.nl-ams.scw.cloud",
+			extraURLs: []string{
-			s3Proxy:    false,
+				"https://s3.nl-ams.scw.cloud",
-			s3Secure:   true,
+			},
 			expected: "default-src 'self'; object-src 'none'; img-src 'self' blob: https://s3.nl-ams.scw.cloud; media-src 'self' https://s3.nl-ams.scw.cloud",
 		},
 		{
-			s3Endpoint: "some-bucket-provider.com",
+			extraURLs: []string{
-			s3Proxy:    true,
+				"https://s3.nl-ams.scw.cloud",
-			s3Secure:   true,
+				"https://s3.somewhere.else.example.org",
-			expected:   "default-src 'self'; object-src 'none'; img-src 'self' blob:",
 			},
-		{
+			expected: "default-src 'self'; object-src 'none'; img-src 'self' blob: https://s3.nl-ams.scw.cloud https://s3.somewhere.else.example.org; media-src 'self' https://s3.nl-ams.scw.cloud https://s3.somewhere.else.example.org",
-			s3Endpoint: "some-bucket-provider.com:6969",
-			s3Proxy:    true,
-			s3Secure:   true,
-			expected:   "default-src 'self'; object-src 'none'; img-src 'self' blob:",
-		},
-		{
-			s3Endpoint: "some-bucket-provider.com:6969",
-			s3Proxy:    true,
-			s3Secure:   true,
-			expected:   "default-src 'self'; object-src 'none'; img-src 'self' blob:",
-		},
-		{
-			s3Endpoint: "s3.nl-ams.scw.cloud",
-			s3Proxy:    true,
-			s3Secure:   true,
-			expected:   "default-src 'self'; object-src 'none'; img-src 'self' blob:",
 		},
 	} {
-		config.SetStorageS3Endpoint(test.s3Endpoint)
+		csp := middleware.BuildContentSecurityPolicy(test.extraURLs...)
-		config.SetStorageS3Proxy(test.s3Proxy)
-		config.SetStorageS3UseSSL(test.s3Secure)
-
-		csp := middleware.BuildContentSecurityPolicy()
 		if csp != test.expected {
 			t.Logf("expected '%s', got '%s'", test.expected, csp)
 			t.Fail()

internal/storage/storage.go

@@ -32,6 +32,8 @@ import (
 	"github.com/minio/minio-go/v7"
 	"github.com/minio/minio-go/v7/pkg/credentials"
 	"github.com/superseriousbusiness/gotosocial/internal/config"
+	"github.com/superseriousbusiness/gotosocial/internal/gtserror"
+	"github.com/superseriousbusiness/gotosocial/internal/log"
 )
 
 const (
@@ -145,6 +147,61 @@ func (d *Driver) URL(ctx context.Context, key string) *PresignedURL {
 	return &psu
 }
 
+// ProbeCSPUri returns a URI string that can be added
+// to a content-security-policy to allow requests to
+// endpoints served by this driver.
+//
+// If the driver is not backed by non-proxying S3,
+// this will return an empty string and no error.
+//
+// Otherwise, this function probes for a CSP URI by
+// doing the following:
+//
+//  1. Create a temporary file in the S3 bucket.
+//  2. Generate a pre-signed URL for that file.
+//  3. Extract '[scheme]://[host]' from the URL.
+//  4. Remove the temporary file.
+//  5. Return the '[scheme]://[host]' string.
+func (d *Driver) ProbeCSPUri(ctx context.Context) (string, error) {
+	// Check whether S3 without proxying
+	// is enabled. If it's not, there's
+	// no need to add anything to the CSP.
+	s3, ok := d.Storage.(*storage.S3Storage)
+	if !ok || d.Proxy {
+		return "", nil
+	}
+
+	const cspKey = "gotosocial-csp-probe"
+
+	// Create an empty file in S3 storage.
+	if _, err := d.Put(ctx, cspKey, make([]byte, 0)); err != nil {
+		return "", gtserror.Newf("error putting file in bucket at key %s: %w", cspKey, err)
+	}
+
+	// Try to clean up file whatever happens.
+	defer func() {
+		if err := d.Delete(ctx, cspKey); err != nil {
+			log.Warnf(ctx, "error deleting file from bucket at key %s (%v); "+
+				"you may want to remove this file manually from your S3 bucket", cspKey, err)
+		}
+	}()
+
+	// Get a presigned URL for that empty file.
+	u, err := s3.Client().PresignedGetObject(ctx, d.Bucket, cspKey, 1*time.Second, nil)
+	if err != nil {
+		return "", err
+	}
+
+	// Create a stripped version of the presigned
+	// URL that includes only the host and scheme.
+	uStripped := &url.URL{
+		Scheme: u.Scheme,
+		Host:   u.Host,
+	}
+
+	return uStripped.String(), nil
+}
+
 func AutoConfig() (*Driver, error) {
 	switch backend := config.GetStorageBackend(); backend {
 	case "s3":

test/envparsing.sh

@@ -11,6 +11,7 @@ EXPECT=$(cat << "EOF"
     "accounts-reason-required": false,
     "accounts-registration-open": true,
     "advanced-cookies-samesite": "strict",
+    "advanced-csp-extra-uris": [],
     "advanced-rate-limit-requests": 6969,
     "advanced-sender-multiplier": -1,
     "advanced-throttling-multiplier": -1,