From 28db15c830a8f129283f43682727b470862572d8 Mon Sep 17 00:00:00 2001
From: Peter Amstutz <peter.amstutz@curoverse.com>
Date: Fri, 16 Dec 2016 11:09:41 -0500
Subject: [PATCH] 10684: Refactor into MakeTLSConfig().  Only look for
 /etc/arvados/ca-certificates.crt and then fall back onto system certs.  Skip
 looking for arvados-specific certs if insecure is true.

---
 sdk/go/arvadosclient/arvadosclient.go | 51 +++++++++++++--------------
 sdk/go/crunchrunner/crunchrunner.go   |  2 --
 sdk/go/keepclient/keepclient.go       |  5 +--
 3 files changed, 25 insertions(+), 33 deletions(-)

diff --git a/sdk/go/arvadosclient/arvadosclient.go b/sdk/go/arvadosclient/arvadosclient.go
index be036c0219..dc3eddba65 100644
--- a/sdk/go/arvadosclient/arvadosclient.go
+++ b/sdk/go/arvadosclient/arvadosclient.go
@@ -105,29 +105,31 @@ type ArvadosClient struct {
 	Retries int
 }
 
-var CertFiles = []string{
-	"/etc/arvados/ca-certificates.crt",   // Arvados specific
-	"/etc/ssl/certs/ca-certificates.crt", // Debian
-	"/etc/pki/tls/certs/ca-bundle.crt",   // Red Hat
-}
-
-// SetupRootCAs loads a set of root certificates into TLSClientConfig by
-// searching a default list of locations.
-func SetupRootCAs(tlsClientConfig *tls.Config) error {
-	// Container may not have certificates installed, so need to look for
-	// /etc/arvados/ca-certificates.crt in addition to normal system certs.
-
-	certs := x509.NewCertPool()
-	for _, file := range CertFiles {
-		data, err := ioutil.ReadFile(file)
-		if err == nil {
-			certs.AppendCertsFromPEM(data)
-			tlsClientConfig.RootCAs = certs
-			return nil
+var CertFiles = []string{"/etc/arvados/ca-certificates.crt"}
+
+// MakeTLSConfig sets up TLS configuration for communicating with Arvados and Keep services.
+func MakeTLSConfig(insecure bool) *tls.Config {
+	tlsconfig := tls.Config{InsecureSkipVerify: insecure}
+
+	if !insecure {
+		// Look for /etc/arvados/ca-certificates.crt in addition to normal system certs.
+		certs := x509.NewCertPool()
+		for _, file := range CertFiles {
+			data, err := ioutil.ReadFile(file)
+			if err == nil {
+				success := certs.AppendCertsFromPEM(data)
+				if !success {
+					fmt.Errorf("Did not load any certificates from %v", file)
+				} else {
+					tlsconfig.RootCAs = certs
+					break
+				}
+			}
 		}
+		// Will use system default CA roots if /etc/arvados/ca-certificates.crt not found.
 	}
 
-	return fmt.Errorf("Unable to find TLS root certificates to use, tried %v", CertFiles)
+	return &tlsconfig
 }
 
 // New returns an ArvadosClient using the given arvados.Client
@@ -135,15 +137,13 @@ func SetupRootCAs(tlsClientConfig *tls.Config) error {
 // fields from configuration files but still need to use the
 // arvadosclient.ArvadosClient package.
 func New(c *arvados.Client) (*ArvadosClient, error) {
-	tlsconfig := &tls.Config{InsecureSkipVerify: c.Insecure}
-	SetupRootCAs(tlsconfig)
 	ac := &ArvadosClient{
 		Scheme:      "https",
 		ApiServer:   c.APIHost,
 		ApiToken:    c.AuthToken,
 		ApiInsecure: c.Insecure,
 		Client: &http.Client{Transport: &http.Transport{
-			TLSClientConfig: tlsconfig}},
+			TLSClientConfig: MakeTLSConfig(c.Insecure)}},
 		External:          false,
 		Retries:           2,
 		lastClosedIdlesAt: time.Now(),
@@ -161,16 +161,13 @@ func MakeArvadosClient() (ac *ArvadosClient, err error) {
 	insecure := matchTrue.MatchString(os.Getenv("ARVADOS_API_HOST_INSECURE"))
 	external := matchTrue.MatchString(os.Getenv("ARVADOS_EXTERNAL_CLIENT"))
 
-	tlsconfig := &tls.Config{InsecureSkipVerify: insecure}
-	SetupRootCAs(tlsconfig)
-
 	ac = &ArvadosClient{
 		Scheme:      "https",
 		ApiServer:   os.Getenv("ARVADOS_API_HOST"),
 		ApiToken:    os.Getenv("ARVADOS_API_TOKEN"),
 		ApiInsecure: insecure,
 		Client: &http.Client{Transport: &http.Transport{
-			TLSClientConfig: tlsconfig}},
+			TLSClientConfig: MakeTLSConfig(insecure)}},
 		External: external,
 		Retries:  2}
 
diff --git a/sdk/go/crunchrunner/crunchrunner.go b/sdk/go/crunchrunner/crunchrunner.go
index 9369036071..5d7e10be4b 100644
--- a/sdk/go/crunchrunner/crunchrunner.go
+++ b/sdk/go/crunchrunner/crunchrunner.go
@@ -1,7 +1,6 @@
 package main
 
 import (
-	"crypto/x509"
 	"encoding/json"
 	"fmt"
 	"git.curoverse.com/arvados.git/sdk/go/arvados"
@@ -10,7 +9,6 @@ import (
 	"io"
 	"io/ioutil"
 	"log"
-	"net/http"
 	"os"
 	"os/exec"
 	"os/signal"
diff --git a/sdk/go/keepclient/keepclient.go b/sdk/go/keepclient/keepclient.go
index b03a5fe5f6..1df0fa3f6a 100644
--- a/sdk/go/keepclient/keepclient.go
+++ b/sdk/go/keepclient/keepclient.go
@@ -99,14 +99,11 @@ func New(arv *arvadosclient.ArvadosClient) *KeepClient {
 		}
 	}
 
-	tlsconfig := &tls.Config{InsecureSkipVerify: arv.ApiInsecure}
-	arvadosclient.SetupRootCAs(tlsconfig)
-
 	kc := &KeepClient{
 		Arvados:       arv,
 		Want_replicas: defaultReplicationLevel,
 		Client: &http.Client{Transport: &http.Transport{
-			TLSClientConfig: tlsconfig}},
+			TLSClientConfig: arvadosclient.MakeTLSConfig(arv.ApiInsecure)}},
 		Retries: 2,
 	}
 	return kc
-- 
2.30.2