From 351a7313df35e8de9e9474fd56a1a905cd51e0c1 Mon Sep 17 00:00:00 2001
From: James Peach <jpeach@vmware.com>
Date: Thu, 26 Mar 2020 08:16:41 +1100
Subject: [PATCH] topdown: Fix lost `tls_insecure_skip_verify` setting

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>
---
 topdown/http.go      | 9 +++++----
 topdown/http_test.go | 5 +++++
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/topdown/http.go b/topdown/http.go
index 7c6cae2ade..d16b65287a 100644
--- a/topdown/http.go
+++ b/topdown/http.go
@@ -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 {
@@ -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...)
@@ -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
diff --git a/topdown/http_test.go b/topdown/http_test.go
index bfc356d109..a69ae21e85 100644
--- a/topdown/http_test.go
+++ b/topdown/http_test.go
@@ -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()