vault icon indicating copy to clipboard operation
vault copied to clipboard

Stop sending CORS when Vault is manually sealed

Open cipherboy opened this issue 1 year ago • 2 comments

When Vault has CORS configuration set, it serves all relevant handlers with CORS request data. This configuration is stored within barrier protected storage, so when Vault starts up with a manual seal method, no CORS headers are initially served. However, once unsealed, Vault will start servicing requests with CORS headers.

When Vault is subsequently manually sealed, CORS headers were previously still sent on outbound requests.

This meant any misbehaving CORS script could request Vault be unsealed and provide bogus Shamir shares, because the browser's CORS validation would permit access Vault, as Vault is still serving valid CORS headers.

By disabling CORS, we fall back to Vault's default logic of servicing the request directly--but note that conforming browsers will still deny the request because Vault does not respond to CORS validation requests successfully after their expiry. This is more secure: most browsers are well-behaved and thus will not institute CORS requests after the initial Access-Control-Max-Age (300 seconds) expires.

Resolves: #12569

Signed-off-by: Alexander Scheel <[email protected]>


What threw me for a loop was the start of wrapCORSHandler:

func wrapCORSHandler(h http.Handler, core *vault.Core) http.Handler {
    return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
        corsConf := core.CORSConfig()

        // If CORS is not enabled or if no Origin header is present (i.e. the request
        // is from the Vault CLI. A browser will always send an Origin header), then
        // just return a 204.
        if !corsConf.IsEnabled() {
            h.ServeHTTP(w, req)
            return
        }

        origin := req.Header.Get("Origin")
        requestMethod := req.Header.Get("Access-Control-Request-Method")

        if origin == "" {
            h.ServeHTTP(w, req)
            return
        }

        // Return a 403 if the origin is not allowed to make cross-origin requests.
        if !corsConf.IsValidOrigin(origin) {
            respondError(w, http.StatusForbidden, fmt.Errorf("origin not allowed"))
            return
        }

I'd expect Vault to check, if CORS is disabled, that if Origin+Access-Control-Request-Method client request headers are provided, we'd deny the request. However, since plugins can't really do METHOD-typed requests, we should still err out from the call (maybe not with http.StatusForbidden but a 404 should behave the same in the end on conforming Browsers).

Non-conforming browsers which don't send these requests don't really matter, as they'd not set Origin on CORS requests either way, so Vault would have nothing to validate.

cipherboy avatar Sep 23 '22 19:09 cipherboy

@cipherboy It might be nice to see some go tests to ensure this change is doing what we claim.

Still need to determine if expectations and reality align and tests might support that.

As far as the code path for c.CORSConfig().Enabled goes, it sounds like sealInternalWithOptions is the correct place (as @hghaf099 suggested).

mpalmi avatar Sep 27 '22 17:09 mpalmi

Marking draft as we're awaiting product decisions and consistency analysis of equivalent actions on seal/unseal.

cipherboy avatar Nov 18 '22 17:11 cipherboy