GH-18288: Allow configuring unsafe-eval and unsafe-inline (#18801)

* GH-18288: Allow configuring unsafe-eval and unsafe-inline * Add developer flags to telemetry * wip * wip * Refactor based on review * Refactor based on review * fix expected vs. actual in assert.Equal * Add unit tests, rework to check only supported flags. * Update model/config.go Co-authored-by: Jesse Hallam <jesse@thehallams.ca> * Fix failing tests * Refactor based on review * Refactor based on review * Refactor based on review Co-authored-by: Jesse Hallam <jesse.hallam@gmail.com> Co-authored-by: Jesse Hallam <jesse@thehallams.ca>
2021-11-12 03:11:11 +05:30
--- a/model/config.go
+++ b/model/config.go
@@ -106,6 +106,7 @@ const (
 	ServiceSettingsDefaultListenAndAddress = ":8065"
 	ServiceSettingsDefaultGfycatAPIKey     = "2_KtH_W5"
 	ServiceSettingsDefaultGfycatAPISecret  = "3wLVZPiswc3DnaiaFoLkDvB4X0IV6CpMkj4tf2inJRsBY6-FnkT08zGmppWFgeof"
+	ServiceSettingsDefaultDeveloperFlags   = ""

 	TeamSettingsDefaultSiteName              = "Mattermost"
 	TeamSettingsDefaultMaxUsersPerTeam       = 50
@@ -302,6 +303,7 @@ type ServiceSettings struct {
 	RestrictLinkPreviews                              *string  `access:"site_posts"`
 	EnableTesting                                     *bool    `access:"environment_developer,write_restrictable,cloud_restrictable"`
 	EnableDeveloper                                   *bool    `access:"environment_developer,write_restrictable,cloud_restrictable"`
+	DeveloperFlags                                    *string  `access:"environment_developer"`
 	EnableOpenTracing                                 *bool    `access:"write_restrictable,cloud_restrictable"`
 	EnableSecurityFixAlert                            *bool    `access:"environment_smtp,write_restrictable,cloud_restrictable"`
 	EnableInsecureOutgoingConnections                 *bool    `access:"environment_web_server,write_restrictable,cloud_restrictable"`
@@ -416,6 +418,10 @@ func (s *ServiceSettings) SetDefaults(isUpdate bool) {
 		s.EnableDeveloper = NewBool(false)
 	}

+	if s.DeveloperFlags == nil {
+		s.DeveloperFlags = NewString("")
+	}
+
 	if s.EnableOpenTracing == nil {
 		s.EnableOpenTracing = NewBool(false)
 	}
--- a/services/telemetry/telemetry.go
+++ b/services/telemetry/telemetry.go
@@ -373,6 +373,7 @@ func (ts *TelemetryService) trackConfig() {
 		"experimental_enable_authentication_transfer":             *cfg.ServiceSettings.ExperimentalEnableAuthenticationTransfer,
 		"enable_testing":                                          cfg.ServiceSettings.EnableTesting,
 		"enable_developer":                                        *cfg.ServiceSettings.EnableDeveloper,
+		"developer_flags":                                         isDefault(*cfg.ServiceSettings.DeveloperFlags, model.ServiceSettingsDefaultDeveloperFlags),
 		"enable_multifactor_authentication":                       *cfg.ServiceSettings.EnableMultifactorAuthentication,
 		"enforce_multifactor_authentication":                      *cfg.ServiceSettings.EnforceMultifactorAuthentication,
 		"enable_oauth_service_provider":                           cfg.ServiceSettings.EnableOAuthServiceProvider,
--- a/web/handlers.go
+++ b/web/handlers.go
@@ -86,6 +86,53 @@ type Handler struct {
 	cspShaDirective string
 }

+func generateDevCSP(c Context) string {
+	// Add unsafe-eval to the content security policy for faster source maps in development mode
+	devCSPMap := make(map[string]bool)
+	if model.BuildNumber == "dev" {
+		devCSPMap["unsafe-eval"] = true
+	}
+
+	// Add unsafe-inline to unlock extensions like React & Redux DevTools in Firefox
+	// see https://github.com/reduxjs/redux-devtools/issues/380
+	if model.BuildNumber == "dev" {
+		devCSPMap["unsafe-inline"] = true
+	}
+
+	// Add supported flags for debugging during development, even if not on a dev build.
+	for _, devFlagKVStr := range strings.Split(*c.App.Config().ServiceSettings.DeveloperFlags, ",") {
+		devFlagKVSplit := strings.SplitN(devFlagKVStr, "=", 2)
+		if len(devFlagKVSplit) != 2 {
+			c.Logger.Warn("Unable to parse developer flag", mlog.String("developer_flag", devFlagKVStr))
+			continue
+		}
+		devFlagKey := devFlagKVSplit[0]
+		devFlagValue := devFlagKVSplit[1]
+
+		// Ignore disabled keys
+		if devFlagValue != "true" {
+			continue
+		}
+
+		// Honour only supported keys
+		switch devFlagKey {
+		case "unsafe-eval", "unsafe-inline":
+			devCSPMap[devFlagKey] = true
+		default:
+			c.Logger.Warn("Unrecognized developer flag", mlog.String("developer_flag", devFlagKVStr))
+		}
+	}
+	var devCSP string
+	supportedCSPFlags := []string{"unsafe-eval", "unsafe-inline"}
+	for _, devCSPFlag := range supportedCSPFlags {
+		if devCSPMap[devCSPFlag] {
+			devCSP += fmt.Sprintf(" '%s'", devCSPFlag)
+		}
+	}
+
+	return devCSP
+}
+
 func (h Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	w = newWrappedWriter(w)
 	now := time.Now()
@@ -176,17 +223,7 @@ func (h Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		// Instruct the browser not to display us in an iframe unless is the same origin for anti-clickjacking
 		w.Header().Set("X-Frame-Options", "SAMEORIGIN")

-		// Add unsafe-eval to the content security policy for faster source maps in development mode
-		devCSP := ""
-		if model.BuildNumber == "dev" {
-			devCSP += " 'unsafe-eval'"
-		}
-
-		// Add unsafe-inline to unlock extensions like React & Redux DevTools in Firefox
-		// see https://github.com/reduxjs/redux-devtools/issues/380
-		if model.BuildNumber == "dev" {
-			devCSP += " 'unsafe-inline'"
-		}
+		devCSP := generateDevCSP(*c)

 		// Set content security policy. This is also specified in the root.html of the webapp in a meta tag.
 		w.Header().Set("Content-Security-Policy", fmt.Sprintf(
--- a/web/handlers_test.go
+++ b/web/handlers_test.go
@@ -300,7 +300,7 @@ func TestHandlerServeCSPHeader(t *testing.T) {
 		response := httptest.NewRecorder()
 		handler.ServeHTTP(response, request)
 		assert.Equal(t, 200, response.Code)
-		assert.Equal(t, response.Header()["Content-Security-Policy"], []string{"frame-ancestors 'self'; script-src 'self' cdn.rudderlabs.com"})
+		assert.Equal(t, []string{"frame-ancestors 'self'; script-src 'self' cdn.rudderlabs.com"}, response.Header()["Content-Security-Policy"])
 	})

 	t.Run("static, with subpath", func(t *testing.T) {
@@ -340,7 +340,7 @@ func TestHandlerServeCSPHeader(t *testing.T) {
 		response := httptest.NewRecorder()
 		handler.ServeHTTP(response, request)
 		assert.Equal(t, 200, response.Code)
-		assert.Equal(t, response.Header()["Content-Security-Policy"], []string{"frame-ancestors 'self'; script-src 'self' cdn.rudderlabs.com"})
+		assert.Equal(t, []string{"frame-ancestors 'self'; script-src 'self' cdn.rudderlabs.com"}, response.Header()["Content-Security-Policy"])

 		// TODO: It's hard to unit test this now that the CSP directive is effectively
 		// decided in Setup(). Circle back to this in master once the memory store is
@@ -355,10 +355,136 @@ func TestHandlerServeCSPHeader(t *testing.T) {
 		response = httptest.NewRecorder()
 		handler.ServeHTTP(response, request)
 		assert.Equal(t, 200, response.Code)
-		assert.Equal(t, response.Header()["Content-Security-Policy"], []string{"frame-ancestors 'self'; script-src 'self' cdn.rudderlabs.com"})
+		assert.Equal(t, []string{"frame-ancestors 'self'; script-src 'self' cdn.rudderlabs.com"}, response.Header()["Content-Security-Policy"])
 		// TODO: See above.
 		// assert.Contains(t, response.Header()["Content-Security-Policy"], "frame-ancestors 'self'; script-src 'self' cdn.rudderlabs.com 'sha256-tPOjw+tkVs9axL78ZwGtYl975dtyPHB6LYKAO2R3gR4='", "csp header incorrectly changed after subpath changed")
 	})
+
+	t.Run("dev mode", func(t *testing.T) {
+		th := Setup(t)
+		defer th.TearDown()
+
+		oldBuildNumber := model.BuildNumber
+		model.BuildNumber = "dev"
+		defer func() {
+			model.BuildNumber = oldBuildNumber
+		}()
+
+		web := New(th.Server)
+
+		handler := Handler{
+			Srv:            web.srv,
+			HandleFunc:     handlerForCSPHeader,
+			RequireSession: false,
+			TrustRequester: false,
+			RequireMfa:     false,
+			IsStatic:       true,
+		}
+
+		request := httptest.NewRequest("POST", "/", nil)
+		response := httptest.NewRecorder()
+		handler.ServeHTTP(response, request)
+		assert.Equal(t, 200, response.Code)
+		assert.Equal(t, []string{"frame-ancestors 'self'; script-src 'self' cdn.rudderlabs.com 'unsafe-eval' 'unsafe-inline'"}, response.Header()["Content-Security-Policy"])
+	})
+
+}
+
+func TestGenerateDevCSP(t *testing.T) {
+	t.Run("dev mode", func(t *testing.T) {
+		th := Setup(t)
+		defer th.TearDown()
+
+		oldBuildNumber := model.BuildNumber
+		model.BuildNumber = "dev"
+		defer func() {
+			model.BuildNumber = oldBuildNumber
+		}()
+		c := &Context{
+			App:        th.App,
+			AppContext: th.Context,
+			Logger:     th.App.Log(),
+		}
+
+		devCSP := generateDevCSP(*c)
+
+		assert.Equal(t, " 'unsafe-eval' 'unsafe-inline'", devCSP)
+
+	})
+	t.Run("allowed dev flags", func(t *testing.T) {
+		th := Setup(t)
+		defer th.TearDown()
+
+		oldBuildNumber := model.BuildNumber
+		model.BuildNumber = "0"
+		defer func() {
+			model.BuildNumber = oldBuildNumber
+		}()
+
+		th.App.UpdateConfig(func(cfg *model.Config) {
+			*cfg.ServiceSettings.DeveloperFlags = "unsafe-inline=true,unsafe-eval=true"
+		})
+
+		c := &Context{
+			App:        th.App,
+			AppContext: th.Context,
+			Logger:     th.App.Log(),
+		}
+
+		devCSP := generateDevCSP(*c)
+
+		assert.Equal(t, " 'unsafe-eval' 'unsafe-inline'", devCSP)
+	})
+
+	t.Run("partial dev flags", func(t *testing.T) {
+		th := Setup(t)
+		defer th.TearDown()
+
+		oldBuildNumber := model.BuildNumber
+		model.BuildNumber = "0"
+		defer func() {
+			model.BuildNumber = oldBuildNumber
+		}()
+
+		th.App.UpdateConfig(func(cfg *model.Config) {
+			*cfg.ServiceSettings.DeveloperFlags = "unsafe-inline=false,unsafe-eval=true"
+		})
+
+		c := &Context{
+			App:        th.App,
+			AppContext: th.Context,
+			Logger:     th.App.Log(),
+		}
+
+		devCSP := generateDevCSP(*c)
+
+		assert.Equal(t, " 'unsafe-eval'", devCSP)
+	})
+
+	t.Run("unknown dev flags", func(t *testing.T) {
+		th := Setup(t)
+		defer th.TearDown()
+
+		oldBuildNumber := model.BuildNumber
+		model.BuildNumber = "0"
+		defer func() {
+			model.BuildNumber = oldBuildNumber
+		}()
+
+		th.App.UpdateConfig(func(cfg *model.Config) {
+			*cfg.ServiceSettings.DeveloperFlags = "unknown=true,unsafe-inline=false,unsafe-eval=true"
+		})
+
+		c := &Context{
+			App:        th.App,
+			AppContext: th.Context,
+			Logger:     th.App.Log(),
+		}
+
+		devCSP := generateDevCSP(*c)
+
+		assert.Equal(t, " 'unsafe-eval'", devCSP)
+	})
 }

 func TestHandlerServeInvalidToken(t *testing.T) {