Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(client): race in cert renewal #42

Merged
merged 3 commits into from
Jan 22, 2025
Merged

fix(client): race in cert renewal #42

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
660a36b
fix: race in cert renewal
lidel Jan 17, 2025
60f66f8
refactor: named acme-broker logger
lidel Jan 17, 2025
d44d4af
test: OnCertRenewed
lidel Jan 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
147 changes: 83 additions & 64 deletions client/acme.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ type P2PForgeCertMgr struct {
ProvideHost func(host.Host)
hostFn func() host.Host
hasHost func() bool
cfg *certmagic.Config
certmagic *certmagic.Config
log *zap.SugaredLogger
allowPrivateForgeAddresses bool
produceShortAddrs bool
Expand All @@ -45,11 +45,6 @@ type P2PForgeCertMgr struct {
certCheckMx sync.RWMutex
}

var (
defaultCertCache *certmagic.Cache
defaultCertCacheMu sync.Mutex
)

func isRelayAddr(a multiaddr.Multiaddr) bool {
found := false
multiaddr.ForEach(a, func(c multiaddr.Component) bool {
Expand Down Expand Up @@ -84,9 +79,11 @@ type P2PForgeCertMgrConfig struct {
storage certmagic.Storage
modifyForgeRequest func(r *http.Request) error
onCertLoaded func()
onCertRenewed func()
log *zap.SugaredLogger
allowPrivateForgeAddresses bool
produceShortAddrs bool
renewCheckInterval time.Duration
}

type P2PForgeCertMgrOptions func(*P2PForgeCertMgrConfig) error
Expand Down Expand Up @@ -176,6 +173,22 @@ func WithTrustedRoots(trustedRoots *x509.CertPool) P2PForgeCertMgrOptions {
}
}

// WithOnCertRenewed is optional callback executed on cert renewal event
func WithOnCertRenewed(fn func()) P2PForgeCertMgrOptions {
return func(config *P2PForgeCertMgrConfig) error {
config.onCertRenewed = fn
return nil
}
}

// WithRenewCheckInterval is meant for testing
func WithRenewCheckInterval(renewCheckInterval time.Duration) P2PForgeCertMgrOptions {
return func(config *P2PForgeCertMgrConfig) error {
config.renewCheckInterval = renewCheckInterval
return nil
}
}

// WithAllowPrivateForgeAddrs is meant for testing or skipping all the
// connectivity checks libp2p node needs to pass before it can request domain
// and start ACME DNS-01 challenge.
Expand Down Expand Up @@ -210,41 +223,13 @@ func WithLogger(log *zap.SugaredLogger) P2PForgeCertMgrOptions {
}
}

// newCertmagicConfig is p2p-forge/client-specific version of
// certmagic.NewDefault() that ensures we have our own cert cache. This is
// necessary to ensure cert maintenance spawned by NewCache does not share
// global certmagic.Default.Storage, and certmagic.Default.Logger and uses
// storage path specific to p2p-forge, and no other instance of certmagic in
// golang application.
func newCertmagicConfig(mgrCfg *P2PForgeCertMgrConfig) *certmagic.Config {
clog := mgrCfg.log.Desugar()

defaultCertCacheMu.Lock()
if defaultCertCache == nil {
defaultCertCache = certmagic.NewCache(certmagic.CacheOptions{
GetConfigForCert: func(certmagic.Certificate) (*certmagic.Config, error) {
// default getter that does not depend on certmagic defaults
// and respects Config.Storage path
return newCertmagicConfig(mgrCfg), nil
},
Logger: clog,
})
}
certCache := defaultCertCache
defaultCertCacheMu.Unlock()

return certmagic.New(certCache, certmagic.Config{
Storage: mgrCfg.storage,
Logger: clog,
})
}

// NewP2PForgeCertMgr handles the creation and management of certificates that are automatically granted by a forge
// to a libp2p host.
//
// Calling this function signifies your acceptance to
// the CA's Subscriber Agreement and/or Terms of Service. Let's Encrypt is the default CA.
func NewP2PForgeCertMgr(opts ...P2PForgeCertMgrOptions) (*P2PForgeCertMgr, error) {
// Init config + apply optional user settings
mgrCfg := &P2PForgeCertMgrConfig{}
for _, opt := range opts {
if err := opt(mgrCfg); err != nil {
Expand All @@ -270,14 +255,11 @@ func NewP2PForgeCertMgr(opts ...P2PForgeCertMgrOptions) (*P2PForgeCertMgr, error
return nil, fmt.Errorf("must specify the forge registration endpoint if using a non-default forge")
}
}

const defaultStorageLocation = "p2p-forge-certs"
if mgrCfg.storage == nil {
mgrCfg.storage = &certmagic.FileStorage{Path: defaultStorageLocation}
mgrCfg.storage = &certmagic.FileStorage{Path: DefaultStorageLocation}
}

certCfg := newCertmagicConfig(mgrCfg)

// Wire up p2p-forge manager instance
hostChan := make(chan host.Host, 1)
provideHost := func(host host.Host) { hostChan <- host }
hasHostChan := make(chan struct{})
Expand All @@ -293,39 +275,60 @@ func NewP2PForgeCertMgr(opts ...P2PForgeCertMgrOptions) (*P2PForgeCertMgr, error
defer close(hasHostChan)
return <-hostChan
})
mgr := &P2PForgeCertMgr{
forgeDomain: mgrCfg.forgeDomain,
forgeRegistrationEndpoint: mgrCfg.forgeRegistrationEndpoint,
ProvideHost: provideHost,
hostFn: hostFn,
hasHost: hasHostFn,
log: mgrCfg.log,
allowPrivateForgeAddresses: mgrCfg.allowPrivateForgeAddresses,
produceShortAddrs: mgrCfg.produceShortAddrs,
}

// NOTE: callback getter is necessary to avoid circular dependency
// but also structure code to avoid issues like https://github.com/ipshipyard/p2p-forge/issues/28
configGetter := func(cert certmagic.Certificate) (*certmagic.Config, error) {
if mgr.certmagic == nil {
return nil, errors.New("P2PForgeCertmgr.certmagic is not set")
}
return mgr.certmagic, nil
}

magicCache := certmagic.NewCache(certmagic.CacheOptions{
GetConfigForCert: configGetter,
RenewCheckInterval: mgrCfg.renewCheckInterval,
Logger: mgrCfg.log.Desugar(),
})

myACME := certmagic.NewACMEIssuer(certCfg, certmagic.ACMEIssuer{ // TODO: UX around user passed emails + agreement
// Wire up final certmagic config by calling upstream New with sanity checks
mgr.certmagic = certmagic.New(magicCache, certmagic.Config{
Storage: mgrCfg.storage,
Logger: mgrCfg.log.Desugar(),
})

// Wire up Issuer that does brokered DNS-01 ACME challenge
acmeLog := mgrCfg.log.Named("acme-broker")
brokeredDNS01Issuer := certmagic.NewACMEIssuer(mgr.certmagic, certmagic.ACMEIssuer{
CA: mgrCfg.caEndpoint,
Email: mgrCfg.userEmail,
Agreed: true,
DNS01Solver: &dns01P2PForgeSolver{
forgeRegistrationEndpoint: mgrCfg.forgeRegistrationEndpoint,
forgeAuth: mgrCfg.forgeAuth,
hostFn: hostFn,
hostFn: mgr.hostFn,
modifyForgeRequest: mgrCfg.modifyForgeRequest,
userAgent: mgrCfg.userAgent,
allowPrivateForgeAddresses: mgrCfg.allowPrivateForgeAddresses,
log: mgrCfg.log.Named("dns01solver"),
log: acmeLog.Named("dns01solver"),
},
TrustedRoots: mgrCfg.trustedRoots,
Logger: certCfg.Logger,
Logger: acmeLog.Desugar(),
})
mgr.certmagic.Issuers = []certmagic.Issuer{brokeredDNS01Issuer}

certCfg.Issuers = []certmagic.Issuer{myACME}

mgr := &P2PForgeCertMgr{
forgeDomain: mgrCfg.forgeDomain,
forgeRegistrationEndpoint: mgrCfg.forgeRegistrationEndpoint,
ProvideHost: provideHost,
hostFn: hostFn,
hasHost: hasHostFn,
cfg: certCfg,
log: mgrCfg.log,
allowPrivateForgeAddresses: mgrCfg.allowPrivateForgeAddresses,
produceShortAddrs: mgrCfg.produceShortAddrs,
}

certCfg.OnEvent = func(ctx context.Context, event string, data map[string]any) error {
// Wire up onCertLoaded callback
mgr.certmagic.OnEvent = func(ctx context.Context, event string, data map[string]any) error {
if event == "cached_managed_cert" {
sans, ok := data["sans"]
if !ok {
Expand All @@ -352,24 +355,39 @@ func NewP2PForgeCertMgr(opts ...P2PForgeCertMgrOptions) (*P2PForgeCertMgr, error
}
return nil
}

// Execute user function for on certificate cert renewal
if event == "cert_obtained" && mgrCfg.onCertRenewed != nil {
if renewal, ok := data["renewal"].(bool); ok && renewal {
name := certName(hostFn().ID(), mgrCfg.forgeDomain)
if id, ok := data["identifier"].(string); ok && id == name {
mgrCfg.onCertRenewed()
}
}
return nil
}

return nil
}

return mgr, nil
}

func (m *P2PForgeCertMgr) Start() error {
if m.cfg == nil || m.hostFn == nil {
if m.certmagic == nil || m.hostFn == nil {
return errors.New("unable to start without a certmagic and libp2p host")
}
if m.certmagic.Storage == nil {
return errors.New("unable to start without a certmagic Cache and Storage set up")
}
m.ctx, m.cancel = context.WithCancel(context.Background())
go func() {
log := m.log.Named("start")
h := m.hostFn()
name := certName(h.ID(), m.forgeDomain)
certExists := localCertExists(m.ctx, m.cfg, name)
certExists := localCertExists(m.ctx, m.certmagic, name)
startCertManagement := func() {
if err := m.cfg.ManageAsync(m.ctx, []string{name}); err != nil {
if err := m.certmagic.ManageAsync(m.ctx, []string{name}); err != nil {
log.Error(err)
}
}
Expand Down Expand Up @@ -433,8 +451,9 @@ func (m *P2PForgeCertMgr) Stop() {

// TLSConfig returns a tls.Config that managed by the P2PForgeCertMgr
func (m *P2PForgeCertMgr) TLSConfig() *tls.Config {
tlsCfg := m.cfg.TLSConfig()
tlsCfg := m.certmagic.TLSConfig()
tlsCfg.NextProtos = nil // remove the ACME ALPN
tlsCfg.GetCertificate = m.certmagic.GetCertificate
return tlsCfg
}

Expand All @@ -449,7 +468,7 @@ func (m *P2PForgeCertMgr) AddrStrings() []string {
// This should be used with the libp2p.AddrsFactory option to ensure that a libp2p host with forge managed addresses
// only announces those that are active and valid.
func (m *P2PForgeCertMgr) AddressFactory() config.AddrsFactory {
tlsCfg := m.cfg.TLSConfig()
tlsCfg := m.certmagic.TLSConfig()
tlsCfg.NextProtos = []string{"h2", "http/1.1"} // remove the ACME ALPN and set the HTTP 1.1 and 2 ALPNs

return m.createAddrsFactory(m.allowPrivateForgeAddresses, m.produceShortAddrs)
Expand Down
2 changes: 2 additions & 0 deletions client/defaults.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ const (
// ForgeAuthHeader optional HTTP header that client should include when
// talking to a limited access registration endpoint
ForgeAuthHeader = "Forge-Authorization"

DefaultStorageLocation = "p2p-forge-certs"
)

// defaultUserAgent is used as a fallback to inform HTTP server which library
Expand Down
Loading
Loading