fix the server's 0-RTT rejection logic when using GetConfigForClient (#…

…4550)

http3/server.go

@@ -94,6 +94,10 @@ func ConfigureTLSConfig(tlsConf *tls.Config) *tls.Config {
  if config == nil {
  return nil, nil
  }
+ // Workaround for https://github.com/golang/go/issues/60506.
+ // This initializes the session tickets _before_ cloning the config.
+ _, _ = config.DecryptTicket(nil, tls.ConnectionState{})
+
  config = config.Clone()
  config.NextProtos = []string{proto}
  return config, nil

integrationtests/self/zero_rtt_test.go

@@ -813,87 +813,93 @@ var _ = Describe("0-RTT", func() {
  Entry("doesn't reject 0-RTT when the server's transport connection flow control limit increased", func(c *quic.Config, limit uint64) { c.InitialConnectionReceiveWindow = limit }),
  )
 
- for _, l := range []int{0, 15} {
- connIDLen := l
-
- It(fmt.Sprintf("correctly deals with 0-RTT rejections, for %d byte connection IDs", connIDLen), func() {
- tlsConf := getTLSConfig()
- clientConf := getTLSClientConfig()
- dialAndReceiveSessionTicket(tlsConf, nil, clientConf)
- // now dial new connection with different transport parameters
- counter, tracer := newPacketTracer()
- ln, err := quic.ListenAddrEarly(
- "localhost:0",
- tlsConf,
- getQuicConfig(&quic.Config{
- MaxIncomingUniStreams: 1,
- Tracer: newTracer(tracer),
- }),
- )
- Expect(err).ToNot(HaveOccurred())
- defer ln.Close()
- proxy, num0RTTPackets := runCountingProxy(ln.Addr().(*net.UDPAddr).Port)
- defer proxy.Close()
+ test0RTTRejection := func(tlsConf *tls.Config) {
+ clientConf := getTLSClientConfig()
+ dialAndReceiveSessionTicket(tlsConf, nil, clientConf)
+ // now dial new connection with different transport parameters
+ counter, tracer := newPacketTracer()
+ ln, err := quic.ListenAddrEarly(
+ "localhost:0",
+ tlsConf,
+ getQuicConfig(&quic.Config{
+ MaxIncomingUniStreams: 1,
+ Tracer: newTracer(tracer),
+ }),
+ )
+ Expect(err).ToNot(HaveOccurred())
+ defer ln.Close()
+ proxy, num0RTTPackets := runCountingProxy(ln.Addr().(*net.UDPAddr).Port)
+ defer proxy.Close()
 
- conn, err := quic.DialAddrEarly(
- context.Background(),
- fmt.Sprintf("localhost:%d", proxy.LocalPort()),
- clientConf,
- getQuicConfig(nil),
- )
- Expect(err).ToNot(HaveOccurred())
- // The client remembers that it was allowed to open 2 uni-directional streams.
- firstStr, err := conn.OpenUniStream()
+ conn, err := quic.DialAddrEarly(
+ context.Background(),
+ fmt.Sprintf("localhost:%d", proxy.LocalPort()),
+ clientConf,
+ getQuicConfig(&quic.Config{}),
+ )
+ Expect(err).ToNot(HaveOccurred())
+ // The client remembers that it was allowed to open 2 uni-directional streams.
+ firstStr, err := conn.OpenUniStream()
+ Expect(err).ToNot(HaveOccurred())
+ written := make(chan struct{}, 2)
+ go func() {
+ defer GinkgoRecover()
+ defer func() { written <- struct{}{} }()
+ _, err := firstStr.Write([]byte("first flight"))
  Expect(err).ToNot(HaveOccurred())
- written := make(chan struct{}, 2)
- go func() {
- defer GinkgoRecover()
- defer func() { written <- struct{}{} }()
- _, err := firstStr.Write([]byte("first flight"))
- Expect(err).ToNot(HaveOccurred())
- }()
- secondStr, err := conn.OpenUniStream()
+ }()
+ secondStr, err := conn.OpenUniStream()
+ Expect(err).ToNot(HaveOccurred())
+ go func() {
+ defer GinkgoRecover()
+ defer func() { written <- struct{}{} }()
+ _, err := secondStr.Write([]byte("first flight"))
  Expect(err).ToNot(HaveOccurred())
- go func() {
- defer GinkgoRecover()
- defer func() { written <- struct{}{} }()
- _, err := secondStr.Write([]byte("first flight"))
- Expect(err).ToNot(HaveOccurred())
- }()
+ }()
 
- ctx, cancel := context.WithTimeout(context.Background(), time.Second)
- defer cancel()
- _, err = conn.AcceptStream(ctx)
- Expect(err).To(MatchError(quic.Err0RTTRejected))
- Eventually(written).Should(Receive())
- Eventually(written).Should(Receive())
- _, err = firstStr.Write([]byte("foobar"))
- Expect(err).To(MatchError(quic.Err0RTTRejected))
- _, err = conn.OpenUniStream()
- Expect(err).To(MatchError(quic.Err0RTTRejected))
-
- _, err = conn.AcceptStream(ctx)
- Expect(err).To(Equal(quic.Err0RTTRejected))
-
- newConn := conn.NextConnection()
- str, err := newConn.OpenUniStream()
- Expect(err).ToNot(HaveOccurred())
- _, err = newConn.OpenUniStream()
- Expect(err).To(HaveOccurred())
- Expect(err.Error()).To(ContainSubstring("too many open streams"))
- _, err = str.Write([]byte("second flight"))
- Expect(err).ToNot(HaveOccurred())
- Expect(str.Close()).To(Succeed())
- Expect(conn.CloseWithError(0, "")).To(Succeed())
+ ctx, cancel := context.WithTimeout(context.Background(), time.Second)
+ defer cancel()
+ _, err = conn.AcceptStream(ctx)
+ Expect(err).To(MatchError(quic.Err0RTTRejected))
+ Eventually(written).Should(Receive())
+ Eventually(written).Should(Receive())
+ _, err = firstStr.Write([]byte("foobar"))
+ Expect(err).To(MatchError(quic.Err0RTTRejected))
+ _, err = conn.OpenUniStream()
+ Expect(err).To(MatchError(quic.Err0RTTRejected))
 
- // The client should send 0-RTT packets, but the server doesn't process them.
- num0RTT := num0RTTPackets.Load()
- fmt.Fprintf(GinkgoWriter, "Sent %d 0-RTT packets.", num0RTT)
- Expect(num0RTT).ToNot(BeZero())
- Expect(get0RTTPackets(counter.getRcvdLongHeaderPackets())).To(BeEmpty())
- })
+ _, err = conn.AcceptStream(ctx)
+ Expect(err).To(Equal(quic.Err0RTTRejected))
+
+ newConn := conn.NextConnection()
+ str, err := newConn.OpenUniStream()
+ Expect(err).ToNot(HaveOccurred())
+ _, err = newConn.OpenUniStream()
+ Expect(err).To(HaveOccurred())
+ Expect(err.Error()).To(ContainSubstring("too many open streams"))
+ _, err = str.Write([]byte("second flight"))
+ Expect(err).ToNot(HaveOccurred())
+ Expect(str.Close()).To(Succeed())
+ Expect(conn.CloseWithError(0, "")).To(Succeed())
+
+ // The client should send 0-RTT packets, but the server doesn't process them.
+ num0RTT := num0RTTPackets.Load()
+ fmt.Fprintf(GinkgoWriter, "Sent %d 0-RTT packets.", num0RTT)
+ Expect(num0RTT).ToNot(BeZero())
+ Expect(get0RTTPackets(counter.getRcvdLongHeaderPackets())).To(BeEmpty())
  }
 
+ It("correctly deals with 0-RTT rejections", func() {
+ test0RTTRejection(getTLSConfig())
+ })
+
+ It("correctly deals with 0-RTT rejections, when the server uses GetConfigForClient", func() {
+ tlsConf := getTLSConfig()
+ test0RTTRejection(&tls.Config{
+ GetConfigForClient: func(*tls.ClientHelloInfo) (*tls.Config, error) { return tlsConf, nil },
+ })
+ })
+
  It("queues 0-RTT packets, if the Initial is delayed", func() {
  tlsConf := getTLSConfig()
  clientConf := getTLSClientConfig()

internal/handshake/crypto_setup.go

@@ -123,44 +123,12 @@ func NewCryptoSetupServer(
  )
  cs.allow0RTT = allow0RTT
 
- quicConf := &tls.QUICConfig{TLSConfig: tlsConf}
- qtls.SetupConfigForServer(quicConf, cs.getDataForSessionTicket, cs.handleSessionTicket)
- addConnToClientHelloInfo(quicConf.TLSConfig, localAddr, remoteAddr)
-
- cs.tlsConf = quicConf.TLSConfig
- cs.conn = tls.QUICServer(quicConf)
-
+ tlsConf = qtls.SetupConfigForServer(tlsConf, localAddr, remoteAddr, cs.getDataForSessionTicket, cs.handleSessionTicket)
+ cs.tlsConf = tlsConf
+ cs.conn = tls.QUICServer(&tls.QUICConfig{TLSConfig: tlsConf})
  return cs
 }
 
-// The tls.Config contains two callbacks that pass in a tls.ClientHelloInfo.
-// Since crypto/tls doesn't do it, we need to make sure to set the Conn field with a fake net.Conn
-// that allows the caller to get the local and the remote address.
-func addConnToClientHelloInfo(conf *tls.Config, localAddr, remoteAddr net.Addr) {
- if conf.GetConfigForClient != nil {
- gcfc := conf.GetConfigForClient
- conf.GetConfigForClient = func(info *tls.ClientHelloInfo) (*tls.Config, error) {
- info.Conn = &conn{localAddr: localAddr, remoteAddr: remoteAddr}
- c, err := gcfc(info)
- if c != nil {
- c = c.Clone()
- // This won't be necessary anymore once https://github.com/golang/go/issues/63722 is accepted.
- c.MinVersion = tls.VersionTLS13
- // We're returning a tls.Config here, so we need to apply this recursively.
- addConnToClientHelloInfo(c, localAddr, remoteAddr)
- }
- return c, err
- }
- }
- if conf.GetCertificate != nil {
- gc := conf.GetCertificate
- conf.GetCertificate = func(info *tls.ClientHelloInfo) (*tls.Certificate, error) {
- info.Conn = &conn{localAddr: localAddr, remoteAddr: remoteAddr}
- return gc(info)
- }
- }
-}
-
 func newCryptoSetup(
  connID protocol.ConnectionID,
  tp *wire.TransportParameters,
@@ -376,9 +344,7 @@ func (h *cryptoSetup) getDataForSessionTicket() []byte {
 // Due to limitations in crypto/tls, it's only possible to generate a single session ticket per connection.
 // It is only valid for the server.
 func (h *cryptoSetup) GetSessionTicket() ([]byte, error) {
- if err := h.conn.SendSessionTicket(tls.QUICSessionTicketOptions{
- EarlyData: h.allow0RTT,
- }); err != nil {
+ if err := h.conn.SendSessionTicket(tls.QUICSessionTicketOptions{EarlyData: h.allow0RTT}); err != nil {
  // Session tickets might be disabled by tls.Config.SessionTicketsDisabled.
  // We can't check h.tlsConfig here, since the actual config might have been obtained from
  // the GetConfigForClient callback.

internal/handshake/crypto_setup_test.go

@@ -9,7 +9,6 @@ import (
  "crypto/x509/pkix"
  "math/big"
  "net"
- "reflect"
  "time"
 
  mocktls "github.com/quic-go/quic-go/internal/mocks/tls"
@@ -106,79 +105,6 @@ var _ = Describe("Crypto Setup TLS", func() {
  Expect(err.Error()).To(ContainSubstring("tls: handshake data received at wrong level"))
  })
 
- Context("filling in a net.Conn in tls.ClientHelloInfo", func() {
- var (
- local = &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 42}
- remote = &net.UDPAddr{IP: net.IPv4(192, 168, 0, 1), Port: 1337}
- )
-
- It("wraps GetCertificate", func() {
- var localAddr, remoteAddr net.Addr
- tlsConf := &tls.Config{
- GetCertificate: func(info *tls.ClientHelloInfo) (*tls.Certificate, error) {
- localAddr = info.Conn.LocalAddr()
- remoteAddr = info.Conn.RemoteAddr()
- cert := generateCert()
- return &cert, nil
- },
- }
- addConnToClientHelloInfo(tlsConf, local, remote)
- _, err := tlsConf.GetCertificate(&tls.ClientHelloInfo{})
- Expect(err).ToNot(HaveOccurred())
- Expect(localAddr).To(Equal(local))
- Expect(remoteAddr).To(Equal(remote))
- })
-
- It("wraps GetConfigForClient", func() {
- var localAddr, remoteAddr net.Addr
- tlsConf := &tls.Config{
- GetConfigForClient: func(info *tls.ClientHelloInfo) (*tls.Config, error) {
- localAddr = info.Conn.LocalAddr()
- remoteAddr = info.Conn.RemoteAddr()
- return &tls.Config{}, nil
- },
- }
- addConnToClientHelloInfo(tlsConf, local, remote)
- conf, err := tlsConf.GetConfigForClient(&tls.ClientHelloInfo{})
- Expect(err).ToNot(HaveOccurred())
- Expect(localAddr).To(Equal(local))
- Expect(remoteAddr).To(Equal(remote))
- Expect(conf).ToNot(BeNil())
- Expect(conf.MinVersion).To(BeEquivalentTo(tls.VersionTLS13))
- })
-
- It("wraps GetConfigForClient, recursively", func() {
- var localAddr, remoteAddr net.Addr
- tlsConf := &tls.Config{}
- var innerConf *tls.Config
- getCert := func(info *tls.ClientHelloInfo) (*tls.Certificate, error) { //nolint:unparam
- localAddr = info.Conn.LocalAddr()
- remoteAddr = info.Conn.RemoteAddr()
- cert := generateCert()
- return &cert, nil
- }
- tlsConf.GetConfigForClient = func(info *tls.ClientHelloInfo) (*tls.Config, error) {
- innerConf = tlsConf.Clone()
- // set the MaxVersion, so we can check that quic-go doesn't overwrite the user's config
- innerConf.MaxVersion = tls.VersionTLS12
- innerConf.GetCertificate = getCert
- return innerConf, nil
- }
- addConnToClientHelloInfo(tlsConf, local, remote)
- conf, err := tlsConf.GetConfigForClient(&tls.ClientHelloInfo{})
- Expect(err).ToNot(HaveOccurred())
- Expect(conf).ToNot(BeNil())
- Expect(conf.MinVersion).To(BeEquivalentTo(tls.VersionTLS13))
- _, err = conf.GetCertificate(&tls.ClientHelloInfo{})
- Expect(err).ToNot(HaveOccurred())
- Expect(localAddr).To(Equal(local))
- Expect(remoteAddr).To(Equal(remote))
- // make sure that the tls.Config returned by GetConfigForClient isn't modified
- Expect(reflect.ValueOf(innerConf.GetCertificate).Pointer() == reflect.ValueOf(getCert).Pointer()).To(BeTrue())
- Expect(innerConf.MaxVersion).To(BeEquivalentTo(tls.VersionTLS12))
- })
- })
-
  Context("doing the handshake", func() {
  newRTTStatsWithRTT := func(rtt time.Duration) *utils.RTTStats {
  rttStats := &utils.RTTStats{}

internal/handshake/conn.go → internal/qtls/conn.go

@@ -1,4 +1,4 @@
-package handshake
+package qtls
 
 import (
  "net"

internal/qtls/qtls.go

@@ -4,20 +4,23 @@ import (
  "bytes"
  "crypto/tls"
  "fmt"
+ "net"
 
  "github.com/quic-go/quic-go/internal/protocol"
 )
 
-func SetupConfigForServer(qconf *tls.QUICConfig, getData func() []byte, handleSessionTicket func([]byte, bool) bool) {
- conf := qconf.TLSConfig
-
+func SetupConfigForServer(
+ conf *tls.Config,
+ localAddr, remoteAddr net.Addr,
+ getData func() []byte,
+ handleSessionTicket func([]byte, bool) bool,
+) *tls.Config {
  // Workaround for https://github.com/golang/go/issues/60506.
  // This initializes the session tickets _before_ cloning the config.
  _, _ = conf.DecryptTicket(nil, tls.ConnectionState{})
 
  conf = conf.Clone()
  conf.MinVersion = tls.VersionTLS13
- qconf.TLSConfig = conf
 
  // add callbacks to save transport parameters into the session ticket
  origWrapSession := conf.WrapSession
@@ -58,6 +61,29 @@ func SetupConfigForServer(qconf *tls.QUICConfig, getData func() []byte, handleSe
 
  return state, nil
  }
+ // The tls.Config contains two callbacks that pass in a tls.ClientHelloInfo.
+ // Since crypto/tls doesn't do it, we need to make sure to set the Conn field with a fake net.Conn
+ // that allows the caller to get the local and the remote address.
+ if conf.GetConfigForClient != nil {
+ gcfc := conf.GetConfigForClient
+ conf.GetConfigForClient = func(info *tls.ClientHelloInfo) (*tls.Config, error) {
+ info.Conn = &conn{localAddr: localAddr, remoteAddr: remoteAddr}
+ c, err := gcfc(info)
+ if c != nil {
+ // We're returning a tls.Config here, so we need to apply this recursively.
+ c = SetupConfigForServer(c, localAddr, remoteAddr, getData, handleSessionTicket)
+ }
+ return c, err
+ }
+ }
+ if conf.GetCertificate != nil {
+ gc := conf.GetCertificate
+ conf.GetCertificate = func(info *tls.ClientHelloInfo) (*tls.Certificate, error) {
+ info.Conn = &conn{localAddr: localAddr, remoteAddr: remoteAddr}
+ return gc(info)
+ }
+ }
+ return conf
 }
 
 func SetupConfigForClient(