From 0ed6751456bc51abc8b2a78370ffd2e14ccd4702 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Sat, 10 Oct 2015 01:04:02 -0400 Subject: [PATCH] Simplify TLSConfigFor, honor Insecure with client certs --- pkg/client/unversioned/transport.go | 60 +++++++----------------- pkg/client/unversioned/transport_test.go | 7 --- 2 files changed, 17 insertions(+), 50 deletions(-) diff --git a/pkg/client/unversioned/transport.go b/pkg/client/unversioned/transport.go index ecb73dc1e7a..e3131e1a4d4 100644 --- a/pkg/client/unversioned/transport.go +++ b/pkg/client/unversioned/transport.go @@ -90,28 +90,32 @@ func TLSConfigFor(config *Config) (*tls.Config, error) { hasCA := len(config.CAFile) > 0 || len(config.CAData) > 0 hasCert := len(config.CertFile) > 0 || len(config.CertData) > 0 + if !hasCA && !hasCert && !config.Insecure { + return nil, nil + } if hasCA && config.Insecure { return nil, fmt.Errorf("specifying a root certificates file with the insecure flag is not allowed") } if err := LoadTLSFiles(config); err != nil { return nil, err } - var tlsConfig *tls.Config - switch { - case hasCert: - cfg, err := NewClientCertTLSConfig(config.CertData, config.KeyData, config.CAData) + + tlsConfig := &tls.Config{ + // Change default from SSLv3 to TLSv1.0 (because of POODLE vulnerability) + MinVersion: tls.VersionTLS10, + InsecureSkipVerify: config.Insecure, + } + + if hasCA { + tlsConfig.RootCAs = rootCertPool(config.CAData) + } + + if hasCert { + cert, err := tls.X509KeyPair(config.CertData, config.KeyData) if err != nil { return nil, err } - tlsConfig = cfg - case hasCA: - cfg, err := NewTLSConfig(config.CAData) - if err != nil { - return nil, err - } - tlsConfig = cfg - case config.Insecure: - tlsConfig = NewUnsafeTLSConfig() + tlsConfig.Certificates = []tls.Certificate{cert} } return tlsConfig, nil @@ -166,30 +170,6 @@ func dataFromSliceOrFile(data []byte, file string) ([]byte, error) { return nil, nil } -func NewClientCertTLSConfig(certData, keyData, caData []byte) (*tls.Config, error) { - cert, err := tls.X509KeyPair(certData, keyData) - if err != nil { - return nil, err - } - - return &tls.Config{ - // Change default from SSLv3 to TLSv1.0 (because of POODLE vulnerability) - MinVersion: tls.VersionTLS10, - Certificates: []tls.Certificate{ - cert, - }, - RootCAs: rootCertPool(caData), - }, nil -} - -func NewTLSConfig(caData []byte) (*tls.Config, error) { - return &tls.Config{ - // Change default from SSLv3 to TLSv1.0 (because of POODLE vulnerability) - MinVersion: tls.VersionTLS10, - RootCAs: rootCertPool(caData), - }, nil -} - // rootCertPool returns nil if caData is empty. When passed along, this will mean "use system CAs". // When caData is not empty, it will be the ONLY information used in the CertPool. func rootCertPool(caData []byte) *x509.CertPool { @@ -206,12 +186,6 @@ func rootCertPool(caData []byte) *x509.CertPool { return certPool } -func NewUnsafeTLSConfig() *tls.Config { - return &tls.Config{ - InsecureSkipVerify: true, - } -} - // cloneRequest returns a clone of the provided *http.Request. // The clone is a shallow copy of the struct and its Header map. func cloneRequest(r *http.Request) *http.Request { diff --git a/pkg/client/unversioned/transport_test.go b/pkg/client/unversioned/transport_test.go index baa09c3e04b..8ba609110f4 100644 --- a/pkg/client/unversioned/transport_test.go +++ b/pkg/client/unversioned/transport_test.go @@ -24,13 +24,6 @@ import ( "k8s.io/kubernetes/pkg/api/testapi" ) -func TestUnsecuredTLSTransport(t *testing.T) { - cfg := NewUnsafeTLSConfig() - if !cfg.InsecureSkipVerify { - t.Errorf("expected config to be insecure") - } -} - type testRoundTripper struct { Request *http.Request Response *http.Response