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

@@ -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
 }

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
 		}

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
+	// sharedKey is the secret to encrypt and authenticate data shared between services
-	sharedSecret []byte
+	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.redirectURL, err = urlutil.DeepCopy(authenticateURL)
-	state.sharedEncoder, err = jws.NewHS256Signer([]byte(cfg.Options.SharedKey))
 	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.sharedKey, err = base64.StdEncoding.DecodeString(cfg.Options.SharedKey)
-	state.sharedCipher, _ = cryptutil.NewAEADCipher(state.sharedSecret)
+	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.cookieSecret, err = base64.StdEncoding.DecodeString(cfg.Options.CookieSecret)
-	state.cookieCipher, _ = cryptutil.NewAEADCipher(state.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 {

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,

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

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))

proxy/state.go

@@ -44,12 +44,23 @@ func newProxyStateFromConfig(cfg *config.Config) (*proxyState, error) {
 	}
 
 	state := new(proxyState)
-	state.sharedKey = []byte(cfg.Options.SharedKey)
+	state.sharedKey, err = base64.StdEncoding.DecodeString(cfg.Options.SharedKey)
-	state.sharedCipher, _ = cryptutil.NewAEADCipherFromBase64(cfg.Options.SharedKey)
+	if err != nil {
-	state.cookieSecret, _ = base64.StdEncoding.DecodeString(cfg.Options.CookieSecret)
+		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})