From 53edba5d41f549027f644832c4fac043bc933e2b Mon Sep 17 00:00:00 2001 From: Anonymous Date: Thu, 14 Apr 2022 16:13:21 +0100 Subject: [PATCH] Implement retransmit backoff according to 4.2.4.1 --- conn.go | 2 +- handshaker.go | 48 +++++++++++++++++++++++++++++++--------------- handshaker_test.go | 12 ++++++------ 3 files changed, 40 insertions(+), 22 deletions(-) diff --git a/conn.go b/conn.go index 7c586e749..0ec108769 100644 --- a/conn.go +++ b/conn.go @@ -170,7 +170,7 @@ func createConn(ctx context.Context, nextConn net.Conn, config *Config, isClient rootCAs: config.RootCAs, clientCAs: config.ClientCAs, customCipherSuites: config.CustomCipherSuites, - retransmitInterval: workerInterval, + initialRetransmitInterval: workerInterval, log: logger, initialEpoch: 0, keyLogWriter: config.KeyLogWriter, diff --git a/handshaker.go b/handshaker.go index 1c7b9ffa2..53b5945f3 100644 --- a/handshaker.go +++ b/handshaker.go @@ -78,13 +78,14 @@ func (s handshakeState) String() string { } type handshakeFSM struct { - currentFlight flightVal - flights []*packet - retransmit bool - state *State - cache *handshakeCache - cfg *handshakeConfig - closed chan struct{} + currentFlight flightVal + flights []*packet + retransmit bool + retransmitInterval time.Duration + state *State + cache *handshakeCache + cfg *handshakeConfig + closed chan struct{} } type handshakeConfig struct { @@ -104,7 +105,7 @@ type handshakeConfig struct { sessionStore SessionStore rootCAs *x509.CertPool clientCAs *x509.CertPool - retransmitInterval time.Duration + initialRetransmitInterval time.Duration customCipherSuites func() []CipherSuite onFlightState func(flightVal, handshakeState) @@ -149,11 +150,12 @@ func newHandshakeFSM( initialFlight flightVal, ) *handshakeFSM { return &handshakeFSM{ - currentFlight: initialFlight, - state: s, - cache: cache, - cfg: cfg, - closed: make(chan struct{}), + currentFlight: initialFlight, + state: s, + cache: cache, + cfg: cfg, + retransmitInterval: cfg.initialRetransmitInterval, + closed: make(chan struct{}), } } @@ -260,11 +262,12 @@ func (s *handshakeFSM) wait(ctx context.Context, c flightConn) (handshakeState, return handshakeErrored, errFlight } - retransmitTimer := time.NewTimer(s.cfg.retransmitInterval) + retransmitTimer := time.NewTimer(s.retransmitInterval) for { select { case done := <-c.recvHandshake(): nextFlight, alert, err := parse(ctx, c, s.state, s.cache, s.cfg) + s.retransmitInterval = s.cfg.initialRetransmitInterval close(done) if alert != nil { if alertErr := c.notify(ctx, alert.Level, alert.Description); alertErr != nil { @@ -287,11 +290,19 @@ func (s *handshakeFSM) wait(ctx context.Context, c flightConn) (handshakeState, return handshakePreparing, nil case <-retransmitTimer.C: + // RFC 4347 4.2.4.1: + // Implementations SHOULD use an initial timer value of 1 second (the minimum defined in RFC 2988 [RFC2988]) + // and double the value at each retransmission, up to no less than the RFC 2988 maximum of 60 seconds. + s.retransmitInterval *= 2 + if s.retransmitInterval > time.Second*60 { + s.retransmitInterval = time.Second * 60 + } if !s.retransmit { return handshakeWaiting, nil } return handshakeSending, nil case <-ctx.Done(): + s.retransmitInterval = s.cfg.initialRetransmitInterval return handshakeErrored, ctx.Err() } } @@ -308,11 +319,12 @@ func (s *handshakeFSM) finish(ctx context.Context, c flightConn) (handshakeState return handshakeErrored, errFlight } - retransmitTimer := time.NewTimer(s.cfg.retransmitInterval) + retransmitTimer := time.NewTimer(s.retransmitInterval) select { case done := <-c.recvHandshake(): nextFlight, alert, err := parse(ctx, c, s.state, s.cache, s.cfg) close(done) + s.retransmitInterval = s.cfg.initialRetransmitInterval if alert != nil { if alertErr := c.notify(ctx, alert.Level, alert.Description); alertErr != nil { if err != nil { @@ -330,10 +342,16 @@ func (s *handshakeFSM) finish(ctx context.Context, c flightConn) (handshakeState return handshakeFinished, nil } <-retransmitTimer.C + // RFC 4347 4.2.4.1 + s.retransmitInterval *= 2 + if s.retransmitInterval > time.Second*60 { + s.retransmitInterval = time.Second * 60 + } // Retransmit last flight return handshakeSending, nil case <-ctx.Done(): + s.retransmitInterval = s.cfg.initialRetransmitInterval return handshakeErrored, ctx.Err() } return handshakeFinished, nil diff --git a/handshaker_test.go b/handshaker_test.go index 37bda9037..119dc2788 100644 --- a/handshaker_test.go +++ b/handshaker_test.go @@ -212,10 +212,10 @@ func TestHandshaker(t *testing.T) { } report := func(t *testing.T) { - // with one second server delay and 100 ms retransmit, there should be close to 10 `Finished` from client - // using a range of 9 - 11 for checking - if cntClientFinished < 8 || cntClientFinished > 11 { - t.Errorf("Number of client finished is wrong, expected: %d - %d times, got: %d times", 9, 11, cntClientFinished) + // with one second server delay and 100 ms retransmit (+ exponential backoff), there should be close to 4 `Finished` from client + // using a range of 3 - 5 for checking + if cntClientFinished < 3 || cntClientFinished > 5 { + t.Errorf("Number of client finished is wrong, expected: %d - %d times, got: %d times", 3, 5, cntClientFinished) } if !isClientFinished { t.Errorf("Client is not finished") @@ -276,7 +276,7 @@ func TestHandshaker(t *testing.T) { }) } }, - retransmitInterval: nonZeroRetransmitInterval, + initialRetransmitInterval: nonZeroRetransmitInterval, } fsm := newHandshakeFSM(&ca.state, ca.handshakeCache, cfg, flight1) @@ -307,7 +307,7 @@ func TestHandshaker(t *testing.T) { }) } }, - retransmitInterval: nonZeroRetransmitInterval, + initialRetransmitInterval: nonZeroRetransmitInterval, } fsm := newHandshakeFSM(&cb.state, cb.handshakeCache, cfg, flight0)