From 9d626265e8c2cef0917702f653047f486040a018 Mon Sep 17 00:00:00 2001
From: Nick Meves <nick.meves@greenhouse.io>
Date: Tue, 5 May 2020 18:41:48 -0700
Subject: [PATCH] Migrate cookie signing to SHA256 from SHA1 (#524)

Also, cleanup the code & make the specific
hashing algorithm chosen a function variable.

Co-authored-by: Henry Jenkins <henry@henryjenkins.name>
---
 CHANGELOG.md                  |  1 +
 pkg/encryption/cipher.go      | 22 +++++++++++++++++-----
 pkg/encryption/cipher_test.go | 19 +++++++++++++++++++
 3 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index ada558bf..6d522260 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -25,6 +25,7 @@
 
 ## Changes since v5.1.1
 
+- [#524](https://github.com/oauth2-proxy/oauth2-proxy/pull/524) Sign cookies with SHA256 (@NickMeves)
 - [#515](https://github.com/oauth2-proxy/oauth2-proxy/pull/515) Drop configure script in favour of native Makefile env and checks (@JoelSpeed)
 - [#487](https://github.com/oauth2-proxy/oauth2-proxy/pull/487) Switch flags to PFlag to remove StringArray (@JoelSpeed)
 - [#484](https://github.com/oauth2-proxy/oauth2-proxy/pull/484) Replace configuration loading with Viper (@JoelSpeed)
diff --git a/pkg/encryption/cipher.go b/pkg/encryption/cipher.go
index c308330f..9c438bce 100644
--- a/pkg/encryption/cipher.go
+++ b/pkg/encryption/cipher.go
@@ -6,8 +6,10 @@ import (
 	"crypto/hmac"
 	"crypto/rand"
 	"crypto/sha1"
+	"crypto/sha256"
 	"encoding/base64"
 	"fmt"
+	"hash"
 	"io"
 	"net/http"
 	"strconv"
@@ -25,8 +27,7 @@ func Validate(cookie *http.Cookie, seed string, expiration time.Duration) (value
 	if len(parts) != 3 {
 		return
 	}
-	sig := cookieSignature(seed, cookie.Name, parts[0], parts[1])
-	if checkHmac(parts[2], sig) {
+	if checkSignature(parts[2], seed, cookie.Name, parts[0], parts[1]) {
 		ts, err := strconv.Atoi(parts[1])
 		if err != nil {
 			return
@@ -53,13 +54,13 @@ func Validate(cookie *http.Cookie, seed string, expiration time.Duration) (value
 func SignedValue(seed string, key string, value string, now time.Time) string {
 	encodedValue := base64.URLEncoding.EncodeToString([]byte(value))
 	timeStr := fmt.Sprintf("%d", now.Unix())
-	sig := cookieSignature(seed, key, encodedValue, timeStr)
+	sig := cookieSignature(sha256.New, seed, key, encodedValue, timeStr)
 	cookieVal := fmt.Sprintf("%s|%s|%s", encodedValue, timeStr, sig)
 	return cookieVal
 }
 
-func cookieSignature(args ...string) string {
-	h := hmac.New(sha1.New, []byte(args[0]))
+func cookieSignature(signer func() hash.Hash, args ...string) string {
+	h := hmac.New(signer, []byte(args[0]))
 	for _, arg := range args[1:] {
 		h.Write([]byte(arg))
 	}
@@ -68,6 +69,17 @@ func cookieSignature(args ...string) string {
 	return base64.URLEncoding.EncodeToString(b)
 }
 
+func checkSignature(signature string, args ...string) bool {
+	checkSig := cookieSignature(sha256.New, args...)
+	if checkHmac(signature, checkSig) {
+		return true
+	}
+
+	// TODO: After appropriate rollout window, remove support for SHA1
+	legacySig := cookieSignature(sha1.New, args...)
+	return checkHmac(signature, legacySig)
+}
+
 func checkHmac(input, expected string) bool {
 	inputMAC, err1 := base64.URLEncoding.DecodeString(input)
 	if err1 == nil {
diff --git a/pkg/encryption/cipher_test.go b/pkg/encryption/cipher_test.go
index fb6a4aa7..37026ab1 100644
--- a/pkg/encryption/cipher_test.go
+++ b/pkg/encryption/cipher_test.go
@@ -1,12 +1,31 @@
 package encryption
 
 import (
+	"crypto/sha1"
+	"crypto/sha256"
 	"encoding/base64"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 )
 
+func TestSignAndValidate(t *testing.T) {
+	seed := "0123456789abcdef"
+	key := "cookie-name"
+	value := base64.URLEncoding.EncodeToString([]byte("I am soooo encoded"))
+	epoch := "123456789"
+
+	sha256sig := cookieSignature(sha256.New, seed, key, value, epoch)
+	sha1sig := cookieSignature(sha1.New, seed, key, value, epoch)
+
+	assert.True(t, checkSignature(sha256sig, seed, key, value, epoch))
+	// This should be switched to False after fully deprecating SHA1
+	assert.True(t, checkSignature(sha1sig, seed, key, value, epoch))
+
+	assert.False(t, checkSignature(sha256sig, seed, key, "tampered", epoch))
+	assert.False(t, checkSignature(sha1sig, seed, key, "tampered", epoch))
+}
+
 func TestEncodeAndDecodeAccessToken(t *testing.T) {
 	const secret = "0123456789abcdefghijklmnopqrstuv"
 	const token = "my access token"