From 7ba613c3b9b45c076e552d16c33abaef7658a219 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Tue, 5 Dec 2017 10:20:14 +0700 Subject: [PATCH] use the mint default cookie protector to encrypt and decrypt cookies --- internal/crypto/source_address_token.go | 76 ------------------- internal/crypto/source_address_token_test.go | 41 ---------- internal/handshake/cookie_generator.go | 12 +-- internal/handshake/cookie_generator_test.go | 6 +- .../handshake/crypto_setup_server_test.go | 14 ++-- 5 files changed, 17 insertions(+), 132 deletions(-) delete mode 100644 internal/crypto/source_address_token.go delete mode 100644 internal/crypto/source_address_token_test.go diff --git a/internal/crypto/source_address_token.go b/internal/crypto/source_address_token.go deleted file mode 100644 index 3dcb26a75..000000000 --- a/internal/crypto/source_address_token.go +++ /dev/null @@ -1,76 +0,0 @@ -package crypto - -import ( - "crypto/aes" - "crypto/cipher" - "crypto/rand" - "crypto/sha256" - "fmt" - "io" - - "golang.org/x/crypto/hkdf" -) - -// StkSource is used to create and verify source address tokens -type StkSource interface { - // NewToken creates a new token - NewToken([]byte) ([]byte, error) - // DecodeToken decodes a token - DecodeToken([]byte) ([]byte, error) -} - -type stkSource struct { - aead cipher.AEAD -} - -const stkKeySize = 16 - -// Chrome currently sets this to 12, but discusses changing it to 16. We start -// at 16 :) -const stkNonceSize = 16 - -// NewStkSource creates a source for source address tokens -func NewStkSource() (StkSource, error) { - secret := make([]byte, 32) - if _, err := rand.Read(secret); err != nil { - return nil, err - } - key, err := deriveKey(secret) - if err != nil { - return nil, err - } - c, err := aes.NewCipher(key) - if err != nil { - return nil, err - } - aead, err := cipher.NewGCMWithNonceSize(c, stkNonceSize) - if err != nil { - return nil, err - } - return &stkSource{aead: aead}, nil -} - -func (s *stkSource) NewToken(data []byte) ([]byte, error) { - nonce := make([]byte, stkNonceSize) - if _, err := rand.Read(nonce); err != nil { - return nil, err - } - return s.aead.Seal(nonce, nonce, data, nil), nil -} - -func (s *stkSource) DecodeToken(p []byte) ([]byte, error) { - if len(p) < stkNonceSize { - return nil, fmt.Errorf("STK too short: %d", len(p)) - } - nonce := p[:stkNonceSize] - return s.aead.Open(nil, nonce, p[stkNonceSize:], nil) -} - -func deriveKey(secret []byte) ([]byte, error) { - r := hkdf.New(sha256.New, secret, nil, []byte("QUIC source address token key")) - key := make([]byte, stkKeySize) - if _, err := io.ReadFull(r, key); err != nil { - return nil, err - } - return key, nil -} diff --git a/internal/crypto/source_address_token_test.go b/internal/crypto/source_address_token_test.go deleted file mode 100644 index d25a2ba99..000000000 --- a/internal/crypto/source_address_token_test.go +++ /dev/null @@ -1,41 +0,0 @@ -package crypto - -import ( - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" -) - -var _ = Describe("Source Address Tokens", func() { - It("should generate the encryption key", func() { - Expect(deriveKey([]byte("TESTING"))).To(Equal([]byte{0xee, 0x71, 0x18, 0x9, 0xfd, 0xb8, 0x9a, 0x79, 0x19, 0xfc, 0x5e, 0x1a, 0x97, 0x20, 0xb2, 0x6})) - }) - - Context("tokens", func() { - var source *stkSource - - BeforeEach(func() { - sourceI, err := NewStkSource() - source = sourceI.(*stkSource) - Expect(err).NotTo(HaveOccurred()) - }) - - It("serializes", func() { - token, err := source.NewToken([]byte("foobar")) - Expect(err).ToNot(HaveOccurred()) - data, err := source.DecodeToken(token) - Expect(err).ToNot(HaveOccurred()) - Expect(data).To(Equal([]byte("foobar"))) - }) - - It("rejects invalid tokens", func() { - _, err := source.DecodeToken([]byte("invalid source address token")) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("message authentication failed")) - }) - - It("rejects tokens of wrong size", func() { - _, err := source.DecodeToken(nil) - Expect(err).To(MatchError("STK too short: 0")) - }) - }) -}) diff --git a/internal/handshake/cookie_generator.go b/internal/handshake/cookie_generator.go index 10281fa68..97accb73d 100644 --- a/internal/handshake/cookie_generator.go +++ b/internal/handshake/cookie_generator.go @@ -6,7 +6,7 @@ import ( "net" "time" - "github.com/lucas-clemente/quic-go/internal/crypto" + "github.com/bifurcation/mint" ) const ( @@ -29,17 +29,17 @@ type token struct { // A CookieGenerator generates Cookies type CookieGenerator struct { - cookieSource crypto.StkSource + cookieProtector mint.CookieProtector } // NewCookieGenerator initializes a new CookieGenerator func NewCookieGenerator() (*CookieGenerator, error) { - stkSource, err := crypto.NewStkSource() + cookieProtector, err := mint.NewDefaultCookieProtector() if err != nil { return nil, err } return &CookieGenerator{ - cookieSource: stkSource, + cookieProtector: cookieProtector, }, nil } @@ -52,7 +52,7 @@ func (g *CookieGenerator) NewToken(raddr net.Addr) ([]byte, error) { if err != nil { return nil, err } - return g.cookieSource.NewToken(data) + return g.cookieProtector.NewToken(data) } // DecodeToken decodes a Cookie @@ -62,7 +62,7 @@ func (g *CookieGenerator) DecodeToken(encrypted []byte) (*Cookie, error) { return nil, nil } - data, err := g.cookieSource.DecodeToken(encrypted) + data, err := g.cookieProtector.DecodeToken(encrypted) if err != nil { return nil, err } diff --git a/internal/handshake/cookie_generator_test.go b/internal/handshake/cookie_generator_test.go index 7f908e6b1..f04807019 100644 --- a/internal/handshake/cookie_generator_test.go +++ b/internal/handshake/cookie_generator_test.go @@ -49,7 +49,7 @@ var _ = Describe("Cookie Generator", func() { }) It("rejects tokens that cannot be decoded", func() { - token, err := cookieGen.cookieSource.NewToken([]byte("foobar")) + token, err := cookieGen.cookieProtector.NewToken([]byte("foobar")) Expect(err).ToNot(HaveOccurred()) _, err = cookieGen.DecodeToken(token) Expect(err).To(HaveOccurred()) @@ -59,7 +59,7 @@ var _ = Describe("Cookie Generator", func() { t, err := asn1.Marshal(token{Data: []byte("foobar")}) Expect(err).ToNot(HaveOccurred()) t = append(t, []byte("rest")...) - enc, err := cookieGen.cookieSource.NewToken(t) + enc, err := cookieGen.cookieProtector.NewToken(t) Expect(err).ToNot(HaveOccurred()) _, err = cookieGen.DecodeToken(enc) Expect(err).To(MatchError("rest when unpacking token: 4")) @@ -69,7 +69,7 @@ var _ = Describe("Cookie Generator", func() { It("doesn't panic if a tokens has no data", func() { t, err := asn1.Marshal(token{Data: []byte("")}) Expect(err).ToNot(HaveOccurred()) - enc, err := cookieGen.cookieSource.NewToken(t) + enc, err := cookieGen.cookieProtector.NewToken(t) Expect(err).ToNot(HaveOccurred()) _, err = cookieGen.DecodeToken(enc) Expect(err).ToNot(HaveOccurred()) diff --git a/internal/handshake/crypto_setup_server_test.go b/internal/handshake/crypto_setup_server_test.go index 77ec1cef3..e979ceb19 100644 --- a/internal/handshake/crypto_setup_server_test.go +++ b/internal/handshake/crypto_setup_server_test.go @@ -7,6 +7,8 @@ import ( "net" "time" + "github.com/bifurcation/mint" + "github.com/lucas-clemente/quic-go/internal/crypto" "github.com/lucas-clemente/quic-go/internal/mocks/crypto" "github.com/lucas-clemente/quic-go/internal/protocol" @@ -91,18 +93,18 @@ func (s *mockStream) Reset(error) { panic("not implemente func (mockStream) CloseRemote(offset protocol.ByteCount) { panic("not implemented") } func (s mockStream) StreamID() protocol.StreamID { panic("not implemented") } -type mockCookieSource struct { +type mockCookieProtector struct { data []byte decodeErr error } -var _ crypto.StkSource = &mockCookieSource{} +var _ mint.CookieProtector = &mockCookieProtector{} -func (mockCookieSource) NewToken(sourceAddr []byte) ([]byte, error) { +func (mockCookieProtector) NewToken(sourceAddr []byte) ([]byte, error) { return append([]byte("token "), sourceAddr...), nil } -func (s mockCookieSource) DecodeToken(data []byte) ([]byte, error) { +func (s mockCookieProtector) DecodeToken(data []byte) ([]byte, error) { if s.decodeErr != nil { return nil, s.decodeErr } @@ -170,7 +172,7 @@ var _ = Describe("Server Crypto Setup", func() { ) Expect(err).NotTo(HaveOccurred()) cs = csInt.(*cryptoSetupServer) - cs.scfg.cookieGenerator.cookieSource = &mockCookieSource{} + cs.scfg.cookieGenerator.cookieProtector = &mockCookieProtector{} validSTK, err = cs.scfg.cookieGenerator.NewToken(remoteAddr) Expect(err).NotTo(HaveOccurred()) sourceAddrValid = true @@ -409,7 +411,7 @@ var _ = Describe("Server Crypto Setup", func() { It("recognizes inchoate CHLOs with an invalid STK", func() { testErr := errors.New("STK invalid") - cs.scfg.cookieGenerator.cookieSource.(*mockCookieSource).decodeErr = testErr + cs.scfg.cookieGenerator.cookieProtector.(*mockCookieProtector).decodeErr = testErr Expect(cs.isInchoateCHLO(fullCHLO, cert)).To(BeTrue()) })