From 29d8fbbef6facc96d03be3e07473d12e38acd843 Mon Sep 17 00:00:00 2001 From: James Peach Date: Thu, 26 Mar 2020 10:02:04 +1100 Subject: [PATCH] topdown: Add http.send support for setting the TLS server name If the caller sets the Host header, then we can reasonably impute that they wanted to use that at the TLS layer as well. To handle test cases where the caller needs to explicitly set the TLS server name, add a `tls_server_name` parameter that overrides both the `Host` header and the Go default. Signed-off-by: James Peach --- docs/content/policy-reference.md | 3 +- topdown/http.go | 62 +++++++++++++++++++++----------- topdown/http_test.go | 36 +++++++++++++++++++ 3 files changed, 79 insertions(+), 22 deletions(-) diff --git a/docs/content/policy-reference.md b/docs/content/policy-reference.md index 1997c13205..7b0e0fd001 100644 --- a/docs/content/policy-reference.md +++ b/docs/content/policy-reference.md @@ -572,7 +572,8 @@ The `request` object parameter may contain the following fields: | `tls_client_cert_file` | no | `string` | Path to file containing a client certificate in PEM encoded format. | | `tls_client_key_file` | no | `string` | Path to file containing a key in PEM encoded format. | | `timeout` | no | `string` or `number` | Timeout for the HTTP request with a default of 5 seconds (`5s`). Numbers provided are in nanoseconds. Strings must be a valid duration string where a duration string is a possibly signed sequence of decimal numbers, each with optional fraction and a unit suffix, such as "300ms", "-1.5h" or "2h45m". Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h". A zero timeout means no timeout.| -| `tls_insecure_skip_verify` | no | `bool` | Allows for skipping TLS verification when calling a network endpoint. Not recommended for production | +| `tls_insecure_skip_verify` | no | `bool` | Allows for skipping TLS verification when calling a network endpoint. Not recommended for production. | +| `tls_server_name` | no | `string` | Sets the hostname that is sent in the client Server Name Indication and that be will be used for server certificate validation. If this is not set, the value of the `Host` header (if present) will be used. If neither are set, the host name from the requested URL is used. | If the `Host` header is included in `headers`, its value will be used as the `Host` header of the request. The `url` parameter will continue to specify the server to connect to. diff --git a/topdown/http.go b/topdown/http.go index d16b65287a..63116a3f29 100644 --- a/topdown/http.go +++ b/topdown/http.go @@ -45,6 +45,7 @@ var allowedKeyNames = [...]string{ "tls_client_cert_file", "tls_client_key_file", "tls_insecure_skip_verify", + "tls_server_name", "timeout", } var allowedKeys = ast.NewSet() @@ -134,28 +135,16 @@ func validateHTTPRequestOperand(term *ast.Term, pos int) (ast.Object, error) { } -// Adds custom headers to a new HTTP request. -func addHeaders(req *http.Request, headers map[string]interface{}) (bool, error) { - for k, v := range headers { - // Type assertion - header, ok := v.(string) - if !ok { - return false, fmt.Errorf("invalid type for headers value %q", v) - } +// canonicalizeHeaders returns a copy of the headers where the keys are in +// canonical HTTP form. +func canonicalizeHeaders(headers map[string]interface{}) map[string]interface{} { + canonicalized := map[string]interface{}{} - // If the Host header is given, bump that up to - // the request. Otherwise, just collect it in the - // headers. - k := http.CanonicalHeaderKey(k) - switch k { - case "Host": - req.Host = header - default: - req.Header.Add(k, header) - } + for k, v := range headers { + canonicalized[http.CanonicalHeaderKey(k)] = v } - return true, nil + return canonicalized } func executeHTTPRequest(bctx BuiltinContext, obj ast.Object) (ast.Value, error) { @@ -167,6 +156,7 @@ func executeHTTPRequest(bctx BuiltinContext, obj ast.Object) (ast.Value, error) var tlsClientCertEnvVar []byte var tlsClientCertFile string var tlsClientKeyFile string + var tlsServerName string var body *bytes.Buffer var rawBody *bytes.Buffer var enableRedirect bool @@ -246,6 +236,9 @@ func executeHTTPRequest(bctx BuiltinContext, obj ast.Object) (ast.Value, error) case "tls_client_key_file": tlsClientKeyFile = obj.Get(val).String() tlsClientKeyFile = strings.Trim(tlsClientKeyFile, "\"") + case "tls_server_name": + tlsServerName = obj.Get(val).String() + tlsServerName = strings.Trim(tlsServerName, "\"") case "headers": headersVal := obj.Get(val).Value headersValInterface, err := ast.JSON(headersVal) @@ -342,13 +335,40 @@ func executeHTTPRequest(bctx BuiltinContext, obj ast.Object) (ast.Value, error) // Add custom headers if len(customHeaders) != 0 { - if ok, err := addHeaders(req, customHeaders); !ok { - return nil, err + customHeaders = canonicalizeHeaders(customHeaders) + + for k, v := range customHeaders { + header, ok := v.(string) + if !ok { + return nil, fmt.Errorf("invalid type for headers value %q", v) + } + + req.Header.Add(k, header) } + // Don't overwrite or append to one that was set in the custom headers if _, hasUA := customHeaders["User-Agent"]; !hasUA { req.Header.Add("User-Agent", version.UserAgent) } + + // If the caller specifies the Host header, use it for the HTTP + // request host and the TLS server name. + if host, hasHost := customHeaders["Host"]; hasHost { + host := host.(string) // We already checked that it's a string. + req.Host = host + + // Only default the ServerName if the caller has + // specified the host. If we don't specify anything, + // Go will default to the target hostname. This name + // is not the same as the default that Go populates + // `req.Host` with, which is why we don't just set + // this unconditionally. + tlsConfig.ServerName = host + } + } + + if tlsServerName != "" { + tlsConfig.ServerName = tlsServerName } // execute the http request diff --git a/topdown/http_test.go b/topdown/http_test.go index a69ae21e85..5bcfcb463f 100644 --- a/topdown/http_test.go +++ b/topdown/http_test.go @@ -1092,6 +1092,42 @@ func TestHTTPSClient(t *testing.T) { // run the test runTopDownTestCase(t, data, "http.send", rule, expectedResult) }) + + // Expect that setting the Host header causes TLS server validation + // to fail because the server sends a different certificate. + t.Run("Client Host is also ServerName", func(t *testing.T) { + url := s.URL + "/cert" + hostname := "notpresent" + + expected := &Error{Code: BuiltinErr, Message: fmt.Sprintf( + "http.send: Get %s: x509: certificate is valid for localhost, not %s", + url, hostname)} + + data := loadSmallTestData() + rule := []string{fmt.Sprintf( + `p = x { http.send({"method": "get", "url": "%s", "tls_ca_cert_file": "%s", "tls_client_cert_file": "%s", "tls_client_key_file": "%s", "headers": {"host": "%s"}}, x) }`, url, localCaFile, localClientCertFile, localClientKeyFile, hostname)} + + // run the test + runTopDownTestCase(t, data, "http.send", rule, expected) + }) + + // Expect that setting `tls_server_name` causes TLS server validation + // to fail because the server sends a different certificate. + t.Run("Client can set ServerName", func(t *testing.T) { + url := s.URL + "/cert" + hostname := "notpresent" + + expected := &Error{Code: BuiltinErr, Message: fmt.Sprintf( + "http.send: Get %s: x509: certificate is valid for localhost, not %s", + url, hostname)} + + data := loadSmallTestData() + rule := []string{fmt.Sprintf( + `p = x { http.send({"method": "get", "url": "%s", "tls_ca_cert_file": "%s", "tls_client_cert_file": "%s", "tls_client_key_file": "%s", "tls_server_name": "%s"}, x) }`, url, localCaFile, localClientCertFile, localClientKeyFile, hostname)} + + // run the test + runTopDownTestCase(t, data, "http.send", rule, expected) + }) } func TestHTTPSNoClientCerts(t *testing.T) {