Skip to content

Commit

Permalink
topdown: Add http.send support for setting the TLS server name
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
jpeach authored and patrick-east committed Mar 26, 2020
1 parent 5234aae commit 29d8fbb
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 22 deletions.
3 changes: 2 additions & 1 deletion docs/content/policy-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
62 changes: 41 additions & 21 deletions topdown/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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) {
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
36 changes: 36 additions & 0 deletions topdown/http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit 29d8fbb

Please sign in to comment.