feat: add CSP and other security related headers in the oCIS proxy se…

…rvice (#8777) * feat: add CSP and other security related headers in the oCIS proxy service * fix: consolidate security related headers - drop middleware.Secure * fix: use github.com/DeepDiver1975/secure * fix: acceptance tests * feat: support env var replacements in csp.yaml
owncloud · Apr 26, 2024 · bdbba92 · bdbba92
1 parent d3415a8
commit bdbba92
Show file tree

Hide file tree

Showing 47 changed files with 2,357 additions and 36 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -49,6 +49,7 @@ linters-settings:
     replace-allow-list:
       - github.com/studio-b12/gowebdav
       - github.com/egirna/icap-client
+      - github.com/unrolled/secure
   interfacebloat:
     max: 15
 

diff --git a/changelog/unreleased/csp.md b/changelog/unreleased/csp.md
@@ -0,0 +1,5 @@
+Enhancement: Add CSP and other security related headers to oCIS
+
+General hardening of oCIS
+
+https://github.com/owncloud/ocis/pull/8777
diff --git a/deployments/examples/ocis_wopi/config/ocis/csp.yaml b/deployments/examples/ocis_wopi/config/ocis/csp.yaml
@@ -0,0 +1,35 @@
+directives:
+  child-src:
+    - '''self'''
+  connect-src:
+    - '''self'''
+  default-src:
+    - '''none'''
+  font-src:
+    - '''self'''
+  frame-ancestors:
+    - '''none'''
+  frame-src:
+    - '''self'''
+    - 'https://embed.diagrams.net/'
+    - 'https://${ONLYOFFICE_DOMAIN:-onlyoffice.owncloud.test}/'
+    - 'https://${COLLABORA_DOMAIN:-collabora.owncloud.test}/'
+  img-src:
+    - '''self'''
+    - 'data:'
+    - 'blob:'
+    - 'https://${ONLYOFFICE_DOMAIN:-onlyoffice.owncloud.test}/'
+    - 'https://${COLLABORA_DOMAIN:-collabora.owncloud.test}/'
+  manifest-src:
+    - '''self'''
+  media-src:
+    - '''self'''
+  object-src:
+    - '''self'''
+    - 'blob:'
+  script-src:
+    - '''self'''
+    - '''unsafe-inline'''
+  style-src:
+    - '''self'''
+    - '''unsafe-inline'''
diff --git a/deployments/examples/ocis_wopi/docker-compose.yml b/deployments/examples/ocis_wopi/docker-compose.yml
@@ -88,8 +88,10 @@ services:
       MICRO_REGISTRY_ADDRESS: 127.0.0.1:9233
       NATS_NATS_HOST: 0.0.0.0
       NATS_NATS_PORT: 9233
+      PROXY_CSP_CONFIG_FILE_LOCATION: /etc/ocis/csp.yaml
     volumes:
       - ./config/ocis/app-registry.yaml:/etc/ocis/app-registry.yaml
+      - ./config/ocis/csp.yaml:/etc/ocis/csp.yaml
       - ./config/ocis/${COMPANION_WEB_CONFIG_FILE_NAME:-web.yaml}:/etc/ocis/web.yaml
       - ocis-config:/etc/ocis
       - ocis-data:/var/lib/ocis

diff --git a/go.mod b/go.mod
@@ -9,6 +9,7 @@ require (
 	github.com/Masterminds/semver v1.5.0
 	github.com/MicahParks/keyfunc v1.9.0
 	github.com/Nerzal/gocloak/v13 v13.9.0
+	github.com/a8m/envsubst v1.4.2
 	github.com/bbalet/stopwords v1.0.0
 	github.com/blevesearch/bleve/v2 v2.4.0
 	github.com/cenkalti/backoff v2.2.1+incompatible
@@ -86,6 +87,7 @@ require (
 	github.com/thejerf/suture/v4 v4.0.5
 	github.com/tidwall/gjson v1.17.1
 	github.com/tus/tusd v1.13.0
+	github.com/unrolled/secure v1.14.0
 	github.com/urfave/cli/v2 v2.27.1
 	github.com/xhit/go-simple-mail/v2 v2.16.0
 	go-micro.dev/v4 v4.10.2
@@ -355,6 +357,8 @@ replace github.com/studio-b12/gowebdav => github.com/aduffeck/gowebdav v0.0.0-20
 
 replace github.com/egirna/icap-client => github.com/fschade/icap-client v0.0.0-20240123094924-5af178158eaf
 
+replace github.com/unrolled/secure => github.com/DeepDiver1975/secure v0.0.0-20240424132259-5b29166734cb
+
 // exclude the v2 line of go-sqlite3 which was released accidentally and prevents pulling in newer versions of go-sqlite3
 // see https://github.com/mattn/go-sqlite3/issues/965 for more details
 exclude github.com/mattn/go-sqlite3 v2.0.3+incompatible
diff --git a/go.sum b/go.sum
@@ -798,6 +798,8 @@ github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym
 github.com/CiscoM31/godata v1.0.10 h1:DZdJ6M8QNh4HquvDDOqNLu6h77Wl86KGK7Qlbmb90sk=
 github.com/CiscoM31/godata v1.0.10/go.mod h1:ZMiT6JuD3Rm83HEtiTx4JEChsd25YCrxchKGag/sdTc=
 github.com/DataDog/datadog-go v3.2.0+incompatible/go.mod h1:LButxg5PwREeZtORoXG3tL4fMGNddJ+vMq1mwgfaqoQ=
+github.com/DeepDiver1975/secure v0.0.0-20240424132259-5b29166734cb h1:Ugrv7ivJ035zunmhmGEBSXL76tyxRNH5XaBSQUTqf38=
+github.com/DeepDiver1975/secure v0.0.0-20240424132259-5b29166734cb/go.mod h1:BmF5hyM6tXczk3MpQkFf1hpKSRqCyhqcbiQtiAF7+40=
 github.com/JohnCGriffin/overflow v0.0.0-20211019200055-46fa312c352c/go.mod h1:X0CRv0ky0k6m906ixxpzmDRLvX58TFUKS2eePweuyxk=
 github.com/KimMachineGun/automemlimit v0.6.0 h1:p/BXkH+K40Hax+PuWWPQ478hPjsp9h1CPDhLlA3Z37E=
 github.com/KimMachineGun/automemlimit v0.6.0/go.mod h1:T7xYht7B8r6AG/AqFcUdc7fzd2bIdBKmepfP2S1svPY=
@@ -824,6 +826,8 @@ github.com/RoaringBitmap/roaring v1.2.3 h1:yqreLINqIrX22ErkKI0vY47/ivtJr6n+kMhVO
 github.com/RoaringBitmap/roaring v1.2.3/go.mod h1:plvDsJQpxOC5bw8LRteu/MLWHsHez/3y6cubLI4/1yE=
 github.com/Shopify/sarama v1.19.0/go.mod h1:FVkBWblsNy7DGZRfXLU0O9RCGt5g3g3yEuWXgklEdEo=
 github.com/Shopify/toxiproxy v2.1.4+incompatible/go.mod h1:OXgGpZ6Cli1/URJOF1DMxUHB2q5Ap20/P/eIdh4G0pI=
+github.com/a8m/envsubst v1.4.2 h1:4yWIHXOLEJHQEFd4UjrWDrYeYlV7ncFWJOCBRLOZHQg=
+github.com/a8m/envsubst v1.4.2/go.mod h1:MVUTQNGQ3tsjOOtKCNd+fl8RzhsXcDvvAEzkhGtlsbY=
 github.com/aduffeck/gowebdav v0.0.0-20231215102054-212d4a4374f6 h1:ws0yvsikTQdmheKINP16tBzAHdttrHwbz/q3Fgl9X1Y=
 github.com/aduffeck/gowebdav v0.0.0-20231215102054-212d4a4374f6/go.mod h1:bHA7t77X/QFExdeAnDzK6vKM34kEZAcE1OX4MfiwjkE=
 github.com/agnivade/levenshtein v1.1.1 h1:QY8M92nrzkmr798gCo3kmMyqXFzdQVpxLlGPRBij0P8=

diff --git a/ocis-pkg/middleware/header.go b/ocis-pkg/middleware/header.go
@@ -38,22 +38,3 @@ func Cors(opts ...cors.Option) func(http.Handler) http.Handler {
 		AllowCredentials: options.AllowCredentials,
 	})
 }
-
-// Secure writes required access headers to all requests.
-func Secure(next http.Handler) http.Handler {
-	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		// Indicates whether the browser is allowed to render this page in a <frame>, <iframe>, <embed> or <object>.
-		w.Header().Set("X-Frame-Options", "DENY")
-		// Does basically the same as X-Frame-Options.
-		w.Header().Set("Content-Security-Policy", "frame-ancestors 'none'")
-		// This header inidicates that MIME types advertised in the Content-Type headers should not be changed and be followed.
-		w.Header().Set("X-Content-Type-Options", "nosniff")
-
-		if r.TLS != nil {
-			// Tell browsers that the website should only be accessed  using HTTPS.
-			w.Header().Set("Strict-Transport-Security", "max-age=31536000")
-		}
-
-		next.ServeHTTP(w, r)
-	})
-}
diff --git a/ocis-pkg/service/debug/service.go b/ocis-pkg/service/debug/service.go
@@ -68,7 +68,6 @@ func NewService(opts ...Option) *http.Server {
 				cors.AllowedHeaders(dopts.CorsAllowedHeaders),
 				cors.AllowCredentials(dopts.CorsAllowCredentials),
 			),
-			middleware.Secure,
 			middleware.Version(
 				dopts.Name,
 				dopts.Version,

diff --git a/services/graph/pkg/server/http/server.go b/services/graph/pkg/server/http/server.go
@@ -79,7 +79,6 @@ func Server(opts ...Option) (http.Service, error) {
 			cors.AllowedHeaders(options.Config.HTTP.CORS.AllowedHeaders),
 			cors.AllowCredentials(options.Config.HTTP.CORS.AllowCredentials),
 		),
-		middleware.Secure,
 	}
 	// how do we secure the api?
 	var requireAdminMiddleware func(stdhttp.Handler) stdhttp.Handler

diff --git a/services/idp/pkg/server/http/server.go b/services/idp/pkg/server/http/server.go
@@ -61,7 +61,6 @@ func Server(opts ...Option) (http.Service, error) {
 			chimiddleware.RequestID,
 			middleware.TraceContext,
 			middleware.NoCache,
-			middleware.Secure,
 			middleware.Version(
 				options.Config.Service.Name,
 				version.GetString(),

diff --git a/services/invitations/pkg/server/http/server.go b/services/invitations/pkg/server/http/server.go
@@ -55,7 +55,6 @@ func Server(opts ...Option) (ohttp.Service, error) {
 			cors.AllowedHeaders(options.Config.HTTP.CORS.AllowedHeaders),
 			cors.AllowCredentials(options.Config.HTTP.CORS.AllowCredentials),
 		))
-	mux.Use(middleware.Secure)
 
 	mux.Use(middleware.Version(
 		options.Name,

diff --git a/services/ocs/pkg/server/http/server.go b/services/ocs/pkg/server/http/server.go
@@ -69,7 +69,6 @@ func Server(opts ...Option) (http.Service, error) {
 				cors.AllowedHeaders(options.Config.HTTP.CORS.AllowedHeaders),
 				cors.AllowCredentials(options.Config.HTTP.CORS.AllowCredentials),
 			),
-			middleware.Secure,
 			middleware.Version(
 				options.Config.Service.Name,
 				version.GetString(),

diff --git a/services/proxy/pkg/command/server.go b/services/proxy/pkg/command/server.go
@@ -300,6 +300,11 @@ func loadMiddlewares(ctx context.Context, logger log.Logger, cfg *config.Config,
 		Now:                time.Now,
 	})
 
+	cspConfig, err := middleware.LoadCSPConfig(cfg)
+	if err != nil {
+		logger.Fatal().Err(err).Msg("Failed to load CSP configuration.")
+	}
+
 	return alice.New(
 		// first make sure we log all requests and redirect to https if necessary
 		otelhttp.NewMiddleware("proxy",
@@ -315,6 +320,7 @@ func loadMiddlewares(ctx context.Context, logger log.Logger, cfg *config.Config,
 		chimiddleware.RequestID,
 		middleware.AccessLog(logger),
 		middleware.HTTPSRedirect,
+		middleware.Security(cspConfig),
 		router.Middleware(cfg.PolicySelector, cfg.Policies, logger),
 		middleware.Authentication(
 			authenticators,

diff --git a/services/proxy/pkg/config/config.go b/services/proxy/pkg/config/config.go
@@ -42,6 +42,7 @@ type Config struct {
 	BackendHTTPSCACert    string             `yaml:"backend_https_cacert" env:"PROXY_HTTPS_CACERT" desc:"Path/File for the root CA certificate used to validate the server’s TLS certificate for https enabled backend services." introductionVersion:"pre5.0"`
 	AuthMiddleware        AuthMiddleware     `yaml:"auth_middleware"`
 	PoliciesMiddleware    PoliciesMiddleware `yaml:"policies_middleware"`
+	CSPConfigFileLocation string             `yaml:"csp_config_file_location" env:"PROXY_CSP_CONFIG_FILE_LOCATION" desc:"The location of the CSP configuration file." introductionVersion:"6.0"`
 
 	Context context.Context `yaml:"-" json:"-"`
 }
@@ -140,7 +141,7 @@ type RoleAssignment struct {
 	OIDCRoleMapper OIDCRoleMapper `yaml:"oidc_role_mapper"`
 }
 
-// OIDCRoleMapper contains the configuration for the "oidc" role assignment driber
+// OIDCRoleMapper contains the configuration for the "oidc" role assignment driver
 type OIDCRoleMapper struct {
 	RoleClaim string        `yaml:"role_claim" env:"PROXY_ROLE_ASSIGNMENT_OIDC_CLAIM" desc:"The OIDC claim used to create the users role assignment." introductionVersion:"pre5.0"`
 	RolesMap  []RoleMapping `yaml:"role_mapping" desc:"A list of mappings of ocis role names to PROXY_ROLE_ASSIGNMENT_OIDC_CLAIM claim values. This setting can only be configured in the configuration file and not via environment variables."`

diff --git a/services/proxy/pkg/config/csp.go b/services/proxy/pkg/config/csp.go
@@ -0,0 +1,13 @@
+package config
+
+import (
+	_ "embed"
+)
+
+// CSP defines CSP header directives
+type CSP struct {
+	Directives map[string][]string `yaml:"directives"`
+}
+
+//go:embed csp.yaml
+var DefaultCSPConfig string
diff --git a/services/proxy/pkg/config/csp.yaml b/services/proxy/pkg/config/csp.yaml
@@ -0,0 +1,31 @@
+directives:
+  child-src:
+    - '''self'''
+  connect-src:
+    - '''self'''
+  default-src:
+    - '''none'''
+  font-src:
+    - '''self'''
+  frame-ancestors:
+    - '''none'''
+  frame-src:
+    - '''self'''
+    - 'https://embed.diagrams.net/'
+  img-src:
+    - '''self'''
+    - 'data:'
+    - 'blob:'
+  manifest-src:
+    - '''self'''
+  media-src:
+    - '''self'''
+  object-src:
+    - '''self'''
+    - 'blob:'
+  script-src:
+    - '''self'''
+    - '''unsafe-inline'''
+  style-src:
+    - '''self'''
+    - '''unsafe-inline'''
diff --git a/services/proxy/pkg/config/defaults/defaultconfig.go b/services/proxy/pkg/config/defaults/defaultconfig.go
@@ -86,6 +86,7 @@ func DefaultConfig() *config.Config {
 		AutoprovisionAccounts: false,
 		EnableBasicAuth:       false,
 		InsecureBackends:      false,
+		CSPConfigFileLocation: "",
 	}
 }
 

diff --git a/services/proxy/pkg/middleware/security.go b/services/proxy/pkg/middleware/security.go
@@ -0,0 +1,61 @@
+package middleware
+
+import (
+	"github.com/a8m/envsubst"
+	"github.com/owncloud/ocis/v2/services/proxy/pkg/config"
+	"github.com/unrolled/secure"
+	"github.com/unrolled/secure/cspbuilder"
+	"gopkg.in/yaml.v2"
+	"net/http"
+	"os"
+)
+
+// LoadCSPConfig loads CSP header configuration from a yaml file.
+func LoadCSPConfig(proxyCfg *config.Config) (*config.CSP, error) {
+	yamlContent, err := loadCSPYaml(proxyCfg)
+	if err != nil {
+		return nil, err
+	}
+	// replace env vars ..
+	yamlContent, err = envsubst.Bytes(yamlContent)
+	if err != nil {
+		return nil, err
+	}
+
+	// read yaml
+	cspConfig := config.CSP{}
+	err = yaml.Unmarshal(yamlContent, &cspConfig)
+	if err != nil {
+		return nil, err
+	}
+
+	return &cspConfig, nil
+}
+
+func loadCSPYaml(proxyCfg *config.Config) ([]byte, error) {
+	if proxyCfg.CSPConfigFileLocation == "" {
+		return []byte(config.DefaultCSPConfig), nil
+	}
+	return os.ReadFile(proxyCfg.CSPConfigFileLocation)
+}
+
+// Security is a middleware to apply security relevant http headers like CSP.
+func Security(cspConfig *config.CSP) func(h http.Handler) http.Handler {
+	cspBuilder := cspbuilder.Builder{
+		Directives: cspConfig.Directives,
+	}
+
+	secureMiddleware := secure.New(secure.Options{
+		BrowserXssFilter:        true,
+		ContentSecurityPolicy:   cspBuilder.MustBuild(),
+		ContentTypeNosniff:      true,
+		CustomFrameOptionsValue: "SAMEORIGIN",
+		FrameDeny:               true,
+		ReferrerPolicy:          "strict-origin-when-cross-origin",
+		STSSeconds:              315360000,
+		STSPreload:              true,
+	})
+	return func(next http.Handler) http.Handler {
+		return secureMiddleware.Handler(next)
+	}
+}
diff --git a/services/settings/pkg/server/http/server.go b/services/settings/pkg/server/http/server.go
@@ -52,7 +52,6 @@ func Server(opts ...Option) (ohttp.Service, error) {
 		cors.AllowedHeaders(options.Config.HTTP.CORS.AllowedHeaders),
 		cors.AllowCredentials(options.Config.HTTP.CORS.AllowCredentials),
 	))
-	mux.Use(middleware.Secure)
 	mux.Use(middleware.ExtractAccountUUID(
 		account.Logger(options.Logger),
 		account.JWTSecret(options.Config.TokenManager.JWTSecret)),

diff --git a/services/sse/pkg/server/http/server.go b/services/sse/pkg/server/http/server.go
@@ -64,7 +64,6 @@ func Server(opts ...Option) (http.Service, error) {
 			cors.AllowedHeaders(options.Config.HTTP.CORS.AllowedHeaders),
 			cors.AllowCredentials(options.Config.HTTP.CORS.AllowCredentials),
 		),
-		middleware.Secure,
 	}
 
 	mux := chi.NewMux()

diff --git a/services/thumbnails/pkg/server/http/server.go b/services/thumbnails/pkg/server/http/server.go
@@ -39,7 +39,6 @@ func Server(opts ...Option) (http.Service, error) {
 		svc.Middleware(
 			middleware.RealIP,
 			middleware.RequestID,
-			// ocismiddleware.Secure,
 			ocismiddleware.Version(
 				options.Config.Service.Name,
 				version.GetString(),

diff --git a/services/userlog/pkg/server/http/server.go b/services/userlog/pkg/server/http/server.go
@@ -63,7 +63,6 @@ func Server(opts ...Option) (http.Service, error) {
 			cors.AllowedHeaders(options.Config.HTTP.CORS.AllowedHeaders),
 			cors.AllowCredentials(options.Config.HTTP.CORS.AllowCredentials),
 		),
-		middleware.Secure,
 	}
 
 	mux := chi.NewMux()

diff --git a/services/web/pkg/middleware/silentrefresh.go b/services/web/pkg/middleware/silentrefresh.go
@@ -7,7 +7,6 @@ import (
 // SilentRefresh allows the oidc client lib to silently refresh the token in an iframe
 func SilentRefresh(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		w.Header().Set("X-Frame-Options", "SAMEORIGIN")
 		w.Header().Set("Content-Security-Policy", "frame-ancestors 'self'")
 		next.ServeHTTP(w, r)
 	})

diff --git a/services/web/pkg/server/http/server.go b/services/web/pkg/server/http/server.go
@@ -84,7 +84,6 @@ func Server(opts ...Option) (http.Service, error) {
 			chimiddleware.RealIP,
 			chimiddleware.RequestID,
 			middleware.NoCache,
-			middleware.Secure,
 			webmid.SilentRefresh,
 			middleware.Version(
 				"web",

diff --git a/services/webdav/pkg/server/http/server.go b/services/webdav/pkg/server/http/server.go
@@ -48,7 +48,6 @@ func Server(opts ...Option) (http.Service, error) {
 				cors.AllowedHeaders(options.Config.HTTP.CORS.AllowedHeaders),
 				cors.AllowCredentials(options.Config.HTTP.CORS.AllowCredentials),
 			),
-			middleware.Secure,
 			middleware.Version(
 				options.Config.Service.Name,
 				version.GetString(),

diff --git a/services/webfinger/pkg/server/http/server.go b/services/webfinger/pkg/server/http/server.go
@@ -57,7 +57,6 @@ func Server(opts ...Option) (ohttp.Service, error) {
 			cors.AllowedHeaders(options.Config.HTTP.CORS.AllowedHeaders),
 			cors.AllowCredentials(options.Config.HTTP.CORS.AllowCredentials),
 		))
-	mux.Use(middleware.Secure)
 
 	mux.Use(middleware.Version(
 		options.Name,