From d072232d4a6352459aabb374cf7e78600e9e37e1 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Thu, 29 Jan 2015 17:43:09 -0500 Subject: [PATCH] Allow client.Config to be used for HTTP2 and WebSocket connections client.Config describes how to make a client connection to a server for HTTP traffic, but for connection upgrade scenarios cannot be used because the underlying http.Transport object can't allow the connection to be hijacked. Reorganize the TLS and connection wrapper methods so that a sophisticated client can do: cfg := &client.Config{...} // from somewhere tlsConfig, _ := client.TLSConfigFor(cfg) _ := conn.Dial(...) rt := MyRoundTripper() // some func that implements grabbing requests wrapper, _ := client.HTTPWrappersFor(cfg) req := &http.Request{} req.Header.Set("Connection-Upgrade", ...) _, := wrapper.RoundTrip(req) // rt has been invoked with a fully formed Req with auth rt.Req.Write(conn) // read response for upgrade It would be good to have utility function that does more of this, but mostly enabling the HTTP2/SPDY client exec function right now. --- pkg/client/helper.go | 110 +++++++++++++++-------------------- pkg/client/helper_test.go | 60 ++++++++++++------- pkg/client/kubelet.go | 44 +++++--------- pkg/client/kubelet_test.go | 20 ++++--- pkg/client/transport.go | 86 +++++++++++++++++++-------- pkg/client/transport_test.go | 6 +- 6 files changed, 177 insertions(+), 149 deletions(-) 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 7bbc396d232..32e50bd1056 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 a4ceda47f61..563f04ef4b7 100644 --- a/pkg/client/kubelet_test.go +++ b/pkg/client/kubelet_test.go @@ -147,9 +147,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) @@ -165,11 +167,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") } }