Skip to content

Commit 7681257

Browse files
committed
Try cert Manager before asking permission
Managers are expected to have 'asking permission' built in
1 parent f7ea6fb commit 7681257

File tree

2 files changed

+9
-8
lines changed

2 files changed

+9
-8
lines changed

certmagic.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,9 @@ 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.
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.
352354
GetCertificate(context.Context, *tls.ClientHelloInfo) (*tls.Certificate, error)
353355
}
354356

handshake.go

+6-7
Original file line numberDiff line numberDiff line change
@@ -316,13 +316,6 @@ func (cfg *Config) getCertDuringHandshake(ctx context.Context, hello *tls.Client
316316
}()
317317
}
318318

319-
// Make sure a certificate is allowed for the given name. If not, it doesn't
320-
// make sense to try loading one from storage (issue #185), getting it from a
321-
// certificate manager, or obtaining one from an issuer.
322-
if err := cfg.checkIfCertShouldBeObtained(ctx, name, false); err != nil {
323-
return Certificate{}, fmt.Errorf("certificate is not allowed for server name %s: %w", name, err)
324-
}
325-
326319
// If an external Manager is configured, try to get it from them.
327320
// Only continue to use our own logic if it returns empty+nil.
328321
externalCert, err := cfg.getCertFromAnyCertManager(ctx, hello, logger)
@@ -333,6 +326,12 @@ func (cfg *Config) getCertDuringHandshake(ctx context.Context, hello *tls.Client
333326
return externalCert, nil
334327
}
335328

329+
// Make sure a certificate is allowed for the given name. If not, it doesn't make sense
330+
// to try loading one from storage (issue #185) or obtaining one from an issuer.
331+
if err := cfg.checkIfCertShouldBeObtained(ctx, name, false); err != nil {
332+
return Certificate{}, fmt.Errorf("certificate is not allowed for server name %s: %w", name, err)
333+
}
334+
336335
// We might be able to load or obtain a needed certificate. Load from
337336
// storage if OnDemand is enabled, or if there is the possibility that
338337
// a statically-managed cert was evicted from a full cache.

0 commit comments

Comments
 (0)