From 8c2beac6f1e535c7e190f0ffe03cf5f01d258f1a Mon Sep 17 00:00:00 2001 From: Travis Groth Date: Thu, 30 May 2019 08:48:56 -0400 Subject: [PATCH] Add automatic configuration reloading and policy handling --- authorize/authorize.go | 9 ++++++ authorize/authorize_test.go | 53 ++++++++++++++++++++++++++++--- cmd/pomerium/main.go | 55 ++++++++++++++++++++++++++++++--- cmd/pomerium/main_test.go | 42 +++++++++++++++++++++++++ docs/reference/readme.md | 2 ++ go.mod | 2 ++ go.sum | 2 ++ internal/config/options.go | 40 ++++++++++++++++++------ internal/config/options_test.go | 20 ++++++++++++ proxy/handlers.go | 8 ----- proxy/proxy.go | 40 +++++++++++++++++++----- proxy/proxy_test.go | 48 ++++++++++++++++++++++++++++ 12 files changed, 287 insertions(+), 34 deletions(-) diff --git a/authorize/authorize.go b/authorize/authorize.go index e53d16c27..f276458e5 100644 --- a/authorize/authorize.go +++ b/authorize/authorize.go @@ -5,6 +5,8 @@ import ( "errors" "fmt" + "github.com/pomerium/pomerium/internal/log" + "github.com/pomerium/pomerium/internal/config" "github.com/pomerium/pomerium/internal/policy" @@ -63,3 +65,10 @@ func NewIdentityWhitelist(policies []policy.Policy, admins []string) IdentityVal func (a *Authorize) ValidIdentity(route string, identity *Identity) bool { return a.identityAccess.Valid(route, identity) } + +// UpdateOptions updates internal structres based on config.Options +func (a *Authorize) UpdateOptions(o *config.Options) error { + log.Info().Msg("authorize: updating options") + a.identityAccess = NewIdentityWhitelist(o.Policies, o.Administrators) + return nil +} diff --git a/authorize/authorize_test.go b/authorize/authorize_test.go index 72a5ec89a..71be228e5 100644 --- a/authorize/authorize_test.go +++ b/authorize/authorize_test.go @@ -10,11 +10,7 @@ import ( func TestNew(t *testing.T) { t.Parallel() - goodPolicy := policy.Policy{From: "pomerium.io", To: "httpbin.org"} - goodPolicy.Validate() - policies := []policy.Policy{ - goodPolicy, - } + policies := testPolicies() tests := []struct { name string @@ -46,3 +42,50 @@ func TestNew(t *testing.T) { }) } } + +func testPolicies() []policy.Policy { + testPolicy := policy.Policy{From: "pomerium.io", To: "httpbin.org", AllowedEmails: []string{"test@gmail.com"}} + testPolicy.Validate() + policies := []policy.Policy{ + testPolicy, + } + + return policies +} + +func Test_UpdateOptions(t *testing.T) { + t.Parallel() + policies := testPolicies() + newPolicy := policy.Policy{From: "foo.notatld", To: "bar.notatld", AllowedEmails: []string{"test@gmail.com"}} + newPolicy.Validate() + newPolicies := []policy.Policy{ + newPolicy, + } + identity := &Identity{Email: "test@gmail.com"} + tests := []struct { + name string + SharedKey string + Policies []policy.Policy + newPolices []policy.Policy + route string + wantAllowed bool + }{ + {"good", "gXK6ggrlIW2HyKyUF9rUO4azrDgxhDPWqw9y+lJU7B8=", policies, policies, "pomerium.io", true}, + {"changed", "gXK6ggrlIW2HyKyUF9rUO4azrDgxhDPWqw9y+lJU7B8=", policies, newPolicies, "foo.notatld", true}, + {"changed and missing", "gXK6ggrlIW2HyKyUF9rUO4azrDgxhDPWqw9y+lJU7B8=", policies, newPolicies, "pomerium.io", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + o := &config.Options{SharedKey: tt.SharedKey, Policies: tt.Policies} + authorize, _ := New(o) + o.Policies = tt.newPolices + authorize.UpdateOptions(o) + + allowed := authorize.ValidIdentity(tt.route, identity) + if allowed != tt.wantAllowed { + t.Errorf("New() allowed = %v, wantAllowed %v", allowed, tt.wantAllowed) + return + } + }) + } +} diff --git a/cmd/pomerium/main.go b/cmd/pomerium/main.go index d85b0c924..e81962a0c 100644 --- a/cmd/pomerium/main.go +++ b/cmd/pomerium/main.go @@ -8,8 +8,7 @@ import ( "os" "time" - "google.golang.org/grpc" - + "github.com/fsnotify/fsnotify" "github.com/pomerium/pomerium/authenticate" "github.com/pomerium/pomerium/authorize" "github.com/pomerium/pomerium/internal/config" @@ -21,6 +20,8 @@ import ( pbAuthenticate "github.com/pomerium/pomerium/proto/authenticate" pbAuthorize "github.com/pomerium/pomerium/proto/authorize" "github.com/pomerium/pomerium/proxy" + "github.com/spf13/viper" + "google.golang.org/grpc" ) var versionFlag = flag.Bool("version", false, "prints the version") @@ -48,15 +49,24 @@ func main() { log.Fatal().Err(err).Msg("cmd/pomerium: authenticate") } - _, err = newAuthorizeService(opt, grpcServer) + authz, err := newAuthorizeService(opt, grpcServer) if err != nil { log.Fatal().Err(err).Msg("cmd/pomerium: authorize") } - _, err = newProxyService(opt, mux) + proxy, err := newProxyService(opt, mux) if err != nil { log.Fatal().Err(err).Msg("cmd/pomerium: proxy") } + go viper.WatchConfig() + + viper.OnConfigChange(func(e fsnotify.Event) { + log.Info(). + Str("file", e.Name). + Msg("cmd/pomerium: configuration file changed") + + opt = handleConfigUpdate(opt, []config.OptionsUpdater{authz, proxy}) + }) // defer statements ignored anyway : https://stackoverflow.com/a/17888654 // defer proxyService.AuthenticateClient.Close() // defer proxyService.AuthorizeClient.Close() @@ -181,3 +191,40 @@ func parseOptions(configFile string) (*config.Options, error) { } return o, nil } + +func handleConfigUpdate(opt *config.Options, services []config.OptionsUpdater) *config.Options { + newOpt, err := parseOptions(*configFile) + optChecksum := opt.Checksum() + newOptChecksum := newOpt.Checksum() + + log.Debug(). + Str("old-checksum", optChecksum). + Str("new-checksum", newOptChecksum). + Msg("cmd/pomerium: configuration file changed") + + if newOptChecksum == optChecksum { + log.Debug().Msg("cmd/pomerium: loaded configuration has not changed") + return opt + } + + if err != nil { + log.Error(). + Err(err). + Msg("cmd/pomerium: could not reload configuration") + return opt + } + + log.Info(). + Str("config-checksum", newOptChecksum). + Msg("cmd/pomerium: running configuration has changed") + for _, service := range services { + err := service.UpdateOptions(newOpt) + if err != nil { + log.Error(). + Err(err). + Msg("cmd/pomerium: could not update options") + } + } + + return newOpt +} diff --git a/cmd/pomerium/main_test.go b/cmd/pomerium/main_test.go index 7e5930bbe..6aa27fe15 100644 --- a/cmd/pomerium/main_test.go +++ b/cmd/pomerium/main_test.go @@ -241,3 +241,45 @@ func Test_parseOptions(t *testing.T) { }) } } + +type mockService struct { + fail bool + Updated bool +} + +func (m *mockService) UpdateOptions(o *config.Options) error { + + m.Updated = true + if m.fail { + return fmt.Errorf("Failed") + } + return nil +} + +func Test_handleConfigUpdate(t *testing.T) { + os.Clearenv() + os.Setenv("SHARED_SECRET", "foo") + defer os.Unsetenv("SHARED_SECRET") + + blankOpts := config.NewOptions() + goodOpts, _ := config.OptionsFromViper("") + tests := []struct { + name string + service *mockService + oldOpts *config.Options + wantUpdate bool + }{ + {"good", &mockService{fail: false}, blankOpts, true}, + {"bad", &mockService{fail: true}, blankOpts, true}, + {"no change", &mockService{fail: false}, goodOpts, false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + handleConfigUpdate(tt.oldOpts, []config.OptionsUpdater{tt.service}) + if tt.service.Updated != tt.wantUpdate { + t.Errorf("Failed to update config on service") + } + }) + } +} diff --git a/docs/reference/readme.md b/docs/reference/readme.md index 485bcd4b1..b2f1152a5 100644 --- a/docs/reference/readme.md +++ b/docs/reference/readme.md @@ -15,6 +15,8 @@ If you are coming from a kubernetes or docker background this should feel famili In general, any setting specified by environment variable can also be present in the optional config file as the same name but lower cased. Environment variables take precedence. +Pomerium will automatically reload the configuration file if it is changed. At this time, only policy is re-configured when this reload occurs, but additional options may be added in the future. It is suggested that your policy is stored in a configuration file so that you can take advantage of this feature. + ## Global settings These are configuration variables shared by all services, in all service modes. diff --git a/go.mod b/go.mod index 855cf76d5..11f001d5c 100644 --- a/go.mod +++ b/go.mod @@ -3,10 +3,12 @@ module github.com/pomerium/pomerium go 1.12 require ( + github.com/fsnotify/fsnotify v1.4.7 github.com/golang/mock v1.2.0 github.com/golang/protobuf v1.3.1 github.com/google/go-cmp v0.3.0 github.com/magiconair/properties v1.8.1 // indirect + github.com/mitchellh/hashstructure v1.0.0 github.com/pomerium/go-oidc v2.0.0+incompatible github.com/pquerna/cachecontrol v0.0.0-20180517163645-1555304b9b35 // indirect github.com/rs/zerolog v1.14.3 diff --git a/go.sum b/go.sum index afb9b527d..4dcdf95c5 100644 --- a/go.sum +++ b/go.sum @@ -39,6 +39,8 @@ github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czP github.com/magiconair/properties v1.8.1 h1:ZC2Vc7/ZFkGmsVC9KvOjumD+G5lXy2RtTKyzRKO2BQ4= github.com/magiconair/properties v1.8.1/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ= github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0= +github.com/mitchellh/hashstructure v1.0.0 h1:ZkRJX1CyOoTkar7p/mLS5TZU4nJ1Rn/F8u9dGS02Q3Y= +github.com/mitchellh/hashstructure v1.0.0/go.mod h1:QjSHrPWS+BGUVBYkbTZWEnOh3G1DutKwClXU/ABz6AQ= github.com/mitchellh/mapstructure v1.1.2 h1:fmNYVwqnSfB9mZU6OS2O6GsXM+wcskZDuKQzvN1EDeE= github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y= github.com/openzipkin/zipkin-go v0.1.1/go.mod h1:NtoC/o8u3JlF1lSlyPNswIbeQH9bJTmOf0Erfk+hxe8= diff --git a/internal/config/options.go b/internal/config/options.go index 69fe84ce5..ef5decbff 100644 --- a/internal/config/options.go +++ b/internal/config/options.go @@ -10,11 +10,11 @@ import ( "reflect" "time" - "gopkg.in/yaml.v2" - + "github.com/mitchellh/hashstructure" "github.com/pomerium/pomerium/internal/log" "github.com/pomerium/pomerium/internal/policy" "github.com/spf13/viper" + "gopkg.in/yaml.v2" ) // DisableHeaderKey is the key used to check whether to disable setting header @@ -70,8 +70,8 @@ type Options struct { // AuthenticateURL represents the externally accessible http endpoints // used for authentication requests and callbacks - AuthenticateURLString string `mapstructure:"authenticate_service_url"` - AuthenticateURL *url.URL + AuthenticateURLString string `mapstructure:"authenticate_service_url"` + AuthenticateURL *url.URL `hash:"ignore"` // Session/Cookie management // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie @@ -172,29 +172,32 @@ func OptionsFromViper(configFile string) (*Options, error) { // Load up config o.bindEnvs() if configFile != "" { - log.Info().Msgf("Loading config from '%s'", configFile) + log.Info(). + Str("file", configFile). + Msg("loading config from file") + viper.SetConfigFile(configFile) err := viper.ReadInConfig() if err != nil { - return nil, fmt.Errorf("Failed to read config: %s", err) + return nil, fmt.Errorf("failed to read config: %s", err) } } err := viper.Unmarshal(o) if err != nil { - return nil, fmt.Errorf("Failed to load options from config: %s", err) + return nil, fmt.Errorf("failed to load options from config: %s", err) } // Turn URL strings into url structs err = o.parseURLs() if err != nil { - return nil, fmt.Errorf("Failed to parse URLs: %s", err) + return nil, fmt.Errorf("failed to parse URLs: %s", err) } // Load and initialize policy err = o.parsePolicy() if err != nil { - return nil, fmt.Errorf("Failed to parse Policy: %s", err) + return nil, fmt.Errorf("failed to parse Policy: %s", err) } if o.Debug { @@ -212,6 +215,9 @@ func OptionsFromViper(configFile string) (*Options, error) { return nil, err } + log.Debug(). + Str("config-checksum", o.Checksum()). + Msg("read configuration with checksum") return o, nil } @@ -382,3 +388,19 @@ func IsProxy(s string) bool { } return false } + +// OptionsUpdater updates local state based on an Options struct +type OptionsUpdater interface { + UpdateOptions(*Options) error +} + +// Checksum returns the checksum of the current options struct +func (o *Options) Checksum() string { + hash, err := hashstructure.Hash(o, nil) + + if err != nil { + log.Warn().Msg("could not calculate Option checksum") + return "no checksum availablle" + } + return fmt.Sprintf("%x", hash) +} diff --git a/internal/config/options_test.go b/internal/config/options_test.go index 43caafd5a..a1623b80d 100644 --- a/internal/config/options_test.go +++ b/internal/config/options_test.go @@ -355,3 +355,23 @@ func Test_parsePolicyFile(t *testing.T) { }) } } + +func Test_Checksum(t *testing.T) { + o := NewOptions() + + oldChecksum := o.Checksum() + o.SharedKey = "changemeplease" + newChecksum := o.Checksum() + + if newChecksum == oldChecksum { + t.Errorf("Checksum() failed to update old = %s, new = %s", oldChecksum, newChecksum) + } + + if newChecksum == "" || oldChecksum == "" { + t.Error("Checksum() not returning data") + } + + if o.Checksum() != o.Checksum() { + t.Error("Checksum() inconsistent") + } +} diff --git a/proxy/handlers.go b/proxy/handlers.go index e44809248..016d18980 100644 --- a/proxy/handlers.go +++ b/proxy/handlers.go @@ -423,14 +423,6 @@ func (p *Proxy) authenticate(w http.ResponseWriter, r *http.Request, session *se return nil } -// Handle constructs a route from the given host string and matches it to the provided http.Handler and UpstreamConfig -func (p *Proxy) Handle(host string, handler http.Handler, pol *policy.Policy) { - p.routeConfigs[host] = &routeConfig{ - mux: handler, - policy: *pol, - } -} - // router attempts to find a route for a request. If a route is successfully matched, // it returns the route information and a bool value of `true`. If a route can not be matched, // a nil value for the route and false bool value is returned. diff --git a/proxy/proxy.go b/proxy/proxy.go index 9a632fc38..59f372435 100755 --- a/proxy/proxy.go +++ b/proxy/proxy.go @@ -148,14 +148,9 @@ func New(opts *config.Options) (*Proxy, error) { refreshCooldown: opts.RefreshCooldown, } - for _, route := range opts.Policies { - proxy := NewReverseProxy(route.Destination) - handler, err := NewReverseProxyHandler(opts, proxy, &route) - if err != nil { - return nil, err - } - p.Handle(route.Source.Host, handler, &route) - log.Info().Str("src", route.Source.Host).Str("dst", route.Destination.Host).Msg("proxy: new route") + err = p.UpdatePolicies(opts) + if err != nil { + return nil, err } p.AuthenticateClient, err = clients.NewAuthenticateClient("grpc", @@ -181,6 +176,25 @@ func New(opts *config.Options) (*Proxy, error) { return p, err } +// UpdatePolicies updates the handlers based on the configured policies +func (p *Proxy) UpdatePolicies(opts *config.Options) error { + routeConfigs := make(map[string]*routeConfig) + for _, route := range opts.Policies { + proxy := NewReverseProxy(route.Destination) + handler, err := NewReverseProxyHandler(opts, proxy, &route) + if err != nil { + return err + } + routeConfigs[route.Source.Host] = &routeConfig{ + mux: handler, + policy: route, + } + log.Info().Str("src", route.Source.Host).Str("dst", route.Destination.Host).Msg("proxy: new route") + } + p.routeConfigs = routeConfigs + return nil +} + // UpstreamProxy stores information for proxying the request to the upstream. type UpstreamProxy struct { name string @@ -270,3 +284,13 @@ func urlParse(uri string) (*url.URL, error) { } return url.ParseRequestURI(uri) } + +// UpdateOptions updates internal structres based on config.Options +func (p *Proxy) UpdateOptions(o *config.Options) error { + log.Info().Msg("proxy: updating options") + err := p.UpdatePolicies(o) + if err != nil { + return fmt.Errorf("Could not update policies: %s", err) + } + return nil +} diff --git a/proxy/proxy_test.go b/proxy/proxy_test.go index f0e0b7553..3445a3cc3 100644 --- a/proxy/proxy_test.go +++ b/proxy/proxy_test.go @@ -283,3 +283,51 @@ func TestProxy_OAuthCallback(t *testing.T) { } } + +func Test_UpdateOptions(t *testing.T) { + + good := testOptions() + bad := testOptions() + bad.SigningKey = "f" + newPolicy := policy.Policy{To: "foo.notatld", From: "bar.notatld"} + newPolicy.Validate() + newPolicies := []policy.Policy{ + newPolicy, + } + tests := []struct { + name string + opts *config.Options + newPolicy []policy.Policy + host string + wantErr bool + wantRoute bool + }{ + {"good", good, good.Policies, "https://corp.example.notatld", false, true}, + {"changed", good, newPolicies, "https://bar.notatld", false, true}, + {"changed and missing", good, newPolicies, "https://corp.example.notatld", false, false}, + {"bad options", bad, good.Policies, "https://corp.example.notatld", true, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + o := tt.opts + p, _ := New(o) + + o.Policies = tt.newPolicy + err := p.UpdateOptions(o) + if (err != nil) != tt.wantErr { + t.Errorf("UpdateOptions: err = %v, wantErr = %v", err, tt.wantErr) + return + } + + // This is only safe if we actually can load policies + if err == nil { + req := httptest.NewRequest("GET", tt.host, nil) + _, ok := p.router(req) + if ok != tt.wantRoute { + t.Errorf("Failed to find route handler") + return + } + } + }) + } +}