feat: add csp headers for embedded apps (#18374)

I modified the proxy host cache we already had and were using for
websocket csp headers to also include the wildcard app host, then used
those for frame-src policies.

I did not add frame-ancestors, since if I understand correctly, those
would go on the app, and this middleware does not come into play there.
Maybe we will want to add it on workspace apps like we do with cors, if
we find apps are setting it to `none` or something.

Closes https://github.com/coder/internal/issues/684

coderd/httpmw/csp.go

@@ -5,6 +5,7 @@ import (
 	"net/http"
 	"strings"
 
+	"github.com/coder/coder/v2/coderd/proxyhealth"
 	"github.com/coder/coder/v2/codersdk"
 )
 
@@ -47,18 +48,18 @@ const (
 // for coderd.
 //
 // Arguments:
-//   - websocketHosts: a function that returns a list of supported external websocket hosts.
-//     This is to support the terminal connecting to a workspace proxy.
-//     The origin of the terminal request does not match the url of the proxy,
-//     so the CSP list of allowed hosts must be dynamic and match the current
-//     available proxy urls.
+//   - proxyHosts: a function that returns a list of supported proxy hosts
+//     (including the primary).  This is to support the terminal connecting to a
+//     workspace proxy and for embedding apps in an iframe.  The origin of the
+//     requests do not match the url of the proxy, so the CSP list of allowed
+//     hosts must be dynamic and match the current available proxy urls.
 //   - staticAdditions: a map of CSP directives to append to the default CSP headers.
 //     Used to allow specific static additions to the CSP headers. Allows some niche
 //     use cases, such as embedding Coder in an iframe.
 //     Example: https://github.com/coder/coder/issues/15118
 //
 //nolint:revive
-func CSPHeaders(experiments codersdk.Experiments, telemetry bool, websocketHosts func() []string, staticAdditions map[CSPFetchDirective][]string) func(next http.Handler) http.Handler {
+func CSPHeaders(experiments codersdk.Experiments, telemetry bool, proxyHosts func() []*proxyhealth.ProxyHost, staticAdditions map[CSPFetchDirective][]string) func(next http.Handler) http.Handler {
 	return func(next http.Handler) http.Handler {
 		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 			// Content-Security-Policy disables loading certain content types and can prevent XSS injections.
@@ -97,15 +98,6 @@ func CSPHeaders(experiments codersdk.Experiments, telemetry bool, websocketHosts
 				// "require-trusted-types-for" : []string{"'script'"},
 			}
 
-			if experiments.Enabled(codersdk.ExperimentAITasks) {
-				// AI tasks use iframe embeds of local apps.
-				// TODO: Handle region domains too, not just path based apps
-				cspSrcs.Append(CSPFrameAncestors, `'self'`)
-				cspSrcs.Append(CSPFrameSource, `'self'`)
-			} else {
-				cspSrcs.Append(CSPFrameAncestors, `'none'`)
-			}
-
 			if telemetry {
 				// If telemetry is enabled, we report to coder.com.
 				cspSrcs.Append(CSPDirectiveConnectSrc, "https://coder.com")
@@ -126,19 +118,26 @@ func CSPHeaders(experiments codersdk.Experiments, telemetry bool, websocketHosts
 				cspSrcs.Append(CSPDirectiveConnectSrc, fmt.Sprintf("wss://%[1]s ws://%[1]s", host))
 			}
 
-			// The terminal requires a websocket connection to the workspace proxy.
-			// Make sure we allow this connection to healthy proxies.
-			extraConnect := websocketHosts()
+			// The terminal and iframed apps can use workspace proxies (which includes
+			// the primary).  Make sure we allow connections to healthy proxies.
+			extraConnect := proxyHosts()
 			if len(extraConnect) > 0 {
 				for _, extraHost := range extraConnect {
-					if extraHost == "*" {
+					// Allow embedding the app host.
+					if experiments.Enabled(codersdk.ExperimentAITasks) {
+						cspSrcs.Append(CSPDirectiveFrameSrc, extraHost.AppHost)
+					}
+					if extraHost.Host == "*" {
 						// '*' means all
 						cspSrcs.Append(CSPDirectiveConnectSrc, "*")
 						continue
 					}
-					cspSrcs.Append(CSPDirectiveConnectSrc, fmt.Sprintf("wss://%[1]s ws://%[1]s", extraHost))
+					// Avoid double-adding r.Host.
+					if extraHost.Host != r.Host {
+						cspSrcs.Append(CSPDirectiveConnectSrc, fmt.Sprintf("wss://%[1]s ws://%[1]s", extraHost.Host))
+					}
 					// We also require this to make http/https requests to the workspace proxy for latency checking.
-					cspSrcs.Append(CSPDirectiveConnectSrc, fmt.Sprintf("https://%[1]s http://%[1]s", extraHost))
+					cspSrcs.Append(CSPDirectiveConnectSrc, fmt.Sprintf("https://%[1]s http://%[1]s", extraHost.Host))
 				}
 			}
 

coderd/httpmw/csp_test.go

@@ -1,28 +1,59 @@
 package httpmw_test
 
 import (
-	"fmt"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/require"
 
 	"github.com/coder/coder/v2/coderd/httpmw"
+	"github.com/coder/coder/v2/coderd/proxyhealth"
 	"github.com/coder/coder/v2/codersdk"
 )
 
-func TestCSPConnect(t *testing.T) {
+func TestCSP(t *testing.T) {
 	t.Parallel()
 
-	expected := []string{"example.com", "coder.com"}
+	proxyHosts := []*proxyhealth.ProxyHost{
+		{
+			Host:    "test.com",
+			AppHost: "*.test.com",
+		},
+		{
+			Host:    "coder.com",
+			AppHost: "*.coder.com",
+		},
+		{
+			// Host is not added because it duplicates the host header.
+			Host:    "example.com",
+			AppHost: "*.coder2.com",
+		},
+	}
 	expectedMedia := []string{"media.com", "media2.com"}
 
+	expected := []string{
+		"frame-src 'self' *.test.com *.coder.com *.coder2.com",
+		"media-src 'self' media.com media2.com",
+		strings.Join([]string{
+			"connect-src", "'self'",
+			// Added from host header.
+			"wss://example.com", "ws://example.com",
+			// Added via proxy hosts.
+			"wss://test.com", "ws://test.com", "https://test.com", "http://test.com",
+			"wss://coder.com", "ws://coder.com", "https://coder.com", "http://coder.com",
+		}, " "),
+	}
+
+	// When the host is empty, it uses example.com.
 	r := httptest.NewRequest(http.MethodGet, "/", nil)
 	rw := httptest.NewRecorder()
 
-	httpmw.CSPHeaders(codersdk.Experiments{}, false, func() []string {
-		return expected
+	httpmw.CSPHeaders(codersdk.Experiments{
+		codersdk.ExperimentAITasks,
+	}, false, func() []*proxyhealth.ProxyHost {
+		return proxyHosts
 	}, map[httpmw.CSPFetchDirective][]string{
 		httpmw.CSPDirectiveMediaSrc: expectedMedia,
 	})(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
@@ -31,10 +62,6 @@ func TestCSPConnect(t *testing.T) {
 
 	require.NotEmpty(t, rw.Header().Get("Content-Security-Policy"), "Content-Security-Policy header should not be empty")
 	for _, e := range expected {
-		require.Containsf(t, rw.Header().Get("Content-Security-Policy"), fmt.Sprintf("ws://%s", e), "Content-Security-Policy header should contain ws://%s", e)
-		require.Containsf(t, rw.Header().Get("Content-Security-Policy"), fmt.Sprintf("wss://%s", e), "Content-Security-Policy header should contain wss://%s", e)
-	}
-	for _, e := range expectedMedia {
-		require.Containsf(t, rw.Header().Get("Content-Security-Policy"), e, "Content-Security-Policy header should contain %s", e)
+		require.Contains(t, rw.Header().Get("Content-Security-Policy"), e)
 	}
 }