internal/aead : replace miscreant with xchacha20poly1305

authenticate/authenticate.go

@@ -149,7 +149,7 @@ func NewAuthenticator(opts *Options, optionFuncs ...func(*Authenticator) error)
 	if err != nil {
 		return nil, err
 	}
-	cipher, err := aead.NewMiscreantCipher([]byte(decodedAuthCodeSecret))
+	cipher, err := aead.New([]byte(decodedAuthCodeSecret))
 	if err != nil {
 		return nil, err
 	}

go.mod

@@ -3,7 +3,6 @@ module github.com/pomerium/pomerium
 require (
 	github.com/benbjohnson/clock v0.0.0-20161215174838-7dc76406b6d3
 	github.com/davecgh/go-spew v1.1.1 // indirect
-	github.com/miscreant/miscreant-go v0.0.0-20181010193435-325cbd69228b
 	github.com/pmezard/go-difflib v1.0.0 // indirect
 	github.com/pomerium/envconfig v1.3.1-0.20190112072701-14cbcf832d31
 	github.com/pomerium/go-oidc v2.0.0+incompatible
@@ -14,6 +13,7 @@ require (
 	golang.org/x/net v0.0.0-20181220203305-927f97764cc3 // indirect
 	golang.org/x/oauth2 v0.0.0-20181203162652-d668ce993890
 	golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4 // indirect
+	golang.org/x/sys v0.0.0-20190116161447-11f53e031339 // indirect
 	google.golang.org/appengine v1.4.0 // indirect
 	gopkg.in/square/go-jose.v2 v2.2.1 // indirect
 )

go.sum

@@ -4,12 +4,8 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/golang/protobuf v1.2.0 h1:P3YflyNX/ehuJFLhxviNdFxQPkGK5cDcApsge1SqnvM=
 github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
-github.com/miscreant/miscreant-go v0.0.0-20181010193435-325cbd69228b h1:VPhrxAgvd0d0xSZP4P8zzWZntso2NE0m69dmDEXM53I=
-github.com/miscreant/miscreant-go v0.0.0-20181010193435-325cbd69228b/go.mod h1:Vj6lPE3LxPymcFxg7hm9aDIJWCyhJMnxSNC/y9ZHtN8=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
-github.com/pomerium/envconfig v1.3.1-0.20180517194557-dd1402a4d99d h1:iAavGhspmmDa8LCcQLpHV/JBPWKL7EfvMN7YrGHBN3o=
-github.com/pomerium/envconfig v1.3.1-0.20180517194557-dd1402a4d99d/go.mod h1:1Kz8Ca8PhJDtLYqgvbDZGn6GsJCvrT52SxQ3sPNJkDc=
 github.com/pomerium/envconfig v1.3.1-0.20190112072701-14cbcf832d31 h1:bNqUesLWa+RUxQvSaV3//dEFviXdCSvMF9GKDOopFLU=
 github.com/pomerium/envconfig v1.3.1-0.20190112072701-14cbcf832d31/go.mod h1:1Kz8Ca8PhJDtLYqgvbDZGn6GsJCvrT52SxQ3sPNJkDc=
 github.com/pomerium/go-oidc v2.0.0+incompatible h1:gVvG/ExWsHQqatV+uceROnGmbVYF44mDNx5nayBhC0o=
@@ -29,6 +25,8 @@ golang.org/x/oauth2 v0.0.0-20181203162652-d668ce993890 h1:uESlIz09WIHT2I+pasSXcp
 golang.org/x/oauth2 v0.0.0-20181203162652-d668ce993890/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
 golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4 h1:YUO/7uOKsKeq9UokNS62b8FYywz3ker1l1vDZRCRefw=
 golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
+golang.org/x/sys v0.0.0-20190116161447-11f53e031339 h1:g/Jesu8+QLnA0CPzF3E1pURg0Byr7i6jLoX5sqjcAh0=
+golang.org/x/sys v0.0.0-20190116161447-11f53e031339/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 google.golang.org/appengine v1.4.0 h1:/wp5JvzpHIxhs/dumFmF7BXTf3Z+dd4uXta4kVyO508=
 google.golang.org/appengine v1.4.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4=

internal/aead/aead.go

@@ -4,19 +4,16 @@ import (
 	"bytes"
 	"compress/gzip"
 	"crypto/cipher"
+	"crypto/rand"
 	"encoding/base64"
 	"encoding/json"
 	"fmt"
 	"io"
 	"sync"
 
-	miscreant "github.com/miscreant/miscreant-go"
+	"golang.org/x/crypto/chacha20poly1305"
 )
 
-const miscreantNonceSize = 16
-
-var algorithmType = "AES-CMAC-SIV"
-
 // Cipher provides methods to encrypt and decrypt values.
 type Cipher interface {
 	Encrypt([]byte) ([]byte, error)
@@ -25,42 +22,58 @@ type Cipher interface {
 	Unmarshal(string, interface{}) error
 }
 
-// MiscreantCipher provides methods to encrypt and decrypt values.
+// XChaCha20Cipher provides methods to encrypt and decrypt values.
 // Using an AEAD is a cipher providing authenticated encryption with associated data.
 // For a description of the methodology, see https://en.wikipedia.org/wiki/Authenticated_encryption
-type MiscreantCipher struct {
+type XChaCha20Cipher struct {
 	aead cipher.AEAD
 
-	mux sync.Mutex
+	mu sync.Mutex
 }
 
-// NewMiscreantCipher returns a new AES Cipher for encrypting values
+// New returns a new AES Cipher for encrypting values
-func NewMiscreantCipher(secret []byte) (*MiscreantCipher, error) {
+func New(secret []byte) (*XChaCha20Cipher, error) {
-	aead, err := miscreant.NewAEAD(algorithmType, secret, miscreantNonceSize)
+	aead, err := chacha20poly1305.NewX(secret)
 	if err != nil {
 		return nil, err
 	}
-	return &MiscreantCipher{
+	return &XChaCha20Cipher{
 		aead: aead,
 	}, nil
 }
 
-// GenerateKey wraps miscreant's GenerateKey function
+// GenerateKey generates a random 32-byte encryption key.
+// Panics if the key size is unsupported or source of randomness fails.
 func GenerateKey() []byte {
-	return miscreant.GenerateKey(32)
+	nonce := make([]byte, chacha20poly1305.KeySize)
+	if _, err := rand.Read(nonce); err != nil {
+		panic(err)
+	}
+	return nonce
+}
+
+// GenerateNonce generates a random 24-byte nonce for XChaCha20-Poly1305.
+// Panics if the key size is unsupported or source of randomness fails.
+func GenerateNonce() []byte {
+	nonce := make([]byte, chacha20poly1305.NonceSizeX)
+	if _, err := rand.Read(nonce); err != nil {
+		panic(err)
+	}
+	return nonce
 }
 
 // Encrypt a value using AES-CMAC-SIV
-func (c *MiscreantCipher) Encrypt(plaintext []byte) (joined []byte, err error) {
+func (c *XChaCha20Cipher) Encrypt(plaintext []byte) (joined []byte, err error) {
-	c.mux.Lock()
+	c.mu.Lock()
-	defer c.mux.Unlock()
+	defer c.mu.Unlock()
 
 	defer func() {
 		if r := recover(); r != nil {
-			err = fmt.Errorf("miscreant error encrypting bytes: %v", r)
+			err = fmt.Errorf("internal/aead: error encrypting bytes: %v", r)
 		}
 	}()
-	nonce := miscreant.GenerateNonce(c.aead)
+	nonce := GenerateNonce()
+
 	ciphertext := c.aead.Seal(nil, nonce, plaintext, nil)
 
 	// we return the nonce as part of the returned value
@@ -69,15 +82,15 @@ func (c *MiscreantCipher) Encrypt(plaintext []byte) (joined []byte, err error) {
 }
 
 // Decrypt a value using AES-CMAC-SIV
-func (c *MiscreantCipher) Decrypt(joined []byte) ([]byte, error) {
+func (c *XChaCha20Cipher) Decrypt(joined []byte) ([]byte, error) {
-	c.mux.Lock()
+	c.mu.Lock()
-	defer c.mux.Unlock()
+	defer c.mu.Unlock()
 
-	if len(joined) <= miscreantNonceSize {
+	if len(joined) <= chacha20poly1305.NonceSizeX {
-		return nil, fmt.Errorf("invalid input size: %d", len(joined))
+		return nil, fmt.Errorf("internal/aead: invalid input size: %d", len(joined))
 	}
 	// grab out the nonce
-	pivot := len(joined) - miscreantNonceSize
+	pivot := len(joined) - chacha20poly1305.NonceSizeX
 	ciphertext := joined[:pivot]
 	nonce := joined[pivot:]
 
@@ -91,22 +104,22 @@ func (c *MiscreantCipher) Decrypt(joined []byte) ([]byte, error) {
 
 // Marshal marshals the interface state as JSON, encrypts the JSON using the cipher
 // and base64 encodes the binary value as a string and returns the result
-func (c *MiscreantCipher) Marshal(s interface{}) (string, error) {
+func (c *XChaCha20Cipher) Marshal(s interface{}) (string, error) {
 	// encode json value
 	plaintext, err := json.Marshal(s)
 	if err != nil {
 		return "", err
 	}
+	// compress the plaintext bytes
 	compressed, err := compress(plaintext)
 	if err != nil {
 		return "", err
 	}
-	// encrypt the JSON
+	// encrypt the compressed JSON bytes
 	ciphertext, err := c.Encrypt(compressed)
 	if err != nil {
 		return "", err
 	}
-
 	// base64-encode the result
 	encoded := base64.RawURLEncoding.EncodeToString(ciphertext)
 	return encoded, nil
@@ -114,24 +127,23 @@ func (c *MiscreantCipher) Marshal(s interface{}) (string, error) {
 
 // Unmarshal takes the marshaled string, base64-decodes into a byte slice, decrypts the
 // byte slice the passed cipher, and unmarshals the resulting JSON into the struct pointer passed
-func (c *MiscreantCipher) Unmarshal(value string, s interface{}) error {
+func (c *XChaCha20Cipher) Unmarshal(value string, s interface{}) error {
 	// convert base64 string value to bytes
 	ciphertext, err := base64.RawURLEncoding.DecodeString(value)
 	if err != nil {
 		return err
 	}
-
 	// decrypt the bytes
 	compressed, err := c.Decrypt(ciphertext)
 	if err != nil {
 		return err
 	}
-	// decompress
+	// decompress the unencrypted bytes
 	plaintext, err := decompress(compressed)
 	if err != nil {
 		return err
 	}
-	// unmarshal bytes
+	// unmarshal the unencrypted bytes
 	err = json.Unmarshal(plaintext, s)
 	if err != nil {
 		return err
@@ -144,13 +156,13 @@ func compress(data []byte) ([]byte, error) {
 	var buf bytes.Buffer
 	writer, err := gzip.NewWriterLevel(&buf, gzip.DefaultCompression)
 	if err != nil {
-		return nil, fmt.Errorf("aead/compress: failed to create a gzip writer: %q", err)
+		return nil, fmt.Errorf("internal/aead: failed to create a gzip writer: %q", err)
 	}
 	if writer == nil {
-		return nil, fmt.Errorf("aead/compress: failed to create a gzip writer")
+		return nil, fmt.Errorf("internal/aead: failed to create a gzip writer")
 	}
 	if _, err = writer.Write(data); err != nil {
-		return nil, fmt.Errorf("aead/compress: failed to compress data with err: %q", err)
+		return nil, fmt.Errorf("internal/aead: failed to compress data with err: %q", err)
 	}
 	if err = writer.Close(); err != nil {
 		return nil, err
@@ -161,7 +173,7 @@ func compress(data []byte) ([]byte, error) {
 func decompress(data []byte) ([]byte, error) {
 	reader, err := gzip.NewReader(bytes.NewReader(data))
 	if err != nil {
-		return nil, fmt.Errorf("aead/compress: failed to create a gzip reader: %q", err)
+		return nil, fmt.Errorf("internal/aead: failed to create a gzip reader: %q", err)
 	}
 	defer reader.Close()
 	var buf bytes.Buffer

internal/aead/aead_test.go

@@ -13,7 +13,7 @@ func TestEncodeAndDecodeAccessToken(t *testing.T) {
 	plaintext := []byte("my plain text value")
 
 	key := GenerateKey()
-	c, err := NewMiscreantCipher([]byte(key))
+	c, err := New([]byte(key))
 	if err != nil {
 		t.Fatalf("unexpected err: %v", err)
 	}
@@ -42,7 +42,7 @@ func TestEncodeAndDecodeAccessToken(t *testing.T) {
 func TestMarshalAndUnmarshalStruct(t *testing.T) {
 	key := GenerateKey()
 
-	c, err := NewMiscreantCipher([]byte(key))
+	c, err := New([]byte(key))
 	if err != nil {
 		t.Fatalf("unexpected err: %v", err)
 	}
@@ -94,17 +94,8 @@ func TestMarshalAndUnmarshalStruct(t *testing.T) {
 	}
 }
 
-// TestCipherDataRace exercises a simple concurrency test for the MicscreantCipher.
-// In https://github.com/pomerium/pomerium/pull/75 we investigated why, on random occasion,
-// unmarshalling session states would fail, triggering users to get kicked out of
-// authenticated states. We narrowed our investigation to a data race we uncovered
-// from our misuse of the underlying miscreant library which makes no attempt
-// at thread-safety.
-//
-// In https://github.com/pomerium/pomerium/pull/77 we added this test to exercise the
-// data race condition and resolved said race by introducing a simple mutex.
 func TestCipherDataRace(t *testing.T) {
-	miscreantCipher, err := NewMiscreantCipher(GenerateKey())
+	miscreantCipher, err := New(GenerateKey())
 	if err != nil {
 		t.Fatalf("unexpected generating cipher err: %v", err)
 	}
@@ -116,7 +107,7 @@ func TestCipherDataRace(t *testing.T) {
 	wg := &sync.WaitGroup{}
 	for i := 0; i < 100; i++ {
 		wg.Add(1)
-		go func(c *MiscreantCipher, wg *sync.WaitGroup) {
+		go func(c *XChaCha20Cipher, wg *sync.WaitGroup) {
 			defer wg.Done()
 			b := make([]byte, 32)
 			_, err := rand.Read(b)

internal/sessions/cookie_store.go

@@ -43,7 +43,7 @@ type CookieStore struct {
 // CreateMiscreantCookieCipher creates a new miscreant cipher with the cookie secret
 func CreateMiscreantCookieCipher(cookieSecret []byte) func(s *CookieStore) error {
 	return func(s *CookieStore) error {
-		cipher, err := aead.NewMiscreantCipher(cookieSecret)
+		cipher, err := aead.New(cookieSecret)
 		if err != nil {
 			return fmt.Errorf("miscreant cookie-secret error: %s", err.Error())
 		}

internal/sessions/session_state_test.go

@@ -10,7 +10,7 @@ import (
 
 func TestSessionStateSerialization(t *testing.T) {
 	secret := aead.GenerateKey()
-	c, err := aead.NewMiscreantCipher([]byte(secret))
+	c, err := aead.New([]byte(secret))
 	if err != nil {
 		t.Fatalf("expected to be able to create cipher: %v", err)
 	}

proxy/proxy.go

@@ -144,7 +144,7 @@ func NewProxy(opts *Options) (*Proxy, error) {
 
 	// error explicitly handled by validate
 	decodedSecret, _ := base64.StdEncoding.DecodeString(opts.CookieSecret)
-	cipher, err := aead.NewMiscreantCipher(decodedSecret)
+	cipher, err := aead.New(decodedSecret)
 	if err != nil {
 		return nil, fmt.Errorf("cookie-secret error: %s", err.Error())
 	}