From 6d1d2bec540d3bc98a0978bf7e0432a72a092f87 Mon Sep 17 00:00:00 2001 From: Caleb Doxsey Date: Thu, 8 Apr 2021 20:04:01 -0600 Subject: [PATCH] crypto: use actual bytes of shared secret, not the base64 encoded representation (#2075) * crypto: use actual bytes of shared secret, not the base64 encoded representation * return errors * return errors --- authenticate/handlers.go | 7 +++-- authenticate/middleware.go | 4 +-- authenticate/state.go | 45 +++++++++++++++++++++++-------- authorize/check_response.go | 3 ++- authorize/state.go | 14 ++++++++-- internal/middleware/middleware.go | 4 +-- proxy/state.go | 20 +++++++++++--- 7 files changed, 71 insertions(+), 26 deletions(-) diff --git a/authenticate/handlers.go b/authenticate/handlers.go index c70472535..67194e4e8 100644 --- a/authenticate/handlers.go +++ b/authenticate/handlers.go @@ -79,7 +79,7 @@ func (a *Authenticate) mountDashboard(r *mux.Router) { c := cors.New(cors.Options{ AllowOriginRequestFunc: func(r *http.Request, _ string) bool { state := a.state.Load() - err := middleware.ValidateRequestURL(r, state.sharedSecret) + err := middleware.ValidateRequestURL(r, state.sharedKey) if err != nil { log.FromRequest(r).Info().Err(err).Msg("authenticate: origin blocked") } @@ -175,7 +175,6 @@ func (a *Authenticate) SignIn(w http.ResponseWriter, r *http.Request) error { ctx, span := trace.StartSpan(r.Context(), "authenticate.SignIn") defer span.End() - options := a.options.Load() state := a.state.Load() redirectURL, err := urlutil.ParseAndValidateURL(r.FormValue(urlutil.QueryRedirectURI)) @@ -243,7 +242,7 @@ func (a *Authenticate) SignIn(w http.ResponseWriter, r *http.Request) error { // build our hmac-d redirect URL with our session, pointing back to the // proxy's callback URL which is responsible for setting our new route-session - uri := urlutil.NewSignedURL([]byte(options.SharedKey), callbackURL) + uri := urlutil.NewSignedURL(state.sharedKey, callbackURL) httputil.Redirect(w, r, uri.String(), http.StatusFound) return nil } @@ -606,5 +605,5 @@ func (a *Authenticate) getSignOutURL(r *http.Request) (*url.URL, error) { urlutil.QueryRedirectURI: {redirectURI}, }).Encode() } - return urlutil.NewSignedURL([]byte(a.options.Load().SharedKey), uri).Sign(), nil + return urlutil.NewSignedURL(a.state.Load().sharedKey, uri).Sign(), nil } diff --git a/authenticate/middleware.go b/authenticate/middleware.go index b32437fec..2989f2a4b 100644 --- a/authenticate/middleware.go +++ b/authenticate/middleware.go @@ -13,7 +13,7 @@ import ( func (a *Authenticate) requireValidSignatureOnRedirect(next httputil.HandlerFunc) http.Handler { return httputil.HandlerFunc(func(w http.ResponseWriter, r *http.Request) error { if r.FormValue(urlutil.QueryRedirectURI) != "" || r.FormValue(urlutil.QueryHmacSignature) != "" { - err := middleware.ValidateRequestURL(r, []byte(a.options.Load().SharedKey)) + err := middleware.ValidateRequestURL(r, a.state.Load().sharedKey) if err != nil { return httputil.NewError(http.StatusBadRequest, err) } @@ -25,7 +25,7 @@ func (a *Authenticate) requireValidSignatureOnRedirect(next httputil.HandlerFunc // requireValidSignature validates the pomerium_signature. func (a *Authenticate) requireValidSignature(next httputil.HandlerFunc) http.Handler { return httputil.HandlerFunc(func(w http.ResponseWriter, r *http.Request) error { - err := middleware.ValidateRequestURL(r, []byte(a.options.Load().SharedKey)) + err := middleware.ValidateRequestURL(r, a.state.Load().sharedKey) if err != nil { return err } diff --git a/authenticate/state.go b/authenticate/state.go index 6c7628414..3a5572de1 100644 --- a/authenticate/state.go +++ b/authenticate/state.go @@ -29,8 +29,8 @@ type authenticateState struct { // sharedEncoder is the encoder to use to serialize data to be consumed // by other services sharedEncoder encoding.MarshalUnmarshaler - // sharedSecret is the secret to encrypt and authenticate data shared between services - sharedSecret []byte + // sharedKey is the secret to encrypt and authenticate data shared between services + sharedKey []byte // sharedCipher is the cipher to use to encrypt/decrypt data shared between services sharedCipher cipher.AEAD // cookieSecret is the secret to encrypt and authenticate session data @@ -69,22 +69,42 @@ func newAuthenticateStateFromConfig(cfg *config.Config) (*authenticateState, err if err != nil { return nil, err } - state.redirectURL, _ = urlutil.DeepCopy(authenticateURL) - state.redirectURL.Path = cfg.Options.AuthenticateCallbackPath - // shared state encoder setup - state.sharedEncoder, err = jws.NewHS256Signer([]byte(cfg.Options.SharedKey)) + state.redirectURL, err = urlutil.DeepCopy(authenticateURL) if err != nil { return nil, err } + state.redirectURL.Path = cfg.Options.AuthenticateCallbackPath + // shared cipher to encrypt data before passing data between services - state.sharedSecret, _ = base64.StdEncoding.DecodeString(cfg.Options.SharedKey) - state.sharedCipher, _ = cryptutil.NewAEADCipher(state.sharedSecret) + state.sharedKey, err = base64.StdEncoding.DecodeString(cfg.Options.SharedKey) + if err != nil { + return nil, err + } + + state.sharedCipher, err = cryptutil.NewAEADCipher(state.sharedKey) + if err != nil { + return nil, err + } + + // shared state encoder setup + state.sharedEncoder, err = jws.NewHS256Signer(state.sharedKey) + if err != nil { + return nil, err + } // private state encoder setup, used to encrypt oauth2 tokens - state.cookieSecret, _ = base64.StdEncoding.DecodeString(cfg.Options.CookieSecret) - state.cookieCipher, _ = cryptutil.NewAEADCipher(state.cookieSecret) + state.cookieSecret, err = base64.StdEncoding.DecodeString(cfg.Options.CookieSecret) + if err != nil { + return nil, err + } + + state.cookieCipher, err = cryptutil.NewAEADCipher(state.cookieSecret) + if err != nil { + return nil, err + } + state.encryptedEncoder = ecjson.New(state.cookieCipher) headerStore := header.NewStore(state.encryptedEncoder, httputil.AuthorizationTypePomerium) @@ -120,7 +140,10 @@ func newAuthenticateStateFromConfig(cfg *config.Config) (*authenticateState, err state.jwk.Keys = append(state.jwk.Keys, *jwk) } - sharedKey, _ := base64.StdEncoding.DecodeString(cfg.Options.SharedKey) + sharedKey, err := base64.StdEncoding.DecodeString(cfg.Options.SharedKey) + if err != nil { + return nil, err + } urls, err := cfg.Options.GetDataBrokerURLs() if err != nil { diff --git a/authorize/check_response.go b/authorize/check_response.go index bfa5878fd..550dbdf81 100644 --- a/authorize/check_response.go +++ b/authorize/check_response.go @@ -151,6 +151,7 @@ func (a *Authorize) plainTextDeniedResponse(code int32, reason string, headers m func (a *Authorize) redirectResponse(in *envoy_service_auth_v3.CheckRequest) (*envoy_service_auth_v3.CheckResponse, error) { opts := a.currentOptions.Load() + state := a.state.Load() authenticateURL, err := opts.GetAuthenticateURL() if err != nil { return nil, err @@ -167,7 +168,7 @@ func (a *Authorize) redirectResponse(in *envoy_service_auth_v3.CheckRequest) (*e q.Set(urlutil.QueryRedirectURI, url.String()) signinURL.RawQuery = q.Encode() - redirectTo := urlutil.NewSignedURL([]byte(opts.SharedKey), signinURL).String() + redirectTo := urlutil.NewSignedURL(state.sharedKey, signinURL).String() return a.deniedResponse(in, http.StatusFound, "Login", map[string]string{ "Location": redirectTo, diff --git a/authorize/state.go b/authorize/state.go index 5a5a6fac7..3aebf6253 100644 --- a/authorize/state.go +++ b/authorize/state.go @@ -15,6 +15,7 @@ import ( ) type authorizeState struct { + sharedKey []byte evaluator *evaluator.Evaluator encoder encoding.MarshalUnmarshaler dataBrokerClient databroker.DataBrokerServiceClient @@ -35,12 +36,21 @@ func newAuthorizeStateFromConfig(cfg *config.Config, store *evaluator.Store) (*a return nil, fmt.Errorf("authorize: failed to update policy with options: %w", err) } - state.encoder, err = jws.NewHS256Signer([]byte(cfg.Options.SharedKey)) + state.sharedKey, err = base64.StdEncoding.DecodeString(cfg.Options.SharedKey) + if err != nil { + return nil, err + } + + state.encoder, err = jws.NewHS256Signer(state.sharedKey) + if err != nil { + return nil, err + } + + sharedKey, err := base64.StdEncoding.DecodeString(cfg.Options.SharedKey) if err != nil { return nil, err } - sharedKey, _ := base64.StdEncoding.DecodeString(cfg.Options.SharedKey) urls, err := cfg.Options.GetDataBrokerURLs() if err != nil { return nil, err diff --git a/internal/middleware/middleware.go b/internal/middleware/middleware.go index dd81faa3b..1de90f028 100644 --- a/internal/middleware/middleware.go +++ b/internal/middleware/middleware.go @@ -28,12 +28,12 @@ func SetHeaders(headers map[string]string) func(next http.Handler) http.Handler // ValidateSignature ensures the request is valid and has been signed with // the correspdoning client secret key -func ValidateSignature(sharedSecret []byte) func(next http.Handler) http.Handler { +func ValidateSignature(sharedKey []byte) func(next http.Handler) http.Handler { return func(next http.Handler) http.Handler { return httputil.HandlerFunc(func(w http.ResponseWriter, r *http.Request) error { ctx, span := trace.StartSpan(r.Context(), "middleware.ValidateSignature") defer span.End() - if err := ValidateRequestURL(r, sharedSecret); err != nil { + if err := ValidateRequestURL(r, sharedKey); err != nil { return httputil.NewError(http.StatusBadRequest, err) } next.ServeHTTP(w, r.WithContext(ctx)) diff --git a/proxy/state.go b/proxy/state.go index 5d53bb2cb..acf6580dd 100644 --- a/proxy/state.go +++ b/proxy/state.go @@ -44,12 +44,23 @@ func newProxyStateFromConfig(cfg *config.Config) (*proxyState, error) { } state := new(proxyState) - state.sharedKey = []byte(cfg.Options.SharedKey) - state.sharedCipher, _ = cryptutil.NewAEADCipherFromBase64(cfg.Options.SharedKey) - state.cookieSecret, _ = base64.StdEncoding.DecodeString(cfg.Options.CookieSecret) + state.sharedKey, err = base64.StdEncoding.DecodeString(cfg.Options.SharedKey) + if err != nil { + return nil, err + } + + state.sharedCipher, err = cryptutil.NewAEADCipherFromBase64(cfg.Options.SharedKey) + if err != nil { + return nil, err + } + + state.cookieSecret, err = base64.StdEncoding.DecodeString(cfg.Options.CookieSecret) + if err != nil { + return nil, err + } // used to load and verify JWT tokens signed by the authenticate service - state.encoder, err = jws.NewHS256Signer([]byte(cfg.Options.SharedKey)) + state.encoder, err = jws.NewHS256Signer(state.sharedKey) if err != nil { return nil, err } @@ -62,6 +73,7 @@ func newProxyStateFromConfig(cfg *config.Config) (*proxyState, error) { if err != nil { return nil, err } + state.authenticateDashboardURL = state.authenticateURL.ResolveReference(&url.URL{Path: "/.pomerium/"}) state.authenticateSigninURL = state.authenticateURL.ResolveReference(&url.URL{Path: signinURL}) state.authenticateRefreshURL = state.authenticateURL.ResolveReference(&url.URL{Path: refreshURL})