Skip to content

Commit aa4d957

Browse files
committed
Return error if cert manager returns error
Don't try to issue certificate. If a cert manager returns an error, it indicates that it was supposed to be able to get a cert for that name but was unable to do so.
1 parent 7681257 commit aa4d957

File tree

2 files changed

+15
-10
lines changed

2 files changed

+15
-10
lines changed

certmagic.go

+6-3
Original file line numberDiff line numberDiff line change
@@ -348,9 +348,12 @@ type Revoker interface {
348348
type Manager interface {
349349
// GetCertificate returns the certificate to use to complete the handshake.
350350
// Since this is called during every TLS handshake, it must be very fast and not block.
351-
// Returning (nil, nil) is valid and is simply treated as a no-op. Return (nil, nil)
352-
// when the Manager has no certificate for this handshake. Return an error or a
353-
// certificate only if the Manager is supposed to get a certificate for this handshake.
351+
// Returning any non-nil value indicates that this Manager manages a certificate
352+
// for the described handshake. Returning (nil, nil) is valid and is simply treated as
353+
// a no-op Return (nil, nil) when the Manager has no certificate for this handshake.
354+
// Return an error or a certificate only if the Manager is supposed to get a certificate
355+
// for this handshake. Returning (nil, nil) other Managers or Issuers to try to get
356+
// a certificate for the handshake.
354357
GetCertificate(context.Context, *tls.ClientHelloInfo) (*tls.Certificate, error)
355358
}
356359

handshake.go

+9-7
Original file line numberDiff line numberDiff line change
@@ -752,31 +752,33 @@ func (cfg *Config) getCertFromAnyCertManager(ctx context.Context, hello *tls.Cli
752752
return Certificate{}, nil
753753
}
754754

755-
var upstreamCert *tls.Certificate
756-
757755
// try all the GetCertificate methods on external managers; use first one that returns a certificate
756+
var upstreamCert *tls.Certificate
757+
var err error
758758
for i, certManager := range cfg.OnDemand.Managers {
759-
var err error
760759
upstreamCert, err = certManager.GetCertificate(ctx, hello)
761760
if err != nil {
762-
logger.Error("getting certificate from external certificate manager",
761+
logger.Error("external certificate manager",
763762
zap.String("sni", hello.ServerName),
764-
zap.Int("cert_manager", i),
763+
zap.String("cert_manager", fmt.Sprintf("%T", certManager)),
764+
zap.Int("cert_manager_idx", i),
765765
zap.Error(err))
766766
continue
767767
}
768768
if upstreamCert != nil {
769769
break
770770
}
771771
}
772+
if err != nil {
773+
return Certificate{}, fmt.Errorf("external certificate manager indicated that it is unable to yield certificate: %v", err)
774+
}
772775
if upstreamCert == nil {
773776
logger.Debug("all external certificate managers yielded no certificates and no errors", zap.String("sni", hello.ServerName))
774777
return Certificate{}, nil
775778
}
776779

777780
var cert Certificate
778-
err := fillCertFromLeaf(&cert, *upstreamCert)
779-
if err != nil {
781+
if err = fillCertFromLeaf(&cert, *upstreamCert); err != nil {
780782
return Certificate{}, fmt.Errorf("external certificate manager: %s: filling cert from leaf: %v", hello.ServerName, err)
781783
}
782784

0 commit comments

Comments
 (0)