Merge pull request #14701 from jhadvig/OCPBUGS-49291

OCPBUGS-49291:  Improving the DevExp for passing the CSP directives to console per flag + Make use of connect-src and object-src directives

Author: openshift-merge-bot[bot]

README.md

@@ -461,6 +461,23 @@ this way, then 'none' will be used. Additionally, violation reporting is throttl
 spamming the telemetry service with repetitive data. Identical violations will not be
 reported more than once a day.
 
+In case of local developement of the dynamic plugin, just pass needed CSP directives address to the console server, using the `--content-security-policy` flag.
+
+Example:
+
+```
+./bin/bridge --content-security-policy script-src='localhost:1234',font-src='localhost:2345 localhost:3456'
+```
+
+List of configurable CSP directives is available in the [openshift/api repository](https://github.com/openshift/api/blob/master/console/v1/types_console_plugin.go#L102-L137).
+
+The list is extended automatically by the console server with following CSP directives:
+- `"frame-src 'none'"`
+- `"frame-ancestors 'none'"`
+- `"object-src 'none'"`
+
+Currently this feature is behind feature gate.
+
 ## Frontend Packages
 - [console-dynamic-plugin-sdk](./frontend/packages/console-dynamic-plugin-sdk/README.md)
 [[API]](./frontend/packages/console-dynamic-plugin-sdk/docs/api.md)

cmd/bridge/main.go

@@ -130,8 +130,10 @@ func main() {
 	fs.Var(&consolePluginsFlags, "plugins", "List of plugin entries that are enabled for the console. Each entry consist of plugin-name as a key and plugin-endpoint as a value.")
 	fPluginProxy := fs.String("plugin-proxy", "", "Defines various service types to which will console proxy plugins requests. (JSON as string)")
 	fI18NamespacesFlags := fs.String("i18n-namespaces", "", "List of namespaces separated by comma. Example --i18n-namespaces=plugin__acm,plugin__kubevirt")
+
 	fContentSecurityPolicyEnabled := fs.Bool("content-security-policy-enabled", false, "Flag to indicate if Content Secrity Policy features should be enabled.")
-	fContentSecurityPolicy := fs.String("content-security-policy", "", "Content security policy for the console. (JSON as string)")
+	consoleCSPFlags := serverconfig.MultiKeyValue{}
+	fs.Var(&consoleCSPFlags, "content-security-policy", "List of CSP directives that are enabled for the console. Each entry consist of csp-directive-name as a key and csp-directive-value as a value. Example --content-security-policy script-src='localhost:9000',font-src='localhost:9001'")
 
 	telemetryFlags := serverconfig.MultiKeyValue{}
 	fs.Var(&telemetryFlags, "telemetry", "Telemetry configuration that can be used by console plugins. Each entry should be a key=value pair.")
@@ -282,8 +284,8 @@ func main() {
 		EnabledConsolePlugins:        consolePluginsFlags,
 		I18nNamespaces:               i18nNamespaces,
 		PluginProxy:                  *fPluginProxy,
-		ContentSecurityPolicy:        *fContentSecurityPolicy,
 		ContentSecurityPolicyEnabled: *fContentSecurityPolicyEnabled,
+		ContentSecurityPolicy:        consoleCSPFlags,
 		QuickStarts:                  *fQuickStarts,
 		AddPage:                      *fAddPage,
 		ProjectAccessClusterRoles:    *fProjectAccessClusterRoles,

pkg/server/server.go

@@ -161,8 +161,8 @@ type Server struct {
 	ClusterManagementProxyConfig        *proxy.Config
 	CookieEncryptionKey                 []byte
 	CookieAuthenticationKey             []byte
-	ContentSecurityPolicy               string
 	ContentSecurityPolicyEnabled        bool
+	ContentSecurityPolicy               serverconfig.MultiKeyValue
 	ControlPlaneTopology                string
 	CopiedCSVsDisabled                  bool
 	CSRFVerifier                        *csrfverifier.CSRFVerifier
@@ -281,8 +281,7 @@ func (s *Server) HTTPHandler() (http.Handler, error) {
 		tpl.Delims("[[", "]]")
 		tpls, err := tpl.ParseFiles(path.Join(s.PublicDir, tokenizerPageTemplateName))
 		if err != nil {
-			fmt.Printf("%v not found in configured public-dir path: %v", tokenizerPageTemplateName, err)
-			os.Exit(1)
+			klog.Fatalf("%v not found in configured public-dir path: %v", tokenizerPageTemplateName, err)
 		}
 
 		if err := tpls.ExecuteTemplate(w, tokenizerPageTemplateName, templateData); err != nil {
@@ -542,12 +541,10 @@ func (s *Server) HTTPHandler() (http.Handler, error) {
 		proxyConfig, err := plugins.ParsePluginProxyConfig(s.PluginProxy)
 		if err != nil {
 			klog.Fatalf("Error parsing plugin proxy config: %s", err)
-			os.Exit(1)
 		}
 		proxyServiceHandlers, err := plugins.GetPluginProxyServiceHandlers(proxyConfig, s.PluginsProxyTLSConfig, pluginProxyEndpoint)
 		if err != nil {
 			klog.Fatalf("Error getting plugin proxy handlers: %s", err)
-			os.Exit(1)
 		}
 		if len(proxyServiceHandlers) != 0 {
 			klog.Infoln("The following console endpoints are now proxied to these services:")
@@ -586,7 +583,7 @@ func (s *Server) HTTPHandler() (http.Handler, error) {
 			ConsoleCommit:         os.Getenv("SOURCE_GIT_COMMIT"),
 			Plugins:               pluginsHandler.GetPluginsList(),
 			Capabilities:          s.Capabilities,
-			ContentSecurityPolicy: s.ContentSecurityPolicy,
+			ContentSecurityPolicy: s.ContentSecurityPolicy.String(),
 		})
 	}))
 
@@ -707,7 +704,6 @@ func (s *Server) indexHandler(w http.ResponseWriter, r *http.Request) {
 		)
 		if err != nil {
 			klog.Fatalf("Error building Content Security Policy directives: %s", err)
-			os.Exit(1)
 		}
 		w.Header().Set("Content-Security-Policy-Report-Only", strings.Join(cspDirectives, "; "))
 	}
@@ -791,8 +787,7 @@ func (s *Server) indexHandler(w http.ResponseWriter, r *http.Request) {
 	tpl.Delims("[[", "]]")
 	tpls, err := tpl.ParseFiles(path.Join(s.PublicDir, indexPageTemplateName))
 	if err != nil {
-		fmt.Printf("index.html not found in configured public-dir path: %v", err)
-		os.Exit(1)
+		klog.Fatalf("index.html not found in configured public-dir path: %v", err)
 	}
 
 	if err := tpls.ExecuteTemplate(w, indexPageTemplateName, templateData); err != nil {

pkg/serverconfig/config.go

@@ -176,6 +176,7 @@ func SetFlagsFromConfig(fs *flag.FlagSet, config *Config) (err error) {
 	if err != nil {
 		return err
 	}
+
 	addContentSecurityPolicyEnabled(fs, &config.ContentSecurityPolicyEnabled)
 	addContentSecurityPolicy(fs, config.ContentSecurityPolicy)
 	addTelemetry(fs, config.Telemetry)
@@ -418,6 +419,37 @@ func isAlreadySet(fs *flag.FlagSet, name string) bool {
 	return alreadySet
 }
 
+func addContentSecurityPolicy(fs *flag.FlagSet, csp MultiKeyValue) {
+	for cspDirectiveName, cspDirectiveValue := range csp {
+		directiveName := getDirectiveName(cspDirectiveName)
+		if directiveName == "" {
+			klog.Fatalf("invalid CSP directive: %s", cspDirectiveName)
+		}
+
+		fs.Set("content-security-policy", fmt.Sprintf("%s=%s", directiveName, cspDirectiveValue))
+	}
+}
+
+func getDirectiveName(directive string) string {
+	switch directive {
+	case string(consolev1.DefaultSrc):
+		return "default-src"
+	case string(consolev1.ImgSrc):
+		return "img-src"
+	case string(consolev1.FontSrc):
+		return "font-src"
+	case string(consolev1.ScriptSrc):
+		return "script-src"
+	case string(consolev1.StyleSrc):
+		return "style-src"
+	case string(consolev1.ConnectSrc):
+		return "connect-src"
+	default:
+		klog.Infof("ignored invalid CSP directive: %s", directive)
+		return ""
+	}
+}
+
 func addPlugins(fs *flag.FlagSet, plugins MultiKeyValue) {
 	for pluginName, pluginEndpoint := range plugins {
 		fs.Set("plugins", fmt.Sprintf("%s=%s", pluginName, pluginEndpoint))
@@ -434,18 +466,6 @@ func addI18nNamespaces(fs *flag.FlagSet, i18nNamespaces []string) {
 	fs.Set("i18n-namespaces", strings.Join(i18nNamespaces, ","))
 }
 
-func addContentSecurityPolicy(fs *flag.FlagSet, csp map[consolev1.DirectiveType][]string) error {
-	if csp != nil {
-		marshaledCSP, err := json.Marshal(csp)
-		if err != nil {
-			klog.Fatalf("Could not marshal ConsoleConfig 'content-security-policy' field: %v", err)
-			return err
-		}
-		fs.Set("content-security-policy", string(marshaledCSP))
-	}
-	return nil
-}
-
 func addContentSecurityPolicyEnabled(fs *flag.FlagSet, enabled *bool) {
 	if enabled != nil && *enabled {
 		fs.Set("content-security-policy-enabled", "true")

pkg/serverconfig/config_test.go

@@ -289,13 +289,29 @@ func TestSetFlagsFromConfig(t *testing.T) {
 			},
 			expectedError: nil,
 		},
+		{
+			name: "Should apply CSP configuration",
+			config: Config{
+				APIVersion: "console.openshift.io/v1",
+				Kind:       "ConsoleConfig",
+				ContentSecurityPolicy: MultiKeyValue{
+					"FontSrc":   "value2 value3",
+					"ScriptSrc": "value1",
+				},
+			},
+			expectedFlagValues: map[string]string{
+				"content-security-policy": "font-src=value2 value3, script-src=value1",
+			},
+			expectedError: nil,
+		},
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			fs := &flag.FlagSet{}
 			fs.String("config", "", "")
 			fs.Var(&MultiKeyValue{}, "plugins", "")
 			fs.Var(&MultiKeyValue{}, "telemetry", "")
+			fs.Var(&MultiKeyValue{}, "content-security-policy", "")
 
 			actualError := SetFlagsFromConfig(fs, &test.config)
 			actual := make(map[string]string)

pkg/serverconfig/types.go

@@ -2,7 +2,6 @@ package serverconfig
 
 import (
 	configv1 "github.com/openshift/api/config/v1"
-	v1 "github.com/openshift/api/console/v1"
 	operatorv1 "github.com/openshift/api/operator/v1"
 	authorizationv1 "k8s.io/api/authorization/v1"
 )
@@ -23,12 +22,12 @@ type Config struct {
 	Providers                    `yaml:"providers"`
 	Helm                         `yaml:"helm"`
 	MonitoringInfo               `yaml:"monitoringInfo,omitempty"`
-	Plugins                      MultiKeyValue                 `yaml:"plugins,omitempty"`
-	I18nNamespaces               []string                      `yaml:"i18nNamespaces,omitempty"`
-	Proxy                        Proxy                         `yaml:"proxy,omitempty"`
-	ContentSecurityPolicyEnabled bool                          `yaml:"contentSecurityPolicyEnabled,omitempty"`
-	ContentSecurityPolicy        map[v1.DirectiveType][]string `yaml:"contentSecurityPolicy,omitempty"`
-	Telemetry                    MultiKeyValue                 `yaml:"telemetry,omitempty"`
+	Plugins                      MultiKeyValue `yaml:"plugins,omitempty"`
+	I18nNamespaces               []string      `yaml:"i18nNamespaces,omitempty"`
+	Proxy                        Proxy         `yaml:"proxy,omitempty"`
+	ContentSecurityPolicyEnabled bool          `yaml:"contentSecurityPolicyEnabled,omitempty"`
+	ContentSecurityPolicy        MultiKeyValue `yaml:"contentSecurityPolicy,omitempty"`
+	Telemetry                    MultiKeyValue `yaml:"telemetry,omitempty"`
 }
 
 type Proxy struct {

pkg/utils/utils.go

@@ -3,13 +3,12 @@ package utils
 import (
 	"crypto/rand"
 	"encoding/base64"
-	"encoding/json"
 	"fmt"
 	"strings"
 
 	"k8s.io/klog/v2"
 
-	consolev1 "github.com/openshift/api/console/v1"
+	"github.com/openshift/console/pkg/serverconfig"
 )
 
 const (
@@ -19,13 +18,16 @@ const (
 	fontSrc       = "font-src"
 	scriptSrc     = "script-src"
 	styleSrc      = "style-src"
+	objectSrc     = "object-src"
+	connectSrc    = "connect-src"
 	consoleDot    = "console.redhat.com"
 	httpLocalHost = "http://localhost:8080"
 	wsLocalHost   = "ws://localhost:8080"
 	self          = "'self'"
 	data          = "data:"
 	unsafeEval    = "'unsafe-eval'"
 	unsafeInline  = "'unsafe-inline'"
+	none          = "'none'"
 )
 
 // Generate a cryptographically secure random array of bytes.
@@ -54,7 +56,7 @@ func RandomString(length int) (string, error) {
 // buildCSPDirectives takes the content security policy configuration from the server and constructs
 // a complete set of directives for the Content-Security-Policy-Report-Only header.
 // The constructed directives will include the default sources and the supplied configuration.
-func BuildCSPDirectives(k8sMode, pluginsCSP, indexPageScriptNonce, cspReportingEndpoint string) ([]string, error) {
+func BuildCSPDirectives(k8sMode string, pluginsCSP serverconfig.MultiKeyValue, indexPageScriptNonce string, cspReportingEndpoint string) ([]string, error) {
 	nonce := fmt.Sprintf("'nonce-%s'", indexPageScriptNonce)
 
 	// The default sources are the sources that are allowed for all directives.
@@ -68,38 +70,40 @@ func BuildCSPDirectives(k8sMode, pluginsCSP, indexPageScriptNonce, cspReportingE
 	fontSrcDirective := []string{fontSrc, self}
 	scriptSrcDirective := []string{scriptSrc, self, consoleDot}
 	styleSrcDirective := []string{styleSrc, self}
+	objectSrcDirective := []string{objectSrc, self}
+	connectSrcDirective := []string{connectSrc, self, consoleDot}
+
+	// If running off-cluster, append the localhost sources to the default sources
 	if k8sMode == "off-cluster" {
 		baseUriDirective = append(baseUriDirective, []string{httpLocalHost, wsLocalHost}...)
 		defaultSrcDirective = append(defaultSrcDirective, []string{httpLocalHost, wsLocalHost}...)
 		imgSrcDirective = append(imgSrcDirective, httpLocalHost)
 		fontSrcDirective = append(fontSrcDirective, httpLocalHost)
 		scriptSrcDirective = append(scriptSrcDirective, []string{httpLocalHost, wsLocalHost}...)
 		styleSrcDirective = append(styleSrcDirective, httpLocalHost)
+		objectSrcDirective = append(objectSrcDirective, httpLocalHost)
+		connectSrcDirective = append(connectSrcDirective, httpLocalHost)
 	}
 
 	// If the plugins are providing a content security policy configuration, parse it and add it to
 	// the appropriate directive. The configuration is a string that is parsed into a map of directive types to sources.
 	// The sources are added to the existing sources for each type.
-	if pluginsCSP != "" {
-		parsedCSP, err := ParseContentSecurityPolicyConfig(pluginsCSP)
-		if err != nil {
-			return nil, err
-		}
-		for directive, sources := range *parsedCSP {
-			switch directive {
-			case consolev1.DefaultSrc:
-				defaultSrcDirective = append(defaultSrcDirective, sources...)
-			case consolev1.ImgSrc:
-				imgSrcDirective = append(imgSrcDirective, sources...)
-			case consolev1.FontSrc:
-				fontSrcDirective = append(fontSrcDirective, sources...)
-			case consolev1.ScriptSrc:
-				scriptSrcDirective = append(scriptSrcDirective, sources...)
-			case consolev1.StyleSrc:
-				styleSrcDirective = append(styleSrcDirective, sources...)
-			default:
-				klog.Warningf("ignored invalid CSP directive: %v", directive)
-			}
+	for directive, sources := range pluginsCSP {
+		switch directive {
+		case defaultSrc:
+			defaultSrcDirective = append(defaultSrcDirective, sources)
+		case imgSrc:
+			imgSrcDirective = append(imgSrcDirective, sources)
+		case fontSrc:
+			fontSrcDirective = append(fontSrcDirective, sources)
+		case scriptSrc:
+			scriptSrcDirective = append(scriptSrcDirective, sources)
+		case styleSrc:
+			styleSrcDirective = append(styleSrcDirective, sources)
+		case connectSrc:
+			connectSrcDirective = append(connectSrcDirective, sources)
+		default:
+			klog.Fatalf("invalid CSP directive: %s", directive)
 		}
 	}
 
@@ -120,9 +124,10 @@ func BuildCSPDirectives(k8sMode, pluginsCSP, indexPageScriptNonce, cspReportingE
 		strings.Join(fontSrcDirective, " "),
 		strings.Join(scriptSrcDirective, " "),
 		strings.Join(styleSrcDirective, " "),
+		strings.Join(connectSrcDirective, " "),
+		strings.Join(objectSrcDirective, " "),
 		"frame-src 'none'",
 		"frame-ancestors 'none'",
-		"object-src 'none'",
 	}
 
 	// Support using client provided CSP reporting endpoint for testing purposes.
@@ -132,41 +137,3 @@ func BuildCSPDirectives(k8sMode, pluginsCSP, indexPageScriptNonce, cspReportingE
 
 	return resultDirectives, nil
 }
-
-func ParseContentSecurityPolicyConfig(csp string) (*map[consolev1.DirectiveType][]string, error) {
-	parsedCSP := &map[consolev1.DirectiveType][]string{}
-	err := json.Unmarshal([]byte(csp), parsedCSP)
-	if err != nil {
-		errMsg := fmt.Sprintf("Error unmarshaling ConsoleConfig contentSecurityPolicy field: %v", err)
-		klog.Error(errMsg)
-		return nil, fmt.Errorf(errMsg)
-	}
-
-	// Validate the keys to ensure they are all valid DirectiveTypes
-	for key := range *parsedCSP {
-		// Check if the key is a valid DirectiveType
-		if !isValidDirectiveType(key) {
-			return nil, fmt.Errorf("invalid CSP directive: %v", key)
-		}
-	}
-
-	return parsedCSP, nil
-}
-
-// Helper function to validate DirectiveTypes
-func isValidDirectiveType(d consolev1.DirectiveType) bool {
-	validTypes := []consolev1.DirectiveType{
-		consolev1.DefaultSrc,
-		consolev1.ScriptSrc,
-		consolev1.StyleSrc,
-		consolev1.ImgSrc,
-		consolev1.FontSrc,
-	}
-
-	for _, validType := range validTypes {
-		if d == validType {
-			return true
-		}
-	}
-	return false
-}