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 committed Apr 14, 2022
1 parent a1db639 commit 44c5cbe
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 @@ -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,
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 @@ -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 {
Expand All @@ -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)
Expand Down Expand Up @@ -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{}),
}
}

Expand Down Expand Up @@ -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 {
Expand All @@ -290,8 +293,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
}

Check warning on line 303 in handshaker.go

View check run for this annotation

Codecov / codecov/patch

handshaker.go#L302-L303

Added lines #L302 - L303 were not covered by tests
return handshakeSending, nil
case <-ctx.Done():
s.retransmitInterval = s.cfg.initialRetransmitInterval
return handshakeErrored, ctx.Err()
}
}
Expand All @@ -308,11 +320,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 @@ -330,10 +343,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
}

Check warning on line 350 in handshaker.go

View check run for this annotation

Codecov / codecov/patch

handshaker.go#L349-L350

Added lines #L349 - L350 were not covered by tests
// 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 @@ -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")
Expand Down Expand Up @@ -276,7 +276,7 @@ func TestHandshaker(t *testing.T) {
})
}
},
retransmitInterval: nonZeroRetransmitInterval,
initialRetransmitInterval: nonZeroRetransmitInterval,
}

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

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

0 comments on commit 44c5cbe

Please sign in to comment.