Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add CSP and other security related headers in the oCIS proxy service #8777

Merged
merged 5 commits into from
Apr 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ linters-settings:
- github.com/go-micro/plugins/v4/store/nats-js-kv
- github.com/studio-b12/gowebdav
- github.com/egirna/icap-client
- github.com/unrolled/secure
interfacebloat:
max: 15

Expand Down
5 changes: 5 additions & 0 deletions changelog/unreleased/csp.md
Original file line number Diff line number Diff line change
@@ -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
35 changes: 35 additions & 0 deletions deployments/examples/ocis_wopi/config/ocis/csp.yaml
Original file line number Diff line number Diff line change
@@ -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'''
2 changes: 2 additions & 0 deletions deployments/examples/ocis_wopi/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DeepDiver1975
Shouldn't ONLYOFFICE_DOMAIN and COLLABORA_DOMAIN be set here as well in order for the substitution in etc/ocis/csp.yaml to take effect?

ONLYOFFICE_DOMAIN: ${ONLYOFFICE_DOMAIN:-onlyoffice.owncloud.test}
COLLABORA_DOMAIN: ${COLLABORA_DOMAIN:-collabora.owncloud.test}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DeepDiver1975 Yes, I mean the csp.yaml is being loaded inside the containers environment, so if the ONLYOFFICE_DOMAIN and COLLABORA_DOMAIN are not set in the docker-compose.yml file they will be empty during runtime and the default values are used. (This is at least what happened when I tried it.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah. I see your point now.let me have a look .....

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes - but the env vars are not forwarded into the ocis container and will always be set to the default.
pr coming ...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, right

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

THX a lot @kjeldahl for bringing this to our attention! 🙏

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DeepDiver1975 You are most welcome. And thanks a lot for solving the CSP issue 4 hours before I ran into it 👍

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
Expand Down
4 changes: 4 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -358,6 +360,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
4 changes: 4 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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.5.0 h1:BeOe+BbJc8L5chL3OwzVYjVzyvPALdd5wxVVOWuUZmQ=
github.com/KimMachineGun/automemlimit v0.5.0/go.mod h1:di3GCKiu9Y+1fs92erCbUvKzPkNyViN3mA0vti/ykEQ=
Expand All @@ -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=
Expand Down
19 changes: 0 additions & 19 deletions ocis-pkg/middleware/header.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
1 change: 0 additions & 1 deletion ocis-pkg/service/debug/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 0 additions & 1 deletion services/graph/pkg/server/http/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion services/idp/pkg/server/http/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
1 change: 0 additions & 1 deletion services/invitations/pkg/server/http/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 0 additions & 1 deletion services/ocs/pkg/server/http/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
6 changes: 6 additions & 0 deletions services/proxy/pkg/command/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,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",
Expand All @@ -313,6 +318,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,
Expand Down
3 changes: 2 additions & 1 deletion services/proxy/pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:"-"`
}
Expand Down Expand Up @@ -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."`
Expand Down
13 changes: 13 additions & 0 deletions services/proxy/pkg/config/csp.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package config

import (
_ "embed"
)

// CSP defines CSP header directives
type CSP struct {
DeepDiver1975 marked this conversation as resolved.
Show resolved Hide resolved
Directives map[string][]string `yaml:"directives"`
}

//go:embed csp.yaml
var DefaultCSPConfig string
31 changes: 31 additions & 0 deletions services/proxy/pkg/config/csp.yaml
Original file line number Diff line number Diff line change
@@ -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'''
1 change: 1 addition & 0 deletions services/proxy/pkg/config/defaults/defaultconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ func DefaultConfig() *config.Config {
AutoprovisionAccounts: false,
EnableBasicAuth: false,
InsecureBackends: false,
CSPConfigFileLocation: "",
}
}

Expand Down
61 changes: 61 additions & 0 deletions services/proxy/pkg/middleware/security.go
Original file line number Diff line number Diff line change
@@ -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) {
DeepDiver1975 marked this conversation as resolved.
Show resolved Hide resolved
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)
}
}
1 change: 0 additions & 1 deletion services/settings/pkg/server/http/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
Expand Down
1 change: 0 additions & 1 deletion services/sse/pkg/server/http/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
1 change: 0 additions & 1 deletion services/thumbnails/pkg/server/http/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
1 change: 0 additions & 1 deletion services/userlog/pkg/server/http/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
1 change: 0 additions & 1 deletion services/web/pkg/middleware/silentrefresh.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
Expand Down
1 change: 0 additions & 1 deletion services/web/pkg/server/http/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ func Server(opts ...Option) (http.Service, error) {
chimiddleware.RealIP,
chimiddleware.RequestID,
middleware.NoCache,
middleware.Secure,
webmid.SilentRefresh,
middleware.Version(
"web",
Expand Down
1 change: 0 additions & 1 deletion services/webdav/pkg/server/http/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
1 change: 0 additions & 1 deletion services/webfinger/pkg/server/http/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading