Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set allowed headers via API instead of defaulting to wildcard. #3023

Merged
merged 28 commits into from
Aug 7, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
728ba85
This comment didn't make a lot of sense.
Jul 15, 2017
6c44bcb
Added new field AllowedHeaders. Enable func now accepts a slice of he…
Jul 15, 2017
bd4d006
Cleaned up comment.
Jul 16, 2017
1ee92b3
No need to have the preflight headers as a package global.
Jul 16, 2017
c4ca1e6
Added the headers param.
Jul 16, 2017
31770b6
Specifying the actual headers instead of using the wildcard.
Jul 16, 2017
8aad9e3
Added list of standard headers that are allowed on CORS requests.
Jul 16, 2017
aeae188
Made origin param no longer required to enable CORS. Defaults to wild…
Jul 16, 2017
89ef267
Allowed headers defaults to stdAllowedHeaders.
Jul 16, 2017
ce6d754
Clearing AllowedHeaders when CORS is disabled.
Jul 16, 2017
c1d629b
Added headers to call to Enable.
Jul 16, 2017
558e955
Updated comment.
Jul 16, 2017
ea6deba
Added list of allowed headers to response.
Jul 16, 2017
5d289ed
Added allowed_headers field.
Jul 16, 2017
e8a68e8
Initial commit of test.
Jul 16, 2017
704e7a3
Persisting the allowed headers.
Jul 16, 2017
158f690
Set default allowed headers if they have not been set yet.
Jul 16, 2017
5cd956d
allowed_origins is returned as an array.
Jul 16, 2017
dafb1f8
Added docs for allowed_headers.
Jul 16, 2017
e29e68a
Added allowed_headers to request and expected result.
Jul 16, 2017
7006926
Merge branch 'master' into improvement/cors-allowed-headers
Aug 6, 2017
0f54b4d
Removing unnecessary locking/unlocking.
Aug 6, 2017
0a05154
Fixing logic error.
Aug 6, 2017
88c20e1
Each call to Enable should replace AllowedHeaders, not append to the …
Aug 6, 2017
094e2a8
An origin to allow must be required when calling Enable.
Aug 6, 2017
5abce39
Update test to check that allowed_origins is required.
Aug 6, 2017
6aa4591
Indicating that allowed_origins is required.
Aug 6, 2017
a0b2fe5
Minor adjustments
jefferai Aug 7, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 3 additions & 10 deletions http/cors.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,6 @@ import (
"github.com/hashicorp/vault/vault"
)

var preflightHeaders = map[string]string{
"Access-Control-Allow-Headers": "*",
"Access-Control-Max-Age": "300",
}

var allowedMethods = []string{
http.MethodDelete,
http.MethodGet,
Expand All @@ -38,8 +33,7 @@ func wrapCORSHandler(h http.Handler, core *vault.Core) http.Handler {
return
}

// Return a 403 if the origin is not
// allowed to make cross-origin requests.
// 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
Expand All @@ -56,10 +50,9 @@ func wrapCORSHandler(h http.Handler, core *vault.Core) http.Handler {
// apply headers for preflight requests
if req.Method == http.MethodOptions {
w.Header().Set("Access-Control-Allow-Methods", strings.Join(allowedMethods, ","))
w.Header().Set("Access-Control-Allow-Headers", strings.Join(corsConf.AllowedHeaders, ","))
w.Header().Set("Access-Control-Max-Age", "300")

for k, v := range preflightHeaders {
w.Header().Set(k, v)
}
return
}

Expand Down
5 changes: 3 additions & 2 deletions http/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"net/http"
"net/http/httptest"
"reflect"
"strings"
"testing"

"github.com/hashicorp/go-cleanhttp"
Expand All @@ -21,7 +22,7 @@ func TestHandler_cors(t *testing.T) {

// Enable CORS and allow from any origin for testing.
corsConfig := core.CORSConfig()
err := corsConfig.Enable([]string{addr})
err := corsConfig.Enable([]string{addr}, nil)
if err != nil {
t.Fatalf("Error enabling CORS: %s", err)
}
Expand Down Expand Up @@ -78,7 +79,7 @@ func TestHandler_cors(t *testing.T) {
//
expHeaders := map[string]string{
"Access-Control-Allow-Origin": addr,
"Access-Control-Allow-Headers": "*",
"Access-Control-Allow-Headers": strings.Join(stdAllowedHeaders, ","),
"Access-Control-Max-Age": "300",
"Vary": "Origin",
}
Expand Down
78 changes: 78 additions & 0 deletions http/sys_config_cors_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
package http

import (
"encoding/json"
"net/http"
"reflect"
"testing"

"github.com/hashicorp/vault/vault"
)

func TestSysConfigCors(t *testing.T) {
var resp *http.Response

core, _, token := vault.TestCoreUnsealed(t)
ln, addr := TestServer(t, core)
defer ln.Close()
TestServerAuth(t, addr, token)

corsConf := core.CORSConfig()

// Try to enable CORS without providing a value for allowed_origins
resp = testHttpPut(t, token, addr+"/v1/sys/config/cors", map[string]interface{}{
"allowed_headers": "X-Custom-Header",
})

testResponseStatus(t, resp, 500)

// Enable CORS, but provide an origin this time.
resp = testHttpPut(t, token, addr+"/v1/sys/config/cors", map[string]interface{}{
"allowed_origins": addr,
"allowed_headers": "X-Custom-Header",
})

testResponseStatus(t, resp, 204)

// Read the CORS configuration
resp = testHttpGet(t, token, addr+"/v1/sys/config/cors")
testResponseStatus(t, resp, 200)

var actual map[string]interface{}
var expected map[string]interface{}

lenStdHeaders := len(corsConf.AllowedHeaders)

expectedHeaders := make([]interface{}, lenStdHeaders)

for i := range corsConf.AllowedHeaders {
expectedHeaders[i] = corsConf.AllowedHeaders[i]
}

expected = map[string]interface{}{
"lease_id": "",
"renewable": false,
"lease_duration": json.Number("0"),
"wrap_info": nil,
"warnings": nil,
"auth": nil,
"data": map[string]interface{}{
"enabled": true,
"allowed_origins": []interface{}{addr},
"allowed_headers": expectedHeaders,
},
"enabled": true,
"allowed_origins": []interface{}{addr},
"allowed_headers": expectedHeaders,
}

testResponseStatus(t, resp, 200)

testResponseBody(t, resp, &actual)
expected["request_id"] = actual["request_id"]

if !reflect.DeepEqual(actual, expected) {
t.Fatalf("bad: expected: %#v\nactual: %#v", expected, actual)
}

}
2 changes: 1 addition & 1 deletion vault/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -459,8 +459,8 @@ func NewCore(conf *CoreConfig) (*Core, error) {
enableMlock: !conf.DisableMlock,
}

// Load CORS config and provide core
c.corsConfig = &CORSConfig{core: c}
// Load CORS config and provide a value for the core field.

// Wrap the physical backend in a cache layer if enabled and not already wrapped
if _, isCache := conf.Physical.(*physical.Cache); !conf.DisableCache && !isCache {
Expand Down
34 changes: 30 additions & 4 deletions vault/cors.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,24 @@ const (
CORSEnabled
)

var stdAllowedHeaders = []string{
"Content-Type",
"X-Requested-With",
"X-Vault-AWS-IAM-Server-ID",
"X-Vault-MFA",
"X-Vault-No-Request-Forwarding",
"X-Vault-Token",
"X-Vault-Wrap-Format",
"X-Vault-Wrap-TTL",
}

// CORSConfig stores the state of the CORS configuration.
type CORSConfig struct {
sync.RWMutex `json:"-"`
core *Core
Enabled uint32 `json:"enabled"`
AllowedOrigins []string `json:"allowed_origins,omitempty"`
AllowedHeaders []string `json:"allowed_headers,omitempty"`
}

func (c *Core) saveCORSConfig() error {
Expand All @@ -31,6 +43,7 @@ func (c *Core) saveCORSConfig() error {
}
c.corsConfig.RLock()
localConfig.AllowedOrigins = c.corsConfig.AllowedOrigins
localConfig.AllowedHeaders = c.corsConfig.AllowedHeaders
c.corsConfig.RUnlock()

entry, err := logical.StorageEntryJSON("cors", localConfig)
Expand Down Expand Up @@ -72,9 +85,9 @@ func (c *Core) loadCORSConfig() error {

// Enable takes either a '*' or a comma-seprated list of URLs that can make
// cross-origin requests to Vault.
func (c *CORSConfig) Enable(urls []string) error {
func (c *CORSConfig) Enable(urls []string, headers []string) error {
if len(urls) == 0 {
return errors.New("the list of allowed origins cannot be empty")
return errors.New("at least one origin or the wildcard must be provided.")
}

if strutil.StrListContains(urls, "*") && len(urls) > 1 {
Expand All @@ -83,6 +96,15 @@ func (c *CORSConfig) Enable(urls []string) error {

c.Lock()
c.AllowedOrigins = urls

// Start with the standard headers to Vault accepts.
c.AllowedHeaders = append(c.AllowedHeaders, stdAllowedHeaders...)

// Allow the user to add additional headers to the list of
// headers allowed on cross-origin requests.
if len(headers) > 0 {
c.AllowedHeaders = append(c.AllowedHeaders, headers...)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there ever a chance that Enable will be called multiple times? It seems like the right thing here would be to start fresh every time since this function is (presuambly) being given the canonical set of allowed headers. So rather than the check above and the check here, simply do:

c.AllowedHeaders = stdAllowedHeaders
if len(headers) > 0 {
c.AllowedHeaders = append(c.AllowedHeaders, headers...)
}

Let me know if I'm missing something here...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enable certainly could be called multiple times, because this code allows new headers to be appended to the existing set. This workflow, however, clashes with the workflow for setting origins. Your suggestion is correct.

}
c.Unlock()

atomic.StoreUint32(&c.Enabled, CORSEnabled)
Expand All @@ -95,12 +117,16 @@ func (c *CORSConfig) IsEnabled() bool {
return atomic.LoadUint32(&c.Enabled) == CORSEnabled
}

// Disable sets CORS to disabled and clears the allowed origins
// Disable sets CORS to disabled and clears the allowed origins & headers.
func (c *CORSConfig) Disable() error {
atomic.StoreUint32(&c.Enabled, CORSDisabled)
c.Lock()
c.AllowedOrigins = []string(nil)

c.AllowedOrigins = nil
c.AllowedHeaders = nil

c.Unlock()

return c.core.saveCORSConfig()
}

Expand Down
12 changes: 9 additions & 3 deletions vault/logical_system.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@ func NewSystemBackend(core *Core) *SystemBackend {
Type: framework.TypeCommaStringSlice,
Description: "A comma-separated string or array of strings indicating origins that may make cross-origin requests.",
},
"allowed_headers": &framework.FieldSchema{
Type: framework.TypeCommaStringSlice,
Description: "A comma-separated string or array of strings indicating headers that are allowed on cross-origin requests.",
},
},

Callbacks: map[logical.Operation]framework.OperationFunc{
Expand Down Expand Up @@ -854,6 +858,7 @@ func (b *SystemBackend) handleCORSRead(req *logical.Request, d *framework.FieldD
if enabled {
corsConf.RLock()
resp.Data["allowed_origins"] = corsConf.AllowedOrigins
resp.Data["allowed_headers"] = corsConf.AllowedHeaders
corsConf.RUnlock()
}

Expand All @@ -864,12 +869,13 @@ func (b *SystemBackend) handleCORSRead(req *logical.Request, d *framework.FieldD
// cross-origin requests and sets the CORS enabled flag to true
func (b *SystemBackend) handleCORSUpdate(req *logical.Request, d *framework.FieldData) (*logical.Response, error) {
origins := d.Get("allowed_origins").([]string)
headers := d.Get("allowed_headers").([]string)

return nil, b.Core.corsConfig.Enable(origins)
return nil, b.Core.corsConfig.Enable(origins, headers)
}

// handleCORSDelete clears the allowed origins and sets the CORS enabled flag
// to false
// handleCORSDelete sets the CORS enabled flag to false and clears the list of
// allowed origins & headers.
func (b *SystemBackend) handleCORSDelete(req *logical.Request, d *framework.FieldData) (*logical.Response, error) {
return nil, b.Core.corsConfig.Disable()
}
Expand Down
2 changes: 2 additions & 0 deletions vault/logical_system_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ func TestSystemConfigCORS(t *testing.T) {

req := logical.TestRequest(t, logical.UpdateOperation, "config/cors")
req.Data["allowed_origins"] = "http://www.example.com"
req.Data["allowed_headers"] = "X-Custom-Header"
_, err := b.HandleRequest(req)
if err != nil {
t.Fatal(err)
Expand All @@ -65,6 +66,7 @@ func TestSystemConfigCORS(t *testing.T) {
Data: map[string]interface{}{
"enabled": true,
"allowed_origins": []string{"http://www.example.com"},
"allowed_headers": append(stdAllowedHeaders, "X-Custom-Header"),
},
}

Expand Down
20 changes: 16 additions & 4 deletions website/source/api/system/config-cors.html.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,28 +34,40 @@ $ curl \
```json
{
"enabled": true,
"allowed_origins": "http://www.example.com"
"allowed_origins": ["http://www.example.com"],
"allowed_headers": [
"Content-Type",
"X-Requested-With",
"X-Vault-AWS-IAM-Server-ID",
"X-Vault-No-Request-Forwarding",
"X-Vault-Token",
"X-Vault-Wrap-Format",
"X-Vault-Wrap-TTL",
]
}
```

## Configure CORS Settings

This endpoint allows configuring the origins that are permitted to make
cross-origin requests.
cross-origin requests, as well as headers that are allowed on cross-origin requests.

| Method | Path | Produces |
| :------- | :--------------------------- | :--------------------- |
| `PUT` | `/sys/config/cors` | `204 (empty body)` |

### Parameters

- `allowed_origins` `(string or string array: "" or [])` – A wildcard (`*`), comma-delimited string, or array of strings specifying the origins that are permitted to make cross-origin requests.
- `allowed_origins` `(string or string array: <required>)` – A wildcard (`*`), comma-delimited string, or array of strings specifying the origins that are permitted to make cross-origin requests.

- `allowed_headers` `(string or string array: "" or [])` – A comma-delimited string or array of strings specifying headers that are permitted to be on cross-origin requests. Headers set via this parameter will be appended to the list of headers that Vault allows by default.

### Sample Payload

```json
{
"allowed_origins": "*"
"allowed_origins": "*",
"allowed_headers": "X-Custom-Header"
}
```

Expand Down