Skip to content

Commit

Permalink
Implement retransmit backoff according to 4.2.4.1
Browse files Browse the repository at this point in the history
  • Loading branch information
Anonymous authored and eh-steve committed Feb 15, 2023
1 parent 9e922d5 commit eb8069f
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 24 deletions.
2 changes: 1 addition & 1 deletion conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,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,
Expand Down
4 changes: 2 additions & 2 deletions e2e/e2e_lossy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import (
)

const (
flightInterval = time.Millisecond * 100
lossyTestTimeout = 30 * time.Second
flightInterval = time.Millisecond * 10
lossyTestTimeout = 60 * time.Second
)

/*
Expand Down
49 changes: 34 additions & 15 deletions handshaker.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,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 {
Expand All @@ -106,7 +107,7 @@ type handshakeConfig struct {
sessionStore SessionStore
rootCAs *x509.CertPool
clientCAs *x509.CertPool
retransmitInterval time.Duration
initialRetransmitInterval time.Duration
customCipherSuites func() []CipherSuite
ellipticCurves []elliptic.Curve
insecureSkipHelloVerify bool
Expand Down Expand Up @@ -156,11 +157,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{}),
}
}

Expand Down Expand Up @@ -267,11 +269,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 {
Expand All @@ -297,8 +300,17 @@ func (s *handshakeFSM) wait(ctx context.Context, c flightConn) (handshakeState,
if !s.retransmit {
return handshakeWaiting, nil
}

// 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
}
return handshakeSending, nil
case <-ctx.Done():
s.retransmitInterval = s.cfg.initialRetransmitInterval
return handshakeErrored, ctx.Err()
}
}
Expand All @@ -315,11 +327,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 {
Expand All @@ -337,10 +350,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
Expand Down
12 changes: 6 additions & 6 deletions handshaker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,10 +214,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")
Expand Down Expand Up @@ -279,7 +279,7 @@ func TestHandshaker(t *testing.T) {
})
}
},
retransmitInterval: nonZeroRetransmitInterval,
initialRetransmitInterval: nonZeroRetransmitInterval,
}

fsm := newHandshakeFSM(&ca.state, ca.handshakeCache, cfg, flight1)
Expand Down Expand Up @@ -312,7 +312,7 @@ func TestHandshaker(t *testing.T) {
})
}
},
retransmitInterval: nonZeroRetransmitInterval,
initialRetransmitInterval: nonZeroRetransmitInterval,
}

fsm := newHandshakeFSM(&cb.state, cb.handshakeCache, cfg, flight0)
Expand Down

0 comments on commit eb8069f

Please sign in to comment.