WFE: Return bare expired certs with no issuer (#5362)
When we receive a request for a certificate for which the WFE no longer has the issuer configured in its certificate chains, and the requested certificate is expired, return just the bare cert rather than returning a 500 error. To enable this, refactor the chain-construction logic to occur inside a closure, so that both error-path and non-error-path early returns are possible. This also simplifies the chain construction logic to be more straight-line and readable, despite taking place inside a closure. Fixes #5345
This commit is contained in:
parent
cdce9f0f2f
commit
69aed25cc6
55
wfe2/wfe.go
55
wfe2/wfe.go
|
|
@ -1573,64 +1573,63 @@ func (wfe *WebFrontEndImpl) Certificate(ctx context.Context, logEvent *web.Reque
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
responsePEM, prob := func() ([]byte, *probs.ProblemDetails) {
|
||||||
leafPEM := pem.EncodeToMemory(&pem.Block{
|
leafPEM := pem.EncodeToMemory(&pem.Block{
|
||||||
Type: "CERTIFICATE",
|
Type: "CERTIFICATE",
|
||||||
Bytes: cert.DER,
|
Bytes: cert.DER,
|
||||||
})
|
})
|
||||||
|
|
||||||
var responsePEM []byte
|
// If we don't have any certificateChains configured, just return the cert.
|
||||||
|
if len(wfe.certificateChains) == 0 {
|
||||||
|
return leafPEM, nil
|
||||||
|
}
|
||||||
|
|
||||||
// If the WFE is configured with certificateChains, construct a chain for this
|
|
||||||
// certificate using its IssuerNameID.
|
|
||||||
if len(wfe.certificateChains) > 0 {
|
|
||||||
parsedCert, err := x509.ParseCertificate(cert.DER)
|
parsedCert, err := x509.ParseCertificate(cert.DER)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
// If we can't parse one of our own certs there's a serious problem
|
// If we can't parse one of our own certs there's a serious problem
|
||||||
wfe.sendError(response, logEvent, probs.ServerInternal(
|
return nil, probs.ServerInternal(
|
||||||
fmt.Sprintf(
|
fmt.Sprintf(
|
||||||
"unable to parse Boulder issued certificate with serial %#v",
|
"unable to parse Boulder issued certificate with serial %#v: %s",
|
||||||
serial),
|
serial,
|
||||||
), err)
|
err),
|
||||||
return
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
issuerNameID := issuance.GetIssuerNameID(parsedCert)
|
issuerNameID := issuance.GetIssuerNameID(parsedCert)
|
||||||
availableChains, ok := wfe.certificateChains[issuerNameID]
|
availableChains, ok := wfe.certificateChains[issuerNameID]
|
||||||
if !ok || len(availableChains) == 0 {
|
if !ok || len(availableChains) == 0 {
|
||||||
// If there is no wfe.certificateChains entry for the IssuerNameID there
|
// If there is no wfe.certificateChains entry for the IssuerNameID then
|
||||||
// is probably a misconfiguration and we should treat it as an internal
|
// we can't provide a chain for this cert. If the certificate is expired,
|
||||||
// server error.
|
// just return the bare cert. If the cert is still valid, then there is
|
||||||
wfe.sendError(response, logEvent, probs.ServerInternal(
|
// a misconfiguration and we should treat it as an internal server error.
|
||||||
|
if parsedCert.NotAfter.Before(wfe.clk.Now()) {
|
||||||
|
return leafPEM, nil
|
||||||
|
}
|
||||||
|
return nil, probs.ServerInternal(
|
||||||
fmt.Sprintf(
|
fmt.Sprintf(
|
||||||
"Certificate serial %#v has an unknown IssuerNameID %d - no PEM certificate chain associated.",
|
"Certificate serial %#v has an unknown IssuerNameID %d - no PEM certificate chain associated.",
|
||||||
serial,
|
serial,
|
||||||
issuerNameID),
|
issuerNameID),
|
||||||
), nil)
|
)
|
||||||
return
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// If the requested chain is outside the bounds of the available chains,
|
// If the requested chain is outside the bounds of the available chains,
|
||||||
// then it is an error by the client - not found.
|
// then it is an error by the client - not found.
|
||||||
if requestedChain < 0 || requestedChain >= len(availableChains) {
|
if requestedChain < 0 || requestedChain >= len(availableChains) {
|
||||||
wfe.sendError(response, logEvent, probs.NotFound("Unknown issuance chain"), nil)
|
return nil, probs.NotFound("Unknown issuance chain")
|
||||||
return
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// Double check that the signature validates.
|
// Double check that the signature validates.
|
||||||
err = parsedCert.CheckSignatureFrom(wfe.issuerCertificates[issuerNameID].Certificate)
|
err = parsedCert.CheckSignatureFrom(wfe.issuerCertificates[issuerNameID].Certificate)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
wfe.sendError(response, logEvent, probs.ServerInternal(
|
return nil, probs.ServerInternal(
|
||||||
fmt.Sprintf(
|
fmt.Sprintf(
|
||||||
"Certificate serial %#v has a signature which cannot be verified from issuer %d.",
|
"Certificate serial %#v has a signature which cannot be verified from issuer %d.",
|
||||||
serial,
|
serial,
|
||||||
issuerNameID),
|
issuerNameID),
|
||||||
), nil)
|
)
|
||||||
return
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// Prepend the chain with the leaf certificate
|
|
||||||
responsePEM = append(leafPEM, availableChains[requestedChain]...)
|
|
||||||
|
|
||||||
// Add rel="alternate" links for every chain available for this issuer,
|
// Add rel="alternate" links for every chain available for this issuer,
|
||||||
// excluding the currently requested chain.
|
// excluding the currently requested chain.
|
||||||
for chainID := range availableChains {
|
for chainID := range availableChains {
|
||||||
|
|
@ -1642,10 +1641,12 @@ func (wfe *WebFrontEndImpl) Certificate(ctx context.Context, logEvent *web.Reque
|
||||||
response.Header().Add("Link", link(chainURL, "alternate"))
|
response.Header().Add("Link", link(chainURL, "alternate"))
|
||||||
}
|
}
|
||||||
|
|
||||||
} else {
|
// Prepend the chain with the leaf certificate
|
||||||
// Otherwise, with no configured certificateChains just serve the leaf
|
return append(leafPEM, availableChains[requestedChain]...), nil
|
||||||
// certificate.
|
}()
|
||||||
responsePEM = leafPEM
|
if prob != nil {
|
||||||
|
wfe.sendError(response, logEvent, prob, nil)
|
||||||
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
// NOTE(@cpu): We must explicitly set the Content-Length header here. The Go
|
// NOTE(@cpu): We must explicitly set the Content-Length header here. The Go
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue