From 332dc83c46b1cb135bc42b7afa1c9b50629e0e15 Mon Sep 17 00:00:00 2001
From: Morgan Parry <m.parry@winton.com>
Date: Wed, 13 Dec 2017 15:50:25 +0000
Subject: [PATCH] (fix) Handle caFile alone being set for repos

Previously it was only respected if certFile and keyFile were also
specified. However, these are independent features.
---
 pkg/getter/httpgetter.go      | 15 +++----------
 pkg/getter/httpgetter_test.go | 20 +++++++++++++----
 pkg/tlsutil/tls.go            | 42 ++++++++++++++++++++++++++++-------
 3 files changed, 53 insertions(+), 24 deletions(-)

diff --git a/pkg/getter/httpgetter.go b/pkg/getter/httpgetter.go
index dd462ce5f..d04b1076c 100644
--- a/pkg/getter/httpgetter.go
+++ b/pkg/getter/httpgetter.go
@@ -23,7 +23,6 @@ import (
 	"strings"
 
 	"k8s.io/helm/pkg/tlsutil"
-	"k8s.io/helm/pkg/urlutil"
 	"k8s.io/helm/pkg/version"
 )
 
@@ -80,19 +79,11 @@ func newHTTPGetter(URL, CertFile, KeyFile, CAFile string) (Getter, error) {
 // NewHTTPGetter constructs a valid http/https client as HttpGetter
 func NewHTTPGetter(URL, CertFile, KeyFile, CAFile string) (*HttpGetter, error) {
 	var client HttpGetter
-	if CertFile != "" && KeyFile != "" {
-		tlsConf, err := tlsutil.NewClientTLS(CertFile, KeyFile, CAFile)
+	if (CertFile != "" && KeyFile != "") || CAFile != "" {
+		tlsConf, err := tlsutil.NewTLSConfig(URL, CertFile, KeyFile, CAFile)
 		if err != nil {
-			return &client, fmt.Errorf("can't create TLS config for client: %s", err.Error())
+			return &client, fmt.Errorf("can't create TLS config: %s", err.Error())
 		}
-		tlsConf.BuildNameToCertificate()
-
-		sni, err := urlutil.ExtractHostname(URL)
-		if err != nil {
-			return &client, err
-		}
-		tlsConf.ServerName = sni
-
 		client.client = &http.Client{
 			Transport: &http.Transport{
 				TLSClientConfig: tlsConf,
diff --git a/pkg/getter/httpgetter_test.go b/pkg/getter/httpgetter_test.go
index fe3fde22a..ec730f401 100644
--- a/pkg/getter/httpgetter_test.go
+++ b/pkg/getter/httpgetter_test.go
@@ -28,7 +28,7 @@ func TestHTTPGetter(t *testing.T) {
 	}
 
 	if hg, ok := g.(*HttpGetter); !ok {
-		t.Fatal("Expected newHTTPGetter to produce an httpGetter")
+		t.Fatal("Expected newHTTPGetter to produce an HttpGetter")
 	} else if hg.client != http.DefaultClient {
 		t.Fatal("Expected newHTTPGetter to return a default HTTP client.")
 	}
@@ -37,12 +37,24 @@ func TestHTTPGetter(t *testing.T) {
 	cd := "../../testdata"
 	join := filepath.Join
 	ca, pub, priv := join(cd, "ca.pem"), join(cd, "crt.pem"), join(cd, "key.pem")
-	g, err = newHTTPGetter("http://example.com/", pub, priv, ca)
+	g, err = newHTTPGetter("https://example.com/", pub, priv, ca)
 	if err != nil {
 		t.Fatal(err)
 	}
+	if hg, ok := g.(*HttpGetter); !ok {
+		t.Fatal("Expected newHTTPGetter to produce an HttpGetter")
+	} else if hg.client == http.DefaultClient {
+		t.Fatal("Expected newHTTPGetter to return a non-default HTTP client")
+	}
 
-	if _, ok := g.(*HttpGetter); !ok {
-		t.Fatal("Expected newHTTPGetter to produce an httpGetter")
+	// Test with SSL, caFile only
+	g, err = newHTTPGetter("https://example.com/", "", "", ca)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if hg, ok := g.(*HttpGetter); !ok {
+		t.Fatal("Expected newHTTPGetter to produce an HttpGetter")
+	} else if hg.client == http.DefaultClient {
+		t.Fatal("Expected newHTTPGetter to return a non-default HTTP client")
 	}
 }
diff --git a/pkg/tlsutil/tls.go b/pkg/tlsutil/tls.go
index df698fd4e..c166a1662 100644
--- a/pkg/tlsutil/tls.go
+++ b/pkg/tlsutil/tls.go
@@ -21,17 +21,20 @@ import (
 	"crypto/x509"
 	"fmt"
 	"io/ioutil"
+	"k8s.io/helm/pkg/urlutil"
 )
 
-// NewClientTLS returns tls.Config appropriate for client auth.
-func NewClientTLS(certFile, keyFile, caFile string) (*tls.Config, error) {
-	cert, err := CertFromFilePair(certFile, keyFile)
-	if err != nil {
-		return nil, err
-	}
-	config := tls.Config{
-		Certificates: []tls.Certificate{*cert},
+func newTLSConfigCommon(certFile, keyFile, caFile string) (*tls.Config, error) {
+	config := tls.Config{}
+
+	if certFile != "" && keyFile != "" {
+		cert, err := CertFromFilePair(certFile, keyFile)
+		if err != nil {
+			return nil, err
+		}
+		config.Certificates = []tls.Certificate{*cert}
 	}
+
 	if caFile != "" {
 		cp, err := CertPoolFromFile(caFile)
 		if err != nil {
@@ -39,9 +42,32 @@ func NewClientTLS(certFile, keyFile, caFile string) (*tls.Config, error) {
 		}
 		config.RootCAs = cp
 	}
+
 	return &config, nil
 }
 
+// NewClientTLS returns tls.Config appropriate for client auth.
+func NewClientTLS(certFile, keyFile, caFile string) (*tls.Config, error) {
+	return newTLSConfigCommon(certFile, keyFile, caFile)
+}
+
+// NewTLSConfig returns tls.Config appropriate for client and/or server auth.
+func NewTLSConfig(url, certFile, keyFile, caFile string) (*tls.Config, error) {
+	config, err := newTLSConfigCommon(certFile, keyFile, caFile)
+	if err != nil {
+		return nil, err
+	}
+	config.BuildNameToCertificate()
+
+	serverName, err := urlutil.ExtractHostname(url)
+	if err != nil {
+		return nil, err
+	}
+	config.ServerName = serverName
+
+	return config, nil
+}
+
 // CertPoolFromFile returns an x509.CertPool containing the certificates
 // in the given PEM-encoded file.
 // Returns an error if the file could not be read, a certificate could not