From b05b98fc30c615f8c45be6465411fa37c8dcdcff Mon Sep 17 00:00:00 2001 From: Vladimir Shushlin Date: Tue, 30 Jul 2019 10:28:44 +0300 Subject: [PATCH 1/4] Release 1.5.1 --- CHANGELOG | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index 72030bd2a..01f8d0adc 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -15,6 +15,10 @@ v 1.6.0 - Disable 3DES and other insecure cipher suites !145 - Provide ability to disable old TLS versions !146 +v 1.5.1 + +- Security fix for recovering gitlab access token from cookies + v 1.5.0 - Make extensionless URLs work !112 -- GitLab From b392f66121ce7eac8776f98db72ba755d1bd2e37 Mon Sep 17 00:00:00 2001 From: Vladimir Shushlin Date: Tue, 30 Jul 2019 10:28:44 +0300 Subject: [PATCH 2/4] Release 1.6.2 --- CHANGELOG | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index 01f8d0adc..87e86b65d 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -2,6 +2,10 @@ v 1.7.0 - Add support for Sentry error reporting +v 1.6.2 + +- Security fix for recovering gitlab access token from cookies + v 1.6.1 - Fix serving acme challenges with index.html -- GitLab From 5068f12f32548cc0cd866147ec04f755cb56d5d2 Mon Sep 17 00:00:00 2001 From: Vladimir Shushlin Date: Tue, 30 Jul 2019 10:28:44 +0300 Subject: [PATCH 3/4] Release 1.7.1 --- CHANGELOG | 4 ++++ VERSION | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 87e86b65d..7e78a2c96 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,7 @@ +v 1.7.1 + +- Security fix for recovering gitlab access token from cookies + v 1.7.0 - Add support for Sentry error reporting diff --git a/VERSION b/VERSION index bd8bf882d..943f9cbc4 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.7.0 +1.7.1 -- GitLab From d28870f3956c5c04572ccea6388c06aaf916824d Mon Sep 17 00:00:00 2001 From: Vladimir Shushlin Date: Tue, 30 Jul 2019 10:28:25 +0300 Subject: [PATCH 4/4] Fix recovering gitlab api token from session Generate signing and encryption secrets from one auth-secret using hkdf --- internal/auth/auth.go | 26 ++++++- internal/auth/auth_test.go | 42 ++++++----- vendor/golang.org/x/crypto/hkdf/hkdf.go | 93 +++++++++++++++++++++++++ vendor/vendor.json | 6 ++ 4 files changed, 150 insertions(+), 17 deletions(-) create mode 100644 vendor/golang.org/x/crypto/hkdf/hkdf.go diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 6cbf08427..d6cbdff16 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" @@ -20,6 +22,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 ( @@ -555,9 +559,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, @@ -568,6 +592,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 ed130cafd..2fbbb938e 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 000000000..dda3f143b --- /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 2b2f14209..7cbfbe3ae 100644 --- a/vendor/vendor.json +++ b/vendor/vendor.json @@ -332,6 +332,12 @@ "revision": "e8af1d65987739a7ab277dedd9772c7398154ff3", "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", -- GitLab