cryptutil: use bytes for hmac (#2067)

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, string(state.sharedSecret))
+			err := middleware.ValidateRequestURL(r, state.sharedSecret)
 			if err != nil {
 				log.FromRequest(r).Info().Err(err).Msg("authenticate: origin blocked")
 			}
@@ -243,7 +243,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(options.SharedKey, callbackURL)
+	uri := urlutil.NewSignedURL([]byte(options.SharedKey), callbackURL)
 	httputil.Redirect(w, r, uri.String(), http.StatusFound)
 	return nil
 }
@@ -606,5 +606,5 @@ func (a *Authenticate) getSignOutURL(r *http.Request) (*url.URL, error) {
 			urlutil.QueryRedirectURI: {redirectURI},
 		}).Encode()
 	}
-	return urlutil.NewSignedURL(a.options.Load().SharedKey, uri).Sign(), nil
+	return urlutil.NewSignedURL([]byte(a.options.Load().SharedKey), uri).Sign(), nil
 }

authenticate/handlers_test.go

@@ -621,7 +621,7 @@ func TestAuthenticate_userInfo(t *testing.T) {
 		},
 		{
 			"bad signature",
-			urlutil.NewSignedURL("BAD KEY", mustParseURL("/?pomerium_redirect_uri=http://example.com")).Sign(),
+			urlutil.NewSignedURL([]byte("BAD KEY"), mustParseURL("/?pomerium_redirect_uri=http://example.com")).Sign(),
 			http.MethodGet,
 			&mstore.Store{Encrypted: true, Session: &sessions.State{ID: "SESSION_ID", IssuedAt: jwt.NewNumericDate(now)}},
 			http.StatusBadRequest,

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, a.options.Load().SharedKey)
+			err := middleware.ValidateRequestURL(r, []byte(a.options.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, a.options.Load().SharedKey)
+		err := middleware.ValidateRequestURL(r, []byte(a.options.Load().SharedKey))
 		if err != nil {
 			return err
 		}

authorize/check_response.go

@@ -167,7 +167,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(opts.SharedKey, signinURL).String()
+	redirectTo := urlutil.NewSignedURL([]byte(opts.SharedKey), signinURL).String()
 
 	return a.deniedResponse(in, http.StatusFound, "Login", map[string]string{
 		"Location": redirectTo,

internal/httputil/reproxy/reproxy.go

@@ -55,7 +55,7 @@ func (h *Handler) GetPolicyIDFromHeaders(headers http.Header) (uint64, bool) {
 	h.mu.RLock()
 	defer h.mu.RUnlock()
 
-	return policyID, cryptutil.CheckHMAC([]byte(policyStr), hmac, string(h.key))
+	return policyID, cryptutil.CheckHMAC([]byte(policyStr), hmac, h.key)
 }
 
 // GetPolicyIDHeaders returns http headers for the given policy id.
@@ -64,7 +64,7 @@ func (h *Handler) GetPolicyIDHeaders(policyID uint64) [][2]string {
 	defer h.mu.RUnlock()
 
 	s := strconv.FormatUint(policyID, 10)
-	hmac := base64.StdEncoding.EncodeToString(cryptutil.GenerateHMAC([]byte(s), string(h.key)))
+	hmac := base64.StdEncoding.EncodeToString(cryptutil.GenerateHMAC([]byte(s), h.key))
 	return [][2]string{
 		{httputil.HeaderPomeriumReproxyPolicy, s},
 		{httputil.HeaderPomeriumReproxyPolicyHMAC, hmac},

internal/middleware/middleware.go

@@ -28,7 +28,7 @@ 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 string) func(next http.Handler) http.Handler {
+func ValidateSignature(sharedSecret []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")
@@ -44,7 +44,7 @@ func ValidateSignature(sharedSecret string) func(next http.Handler) http.Handler
 
 // ValidateRequestURL validates the current absolute request URL was signed
 // by a given shared key.
-func ValidateRequestURL(r *http.Request, key string) error {
+func ValidateRequestURL(r *http.Request, key []byte) error {
 	return urlutil.NewSignedURL(key, urlutil.GetAbsoluteURL(r)).Validate()
 }
 

internal/middleware/middleware_test.go

@@ -125,13 +125,13 @@ func TestValidateSignature(t *testing.T) {
 
 	tests := []struct {
 		name       string
-		secretA    string
-		secretB    string
+		secretA    []byte
+		secretB    []byte
 		wantStatus int
 		wantBody   string
 	}{
-		{"good", "secret", "secret", http.StatusOK, http.StatusText(http.StatusOK)},
-		{"secret mistmatch", "secret", "hunter42", http.StatusBadRequest, "{\"Status\":400,\"Error\":\"Bad Request: internal/urlutil: hmac failed\"}\n"},
+		{"good", []byte("secret"), []byte("secret"), http.StatusOK, http.StatusText(http.StatusOK)},
+		{"secret mistmatch", []byte("secret"), []byte("hunter42"), http.StatusBadRequest, "{\"Status\":400,\"Error\":\"Bad Request: internal/urlutil: hmac failed\"}\n"},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

internal/urlutil/signed.go

@@ -13,7 +13,7 @@ import (
 // SignedURL is a shared-key HMAC wrapped URL.
 type SignedURL struct {
 	uri    url.URL
-	key    string
+	key    []byte
 	signed bool
 
 	// mockable time for testing
@@ -24,7 +24,7 @@ type SignedURL struct {
 //
 // N.B. It is the user's responsibility to make sure the key is 256 bits and
 // 		the url is not nil.
-func NewSignedURL(key string, uri *url.URL) *SignedURL {
+func NewSignedURL(key []byte, uri *url.URL) *SignedURL {
 	return &SignedURL{uri: *uri, key: key, timeNow: time.Now} // uri is copied
 }
 
@@ -93,7 +93,7 @@ func (su *SignedURL) Validate() error {
 
 // hmacURL takes a redirect url string and timestamp and returns the base64
 // encoded HMAC result.
-func hmacURL(key string, data ...interface{}) string {
+func hmacURL(key []byte, data ...interface{}) string {
 	h := cryptutil.GenerateHMAC([]byte(fmt.Sprint(data...)), key)
 	return base64.URLEncoding.EncodeToString(h)
 }

internal/urlutil/signed_test.go

@@ -31,14 +31,14 @@ func TestSignedURL(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			signedURL := NewSignedURL(tt.key, &tt.uri)
+			signedURL := NewSignedURL([]byte(tt.key), &tt.uri)
 			signedURL.timeNow = tt.origTime
 
 			if diff := cmp.Diff(signedURL.String(), tt.wantStr); diff != "" {
 				t.Errorf("signedURL() = %v", diff)
 			}
 
-			signedURL = NewSignedURL(tt.key, &tt.uri)
+			signedURL = NewSignedURL([]byte(tt.key), &tt.uri)
 			signedURL.timeNow = tt.origTime
 			got := signedURL.Sign()
 
@@ -89,7 +89,7 @@ func TestSignedURL_Validate(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			out := NewSignedURL(tt.key, &tt.uri)
+			out := NewSignedURL([]byte(tt.key), &tt.uri)
 			out.timeNow = tt.timeNow
 
 			if err := out.Validate(); (err != nil) != tt.wantErr {

pkg/cryptutil/hmac.go

@@ -20,15 +20,15 @@ var (
 )
 
 // GenerateHMAC produces a symmetric signature using a shared secret key.
-func GenerateHMAC(data []byte, key string) []byte {
-	h := hmac.New(sha512.New512_256, []byte(key))
+func GenerateHMAC(data, key []byte) []byte {
+	h := hmac.New(sha512.New512_256, key)
 	h.Write(data)
 	return h.Sum(nil)
 }
 
 // CheckHMAC securely checks the supplied MAC against a message using the
 // shared secret key.
-func CheckHMAC(data, suppliedMAC []byte, key string) bool {
+func CheckHMAC(data, suppliedMAC, key []byte) bool {
 	expectedMAC := GenerateHMAC(data, key)
 	return hmac.Equal(expectedMAC, suppliedMAC)
 }

pkg/cryptutil/hmac_test.go

@@ -34,11 +34,11 @@ func TestHMAC(t *testing.T) {
 		keyBytes := &[32]byte{}
 		copy(keyBytes[:], keySlice)
 
-		macDigest := GenerateHMAC(dataBytes, string(keyBytes[:]))
+		macDigest := GenerateHMAC(dataBytes, keyBytes[:])
 		if !bytes.Equal(macDigest, expectedDigest) {
 			t.Errorf("test %d generated unexpected mac", idx)
 		}
-		if !CheckHMAC(dataBytes, macDigest, string(keyBytes[:])) {
+		if !CheckHMAC(dataBytes, macDigest, keyBytes[:]) {
 			t.Errorf("test %d generated unexpected mac", idx)
 		}
 	}

proxy/state.go

@@ -19,7 +19,7 @@ import (
 )
 
 type proxyState struct {
-	sharedKey    string
+	sharedKey    []byte
 	sharedCipher cipher.AEAD
 
 	authenticateURL          *url.URL
@@ -44,7 +44,7 @@ func newProxyStateFromConfig(cfg *config.Config) (*proxyState, error) {
 	}
 
 	state := new(proxyState)
-	state.sharedKey = cfg.Options.SharedKey
+	state.sharedKey = []byte(cfg.Options.SharedKey)
 	state.sharedCipher, _ = cryptutil.NewAEADCipherFromBase64(cfg.Options.SharedKey)
 	state.cookieSecret, _ = base64.StdEncoding.DecodeString(cfg.Options.CookieSecret)