diff --git a/pkg/client/helper.go b/pkg/client/helper.go index f1939f10eb4..a0689073586 100644 --- a/pkg/client/helper.go +++ b/pkg/client/helper.go @@ -61,22 +61,8 @@ type Config struct { // TODO: demonstrate an OAuth2 compatible client. BearerToken string - // Server requires TLS client certificate authentication - CertFile string - // Server requires TLS client certificate authentication - KeyFile string - // Trusted root certificates for server - CAFile string - - // CertData holds PEM-encoded bytes (typically read from a client certificate file). - // CertData takes precedence over CertFile - CertData []byte - // KeyData holds PEM-encoded bytes (typically read from a client certificate key file). - // KeyData takes precedence over KeyFile - KeyData []byte - // CAData holds PEM-encoded bytes (typically read from a root certificates bundle). - // CAData takes precedence over CAFile - CAData []byte + // TLSClientConfig contains settings to enable transport layer security + TLSClientConfig // Server should be accessed without verifying the TLS // certificate. For testing only. @@ -92,11 +78,17 @@ type KubeletConfig struct { Port uint EnableHttps bool - // TLS Configuration, only applies if EnableHttps is true. + // TLSClientConfig contains settings to enable transport layer security + TLSClientConfig +} + +// TLSClientConfig contains settings to enable transport layer security +type TLSClientConfig struct { + // Server requires TLS client certificate authentication CertFile string - // TLS Configuration, only applies if EnableHttps is true. + // Server requires TLS client certificate authentication KeyFile string - // TLS Configuration, only applies if EnableHttps is true. + // Trusted root certificates for server CAFile string // CertData holds PEM-encoded bytes (typically read from a client certificate file). @@ -215,47 +207,40 @@ func TransportFor(config *Config) (http.RoundTripper, error) { if config.Transport != nil && (hasCA || hasCert || config.Insecure) { return nil, fmt.Errorf("using a custom transport with TLS certificate options or the insecure flag is not allowed") } - if hasCA && config.Insecure { - return nil, fmt.Errorf("specifying a root certificates file with the insecure flag is not allowed") - } - var transport http.RoundTripper - switch { - case config.Transport != nil: - transport = config.Transport - case hasCert: - var ( - certData, keyData, caData []byte - err error - ) - if certData, err = dataFromSliceOrFile(config.CertData, config.CertFile); err != nil { - return nil, err - } - if keyData, err = dataFromSliceOrFile(config.KeyData, config.KeyFile); err != nil { - return nil, err - } - if caData, err = dataFromSliceOrFile(config.CAData, config.CAFile); err != nil { - return nil, err - } - if transport, err = NewClientCertTLSTransport(certData, keyData, caData); err != nil { - return nil, err - } - case hasCA: - var ( - caData []byte - err error - ) - if caData, err = dataFromSliceOrFile(config.CAData, config.CAFile); err != nil { - return nil, err - } - if transport, err = NewTLSTransport(caData); err != nil { - return nil, err - } - case config.Insecure: - transport = NewUnsafeTLSTransport() - default: - transport = http.DefaultTransport + + tlsConfig, err := TLSConfigFor(config) + if err != nil { + return nil, err } + var transport http.RoundTripper + if config.Transport != nil { + transport = config.Transport + } else { + if tlsConfig != nil { + transport = &http.Transport{ + TLSClientConfig: tlsConfig, + } + } else { + transport = http.DefaultTransport + } + } + + transport, err = HTTPWrappersForConfig(config, transport) + if err != nil { + return nil, err + } + + // TODO: use the config context to wrap a transport + + return transport, nil +} + +// HTTPWrappersForConfig wraps a round tripper with any relevant layered behavior from the +// config. Exposed to allow more clients that need HTTP-like behavior but then must hijack +// the underlying connection (like WebSocket or HTTP2 clients). Pure HTTP clients should use +// the higher level TransportFor or RESTClientFor methods. +func HTTPWrappersForConfig(config *Config, rt http.RoundTripper) (http.RoundTripper, error) { // Set authentication wrappers hasBasicAuth := config.Username != "" || config.Password != "" if hasBasicAuth && config.BearerToken != "" { @@ -263,14 +248,11 @@ func TransportFor(config *Config) (http.RoundTripper, error) { } switch { case config.BearerToken != "": - transport = NewBearerAuthRoundTripper(config.BearerToken, transport) + rt = NewBearerAuthRoundTripper(config.BearerToken, rt) case hasBasicAuth: - transport = NewBasicAuthRoundTripper(config.Username, config.Password, transport) + rt = NewBasicAuthRoundTripper(config.Username, config.Password, rt) } - - // TODO: use the config context to wrap a transport - - return transport, nil + return rt, nil } // dataFromSliceOrFile returns data from the slice (if non-empty), or from the file, diff --git a/pkg/client/helper_test.go b/pkg/client/helper_test.go index 6eeaaf185da..c15e961a51b 100644 --- a/pkg/client/helper_test.go +++ b/pkg/client/helper_test.go @@ -104,54 +104,68 @@ func TestTransportFor(t *testing.T) { "ca transport": { TLS: true, Config: &Config{ - CAData: []byte(rootCACert), + TLSClientConfig: TLSClientConfig{ + CAData: []byte(rootCACert), + }, }, }, "bad ca file transport": { Err: true, Config: &Config{ - CAFile: "invalid file", + TLSClientConfig: TLSClientConfig{ + CAFile: "invalid file", + }, }, }, "ca data overriding bad ca file transport": { TLS: true, Config: &Config{ - CAData: []byte(rootCACert), - CAFile: "invalid file", + TLSClientConfig: TLSClientConfig{ + CAData: []byte(rootCACert), + CAFile: "invalid file", + }, }, }, "cert transport": { TLS: true, Config: &Config{ - CertData: []byte(certData), - KeyData: []byte(keyData), - CAData: []byte(rootCACert), + TLSClientConfig: TLSClientConfig{ + CertData: []byte(certData), + KeyData: []byte(keyData), + CAData: []byte(rootCACert), + }, }, }, "bad cert data transport": { Err: true, Config: &Config{ - CertData: []byte(certData), - KeyData: []byte("bad key data"), - CAData: []byte(rootCACert), + TLSClientConfig: TLSClientConfig{ + CertData: []byte(certData), + KeyData: []byte("bad key data"), + CAData: []byte(rootCACert), + }, }, }, "bad file cert transport": { Err: true, Config: &Config{ - CertData: []byte(certData), - KeyFile: "invalid file", - CAData: []byte(rootCACert), + TLSClientConfig: TLSClientConfig{ + CertData: []byte(certData), + KeyFile: "invalid file", + CAData: []byte(rootCACert), + }, }, }, "key data overriding bad file cert transport": { TLS: true, Config: &Config{ - CertData: []byte(certData), - KeyData: []byte(keyData), - KeyFile: "invalid file", - CAData: []byte(rootCACert), + TLSClientConfig: TLSClientConfig{ + CertData: []byte(certData), + KeyData: []byte(keyData), + KeyFile: "invalid file", + CAData: []byte(rootCACert), + }, }, }, } @@ -206,15 +220,19 @@ func TestIsConfigTransportTLS(t *testing.T) { }, { Config: &Config{ - Host: "localhost", - CertFile: "foo", + Host: "localhost", + TLSClientConfig: TLSClientConfig{ + CertFile: "foo", + }, }, TransportTLS: true, }, { Config: &Config{ - Host: "///:://localhost", - CertFile: "foo", + Host: "///:://localhost", + TLSClientConfig: TLSClientConfig{ + CertFile: "foo", + }, }, TransportTLS: false, }, diff --git a/pkg/client/kubelet.go b/pkg/client/kubelet.go index c8eea30937f..f5754bb91b9 100644 --- a/pkg/client/kubelet.go +++ b/pkg/client/kubelet.go @@ -59,41 +59,25 @@ type HTTPKubeletClient struct { EnableHttps bool } +// TODO: this structure is questionable, it should be using client.Config and overriding defaults. func NewKubeletClient(config *KubeletConfig) (KubeletClient, error) { transport := http.DefaultTransport - hasCA := len(config.CAFile) > 0 || len(config.CAData) > 0 - hasCert := len(config.CertFile) > 0 || len(config.CertData) > 0 - if hasCert { - var ( - certData, keyData, caData []byte - err error - ) - if certData, err = dataFromSliceOrFile(config.CertData, config.CertFile); err != nil { - return nil, err - } - if keyData, err = dataFromSliceOrFile(config.KeyData, config.KeyFile); err != nil { - return nil, err - } - if caData, err = dataFromSliceOrFile(config.CAData, config.CAFile); err != nil { - return nil, err - } - if transport, err = NewClientCertTLSTransport(certData, keyData, caData); err != nil { - return nil, err - } - } else if hasCA { - var ( - caData []byte - err error - ) - if caData, err = dataFromSliceOrFile(config.CAData, config.CAFile); err != nil { - return nil, err - } - if transport, err = NewTLSTransport(caData); err != nil { - return nil, err + + tlsConfig, err := TLSConfigFor(&Config{ + TLSClientConfig: config.TLSClientConfig, + }) + if err != nil { + return nil, err + } + if tlsConfig != nil { + transport = &http.Transport{ + TLSClientConfig: tlsConfig, } } - c := &http.Client{Transport: transport} + c := &http.Client{ + Transport: transport, + } return &HTTPKubeletClient{ Client: c, Port: config.Port, diff --git a/pkg/client/kubelet_test.go b/pkg/client/kubelet_test.go index 7d4f720d0e8..41ad4b1ab31 100644 --- a/pkg/client/kubelet_test.go +++ b/pkg/client/kubelet_test.go @@ -187,9 +187,11 @@ func TestNewKubeletClientTLSInvalid(t *testing.T) { Port: 9000, EnableHttps: true, //Invalid certificate and key path - CertFile: "./testdata/mycertinvalid.cer", - KeyFile: "./testdata/mycertinvalid.key", - CAFile: "./testdata/myCA.cer", + TLSClientConfig: TLSClientConfig{ + CertFile: "./testdata/mycertinvalid.cer", + KeyFile: "./testdata/mycertinvalid.key", + CAFile: "./testdata/myCA.cer", + }, } client, err := NewKubeletClient(config) @@ -205,11 +207,13 @@ func TestNewKubeletClientTLSValid(t *testing.T) { config := &KubeletConfig{ Port: 9000, EnableHttps: true, - CertFile: "./testdata/mycertvalid.cer", - // TLS Configuration, only applies if EnableHttps is true. - KeyFile: "./testdata/mycertvalid.key", - // TLS Configuration, only applies if EnableHttps is true. - CAFile: "./testdata/myCA.cer", + TLSClientConfig: TLSClientConfig{ + CertFile: "./testdata/mycertvalid.cer", + // TLS Configuration, only applies if EnableHttps is true. + KeyFile: "./testdata/mycertvalid.key", + // TLS Configuration, only applies if EnableHttps is true. + CAFile: "./testdata/myCA.cer", + }, } client, err := NewKubeletClient(config) diff --git a/pkg/client/transport.go b/pkg/client/transport.go index 4d171bea5f2..750b0f42312 100644 --- a/pkg/client/transport.go +++ b/pkg/client/transport.go @@ -54,44 +54,84 @@ func (rt *bearerAuthRoundTripper) RoundTrip(req *http.Request) (*http.Response, return rt.rt.RoundTrip(req) } -func NewClientCertTLSTransport(certData, keyData, caData []byte) (*http.Transport, error) { +// TLSConfigFor returns a tls.Config that will provide the transport level security defined +// by the provided Config. Will return nil if no transport level security is requested. +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 && config.Insecure { + return nil, fmt.Errorf("specifying a root certificates file with the insecure flag is not allowed") + } + var tlsConfig *tls.Config + switch { + case hasCert: + certData, err := dataFromSliceOrFile(config.CertData, config.CertFile) + if err != nil { + return nil, err + } + keyData, err := dataFromSliceOrFile(config.KeyData, config.KeyFile) + if err != nil { + return nil, err + } + caData, err := dataFromSliceOrFile(config.CAData, config.CAFile) + if err != nil { + return nil, err + } + cfg, err := NewClientCertTLSConfig(certData, keyData, caData) + if err != nil { + return nil, err + } + tlsConfig = cfg + case hasCA: + caData, err := dataFromSliceOrFile(config.CAData, config.CAFile) + if err != nil { + return nil, err + } + cfg, err := NewTLSConfig(caData) + if err != nil { + return nil, err + } + tlsConfig = cfg + case config.Insecure: + tlsConfig = NewUnsafeTLSConfig() + } + + return tlsConfig, nil +} + +func NewClientCertTLSConfig(certData, keyData, caData []byte) (*tls.Config, error) { cert, err := tls.X509KeyPair(certData, keyData) if err != nil { return nil, err } certPool := x509.NewCertPool() certPool.AppendCertsFromPEM(caData) - return &http.Transport{ - TLSClientConfig: &tls.Config{ - // Change default from SSLv3 to TLSv1.0 (because of POODLE vulnerability) - MinVersion: tls.VersionTLS10, - Certificates: []tls.Certificate{ - cert, - }, - RootCAs: certPool, - ClientCAs: certPool, - ClientAuth: tls.RequireAndVerifyClientCert, + return &tls.Config{ + // Change default from SSLv3 to TLSv1.0 (because of POODLE vulnerability) + MinVersion: tls.VersionTLS10, + Certificates: []tls.Certificate{ + cert, }, + RootCAs: certPool, + ClientCAs: certPool, + ClientAuth: tls.RequireAndVerifyClientCert, }, nil } -func NewTLSTransport(caData []byte) (*http.Transport, error) { +func NewTLSConfig(caData []byte) (*tls.Config, error) { certPool := x509.NewCertPool() certPool.AppendCertsFromPEM(caData) - return &http.Transport{ - TLSClientConfig: &tls.Config{ - // Change default from SSLv3 to TLSv1.0 (because of POODLE vulnerability) - MinVersion: tls.VersionTLS10, - RootCAs: certPool, - }, + return &tls.Config{ + // Change default from SSLv3 to TLSv1.0 (because of POODLE vulnerability) + MinVersion: tls.VersionTLS10, + RootCAs: certPool, }, nil } -func NewUnsafeTLSTransport() *http.Transport { - return &http.Transport{ - TLSClientConfig: &tls.Config{ - InsecureSkipVerify: true, - }, +func NewUnsafeTLSConfig() *tls.Config { + return &tls.Config{ + InsecureSkipVerify: true, } } diff --git a/pkg/client/transport_test.go b/pkg/client/transport_test.go index f322b934ec7..f31445c0501 100644 --- a/pkg/client/transport_test.go +++ b/pkg/client/transport_test.go @@ -23,9 +23,9 @@ import ( ) func TestUnsecuredTLSTransport(t *testing.T) { - transport := NewUnsafeTLSTransport() - if !transport.TLSClientConfig.InsecureSkipVerify { - t.Errorf("expected transport to be insecure") + cfg := NewUnsafeTLSConfig() + if !cfg.InsecureSkipVerify { + t.Errorf("expected config to be insecure") } }