feat(daemon): csp nonce handling (#6587)
* Adding nonce handling * Nonce token and fix content length mismatch * Updating nonce injection for override situations * nonce handling in Content Security Policy for custom CSP scenarios * return original length in cspWriter to prevent truncation
This commit is contained in:
1 parent
af937ba6b1
commit
bf24d6607f
4 files changed
+239
-2
No files matched your search
@@ -6,6 +6,8 @@ package controller
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"crypto/rand"
|
||||
"encoding/base64"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
@@ -98,7 +100,8 @@ func (c *Controller) apiHandler(props HandlerProperties) (http.Handler, error) {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
corsWrappedHandler := wrapHandlerWithCors(mux, props)
|
||||
cspWrappedHandler := wrapHandlerWithCsp(mux, props, isUiRequest)
|
||||
corsWrappedHandler := wrapHandlerWithCors(cspWrappedHandler, props)
|
||||
commonWrappedHandler := wrapHandlerWithCommonFuncs(corsWrappedHandler, c, props)
|
||||
callbackInterceptingHandler := wrapHandlerWithCallbackInterceptor(commonWrappedHandler, c)
|
||||
printablePathCheckHandler := cleanhttp.PrintablePathCheckHandler(callbackInterceptingHandler, nil)
|
||||
@@ -759,3 +762,55 @@ func getActions(urlPath string) []string {
|
||||
// Split the rest on ":", returning all actions and sub-actions
|
||||
return strings.Split(rest, ":")
|
||||
}
|
||||
|
||||
func wrapHandlerWithCsp(h http.Handler, props HandlerProperties, isUiRequest func(*http.Request) bool) http.Handler {
|
||||
cspKey := "Content-Security-Policy"
|
||||
defaultCsp := ""
|
||||
if headers, ok := props.ListenerConfig.CustomUiResponseHeaders[0]; ok {
|
||||
if vals := headers[cspKey]; len(vals) > 0 {
|
||||
defaultCsp = vals[0]
|
||||
}
|
||||
// Remove CSP from the map so WrapCustomHeadersHandler does not
|
||||
// overwrite the nonce injected value.
|
||||
delete(headers, cspKey)
|
||||
}
|
||||
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
|
||||
if defaultCsp == "" || !isUiRequest(req) {
|
||||
h.ServeHTTP(w, req)
|
||||
return
|
||||
}
|
||||
|
||||
// Static assets get the default CSP without a nonce.
|
||||
if strings.LastIndex(req.URL.Path, ".") != -1 {
|
||||
w.Header().Set(cspKey, defaultCsp)
|
||||
h.ServeHTTP(w, req)
|
||||
return
|
||||
}
|
||||
|
||||
// Generate nonce.
|
||||
b := make([]byte, 16)
|
||||
if _, err := rand.Read(b); err != nil {
|
||||
w.Header().Set(cspKey, defaultCsp)
|
||||
h.ServeHTTP(w, req)
|
||||
return
|
||||
}
|
||||
nonce := base64.StdEncoding.EncodeToString(b)
|
||||
|
||||
// Inject nonce into the style-src directive of the CSP.
|
||||
nonceToken := fmt.Sprintf("'nonce-%s'", nonce)
|
||||
|
||||
csp := defaultCsp
|
||||
|
||||
if strings.Contains(csp, "style-src ") {
|
||||
csp = strings.Replace(csp, "style-src ", fmt.Sprintf("style-src %s ", nonceToken), 1)
|
||||
} else {
|
||||
csp = fmt.Sprintf("%s; style-src %s 'self'", csp, nonceToken)
|
||||
}
|
||||
|
||||
w.Header().Set(cspKey, csp)
|
||||
// Creating a custom key so we can pull it when serving UI page
|
||||
w.Header().Set("X-Boundary-Csp-Nonce", nonce)
|
||||
h.ServeHTTP(w, req)
|
||||
})
|
||||
}
|
||||
@@ -12,6 +12,7 @@ import (
|
||||
"io"
|
||||
"net"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
"strings"
|
||||
"testing"
|
||||
@@ -19,6 +20,7 @@ import (
|
||||
"github.com/grpc-ecosystem/grpc-gateway/v2/runtime"
|
||||
"github.com/hashicorp/boundary/internal/daemon/controller/handlers"
|
||||
"github.com/hashicorp/boundary/internal/errors"
|
||||
"github.com/hashicorp/go-secure-stdlib/listenerutil"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"google.golang.org/genproto/googleapis/api/httpbody"
|
||||
@@ -467,3 +469,98 @@ func TestGetActions(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestWrapHandlerWithCsp(t *testing.T) {
|
||||
defaultCsp := "default-src 'none'; script-src 'self' 'wasm-unsafe-eval'; frame-src 'self'; font-src 'self'; connect-src 'self'; img-src 'self' data:; style-src 'self'; media-src 'self'; manifest-src 'self'; style-src-attr 'self'; frame-ancestors 'self'"
|
||||
|
||||
newProps := func(csp string) HandlerProperties {
|
||||
if csp == "" {
|
||||
csp = defaultCsp
|
||||
}
|
||||
headers := http.Header{}
|
||||
headers.Set("Content-Security-Policy", csp)
|
||||
return HandlerProperties{
|
||||
ListenerConfig: &listenerutil.ListenerConfig{
|
||||
CustomUiResponseHeaders: map[int]http.Header{
|
||||
0: headers,
|
||||
},
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
inner := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
})
|
||||
|
||||
testCases := []struct {
|
||||
name string
|
||||
path string
|
||||
isUi bool
|
||||
customCsp string
|
||||
wantNonce bool
|
||||
wantDefaultCsp bool
|
||||
wantContains []string
|
||||
}{
|
||||
{
|
||||
name: "document request gets nonce",
|
||||
path: "/",
|
||||
isUi: true,
|
||||
wantNonce: true,
|
||||
},
|
||||
{
|
||||
name: "static asset gets default CSP without nonce",
|
||||
path: "/assets/app.js",
|
||||
isUi: true,
|
||||
wantDefaultCsp: true,
|
||||
wantNonce: false,
|
||||
},
|
||||
{
|
||||
name: "non UI request gets no CSP header",
|
||||
path: "/",
|
||||
isUi: false,
|
||||
wantNonce: false,
|
||||
},
|
||||
{
|
||||
name: "custom CSP without style-src gets style-src created",
|
||||
path: "/",
|
||||
isUi: true,
|
||||
customCsp: "default-src 'none'; script-src 'self'",
|
||||
wantContains: []string{"default-src 'none'", "script-src 'self'", "; style-src 'nonce-", "'self'"},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range testCases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
require := require.New(t)
|
||||
assert := assert.New(t)
|
||||
|
||||
isUiFunc := func(*http.Request) bool { return tc.isUi }
|
||||
handler := wrapHandlerWithCsp(inner, newProps(tc.customCsp), isUiFunc)
|
||||
req := httptest.NewRequest("GET", tc.path, nil)
|
||||
rec := httptest.NewRecorder()
|
||||
handler.ServeHTTP(rec, req)
|
||||
|
||||
csp := rec.Header().Get("Content-Security-Policy")
|
||||
|
||||
if len(tc.wantContains) > 0 {
|
||||
require.NotEmpty(csp)
|
||||
for _, want := range tc.wantContains {
|
||||
assert.Contains(csp, want)
|
||||
}
|
||||
return
|
||||
}
|
||||
|
||||
switch {
|
||||
case tc.wantNonce:
|
||||
require.NotEmpty(csp)
|
||||
assert.Contains(csp, "'nonce-")
|
||||
case tc.wantDefaultCsp:
|
||||
require.NotEmpty(csp)
|
||||
assert.NotContains(csp, "'nonce-")
|
||||
require.Equal(defaultCsp, csp)
|
||||
default:
|
||||
require.Empty(csp)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -6,6 +6,7 @@
|
||||
package controller
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"net/http"
|
||||
"strings"
|
||||
@@ -35,6 +36,41 @@ var serveGrantSchema = func(ctx context.Context, w http.ResponseWriter) {
|
||||
w.Write(data)
|
||||
}
|
||||
|
||||
const cspPlaceholder = "__BOUNDARY_CSP_NONCE__"
|
||||
|
||||
// cspWriter wraps an http.ResponseWriter to replace the CSP nonce placeholder
|
||||
// in index.html with the actual nonce value.
|
||||
type cspWriter struct {
|
||||
http.ResponseWriter
|
||||
nonce string
|
||||
done bool
|
||||
}
|
||||
|
||||
// WriteHeader removes the stale Content-Length since the body replacement
|
||||
// changes the size.
|
||||
func (w *cspWriter) WriteHeader(statusCode int) {
|
||||
// We need to force recalculation to avoid content length mismatch
|
||||
w.ResponseWriter.Header().Del("Content-Length")
|
||||
w.ResponseWriter.WriteHeader(statusCode)
|
||||
}
|
||||
|
||||
func (w *cspWriter) Write(b []byte) (int, error) {
|
||||
originalLen := len(b)
|
||||
if !w.done {
|
||||
w.done = true
|
||||
b = bytes.Replace(b, []byte(cspPlaceholder), []byte(w.nonce), 1)
|
||||
}
|
||||
_, err := w.ResponseWriter.Write(b)
|
||||
if err != nil {
|
||||
return 0, err
|
||||
}
|
||||
// We return the original length to maintain the io.Writer contract.
|
||||
// Per io.Writer: "Write must return a non-nil error if it returns n < len(p)."
|
||||
// Since we successfully consumed all input bytes, we must return len(p) with nil error.
|
||||
// Returning the modified length would violate this and break the caller's buffer tracking.
|
||||
return originalLen, nil
|
||||
}
|
||||
|
||||
func handleUiWithAssets(c *Controller) http.Handler {
|
||||
var nextHandler http.Handler
|
||||
if c.conf.RawConfig.DevUiPassthroughDir != "" {
|
||||
@@ -45,6 +81,8 @@ func handleUiWithAssets(c *Controller) http.Handler {
|
||||
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.Method != http.MethodGet {
|
||||
// Lets remove nonce in case we return here
|
||||
w.Header().Del("X-Boundary-Csp-Nonce")
|
||||
w.WriteHeader(http.StatusMethodNotAllowed)
|
||||
return
|
||||
}
|
||||
@@ -80,7 +118,18 @@ func handleUiWithAssets(c *Controller) http.Handler {
|
||||
}
|
||||
}
|
||||
|
||||
// Fall through to the next handler
|
||||
// For document requests, replace the CSP placeholder inside <head>.
|
||||
if r.URL.Path == "/" {
|
||||
if nonce := w.Header().Get("X-Boundary-Csp-Nonce"); nonce != "" {
|
||||
// Remove nonce once we have injected it in the Write() call
|
||||
w.Header().Del("X-Boundary-Csp-Nonce")
|
||||
nextHandler.ServeHTTP(&cspWriter{ResponseWriter: w, nonce: nonce}, r)
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
// Strip internal nonce header for non document requests
|
||||
w.Header().Del("X-Boundary-Csp-Nonce")
|
||||
nextHandler.ServeHTTP(w, r)
|
||||
})
|
||||
}
|
||||
@@ -10,6 +10,7 @@ import (
|
||||
"fmt"
|
||||
"io/ioutil"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
@@ -18,6 +19,41 @@ import (
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func TestCspWriter_Write(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
input string
|
||||
nonce string
|
||||
wantBody string
|
||||
}{
|
||||
{
|
||||
name: "replaces placeholder with nonce",
|
||||
input: "foo __BOUNDARY_CSP_NONCE__ bar",
|
||||
nonce: "'nonce-abc123'",
|
||||
wantBody: "foo 'nonce-abc123' bar",
|
||||
},
|
||||
{
|
||||
name: "no placeholder returns original unchanged",
|
||||
input: "no placeholder here",
|
||||
nonce: "'nonce-abc123'",
|
||||
wantBody: "no placeholder here",
|
||||
},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
rec := httptest.NewRecorder()
|
||||
w := &cspWriter{
|
||||
ResponseWriter: rec,
|
||||
nonce: tc.nonce,
|
||||
}
|
||||
n, err := w.Write([]byte(tc.input))
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, len(tc.input), n, "must return original input length to conform to the io.Writer contract")
|
||||
assert.Equal(t, tc.wantBody, rec.Body.String())
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestUiRouting(t *testing.T) {
|
||||
// Create a temporary directory
|
||||
tempDir, err := ioutil.TempDir("", "boundary-test-")
|
||||
|
||||
Reference in new issue
Block a user