Merge branch 'security-1-5-access-token-recovery' into '1-5-stable'v1.5.11-5-stable-for-com1-5-stable

Encrypt cookies

See merge request gitlab/gitlab-pages!9

6 files changed, 155 insertions, 18 deletions

diff --git a/CHANGELOG b/CHANGELOG
index cbdfc814..2990353c 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,3 +1,7 @@
+v 1.5.1
+
+- Security fix for recovering gitlab access token from cookies
+
 v 1.5.0
 
 - Make extensionless URLs work !112
diff --git a/VERSION b/VERSION
index bc80560f..26ca5946 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-1.5.0
+1.5.1
diff --git a/internal/auth/auth.go b/internal/auth/auth.go
index 852d462b..0ca1936c 100644
--- a/internal/auth/auth.go
+++ b/internal/auth/auth.go
@@ -1,10 +1,12 @@
 package auth
 
 import (
+	"crypto/sha256"
 	"encoding/base64"
 	"encoding/json"
 	"errors"
 	"fmt"
+	"io"
 	"net"
 	"net/http"
 	"net/url"
@@ -19,6 +21,8 @@ import (
 	"gitlab.com/gitlab-org/gitlab-pages/internal/domain"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/httperrors"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/httptransport"
+
+	"golang.org/x/crypto/hkdf"
 )
 
 const (
@@ -499,9 +503,29 @@ func logRequest(r *http.Request) *log.Entry {
 	})
 }
 
+// generateKeyPair returns key pair for secure cookie: signing and encryption key
+func generateKeyPair(storeSecret string) ([]byte, []byte) {
+	hash := sha256.New
+	hkdf := hkdf.New(hash, []byte(storeSecret), []byte{}, []byte("PAGES_SIGNING_AND_ENCRYPTION_KEY"))
+	var keys [][]byte
+	for i := 0; i < 2; i++ {
+		key := make([]byte, 32)
+		if _, err := io.ReadFull(hkdf, key); err != nil {
+			log.WithError(err).Fatal("Can't generate key pair for secure cookies")
+		}
+		keys = append(keys, key)
+	}
+	return keys[0], keys[1]
+}
+
+func createCookieStore(storeSecret string) sessions.Store {
+	return sessions.NewCookieStore(generateKeyPair(storeSecret))
+}
+
 // New when authentication supported this will be used to create authentication handler
 func New(pagesDomain string, storeSecret string, clientID string, clientSecret string,
 	redirectURI string, gitLabServer string) *Auth {
+
 	return &Auth{
 		pagesDomain:  pagesDomain,
 		clientID:     clientID,
@@ -512,6 +536,6 @@ func New(pagesDomain string, storeSecret string, clientID string, clientSecret s
 			Timeout:   5 * time.Second,
 			Transport: httptransport.Transport,
 		},
-		store: sessions.NewCookieStore([]byte(storeSecret)),
+		store: createCookieStore(storeSecret),
 	}
 }
diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go
index ed130caf..2fbbb938 100644
--- a/internal/auth/auth_test.go
+++ b/internal/auth/auth_test.go
@@ -1,4 +1,4 @@
-package auth_test
+package auth
 
 import (
 	"fmt"
@@ -12,12 +12,11 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
-	"gitlab.com/gitlab-org/gitlab-pages/internal/auth"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/domain"
 )
 
-func createAuth(t *testing.T) *auth.Auth {
-	return auth.New("pages.gitlab-example.com",
+func createAuth(t *testing.T) *Auth {
+	return New("pages.gitlab-example.com",
 		"something-very-secret",
 		"id",
 		"secret",
@@ -25,6 +24,10 @@ func createAuth(t *testing.T) *auth.Auth {
 		"http://gitlab-example.com")
 }
 
+func defaultCookieStore() sessions.Store {
+	return createCookieStore("something-very-secret")
+}
+
 func TestTryAuthenticate(t *testing.T) {
 	auth := createAuth(t)
 
@@ -85,8 +88,8 @@ func TestTryAuthenticateWithCodeAndState(t *testing.T) {
 	apiServer.Start()
 	defer apiServer.Close()
 
-	store := sessions.NewCookieStore([]byte("something-very-secret"))
-	auth := auth.New("pages.gitlab-example.com",
+	store := defaultCookieStore()
+	auth := New("pages.gitlab-example.com",
 		"something-very-secret",
 		"id",
 		"secret",
@@ -124,8 +127,8 @@ func TestCheckAuthenticationWhenAccess(t *testing.T) {
 	apiServer.Start()
 	defer apiServer.Close()
 
-	store := sessions.NewCookieStore([]byte("something-very-secret"))
-	auth := auth.New("pages.gitlab-example.com",
+	store := defaultCookieStore()
+	auth := New("pages.gitlab-example.com",
 		"something-very-secret",
 		"id",
 		"secret",
@@ -161,8 +164,8 @@ func TestCheckAuthenticationWhenNoAccess(t *testing.T) {
 	apiServer.Start()
 	defer apiServer.Close()
 
-	store := sessions.NewCookieStore([]byte("something-very-secret"))
-	auth := auth.New("pages.gitlab-example.com",
+	store := defaultCookieStore()
+	auth := New("pages.gitlab-example.com",
 		"something-very-secret",
 		"id",
 		"secret",
@@ -199,8 +202,8 @@ func TestCheckAuthenticationWhenInvalidToken(t *testing.T) {
 	apiServer.Start()
 	defer apiServer.Close()
 
-	store := sessions.NewCookieStore([]byte("something-very-secret"))
-	auth := auth.New("pages.gitlab-example.com",
+	store := defaultCookieStore()
+	auth := New("pages.gitlab-example.com",
 		"something-very-secret",
 		"id",
 		"secret",
@@ -236,8 +239,8 @@ func TestCheckAuthenticationWithoutProject(t *testing.T) {
 	apiServer.Start()
 	defer apiServer.Close()
 
-	store := sessions.NewCookieStore([]byte("something-very-secret"))
-	auth := auth.New("pages.gitlab-example.com",
+	store := defaultCookieStore()
+	auth := New("pages.gitlab-example.com",
 		"something-very-secret",
 		"id",
 		"secret",
@@ -274,8 +277,8 @@ func TestCheckAuthenticationWithoutProjectWhenInvalidToken(t *testing.T) {
 	apiServer.Start()
 	defer apiServer.Close()
 
-	store := sessions.NewCookieStore([]byte("something-very-secret"))
-	auth := auth.New("pages.gitlab-example.com",
+	store := defaultCookieStore()
+	auth := New("pages.gitlab-example.com",
 		"something-very-secret",
 		"id",
 		"secret",
@@ -294,3 +297,10 @@ func TestCheckAuthenticationWithoutProjectWhenInvalidToken(t *testing.T) {
 	assert.Equal(t, true, auth.CheckAuthenticationWithoutProject(result, r))
 	assert.Equal(t, 302, result.Code)
 }
+
+func TestGenerateKeyPair(t *testing.T) {
+	signingSecret, encryptionSecret := generateKeyPair("something-very-secret")
+	assert.NotEqual(t, fmt.Sprint(signingSecret), fmt.Sprint(encryptionSecret))
+	assert.Equal(t, len(signingSecret), 32)
+	assert.Equal(t, len(encryptionSecret), 32)
+}
diff --git a/vendor/golang.org/x/crypto/hkdf/hkdf.go b/vendor/golang.org/x/crypto/hkdf/hkdf.go
new file mode 100644
index 00000000..dda3f143
--- /dev/null
+++ b/vendor/golang.org/x/crypto/hkdf/hkdf.go
@@ -0,0 +1,93 @@
+// Copyright 2014 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Package hkdf implements the HMAC-based Extract-and-Expand Key Derivation
+// Function (HKDF) as defined in RFC 5869.
+//
+// HKDF is a cryptographic key derivation function (KDF) with the goal of
+// expanding limited input keying material into one or more cryptographically
+// strong secret keys.
+package hkdf // import "golang.org/x/crypto/hkdf"
+
+import (
+	"crypto/hmac"
+	"errors"
+	"hash"
+	"io"
+)
+
+// Extract generates a pseudorandom key for use with Expand from an input secret
+// and an optional independent salt.
+//
+// Only use this function if you need to reuse the extracted key with multiple
+// Expand invocations and different context values. Most common scenarios,
+// including the generation of multiple keys, should use New instead.
+func Extract(hash func() hash.Hash, secret, salt []byte) []byte {
+	if salt == nil {
+		salt = make([]byte, hash().Size())
+	}
+	extractor := hmac.New(hash, salt)
+	extractor.Write(secret)
+	return extractor.Sum(nil)
+}
+
+type hkdf struct {
+	expander hash.Hash
+	size     int
+
+	info    []byte
+	counter byte
+
+	prev []byte
+	buf  []byte
+}
+
+func (f *hkdf) Read(p []byte) (int, error) {
+	// Check whether enough data can be generated
+	need := len(p)
+	remains := len(f.buf) + int(255-f.counter+1)*f.size
+	if remains < need {
+		return 0, errors.New("hkdf: entropy limit reached")
+	}
+	// Read any leftover from the buffer
+	n := copy(p, f.buf)
+	p = p[n:]
+
+	// Fill the rest of the buffer
+	for len(p) > 0 {
+		f.expander.Reset()
+		f.expander.Write(f.prev)
+		f.expander.Write(f.info)
+		f.expander.Write([]byte{f.counter})
+		f.prev = f.expander.Sum(f.prev[:0])
+		f.counter++
+
+		// Copy the new batch into p
+		f.buf = f.prev
+		n = copy(p, f.buf)
+		p = p[n:]
+	}
+	// Save leftovers for next run
+	f.buf = f.buf[n:]
+
+	return need, nil
+}
+
+// Expand returns a Reader, from which keys can be read, using the given
+// pseudorandom key and optional context info, skipping the extraction step.
+//
+// The pseudorandomKey should have been generated by Extract, or be a uniformly
+// random or pseudorandom cryptographically strong key. See RFC 5869, Section
+// 3.3. Most common scenarios will want to use New instead.
+func Expand(hash func() hash.Hash, pseudorandomKey, info []byte) io.Reader {
+	expander := hmac.New(hash, pseudorandomKey)
+	return &hkdf{expander, expander.Size(), info, 1, nil, nil}
+}
+
+// New returns a Reader, from which keys can be read, using the given hash,
+// secret, salt and context info. Salt and info can be nil.
+func New(hash func() hash.Hash, secret, salt, info []byte) io.Reader {
+	prk := Extract(hash, secret, salt)
+	return Expand(hash, prk, info)
+}
diff --git a/vendor/vendor.json b/vendor/vendor.json
index 1df7f7ef..b71c15cd 100644
--- a/vendor/vendor.json
+++ b/vendor/vendor.json
@@ -319,6 +319,12 @@
 			"revisionTime": "2018-03-07T00:01:49Z"
 		},
 		{
+			"checksumSHA1": "ELSEW2KG0p3oua5lIxl1xW2oFBo=",
+			"path": "golang.org/x/crypto/hkdf",
+			"revision": "4def268fd1a49955bfb3dda92fe3db4f924f2285",
+			"revisionTime": "2019-07-01T08:59:26Z"
+		},
+		{
 			"checksumSHA1": "6U7dCaxxIMjf5V02iWgyAwppczw=",
 			"path": "golang.org/x/crypto/ssh/terminal",
 			"revision": "91a49db82a88618983a78a06c1cbd4e00ab749ab",