optimize expiry periods of certificates (#21)

p2p/transport/webtransport/cert_manager.go

@@ -15,11 +15,19 @@ import (
  "github.com/multiformats/go-multihash"
 )
 
+// Allow for a bit of clock skew.
+// When we generate a certificate, the NotBefore time is set to clockSkewAllowance before the current time.
+// Similarly, we stop using a certificate one clockSkewAllowance before its expiry time.
+const clockSkewAllowance = time.Hour
+
 type certConfig struct {
  tlsConf *tls.Config
  sha256 [32]byte // cached from the tlsConf
 }
 
+func (c *certConfig) Start() time.Time { return c.tlsConf.Certificates[0].Leaf.NotBefore }
+func (c *certConfig) End() time.Time { return c.tlsConf.Certificates[0].Leaf.NotAfter }
+
 func newCertConfig(start, end time.Time) (*certConfig, error) {
  conf, err := getTLSConf(start, end)
  if err != nil {
@@ -32,22 +40,17 @@ func newCertConfig(start, end time.Time) (*certConfig, error) {
 }
 
 // Certificate renewal logic:
-// 0. To simplify the math, assume the certificate is valid for 10 days (in real life: 14 days).
-// 1. On startup, we generate the first certificate (1).
-// 2. After 4 days, we generate a second certificate (2).
-// We don't use that certificate yet, but we advertise the hashes of (1) and (2).
-// That allows clients to connect to us using addresses that are 4 days old.
-// 3. After another 4 days, we now actually start using (2).
-// We also generate a third certificate (3), and start advertising the hashes of (2) and (3).
-// We continue to remember the hash of (1) for validation during the Noise handshake for another 4 days,
-// as the client might be connecting with a cached address.
+// 1. On startup, we generate one cert that is valid from now (-1h, to allow for clock skew), and another
+// cert that is valid from the expiry date of the first certificate (again, with allowance for clock skew).
+// 2. Once we reach 1h before expiry of the first certificate, we switch over to the second certificate.
+// At the same time, we stop advertising the certhash of the first cert and generate the next cert.
 type certManager struct {
  clock clock.Clock
  ctx context.Context
  ctxCancel context.CancelFunc
  refCount sync.WaitGroup
 
- mx sync.Mutex
+ mx sync.RWMutex
  lastConfig *certConfig // initially nil
  currentConfig *certConfig
  nextConfig *certConfig // nil until we have passed half the certValidity of the current config
@@ -61,64 +64,71 @@ func newCertManager(clock clock.Clock) (*certManager, error) {
  return nil, err
  }
 
- t := m.clock.Ticker(certValidity * 4 / 9) // make sure we're a bit faster than 1/2
- m.refCount.Add(1)
- go func() {
- defer m.refCount.Done()
- defer t.Stop()
- if err := m.background(t); err != nil {
- log.Fatal(err)
- }
- }()
+ m.background()
  return m, nil
 }
 
 func (m *certManager) init() error {
- start := m.clock.Now()
- end := start.Add(certValidity)
- cc, err := newCertConfig(start, end)
+ start := m.clock.Now().Add(-clockSkewAllowance)
+ var err error
+ m.nextConfig, err = newCertConfig(start, start.Add(certValidity))
  if err != nil {
  return err
  }
- m.currentConfig = cc
+ return m.rollConfig()
+}
+
+func (m *certManager) rollConfig() error {
+ // We stop using the current certificate clockSkewAllowance before its expiry time.
+ // At this point, the next certificate needs to be valid for one clockSkewAllowance.
+ nextStart := m.nextConfig.End().Add(-2 * clockSkewAllowance)
+ c, err := newCertConfig(nextStart, nextStart.Add(certValidity))
+ if err != nil {
+ return err
+ }
+ m.lastConfig = m.currentConfig
+ m.currentConfig = m.nextConfig
+ m.nextConfig = c
  return m.cacheAddrComponent()
 }
 
-func (m *certManager) background(t *clock.Ticker) error {
- for {
- select {
- case <-m.ctx.Done():
- return nil
- case start := <-t.C:
- end := start.Add(certValidity)
- cc, err := newCertConfig(start, end)
- if err != nil {
- return err
- }
- m.mx.Lock()
- if m.nextConfig != nil {
- m.lastConfig = m.currentConfig
- m.currentConfig = m.nextConfig
- }
- m.nextConfig = cc
- if err := m.cacheAddrComponent(); err != nil {
+func (m *certManager) background() {
+ d := m.currentConfig.End().Add(-clockSkewAllowance).Sub(m.clock.Now())
+ log.Debugw("setting timer", "duration", d.String())
+ t := m.clock.Timer(d)
+ m.refCount.Add(1)
+
+ go func() {
+ defer m.refCount.Done()
+ defer t.Stop()
+
+ for {
+ select {
+ case <-m.ctx.Done():
+ return
+ case now := <-t.C:
+ m.mx.Lock()
+ if err := m.rollConfig(); err != nil {
+ log.Errorw("rolling config failed", "error", err)
+ }
+ d := m.currentConfig.End().Add(-clockSkewAllowance).Sub(now)
+ log.Debugw("rolling certificates", "next", d.String())
+ t.Reset(d)
  m.mx.Unlock()
- return err
  }
- m.mx.Unlock()
  }
- }
+ }()
 }
 
 func (m *certManager) GetConfig() *tls.Config {
- m.mx.Lock()
- defer m.mx.Unlock()
+ m.mx.RLock()
+ defer m.mx.RUnlock()
  return m.currentConfig.tlsConf
 }
 
 func (m *certManager) AddrComponent() ma.Multiaddr {
- m.mx.Lock()
- defer m.mx.Unlock()
+ m.mx.RLock()
+ defer m.mx.RUnlock()
  return m.addrComp
 }
 

p2p/transport/webtransport/cert_manager_test.go

@@ -46,14 +46,15 @@ func TestInitialCert(t *testing.T) {
  conf := m.GetConfig()
  require.Len(t, conf.Certificates, 1)
  cert := conf.Certificates[0]
- require.Equal(t, cl.Now().UTC(), cert.Leaf.NotBefore)
- require.Equal(t, cl.Now().Add(certValidity).UTC(), cert.Leaf.NotAfter)
+ require.Equal(t, cl.Now().Add(-clockSkewAllowance).UTC(), cert.Leaf.NotBefore)
+ require.Equal(t, cert.Leaf.NotBefore.Add(certValidity), cert.Leaf.NotAfter)
  addr := m.AddrComponent()
  components := splitMultiaddr(addr)
- require.Len(t, components, 1)
+ require.Len(t, components, 2)
  require.Equal(t, ma.P_CERTHASH, components[0].Protocol().Code)
  hash := certificateHashFromTLSConfig(conf)
  require.Equal(t, hash[:], certHashFromComponent(t, components[0]))
+ require.Equal(t, ma.P_CERTHASH, components[1].Protocol().Code)
 }
 
 func TestCertRenewal(t *testing.T) {
@@ -63,21 +64,39 @@ func TestCertRenewal(t *testing.T) {
  defer m.Close()
 
  firstConf := m.GetConfig()
- require.Len(t, splitMultiaddr(m.AddrComponent()), 1)
+ first := splitMultiaddr(m.AddrComponent())
+ require.Len(t, first, 2)
+ require.NotEqual(t, first[0].Value(), first[1].Value(), "the hashes should differ")
  // wait for a new certificate to be generated
- cl.Add(certValidity / 2)
- require.Eventually(t, func() bool { return len(splitMultiaddr(m.AddrComponent())) > 1 }, 200*time.Millisecond, 10*time.Millisecond)
- // the actual config used should still be the same, we're just advertising the hash of the next config
- components := splitMultiaddr(m.AddrComponent())
- require.Len(t, components, 2)
- for _, c := range components {
+ cl.Add(certValidity - 2*clockSkewAllowance - time.Second)
+ require.Never(t, func() bool {
+ for i, c := range splitMultiaddr(m.AddrComponent()) {
+ if c.Value() != first[i].Value() {
+ return true
+ }
+ }
+ return false
+ }, 100*time.Millisecond, 10*time.Millisecond)
+ cl.Add(2 * time.Second)
+ require.Eventually(t, func() bool { return m.GetConfig() != firstConf }, 200*time.Millisecond, 10*time.Millisecond)
+ secondConf := m.GetConfig()
+
+ second := splitMultiaddr(m.AddrComponent())
+ require.Len(t, second, 2)
+ for _, c := range second {
  require.Equal(t, ma.P_CERTHASH, c.Protocol().Code)
  }
- require.Equal(t, firstConf, m.GetConfig())
- cl.Add(certValidity / 2)
- require.Eventually(t, func() bool { return m.GetConfig() != firstConf }, 200*time.Millisecond, 10*time.Millisecond)
- newConf := m.GetConfig()
- // check that the new config now matches the second component
- hash := certificateHashFromTLSConfig(newConf)
- require.Equal(t, hash[:], certHashFromComponent(t, components[1]))
+ // check that the 2nd certificate from the beginning was rolled over to be the 1st certificate
+ require.Equal(t, first[1].Value(), second[0].Value())
+ require.NotEqual(t, first[0].Value(), second[1].Value())
+
+ cl.Add(certValidity - 2*clockSkewAllowance + time.Second)
+ require.Eventually(t, func() bool { return m.GetConfig() != secondConf }, 200*time.Millisecond, 10*time.Millisecond)
+ third := splitMultiaddr(m.AddrComponent())
+ require.Len(t, third, 2)
+ for _, c := range third {
+ require.Equal(t, ma.P_CERTHASH, c.Protocol().Code)
+ }
+ // check that the 2nd certificate from the beginning was rolled over to be the 1st certificate
+ require.Equal(t, second[1].Value(), third[0].Value())
 }

p2p/transport/webtransport/transport_test.go

@@ -140,7 +140,16 @@ func TestHashVerification(t *testing.T) {
 
  t.Run("fails using only a wrong hash", func(t *testing.T) {
  // replace the certificate hash in the multiaddr with a fake hash
- addr, _ := ma.SplitLast(ln.Multiaddr())
+ addr := ln.Multiaddr()
+ // strip off all certhash components
+ for {
+ a, comp := ma.SplitLast(addr)
+ if comp.Protocol().Code != ma.P_CERTHASH {
+ break
+ }
+ addr = a
+ }
+
  addr = addr.Encapsulate(foobarHash)
 
  _, err := tr2.Dial(context.Background(), addr, serverID)
@@ -224,7 +233,7 @@ func TestListenerAddrs(t *testing.T) {
  ln2, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic/webtransport"))
  require.NoError(t, err)
  hashes1 := extractCertHashes(ln1.Multiaddr())
- require.Len(t, hashes1, 1)
+ require.Len(t, hashes1, 2)
  hashes2 := extractCertHashes(ln2.Multiaddr())
  require.Equal(t, hashes1, hashes2)
 }