From 43169e4302a6e0ffcc01d9108965b1b4feaad5d6 Mon Sep 17 00:00:00 2001
From: Matt Bostock <mbostock@cloudflare.com>
Date: Thu, 28 Sep 2017 11:04:01 +0100
Subject: [PATCH] Verify datasource TLS and split client auth and CA

---
 pkg/models/datasource_cache.go                | 21 ++++----
 pkg/models/datasource_cache_test.go           | 12 ++---
 .../plugins/partials/ds_http_settings.html    | 48 ++++++++++---------
 3 files changed, 41 insertions(+), 40 deletions(-)

diff --git a/pkg/models/datasource_cache.go b/pkg/models/datasource_cache.go
index 158018b0f0a..14d3c6c2fc1 100644
--- a/pkg/models/datasource_cache.go
+++ b/pkg/models/datasource_cache.go
@@ -47,8 +47,7 @@ func (ds *DataSource) GetHttpTransport() (*http.Transport, error) {
 
 	transport := &http.Transport{
 		TLSClientConfig: &tls.Config{
-			InsecureSkipVerify: true,
-			Renegotiation:      tls.RenegotiateFreelyAsClient,
+			Renegotiation: tls.RenegotiateFreelyAsClient,
 		},
 		Proxy: http.ProxyFromEnvironment,
 		Dial: (&net.Dialer{
@@ -62,15 +61,13 @@ func (ds *DataSource) GetHttpTransport() (*http.Transport, error) {
 		IdleConnTimeout:       90 * time.Second,
 	}
 
-	var tlsAuth, tlsAuthWithCACert bool
+	var tlsClientAuth, tlsAuthWithCACert bool
 	if ds.JsonData != nil {
-		tlsAuth = ds.JsonData.Get("tlsAuth").MustBool(false)
+		tlsClientAuth = ds.JsonData.Get("tlsClientAuth").MustBool(false)
 		tlsAuthWithCACert = ds.JsonData.Get("tlsAuthWithCACert").MustBool(false)
 	}
 
-	if tlsAuth {
-		transport.TLSClientConfig.InsecureSkipVerify = false
-
+	if tlsClientAuth || tlsAuthWithCACert {
 		decrypted := ds.SecureJsonData.Decrypt()
 
 		if tlsAuthWithCACert && len(decrypted["tlsCACert"]) > 0 {
@@ -81,11 +78,13 @@ func (ds *DataSource) GetHttpTransport() (*http.Transport, error) {
 			}
 		}
 
-		cert, err := tls.X509KeyPair([]byte(decrypted["tlsClientCert"]), []byte(decrypted["tlsClientKey"]))
-		if err != nil {
-			return nil, err
+		if tlsClientAuth {
+			cert, err := tls.X509KeyPair([]byte(decrypted["tlsClientCert"]), []byte(decrypted["tlsClientKey"]))
+			if err != nil {
+				return nil, err
+			}
+			transport.TLSClientConfig.Certificates = []tls.Certificate{cert}
 		}
-		transport.TLSClientConfig.Certificates = []tls.Certificate{cert}
 	}
 
 	ptc.cache[ds.Id] = cachedTransport{
diff --git a/pkg/models/datasource_cache_test.go b/pkg/models/datasource_cache_test.go
index 8b36cc68e2a..bbd1c563ad1 100644
--- a/pkg/models/datasource_cache_test.go
+++ b/pkg/models/datasource_cache_test.go
@@ -36,7 +36,7 @@ func TestDataSourceCache(t *testing.T) {
 		setting.SecretKey = "password"
 
 		json := simplejson.New()
-		json.Set("tlsAuth", true)
+		json.Set("tlsClientAuth", true)
 		json.Set("tlsAuthWithCACert", true)
 
 		t := time.Now()
@@ -49,8 +49,8 @@ func TestDataSourceCache(t *testing.T) {
 		transport, err := ds.GetHttpTransport()
 		So(err, ShouldBeNil)
 
-		Convey("Should disable TLS certificate verification", func() {
-			So(transport.TLSClientConfig.InsecureSkipVerify, ShouldEqual, true)
+		Convey("Should verify TLS certificates by default", func() {
+			So(transport.TLSClientConfig.InsecureSkipVerify, ShouldEqual, false)
 		})
 
 		ds.JsonData = json
@@ -69,7 +69,7 @@ func TestDataSourceCache(t *testing.T) {
 		transport, err = ds.GetHttpTransport()
 		So(err, ShouldBeNil)
 
-		Convey("Should add cert and enable TLS certificate verification", func() {
+		Convey("Should add cert and verify TLS certificates", func() {
 			So(transport.TLSClientConfig.InsecureSkipVerify, ShouldEqual, false)
 			So(len(transport.TLSClientConfig.Certificates), ShouldEqual, 1)
 		})
@@ -81,8 +81,8 @@ func TestDataSourceCache(t *testing.T) {
 		transport, err = ds.GetHttpTransport()
 		So(err, ShouldBeNil)
 
-		Convey("Should remove cert and disable TLS certificate vertification", func() {
-			So(transport.TLSClientConfig.InsecureSkipVerify, ShouldEqual, true)
+		Convey("Should remove cert but still verify TLS certificates", func() {
+			So(transport.TLSClientConfig.InsecureSkipVerify, ShouldEqual, false)
 			So(len(transport.TLSClientConfig.Certificates), ShouldEqual, 0)
 		})
 	})
diff --git a/public/app/features/plugins/partials/ds_http_settings.html b/public/app/features/plugins/partials/ds_http_settings.html
index d10b8cbf9bc..6ea3a3cde1a 100644
--- a/public/app/features/plugins/partials/ds_http_settings.html
+++ b/public/app/features/plugins/partials/ds_http_settings.html
@@ -44,7 +44,7 @@
     <gf-form-switch class="gf-form" label="With Credentials" tooltip="Whether credentials such as cookies or auth headers should be sent with cross-site requests." checked="current.withCredentials" label-class="width-11" switch-class="max-width-6"></gf-form-switch>
   </div>
   <div class="gf-form-inline">
-    <gf-form-switch class="gf-form" ng-if="current.access=='proxy'" label="TLS Client Auth" label-class="width-8" checked="current.jsonData.tlsAuth" switch-class="max-width-6"></gf-form-switch>
+    <gf-form-switch class="gf-form" ng-if="current.access=='proxy'" label="TLS Client Auth" label-class="width-8" checked="current.jsonData.tlsClientAuth" switch-class="max-width-6"></gf-form-switch>
     <gf-form-switch class="gf-form" ng-if="current.access=='proxy'" label="With CA Cert" tooltip="Optional. Needed for self-signed TLS Certs." checked="current.jsonData.tlsAuthWithCACert" label-class="width-11" switch-class="max-width-6"></gf-form-switch>
   </div>
 </div>
@@ -66,7 +66,7 @@
 	</div>
 </div>
 
-<div class="gf-form-group" ng-if="current.jsonData.tlsAuth && current.access=='proxy'">
+<div class="gf-form-group" ng-if="(current.jsonData.tlsClientAuth || current.jsonData.tlsAuthWithCACert) && current.access=='proxy'">
   <div class="gf-form">
     <h6>TLS Auth Details</h6>
     <info-popover mode="header">TLS Certs are encrypted and stored in the Grafana database.</info-popover>
@@ -87,29 +87,31 @@
     </div>
   </div>
 
-  <div class="gf-form-inline">
-    <div class="gf-form gf-form--v-stretch">
-      <label class="gf-form-label width-7">Client Cert</label>
+  <div ng-if="current.jsonData.tlsClientAuth">
+    <div class="gf-form-inline">
+      <div class="gf-form gf-form--v-stretch">
+        <label class="gf-form-label width-7">Client Cert</label>
+      </div>
+      <div class="gf-form gf-form--grow" ng-if="!current.secureJsonFields.tlsClientCert">
+        <textarea rows="7" class="gf-form-input gf-form-textarea" ng-model="current.secureJsonData.tlsClientCert" placeholder="Begins with -----BEGIN CERTIFICATE-----" required></textarea>
+      </div>
+      <div class="gf-form" ng-if="current.secureJsonFields.tlsClientCert">
+        <input type="text" class="gf-form-input max-width-12" disabled="disabled" value="configured">
+        <a class="btn btn-secondary gf-form-btn" href="#" ng-click="current.secureJsonFields.tlsClientCert = false">reset</a>
+      </div>
     </div>
-    <div class="gf-form gf-form--grow" ng-if="!current.secureJsonFields.tlsClientCert">
-      <textarea rows="7" class="gf-form-input gf-form-textarea" ng-model="current.secureJsonData.tlsClientCert" placeholder="Begins with -----BEGIN CERTIFICATE-----" required></textarea>
-    </div>
-    <div class="gf-form" ng-if="current.secureJsonFields.tlsClientCert">
-      <input type="text" class="gf-form-input max-width-12" disabled="disabled" value="configured">
-      <a class="btn btn-secondary gf-form-btn" href="#" ng-click="current.secureJsonFields.tlsClientCert = false">reset</a>
-    </div>
-  </div>
 
-  <div class="gf-form-inline">
-    <div class="gf-form gf-form--v-stretch">
-      <label class="gf-form-label width-7">Client Key</label>
-    </div>
-    <div class="gf-form gf-form--grow" ng-if="!current.secureJsonFields.tlsClientKey">
-      <textarea rows="7" class="gf-form-input gf-form-textarea" ng-model="current.secureJsonData.tlsClientKey" placeholder="Begins with -----BEGIN RSA PRIVATE KEY-----" required></textarea>
-    </div>
-    <div class="gf-form" ng-if="current.secureJsonFields.tlsClientKey">
-      <input type="text" class="gf-form-input max-width-12" disabled="disabled" value="configured">
-      <a class="btn btn-secondary gf-form-btn" href="#" ng-click="current.secureJsonFields.tlsClientKey = false">reset</a>
+    <div class="gf-form-inline">
+      <div class="gf-form gf-form--v-stretch">
+        <label class="gf-form-label width-7">Client Key</label>
+      </div>
+      <div class="gf-form gf-form--grow" ng-if="!current.secureJsonFields.tlsClientKey">
+        <textarea rows="7" class="gf-form-input gf-form-textarea" ng-model="current.secureJsonData.tlsClientKey" placeholder="Begins with -----BEGIN RSA PRIVATE KEY-----" required></textarea>
+      </div>
+      <div class="gf-form" ng-if="current.secureJsonFields.tlsClientKey">
+        <input type="text" class="gf-form-input max-width-12" disabled="disabled" value="configured">
+        <a class="btn btn-secondary gf-form-btn" href="#" ng-click="current.secureJsonFields.tlsClientKey = false">reset</a>
+      </div>
     </div>
   </div>
 </div>