From e2eed967997ffa272d8e42ccb2fc26f09a32c265 Mon Sep 17 00:00:00 2001 From: Bob Aman Date: Fri, 14 Sep 2018 17:11:21 -0700 Subject: [PATCH] Configurable TLS verification of upstream servers --- internal/proxy/oauthproxy.go | 34 +++++-- internal/proxy/oauthproxy_test.go | 72 ++++++++++++++- internal/proxy/proxy_config.go | 5 +- internal/proxy/proxy_config_test.go | 134 +++++++++++++++++++++++++++- 4 files changed, 230 insertions(+), 15 deletions(-) diff --git a/internal/proxy/oauthproxy.go b/internal/proxy/oauthproxy.go index 668567c9..3c06c886 100755 --- a/internal/proxy/oauthproxy.go +++ b/internal/proxy/oauthproxy.go @@ -1,6 +1,7 @@ package proxy import ( + "crypto/tls" "encoding/json" "errors" "fmt" @@ -131,11 +132,26 @@ func (u *UpstreamProxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { // upstreamTransport is used to ensure that upstreams cannot override the // security headers applied by sso_proxy -type upstreamTransport struct{} +type upstreamTransport struct { + InsecureSkipVerify bool +} // RoundTrip round trips the request and deletes security headers before returning the response. func (t *upstreamTransport) RoundTrip(req *http.Request) (*http.Response, error) { - resp, err := http.DefaultTransport.RoundTrip(req) + transport := &http.Transport{ + Proxy: http.ProxyFromEnvironment, + DialContext: (&net.Dialer{ + Timeout: 30 * time.Second, + KeepAlive: 30 * time.Second, + DualStack: true, + }).DialContext, + MaxIdleConns: 100, + IdleConnTimeout: 90 * time.Second, + TLSHandshakeTimeout: 10 * time.Second, + TLSClientConfig: &tls.Config{InsecureSkipVerify: t.InsecureSkipVerify}, + ExpectContinueTimeout: 1 * time.Second, + } + resp, err := transport.RoundTrip(req) if err != nil { logger := log.NewLogEntry() logger.Error(err, "error in upstreamTransport RoundTrip") @@ -149,9 +165,9 @@ func (t *upstreamTransport) RoundTrip(req *http.Request) (*http.Response, error) // NewReverseProxy creates a reverse proxy to a specified url. // It adds an X-Forwarded-Host header that is the request's host. -func NewReverseProxy(to *url.URL) *httputil.ReverseProxy { +func NewReverseProxy(to *url.URL, config *UpstreamConfig) *httputil.ReverseProxy { proxy := httputil.NewSingleHostReverseProxy(to) - proxy.Transport = &upstreamTransport{} + proxy.Transport = &upstreamTransport{InsecureSkipVerify: config.TLSSkipVerify} director := proxy.Director proxy.Director = func(req *http.Request) { req.Header.Add("X-Forwarded-Host", req.Host) @@ -164,9 +180,9 @@ func NewReverseProxy(to *url.URL) *httputil.ReverseProxy { // NewRewriteReverseProxy creates a reverse proxy that is capable of creating upstream // urls on the fly based on a from regex and a templated to field. // It adds an X-Forwarded-Host header to the the upstream's request. -func NewRewriteReverseProxy(route *RewriteRoute) *httputil.ReverseProxy { +func NewRewriteReverseProxy(route *RewriteRoute, config *UpstreamConfig) *httputil.ReverseProxy { proxy := &httputil.ReverseProxy{} - proxy.Transport = &upstreamTransport{} + proxy.Transport = &upstreamTransport{InsecureSkipVerify: config.TLSSkipVerify} proxy.Director = func(req *http.Request) { // we do this to rewrite requests rewritten := route.FromRegex.ReplaceAllString(req.Host, route.ToTemplate.Opaque) @@ -296,15 +312,15 @@ func NewOAuthProxy(opts *Options, optFuncs ...func(*OAuthProxy) error) (*OAuthPr for _, upstreamConfig := range opts.upstreamConfigs { switch route := upstreamConfig.Route.(type) { case *SimpleRoute: - reverseProxy := NewReverseProxy(route.ToURL) + reverseProxy := NewReverseProxy(route.ToURL, upstreamConfig) handler, tags := NewReverseProxyHandler(reverseProxy, opts, upstreamConfig) p.Handle(route.FromURL.Host, handler, tags, upstreamConfig) case *RewriteRoute: - reverseProxy := NewRewriteReverseProxy(route) + reverseProxy := NewRewriteReverseProxy(route, upstreamConfig) handler, tags := NewReverseProxyHandler(reverseProxy, opts, upstreamConfig) p.HandleRegex(route.FromRegex, handler, tags, upstreamConfig) default: - return nil, fmt.Errorf("unkown route type") + return nil, fmt.Errorf("unknown route type") } } diff --git a/internal/proxy/oauthproxy_test.go b/internal/proxy/oauthproxy_test.go index dc4180a6..adab6027 100644 --- a/internal/proxy/oauthproxy_test.go +++ b/internal/proxy/oauthproxy_test.go @@ -77,7 +77,7 @@ func TestNewReverseProxy(t *testing.T) { backendHost := net.JoinHostPort(backendHostname, backendPort) proxyURL, _ := url.Parse(backendURL.Scheme + "://" + backendHost + "/") - proxyHandler := NewReverseProxy(proxyURL) + proxyHandler := NewReverseProxy(proxyURL, &UpstreamConfig{TLSSkipVerify: false}) frontend := httptest.NewServer(proxyHandler) defer frontend.Close() @@ -109,7 +109,7 @@ func TestNewRewriteReverseProxy(t *testing.T) { }, } - rewriteProxy := NewRewriteReverseProxy(route) + rewriteProxy := NewRewriteReverseProxy(route, &UpstreamConfig{TLSSkipVerify: false}) frontend := httptest.NewServer(rewriteProxy) defer frontend.Close() @@ -156,7 +156,7 @@ func TestNewReverseProxyHostname(t *testing.T) { t.Fatalf("expected to parse to url: %s", err) } - reverseProxy := NewReverseProxy(toURL) + reverseProxy := NewReverseProxy(toURL, &UpstreamConfig{TLSSkipVerify: false}) from := httptest.NewServer(reverseProxy) defer from.Close() @@ -201,6 +201,70 @@ func TestNewReverseProxyHostname(t *testing.T) { } +func TestNewReverseProxyTLSSkipVerify(t *testing.T) { + type respStruct struct { + HandshakeComplete bool `json:"handshake-complete"` + } + + testCases := []struct { + name string + skipVerify bool + expectedStatus int + }{ + { + name: "skip verify true", + skipVerify: true, + expectedStatus: 200, + }, + { + name: "skip verify false", + skipVerify: false, + expectedStatus: 502, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + to := httptest.NewTLSServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + body, err := json.Marshal( + // Doesn't really matter what's sent since we should 502 + &respStruct{ + HandshakeComplete: r.TLS.HandshakeComplete, + }, + ) + if err != nil { + t.Fatalf("expected to marshal json: %s", err) + } + rw.Write(body) + })) + defer to.Close() + + toURL, err := url.Parse(to.URL) + if err != nil { + t.Fatalf("expected to parse to url: %s", err) + } + + reverseProxy := NewReverseProxy(toURL, &UpstreamConfig{TLSSkipVerify: tc.skipVerify}) + from := httptest.NewServer(reverseProxy) + defer from.Close() + + res, err := http.Get(from.URL) + if err != nil { + t.Fatalf("expected to be able to make req: %s", err) + } + + if res.StatusCode != tc.expectedStatus { + t.Logf(" got status code: %v", res.StatusCode) + t.Logf("want status code: %d", tc.expectedStatus) + + t.Errorf("got unexpected response code for tls failure") + } + if res.Header.Get("Cookie") != "" { + t.Errorf("expected Cookie header to be empty but was %s", res.Header.Get("Cookie")) + } + }) + } +} + func TestDeleteSSOHeader(t *testing.T) { testCases := []struct { name string @@ -300,7 +364,7 @@ func TestEncodedSlashes(t *testing.T) { defer backend.Close() b, _ := url.Parse(backend.URL) - proxyHandler := NewReverseProxy(b) + proxyHandler := NewReverseProxy(b, &UpstreamConfig{TLSSkipVerify: false}) frontend := httptest.NewServer(proxyHandler) defer frontend.Close() diff --git a/internal/proxy/proxy_config.go b/internal/proxy/proxy_config.go index 9eaeb64b..0842f386 100644 --- a/internal/proxy/proxy_config.go +++ b/internal/proxy/proxy_config.go @@ -52,6 +52,7 @@ type UpstreamConfig struct { SkipAuthCompiledRegex []*regexp.Regexp AllowedGroups []string + TLSSkipVerify bool HMACAuth hmacauth.HmacAuth Timeout time.Duration FlushInterval time.Duration @@ -79,6 +80,7 @@ type OptionsConfig struct { HeaderOverrides map[string]string `yaml:"header_overrides"` SkipAuthRegex []string `yaml:"skip_auth_regex"` AllowedGroups []string `yaml:"allowed_groups"` + TLSSkipVerify bool `yaml:"tls_skip_verify"` Timeout time.Duration `yaml:"timeout"` FlushInterval time.Duration `yaml:"flush_interval"` } @@ -109,7 +111,7 @@ func loadServiceConfigs(raw []byte, cluster, scheme string, configVars map[strin // we don't set this to the len(serviceConfig) since not all service configs // are configured for all clusters, leaving nil tail pointers in the slice. configs := make([]*UpstreamConfig, 0) - // resovle overrides + // resolve overrides for _, service := range serviceConfigs { proxy, err := resolveUpstreamConfig(service, cluster) if err != nil { @@ -362,6 +364,7 @@ func parseOptionsConfig(proxy *UpstreamConfig) error { proxy.Timeout = proxy.RouteConfig.Options.Timeout proxy.FlushInterval = proxy.RouteConfig.Options.FlushInterval proxy.HeaderOverrides = proxy.RouteConfig.Options.HeaderOverrides + proxy.TLSSkipVerify = proxy.RouteConfig.Options.TLSSkipVerify proxy.RouteConfig.Options = nil diff --git a/internal/proxy/proxy_config_test.go b/internal/proxy/proxy_config_test.go index 98cc5d09..506b20f1 100644 --- a/internal/proxy/proxy_config_test.go +++ b/internal/proxy/proxy_config_test.go @@ -471,6 +471,7 @@ func TestUpstreamConfigRoutes(t *testing.T) { Host: "bar-internal.sso.dev", }, }, + TLSSkipVerify: false, }, }, }, @@ -498,6 +499,129 @@ func TestUpstreamConfigRoutes(t *testing.T) { Opaque: "bar--$1.sso.dev", }, }, + TLSSkipVerify: false, + }, + }, + }, + { + name: "handle default route w/ explicit tls_verify: true", + rawConfig: []byte(` +- service: bar + default: + from: bar.{{cluster}}.{{root_domain}} + to: bar-internal.{{cluster}}.{{root_domain}} + options: + tls_verify: true +`), + wantConfigs: []*UpstreamConfig{ + { + Service: "bar", + RouteConfig: RouteConfig{ + From: "bar.sso.dev", + To: "bar-internal.sso.dev", + }, + Route: &SimpleRoute{ + FromURL: &url.URL{ + Scheme: "http", + Host: "bar.sso.dev", + }, + ToURL: &url.URL{ + Scheme: "http", + Host: "bar-internal.sso.dev", + }, + }, + TLSSkipVerify: false, + }, + }, + }, + { + name: "handle rewrite route w/ explicit tls_verify: true", + rawConfig: []byte(` +- service: bar + default: + from: ^bar--(.*).{{cluster}}.{{root_domain}}$ + to: bar--$1.{{cluster}}.{{root_domain}} + type: rewrite + options: + tls_verify: true +`), + wantConfigs: []*UpstreamConfig{ + { + Service: "bar", + RouteConfig: RouteConfig{ + From: "^bar--(.*).sso.dev$", + To: "bar--$1.sso.dev", + Type: "rewrite", + }, + Route: &RewriteRoute{ + FromRegex: regexp.MustCompile("^bar--(.*).sso.dev$"), + ToTemplate: &url.URL{ + Scheme: "http", + Opaque: "bar--$1.sso.dev", + }, + }, + TLSSkipVerify: false, + }, + }, + }, + { + name: "handle default route with explicit tls_verify false", + rawConfig: []byte(` +- service: bar + default: + from: bar.{{cluster}}.{{root_domain}} + to: bar-internal.{{cluster}}.{{root_domain}} + options: + tls_skip_verify: true +`), + wantConfigs: []*UpstreamConfig{ + { + Service: "bar", + RouteConfig: RouteConfig{ + From: "bar.sso.dev", + To: "bar-internal.sso.dev", + }, + Route: &SimpleRoute{ + FromURL: &url.URL{ + Scheme: "http", + Host: "bar.sso.dev", + }, + ToURL: &url.URL{ + Scheme: "http", + Host: "bar-internal.sso.dev", + }, + }, + TLSSkipVerify: true, + }, + }, + }, + { + name: "handle rewrite route with explicit tls_verify false", + rawConfig: []byte(` +- service: bar + default: + from: ^bar--(.*).{{cluster}}.{{root_domain}}$ + to: bar--$1.{{cluster}}.{{root_domain}} + type: rewrite + options: + tls_skip_verify: true +`), + wantConfigs: []*UpstreamConfig{ + { + Service: "bar", + RouteConfig: RouteConfig{ + From: "^bar--(.*).sso.dev$", + To: "bar--$1.sso.dev", + Type: "rewrite", + }, + Route: &RewriteRoute{ + FromRegex: regexp.MustCompile("^bar--(.*).sso.dev$"), + ToTemplate: &url.URL{ + Scheme: "http", + Opaque: "bar--$1.sso.dev", + }, + }, + TLSSkipVerify: true, }, }, }, @@ -529,7 +653,7 @@ func TestUpstreamConfigRoutes(t *testing.T) { for i, gotConfig := range gotConfigs { t.Logf("got %#v", gotConfig) t.Logf("want %#v", tc.wantConfigs[i]) - t.Fatalf("expectec configs to be equal") + t.Fatalf("expected configs to be equal") } } }) @@ -572,6 +696,7 @@ func TestUpstreamConfigExtraRoutes(t *testing.T) { Host: "bar-internal.sso.dev", }, }, + TLSSkipVerify: false, }, { Service: "bar", @@ -589,6 +714,7 @@ func TestUpstreamConfigExtraRoutes(t *testing.T) { Host: "foo-internal.sso.dev", }, }, + TLSSkipVerify: false, }, }, }, @@ -621,6 +747,7 @@ func TestUpstreamConfigExtraRoutes(t *testing.T) { Host: "bar-internal.sso.dev", }, }, + TLSSkipVerify: false, }, { Service: "bar", @@ -636,6 +763,7 @@ func TestUpstreamConfigExtraRoutes(t *testing.T) { Opaque: "bar--$1.sso.dev", }, }, + TLSSkipVerify: false, }, }, }, @@ -681,6 +809,7 @@ func TestUpstreamConfigExtraRoutes(t *testing.T) { HeaderOverrides: map[string]string{ "X-Frame-Options": "DENY", }, + TLSSkipVerify: false, }, { Service: "bar", @@ -703,6 +832,7 @@ func TestUpstreamConfigExtraRoutes(t *testing.T) { HeaderOverrides: map[string]string{ "X-Frame-Options": "DENY", }, + TLSSkipVerify: false, }, }, }, @@ -752,6 +882,7 @@ func TestUpstreamConfigExtraRoutes(t *testing.T) { HeaderOverrides: map[string]string{ "X-Frame-Options": "DENY", }, + TLSSkipVerify: false, }, { Service: "bar", @@ -774,6 +905,7 @@ func TestUpstreamConfigExtraRoutes(t *testing.T) { HeaderOverrides: map[string]string{ "X-Frame-Options": "ALLOW", }, + TLSSkipVerify: false, }, }, },