From eb72b494b24fe7b35c6c0807bd88f60805ee926e Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sat, 13 May 2017 14:51:41 +0800 Subject: [PATCH] generate valid tokens for remote addresses that are not UDP addresses --- crypto/source_address_token.go | 4 +-- crypto/source_address_token_test.go | 2 +- handshake/crypto_setup_server.go | 15 +++----- handshake/crypto_setup_server_test.go | 34 ------------------- handshake/stk_generator.go | 25 +++++++++++--- handshake/stk_generator_test.go | 49 +++++++++++++++++++++++---- 6 files changed, 70 insertions(+), 59 deletions(-) diff --git a/crypto/source_address_token.go b/crypto/source_address_token.go index 5bc5c1e6..df220221 100644 --- a/crypto/source_address_token.go +++ b/crypto/source_address_token.go @@ -37,8 +37,8 @@ func (t *sourceAddressToken) serialize() []byte { } func parseToken(data []byte) (*sourceAddressToken, error) { - if len(data) != 8+4 && len(data) != 8+16 { - return nil, fmt.Errorf("invalid STK length: %d", len(data)) + if len(data) < 8 { + return nil, fmt.Errorf("STK too short: %d", len(data)) } return &sourceAddressToken{ data: data[8:], diff --git a/crypto/source_address_token_test.go b/crypto/source_address_token_test.go index 843b6ed5..e0101925 100644 --- a/crypto/source_address_token_test.go +++ b/crypto/source_address_token_test.go @@ -36,7 +36,7 @@ var _ = Describe("Source Address Tokens", func() { It("rejects tokens of wrong size", func() { _, err := parseToken(nil) - Expect(err).To(MatchError("invalid STK length: 0")) + Expect(err).To(MatchError("STK too short: 0")) }) }) diff --git a/handshake/crypto_setup_server.go b/handshake/crypto_setup_server.go index a5f85766..e55dc7ee 100644 --- a/handshake/crypto_setup_server.go +++ b/handshake/crypto_setup_server.go @@ -25,7 +25,7 @@ type KeyExchangeFunction func() crypto.KeyExchange // The CryptoSetupServer handles all things crypto for the Session type cryptoSetupServer struct { connID protocol.ConnectionID - sourceAddr []byte + remoteAddr net.Addr scfg *ServerConfig stkGenerator *STKGenerator diversificationNonce []byte @@ -74,16 +74,9 @@ func NewCryptoSetup( return nil, err } - var sourceAddr []byte - if udpAddr, ok := remoteAddr.(*net.UDPAddr); ok { - sourceAddr = udpAddr.IP - } else { - sourceAddr = []byte(remoteAddr.String()) - } - return &cryptoSetupServer{ connID: connID, - sourceAddr: sourceAddr, + remoteAddr: remoteAddr, version: version, supportedVersions: supportedVersions, scfg: scfg, @@ -283,7 +276,7 @@ func (h *cryptoSetupServer) isInchoateCHLO(cryptoData map[Tag][]byte, cert []byt } func (h *cryptoSetupServer) verifySTK(stk []byte) bool { - stkTime, err := h.stkGenerator.VerifyToken(h.sourceAddr, stk) + stkTime, err := h.stkGenerator.VerifyToken(h.remoteAddr, stk) if err != nil { utils.Debugf("STK invalid: %s", err.Error()) return false @@ -299,7 +292,7 @@ func (h *cryptoSetupServer) handleInchoateCHLO(sni string, chlo []byte, cryptoDa return nil, qerr.Error(qerr.CryptoInvalidValueLength, "CHLO too small") } - token, err := h.stkGenerator.NewToken(h.sourceAddr) + token, err := h.stkGenerator.NewToken(h.remoteAddr) if err != nil { return nil, err } diff --git a/handshake/crypto_setup_server_test.go b/handshake/crypto_setup_server_test.go index df2ebd88..7af64aa8 100644 --- a/handshake/crypto_setup_server_test.go +++ b/handshake/crypto_setup_server_test.go @@ -228,40 +228,6 @@ var _ = Describe("Server Crypto Setup", func() { }) }) - Context("source address token", func() { - It("uses the IP address when the remote address is a UDP address", func() { - remoteAddr := &net.UDPAddr{IP: net.IPv4(1, 3, 3, 7), Port: 1337} - cs, err := NewCryptoSetup( - protocol.ConnectionID(42), - remoteAddr, - protocol.VersionWhatever, - scfg, - stream, - cpm, - supportedVersions, - aeadChanged, - ) - Expect(err).ToNot(HaveOccurred()) - Expect(cs.(*cryptoSetupServer).sourceAddr).To(BeEquivalentTo(remoteAddr.IP)) - }) - - It("works with remote address that are not UDP", func() { - remoteAddr := &net.TCPAddr{IP: net.IPv4(1, 3, 3, 7), Port: 1337} - cs, err := NewCryptoSetup( - protocol.ConnectionID(42), - remoteAddr, - protocol.VersionWhatever, - scfg, - stream, - cpm, - supportedVersions, - aeadChanged, - ) - Expect(err).ToNot(HaveOccurred()) - Expect(cs.(*cryptoSetupServer).sourceAddr).To(BeEquivalentTo("1.3.3.7:1337")) - }) - }) - Context("when responding to client messages", func() { var cert []byte var xlct []byte diff --git a/handshake/stk_generator.go b/handshake/stk_generator.go index 1b33496b..6083c37d 100644 --- a/handshake/stk_generator.go +++ b/handshake/stk_generator.go @@ -1,8 +1,10 @@ package handshake import ( + "bytes" "crypto/subtle" "errors" + "net" "time" "github.com/lucas-clemente/quic-go/crypto" @@ -25,18 +27,31 @@ func NewSTKGenerator() (*STKGenerator, error) { } // NewToken generates a new STK token for a given source address -func (g *STKGenerator) NewToken(sourceAddr []byte) ([]byte, error) { - return g.stkSource.NewToken(sourceAddr) +func (g *STKGenerator) NewToken(raddr net.Addr) ([]byte, error) { + return g.stkSource.NewToken(encodeRemoteAddr(raddr)) } // VerifyToken verifies an STK token -func (g *STKGenerator) VerifyToken(sourceAddr []byte, data []byte) (time.Time, error) { - tokenAddr, timestamp, err := g.stkSource.DecodeToken(data) +func (g *STKGenerator) VerifyToken(raddr net.Addr, data []byte) (time.Time, error) { + data, timestamp, err := g.stkSource.DecodeToken(data) if err != nil { return time.Time{}, err } - if subtle.ConstantTimeCompare(sourceAddr, tokenAddr) != 1 { + if subtle.ConstantTimeCompare(encodeRemoteAddr(raddr), data) != 1 { return time.Time{}, errors.New("invalid source address in STK") } return timestamp, nil } + +// encodeRemoteAddr encodes a remote address such that it can be saved in the STK +// it ensures that we're binary compatible with Google's implementation of STKs +func encodeRemoteAddr(remoteAddr net.Addr) []byte { + // if the address is a UDP address, just use the byte representation of the IP address + // the length of an IP address is 4 bytes (for IPv4) or 16 bytes (for IPv6) + if udpAddr, ok := remoteAddr.(*net.UDPAddr); ok { + return udpAddr.IP + } + // if the address is not a UDP address, prepend 16 bytes + // that way it can be distinguished from an IP address + return append(bytes.Repeat([]byte{0}, 16), []byte(remoteAddr.String())...) +} diff --git a/handshake/stk_generator_test.go b/handshake/stk_generator_test.go index bcfe82dc..d6ca3dd7 100644 --- a/handshake/stk_generator_test.go +++ b/handshake/stk_generator_test.go @@ -19,26 +19,63 @@ var _ = Describe("STK Generator", func() { It("generates an STK", func() { ip := net.IPv4(127, 0, 0, 1) - token, err := stkGen.NewToken(ip) + token, err := stkGen.NewToken(&net.UDPAddr{IP: ip, Port: 1337}) Expect(err).ToNot(HaveOccurred()) Expect(token).ToNot(BeEmpty()) }) It("accepts a valid STK", func() { - ip := net.IPv4(192, 168, 0, 1) - token, err := stkGen.NewToken(ip) + raddr := &net.UDPAddr{IP: net.IPv4(192, 168, 0, 1), Port: 1337} + token, err := stkGen.NewToken(raddr) Expect(err).ToNot(HaveOccurred()) - t, err := stkGen.VerifyToken(ip, token) + t, err := stkGen.VerifyToken(raddr, token) Expect(err).ToNot(HaveOccurred()) Expect(t).To(BeTemporally("~", time.Now(), time.Second)) }) + It("works with an IPv6 address", func() { + ip := net.ParseIP("2001:db8::68") + Expect(ip).ToNot(BeNil()) + raddr := &net.UDPAddr{IP: ip, Port: 1337} + token, err := stkGen.NewToken(raddr) + Expect(err).ToNot(HaveOccurred()) + t, err := stkGen.VerifyToken(raddr, token) + Expect(err).ToNot(HaveOccurred()) + Expect(t).To(BeTemporally("~", time.Now(), time.Second)) + }) + + It("does not care about the port", func() { + ip := net.IPv4(192, 168, 0, 1) + token, err := stkGen.NewToken(&net.UDPAddr{IP: ip, Port: 1337}) + Expect(err).ToNot(HaveOccurred()) + _, err = stkGen.VerifyToken(&net.UDPAddr{IP: ip, Port: 7331}, token) + Expect(err).ToNot(HaveOccurred()) + }) + It("rejects an STK for the wrong address", func() { ip := net.ParseIP("1.2.3.4") otherIP := net.ParseIP("4.3.2.1") - token, err := stkGen.NewToken(ip) + token, err := stkGen.NewToken(&net.UDPAddr{IP: ip, Port: 1337}) Expect(err).NotTo(HaveOccurred()) - _, err = stkGen.VerifyToken(otherIP, token) + _, err = stkGen.VerifyToken(&net.UDPAddr{IP: otherIP, Port: 1337}, token) + Expect(err).To(MatchError("invalid source address in STK")) + }) + + It("works with an address that is not a UDP address", func() { + raddr := &net.TCPAddr{IP: net.IPv4(192, 168, 0, 1), Port: 1337} + token, err := stkGen.NewToken(raddr) + Expect(err).ToNot(HaveOccurred()) + t, err := stkGen.VerifyToken(raddr, token) + Expect(err).ToNot(HaveOccurred()) + Expect(t).To(BeTemporally("~", time.Now(), time.Second)) + }) + + It("uses the string representation of an address that is not a UDP address", func() { + // when using the string representation, the port matters + ip := net.IPv4(192, 168, 0, 1) + token, err := stkGen.NewToken(&net.TCPAddr{IP: ip, Port: 1337}) + Expect(err).ToNot(HaveOccurred()) + _, err = stkGen.VerifyToken(&net.TCPAddr{IP: ip, Port: 7331}, token) Expect(err).To(MatchError("invalid source address in STK")) }) })