Merge pull request #119 from phillipsj/bugfix/tls-config-issue

Fixing the issue with TLS Config settings for CSI Proxy.

cmd/server/app/run.go

@@ -113,7 +113,7 @@ func _runAction(cliCtx *cli.Context) error {
 	//checking if CSI Proxy has config, if so enables it.
 	if cfg.CSIProxy != nil {
 		logrus.Infof("CSI Proxy will be enabled as a Windows service.")
-		csi, err := csiproxy.New(cfg.CSIProxy)
+		csi, err := csiproxy.New(cfg.CSIProxy, cfg.TLSConfig)
 		if err != nil {
 			return err
 		}

cmd/server/config/config.go

@@ -10,7 +10,7 @@ import (
 	"github.com/rancher/system-agent/pkg/config"
 	"github.com/rancher/wins/pkg/csiproxy"
 	"github.com/rancher/wins/pkg/defaults"
-	"github.com/rancher/wins/pkg/tls"
+	wintls "github.com/rancher/wins/pkg/tls"
 )
 
 func DefaultConfig() *Config {
@@ -25,9 +25,6 @@ func DefaultConfig() *Config {
 			Mode:         "watching",
 			WatchingPath: defaults.UpgradeWatchingPath,
 		},
-		TLSConfig: &tls.Config{
-			CertFilePath: "",
-		},
 	}
 }
 
@@ -39,24 +36,7 @@ type Config struct {
 	Upgrade     UpgradeConfig       `yaml:"upgrade" json:"upgrade"`
 	SystemAgent *config.AgentConfig `yaml:"systemagent" json:"systemagent"`
 	CSIProxy    *csiproxy.Config    `yaml:"csi-proxy" json:"csi-proxy"`
-	TLSConfig   *tls.Config         `yaml:"tls-config" json:"tls-config"`
-}
-
-func (c *Config) ValidateTLSConfig() error {
-	if b, err := ioutil.ReadFile(c.TLSConfig.CertFilePath); b == nil || err != nil {
-		return errors.Wrapf(err, "failed to read certificate from %s", c.TLSConfig.CertFilePath)
-	}
-
-	if c.TLSConfig.CertFilePath != defaults.CertPath {
-		// load non-default certificate file
-		_ = csiproxy.Config{
-			Config: tls.Config{
-				CertFilePath: c.TLSConfig.CertFilePath,
-			},
-		}
-	}
-
-	return nil
+	TLSConfig   *wintls.Config      `yaml:"tls-config" json:"tls-config"`
 }
 
 func (c *Config) Validate() error {
@@ -74,12 +54,6 @@ func (c *Config) Validate() error {
 		return errors.Wrap(err, "[Validate] failed to validate upgrade field")
 	}
 
-	// validate csiProxy config
-	if c.TLSConfig.CertFilePath != "" {
-		if err := c.ValidateTLSConfig(); err != nil {
-			return errors.Wrap(err, "[Validate] failed to validate tls-config field")
-		}
-	}
 	return nil
 }
 

pkg/csiproxy/csi.go

@@ -11,6 +11,8 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/sirupsen/logrus"
+
 	"github.com/pkg/errors"
 	"github.com/rancher/wins/pkg/concierge"
 	winstls "github.com/rancher/wins/pkg/tls"
@@ -26,7 +28,6 @@ type Config struct {
 	URL         string `yaml:"url" json:"url"`
 	Version     string `yaml:"version" json:"version"`
 	KubeletPath string `yaml:"kubeletPath" json:"kubeletPath"`
-	winstls.Config
 }
 
 // Validate ensures that the configuration for CSI Proxy is correct if provided.
@@ -48,14 +49,15 @@ func (c *Config) validate() error {
 // Proxy is for creating and retrieving the Windows Service
 type Proxy struct {
 	cfg         *Config
+	tlsCfg      *winstls.Config
 	serviceName string
 	binaryName  string
 	binaryPath  string
 	concierge   *concierge.Concierge
 }
 
 // New creates a new Proxy struct
-func New(cfg *Config) (*Proxy, error) {
+func New(cfg *Config, tlsCfg *winstls.Config) (*Proxy, error) {
 	if err := cfg.validate(); err != nil {
 		return nil, err
 	}
@@ -79,6 +81,7 @@ func New(cfg *Config) (*Proxy, error) {
 
 	return &Proxy{
 		cfg:         cfg,
+		tlsCfg:      tlsCfg,
 		serviceName: serviceName,
 		binaryName:  exeName,
 		binaryPath:  filepath.Join(cwd, exeName),
@@ -92,16 +95,19 @@ func (p *Proxy) Enable() error {
 		return err
 	}
 	if !ok {
-		if p.cfg.CertFilePath != "" && !*p.cfg.Insecure {
+		if p.tlsCfg != nil && p.tlsCfg.CertFilePath != "" {
 			// CSI Proxy does not need the certpool that is returned
-			_, err := winstls.SetupGenericTLSConfigFromFile()
+			_, err := p.tlsCfg.SetupGenericTLSConfigFromFile()
 			if err != nil {
+
 				return err
 			}
 		}
+		logrus.Infof("CSI Proxy is being downloaded.")
 		if err := p.download(); err != nil {
 			return err
 		}
+		logrus.Infof("CSI Proxy is being started.")
 		if err := p.concierge.Enable(); err != nil {
 			return err
 		}
@@ -130,7 +136,7 @@ func (p *Proxy) download() error {
 	// default to insecure which matches system-agent functionality
 	transport := &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: true}}
 
-	if !*p.cfg.Insecure && p.cfg.CertFilePath != "" {
+	if p.tlsCfg != nil && !*p.tlsCfg.Insecure && p.tlsCfg.CertFilePath != "" {
 		transport.TLSClientConfig.InsecureSkipVerify = false
 	}
 
@@ -172,6 +178,5 @@ func (p *Proxy) download() error {
 			}
 		}
 	}
-
 	return nil
 }

pkg/tls/tls.go

@@ -15,9 +15,9 @@ type Config struct {
 }
 
 // SetupGenericTLSConfigFromFile returns a x509 system certificate pool containing the specified certificate file
-func SetupGenericTLSConfigFromFile() (*x509.CertPool, error) {
-	var config *Config
-	if config.CertFilePath == "" {
+func (c *Config) SetupGenericTLSConfigFromFile() (*x509.CertPool, error) {
+	logrus.Infof("Configuring TLS as insecure: %v and using the following cert: %s", c.Insecure, c.CertFilePath)
+	if c.CertFilePath == "" {
 		logrus.Info("[SetupGenericTLSConfigFromFile] specified certificate file path is empty, not modifying system certificate store")
 		return nil, nil
 	}
@@ -29,21 +29,21 @@ func SetupGenericTLSConfigFromFile() (*x509.CertPool, error) {
 		systemCerts = x509.NewCertPool()
 	}
 
-	localCertPath, err := os.Stat(config.CertFilePath)
+	localCertPath, err := os.Stat(c.CertFilePath)
 	if err != nil {
-		return nil, fmt.Errorf("[SetupGenericTLSConfigFromFile] unable to read certificate %s from %s: %v", localCertPath.Name(), config.CertFilePath, err)
+		return nil, fmt.Errorf("[SetupGenericTLSConfigFromFile] unable to read certificate %s from %s: %v", localCertPath.Name(), c.CertFilePath, err)
 	}
 
-	certs, err := ioutil.ReadFile(config.CertFilePath)
+	certs, err := ioutil.ReadFile(c.CertFilePath)
 	if err != nil {
-		return nil, fmt.Errorf("[SetupGenericTLSConfigFromFile] failed to read local cert file %q: %v", config.CertFilePath, err)
+		return nil, fmt.Errorf("[SetupGenericTLSConfigFromFile] failed to read local cert file %q: %v", c.CertFilePath, err)
 	}
 
 	// Append the specified cert to the system pool
 	if ok := systemCerts.AppendCertsFromPEM(certs); !ok {
-		return nil, fmt.Errorf("[SetupGenericTLSConfigFromFile] unable to append cert %s to store, using system certs only", config.CertFilePath)
+		return nil, fmt.Errorf("[SetupGenericTLSConfigFromFile] unable to append cert %s to store, using system certs only", c.CertFilePath)
 	}
 
-	logrus.Infof("[SetupGenericTLSConfigFromFile] successfully loaded %s certificate into system cert store", config.CertFilePath)
+	logrus.Infof("[SetupGenericTLSConfigFromFile] successfully loaded %s certificate into system cert store", c.CertFilePath)
 	return systemCerts, nil
 }