Skip to content

Commit

Permalink
topdown: Fix lost tls_insecure_skip_verify setting
Browse files Browse the repository at this point in the history
If the `http.send` builtin is called with both `tls_insecure_skip_verify`
and another TLS configuration, the verificate setting will get lost as
the TLS configuration is overwritten. Ensure that we update the same
TLS configuration in all cases so that it is consistent.

Signed-off-by: James Peach <jpeach@vmware.com>
  • Loading branch information
jpeach authored and patrick-east committed Mar 25, 2020
1 parent 5a5d2a4 commit 351a731
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 4 deletions.
9 changes: 5 additions & 4 deletions topdown/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,15 +272,16 @@ func executeHTTPRequest(bctx BuiltinContext, obj ast.Object) (ast.Value, error)
}
}

isTLS := false
client := &http.Client{
Timeout: timeout,
}

if tlsInsecureSkipVerify {
client.Transport = &http.Transport{
TLSClientConfig: &tls.Config{InsecureSkipVerify: tlsInsecureSkipVerify},
}
isTLS = true
tlsConfig.InsecureSkipVerify = tlsInsecureSkipVerify
}

if tlsClientCertFile != "" && tlsClientKeyFile != "" {
clientCertFromFile, err := tls.LoadX509KeyPair(tlsClientCertFile, tlsClientKeyFile)
if err != nil {
Expand All @@ -297,7 +298,6 @@ func executeHTTPRequest(bctx BuiltinContext, obj ast.Object) (ast.Value, error)
clientCerts = append(clientCerts, clientCertFromEnv)
}

isTLS := false
if len(clientCerts) > 0 {
isTLS = true
tlsConfig.Certificates = append(tlsConfig.Certificates, clientCerts...)
Expand Down Expand Up @@ -337,6 +337,7 @@ func executeHTTPRequest(bctx BuiltinContext, obj ast.Object) (ast.Value, error)
if err != nil {
return nil, err
}

req = req.WithContext(bctx.Context)

// Add custom headers
Expand Down
5 changes: 5 additions & 0 deletions topdown/http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,11 @@ func TestHTTPGetRequestTlsInsecureSkipVerify(t *testing.T) {
`p = x { http.send({"method": "get", "url": "%s", "force_json_decode": true}, x) }`, ts.URL)}, expected: &Error{Message: "x509: certificate signed by unknown authority"}},
{note: "http.send", rules: []string{fmt.Sprintf(
`p = x { http.send({"method": "get", "url": "%s", "force_json_decode": true, "tls_insecure_skip_verify": true}, x) }`, ts.URL)}, expected: resultObj.String()},
// This case verifies that `tls_insecure_skip_verify`
// is still applied, even if other TLS settings are
// present.
{note: "http.send", rules: []string{fmt.Sprintf(
`p = x { http.send({"method": "get", "url": "%s", "force_json_decode": true, "tls_insecure_skip_verify": true, "tls_use_system_certs": true,}, x) }`, ts.URL)}, expected: resultObj.String()},
}

data := loadSmallTestData()
Expand Down

0 comments on commit 351a731

Please sign in to comment.