Repository navigation
wfe: New endpoint for serving standalone MTCs #9102
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -36,6 +36,7 @@ import ( | |||||
| "github.com/letsencrypt/boulder/issuance" | ||||||
| blog "github.com/letsencrypt/boulder/log" | ||||||
| "github.com/letsencrypt/boulder/metrics/measured_http" | ||||||
| mtcbpb "github.com/letsencrypt/boulder/mtcb/proto" | ||||||
| "github.com/letsencrypt/boulder/nonce" | ||||||
| "github.com/letsencrypt/boulder/policy" | ||||||
| "github.com/letsencrypt/boulder/probs" | ||||||
|
|
@@ -66,6 +67,7 @@ const ( | |||||
| finalizeOrderPath = "/acme/finalize/" | ||||||
| certPath = "/acme/cert/" | ||||||
| renewalInfoPath = "/acme/renewal-info/" | ||||||
| mtcPath = "/acme/mtc/" | ||||||
|
|
||||||
| // Non-ACME paths. | ||||||
| getCertPath = "/get/cert/" | ||||||
|
|
@@ -89,9 +91,10 @@ var errIncompleteGRPCResponse = errors.New("incomplete gRPC response message") | |||||
| // plus a few other data items used in ACME. Its methods are primarily handlers | ||||||
| // for HTTPS requests for the various ACME functions. | ||||||
| type WebFrontEndImpl struct { | ||||||
| ra rapb.RegistrationAuthorityClient | ||||||
| sa sapb.StorageAuthorityReadOnlyClient | ||||||
| ee emailpb.ExporterClient | ||||||
| mtcb mtcbpb.MTCBClient | ||||||
| // gnc is a nonce-service client used exclusively for the issuance of | ||||||
| // nonces. It's configured to route requests to backends colocated with the | ||||||
| // WFE. | ||||||
|
|
@@ -211,6 +214,7 @@ func NewWebFrontEndImpl( | |||||
| rac rapb.RegistrationAuthorityClient, | ||||||
| sac sapb.StorageAuthorityReadOnlyClient, | ||||||
| eec emailpb.ExporterClient, | ||||||
| mtcbc mtcbpb.MTCBClient, | ||||||
| gnc nonce.Getter, | ||||||
| rnc nonce.Redeemer, | ||||||
| rncKey []byte, | ||||||
|
|
@@ -264,6 +268,7 @@ func NewWebFrontEndImpl( | |||||
| ra: rac, | ||||||
| sa: sac, | ||||||
| ee: eec, | ||||||
| mtcb: mtcbc, | ||||||
| gnc: gnc, | ||||||
| rnc: rnc, | ||||||
| rncKey: rncKey, | ||||||
|
|
@@ -310,7 +315,7 @@ func (wfe *WebFrontEndImpl) HandleFunc(mux *http.ServeMux, pattern string, h web | |||||
| methodsMap["HEAD"] = true | ||||||
| } | ||||||
| methodsStr := strings.Join(methods, ", ") | ||||||
| var handler http.Handler = web.NewTopHandler(wfe.log, | ||||||
| web.WFEHandlerFunc(func(ctx context.Context, logEvent *web.RequestEvent, response http.ResponseWriter, request *http.Request) { | ||||||
| span := trace.SpanFromContext(ctx) | ||||||
| span.SetName(pattern) | ||||||
|
|
@@ -366,7 +371,14 @@ func (wfe *WebFrontEndImpl) HandleFunc(mux *http.ServeMux, pattern string, h web | |||||
| h(ctx, logEvent, response, request) | ||||||
| cancel() | ||||||
| }), | ||||||
| ) | ||||||
| // Detect the lack of at least one PAIR of curly braces as a signal that | ||||||
| // there is no wildcard pattern. This gives us a way to migrate handlers | ||||||
| // individually to use wildcard patterns, as our non-wildcard handlers need | ||||||
| // prefixes stripped. | ||||||
| if !strings.Contains(pattern, `{`) && !strings.Contains(pattern, `}`) { | ||||||
| handler = http.StripPrefix(pattern, handler) | ||||||
| } | ||||||
| mux.Handle(pattern, handler) | ||||||
| } | ||||||
|
|
||||||
|
|
@@ -466,6 +478,9 @@ func (wfe *WebFrontEndImpl) Handler(stats prometheus.Registerer) http.Handler { | |||||
| wfe.HandleFunc(m, certPath, wfe.Certificate, "GET", "POST") | ||||||
| wfe.HandleFunc(m, renewalInfoPath, wfe.RenewalInfo, "GET", "POST") | ||||||
|
|
||||||
| // POST-as-GETable ACME endpoints | ||||||
| wfe.HandleFunc(m, mtcPath+"{acctID}/{orderID}/{logID}/{serial}", wfe.MerkleTreeCertificate, "POST") | ||||||
|
Contributor
There was a problem hiding this comment.
Suggested change
We use the fully-typed-out "mtcLogID" in other places in the codebase and I think it helps (a) to be consistent and (b) to be explicit, for instance this isn't the ID of some other type of log. |
||||||
|
|
||||||
| // Boulder specific endpoints | ||||||
| wfe.HandleFunc(m, getCertPath, wfe.Certificate, "GET") | ||||||
| wfe.HandleFunc(m, getCertInfoPath, wfe.CertificateInfo, "GET") | ||||||
|
|
@@ -1855,6 +1870,126 @@ func (wfe *WebFrontEndImpl) Certificate(ctx context.Context, logEvent *web.Reque | |||||
| } | ||||||
| } | ||||||
|
|
||||||
| // MerkleTreeCertificate is used by client to request a copy of their merkle tree certificate. | ||||||
|
Contributor
There was a problem hiding this comment.
Suggested change
|
||||||
| func (wfe *WebFrontEndImpl) MerkleTreeCertificate(ctx context.Context, logEvent *web.RequestEvent, response http.ResponseWriter, request *http.Request) { | ||||||
| acct, err := wfe.validPOSTAsGETForAccount(request, ctx, logEvent) | ||||||
| if err != nil { | ||||||
| wfe.sendError(response, logEvent, web.ProblemDetailsForError(err, "Unable to validate JWS"), err) | ||||||
| return | ||||||
| } | ||||||
|
|
||||||
| // wfe instance not configured with an MTCB Client | ||||||
| if wfe.mtcb == nil { | ||||||
| wfe.sendError(response, logEvent, probs.Malformed("Not Supported"), nil) | ||||||
|
Contributor
There was a problem hiding this comment. IMO this is a little better as a 404 for now. |
||||||
| return | ||||||
| } | ||||||
|
|
||||||
| reqAcctID, err := parsePositiveInt(request.PathValue("acctID")) | ||||||
| if err != nil { | ||||||
| wfe.sendError(response, logEvent, probs.Malformed("Request URL does not include a valid accountID"), nil) | ||||||
|
Contributor
|
||||||
| return | ||||||
| } | ||||||
| reqOrderID, err := parsePositiveInt(request.PathValue("orderID")) | ||||||
| if err != nil { | ||||||
| wfe.sendError(response, logEvent, probs.Malformed("Request URL does not include a valid orderID"), nil) | ||||||
| return | ||||||
| } | ||||||
| reqSerial, err := parsePositiveUint(request.PathValue("serial")) | ||||||
| if err != nil { | ||||||
| wfe.sendError(response, logEvent, probs.Malformed("Request URL does not include a valid serial number"), nil) | ||||||
| return | ||||||
| } | ||||||
|
|
||||||
| // Verify that the request path account ID matches the POST-as-GET-validated account ID | ||||||
| if reqAcctID != acct.ID { | ||||||
| wfe.sendError(response, logEvent, probs.Unauthorized(fmt.Sprintf("Not allowed for ID %d", acct.ID)), nil) | ||||||
| return | ||||||
| } | ||||||
|
|
||||||
| // MerkleTreeCertificate does not need deployability protection via | ||||||
| // WFECallsRAForGetOrder, and always looks up orders via the RA. | ||||||
| order, err := wfe.ra.GetOrder(ctx, &rapb.GetOrderRequest{OrderID: reqOrderID}) | ||||||
| if err != nil { | ||||||
| if errors.Is(err, berrors.NotFound) { | ||||||
| wfe.sendError(response, logEvent, probs.NotFound(fmt.Sprintf("No order for ID %d", reqOrderID)), nil) | ||||||
| return | ||||||
| } | ||||||
| wfe.sendError(response, logEvent, web.ProblemDetailsForError(err, | ||||||
| fmt.Sprintf("Failed to retrieve order for ID %d", reqOrderID)), err) | ||||||
| return | ||||||
| } | ||||||
|
|
||||||
| if core.IsAnyNilOrZero(order.Id, order.Status, order.RegistrationID, order.Identifiers, order.Created, order.Expires) { | ||||||
| wfe.sendError(response, logEvent, probs.ServerInternal(fmt.Sprintf("Failed to retrieve order for ID %d", reqOrderID)), errIncompleteGRPCResponse) | ||||||
| return | ||||||
| } | ||||||
|
|
||||||
| // If the order is not yet valid, then we did not find what the requester | ||||||
| // wants. | ||||||
|
Comment on lines
+1927
to
+1928
Contributor
There was a problem hiding this comment. IMO this and the below cases fall under CONTRIBUTING.md's "No need to gratuitously comment clear code." They're self-explanatory error cases, no comment needed. |
||||||
| if order.Status != string(core.StatusValid) { | ||||||
| wfe.sendError(response, logEvent, probs.NotFound(fmt.Sprintf("No order for ID %d", reqOrderID)), nil) | ||||||
| return | ||||||
| } | ||||||
|
|
||||||
| // If request Path ID does not match order registration ID, then we did not | ||||||
| // find what the requester wants. | ||||||
| if acct.ID != order.RegistrationID { | ||||||
| wfe.sendError(response, logEvent, probs.NotFound(fmt.Sprintf("No order for ID %d", reqOrderID)), nil) | ||||||
| return | ||||||
| } | ||||||
|
|
||||||
| // If the order does not have merkle tree certificate information, then we did | ||||||
| // not find what the requester wants. | ||||||
| if core.IsAnyNilOrZero(order.MtcLogID, order.MtcSerialNumber, order.MtcSubtreeID) { | ||||||
| wfe.sendError(response, logEvent, probs.NotFound(fmt.Sprintf("No merkle tree certificate for order ID %d", reqOrderID)), nil) | ||||||
| return | ||||||
| } | ||||||
|
|
||||||
| // If the request path is looking for an MTC that doesn't match the | ||||||
| // retrieved order, their request is malformed. | ||||||
| if request.PathValue("logID") != order.MtcLogID || reqSerial != order.MtcSerialNumber { | ||||||
| wfe.sendError(response, logEvent, probs.Malformed("MTC request URL details do not match order details"), nil) | ||||||
| return | ||||||
| } | ||||||
|
|
||||||
| logEvent.Extra["RequestedMtcSerial"] = reqSerial | ||||||
|
|
||||||
| // Now ask the MTCB for the standalone certificate. | ||||||
| standalone, err := wfe.mtcb.GetStandalone(ctx, | ||||||
| &mtcbpb.StandaloneRequest{ | ||||||
| MtcLogID: order.MtcLogID, | ||||||
| MtcSerialNumber: order.MtcSerialNumber, | ||||||
| MtcSubtreeID: order.MtcSubtreeID, | ||||||
| }) | ||||||
| if err != nil { | ||||||
| wfe.sendError(response, logEvent, web.ProblemDetailsForError(err, "Failed to retrieve merkle tree certificate"), err) | ||||||
| return | ||||||
| } | ||||||
|
|
||||||
| if standalone.CertDER == nil { | ||||||
|
Contributor
There was a problem hiding this comment.
|
||||||
| wfe.sendError(response, logEvent, probs.ServerInternal("Problem getting merkle tree certificate"), errIncompleteGRPCResponse) | ||||||
| return | ||||||
| } | ||||||
|
|
||||||
| // And PEM-encode the retrieved certificate for return to the client. | ||||||
| standalonePEM := pem.EncodeToMemory(&pem.Block{ | ||||||
|
Comment on lines
+1974
to
+1975
Contributor
There was a problem hiding this comment. This comment is also unneeded: |
||||||
| Type: "CERTIFICATE", | ||||||
| Bytes: standalone.CertDER, | ||||||
| }) | ||||||
|
|
||||||
| // NOTE(@ezekiel): Similar to Certificate(), we must explicitly set the | ||||||
| // Content-Length header here. The Go HTTP library will only add this header | ||||||
| // if the body is below a certain size and a PEM encoded merkle tree | ||||||
| // certificate will likely exceed this threshold. Since we know the length | ||||||
| // we can reliably set it ourselves and not worry. | ||||||
|
Comment on lines
+1981
to
+1985
Contributor
There was a problem hiding this comment. I see this comment is a copy of an existing comment with Further, with the benefit of hindsight I think we could shorten or remove both comments. I might say Really the comment I wish we had is one that explains why it's important that we set Content-Length here. I think we don't actually need it, and #3366 doesn't explain why it's needed. The purpose of Content-Length is so an HTTP client knows when a response is done and reuse the connection for the next request. However, there's an alternative that Go will use automatically in HTTP/1.1: Anyhow, it's about six of one half dozen of the other: my inclination is to keep setting Content-Length explicitly in both sites, and shorten both comments.
Contributor
There was a problem hiding this comment. Some additional context in #1320: we set Content-Length because otherwise our responses to HEAD will have the wrong Content-Length, per HTTP spec. |
||||||
| response.Header().Set("Content-Length", strconv.Itoa(len(standalonePEM))) | ||||||
| response.Header().Set("Content-Type", "application/pem-certificate-chain") | ||||||
|
Contributor
There was a problem hiding this comment.
We don't have the code for CertificatePropertyList yet but might as well start using the Content-Type. |
||||||
| response.WriteHeader(http.StatusOK) | ||||||
| if _, err = response.Write(standalonePEM); err != nil { | ||||||
| wfe.log.Warningf("Could not write response: %s", err) | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| // BuildID tells the requester what build we're running. | ||||||
| func (wfe *WebFrontEndImpl) BuildID(ctx context.Context, logEvent *web.RequestEvent, response http.ResponseWriter, request *http.Request) { | ||||||
| response.Header().Set("Content-Type", "text/plain") | ||||||
|
|
@@ -2114,9 +2249,19 @@ func (wfe *WebFrontEndImpl) orderToOrderJSON(request *http.Request, order *corep | |||||
| respObj.Authorizations = append(respObj.Authorizations, endpoint) | ||||||
| } | ||||||
| if respObj.Status == core.StatusValid { | ||||||
| if order.CertificateSerial != "" { | ||||||
| respObj.Certificate = web.RelativeEndpoint(request, | ||||||
| certPath, order.CertificateSerial) | ||||||
| } | ||||||
| if !core.IsAnyNilOrZero(order.MtcLogID, order.MtcSerialNumber, order.MtcSubtreeID) && wfe.mtcb != nil { | ||||||
| respObj.Certificate = web.RelativeEndpoint(request, | ||||||
| mtcPath, | ||||||
| fmt.Sprintf("%d", order.RegistrationID), | ||||||
| fmt.Sprintf("%d", order.Id), | ||||||
| order.MtcLogID, | ||||||
| fmt.Sprintf("%d", order.MtcSerialNumber), | ||||||
| ) | ||||||
| } | ||||||
| } | ||||||
| return respObj | ||||||
| } | ||||||
|
|
@@ -2891,3 +3036,13 @@ func parsePositiveInt(v string) (int64, error) { | |||||
| } | ||||||
| return parsed, nil | ||||||
| } | ||||||
|
|
||||||
| // parsePositiveUint returns the input parsed as an uint64, or a | ||||||
| // berrors.NotFoundError on parse failure, zero, or negative numbers. | ||||||
| func parsePositiveUint(v string) (uint64, error) { | ||||||
| parsed, err := strconv.ParseUint(v, 10, 64) | ||||||
| if err != nil || parsed == 0 { | ||||||
| return 0, berrors.NotFoundError("") | ||||||
| } | ||||||
| return parsed, nil | ||||||
| } | ||||||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PAIR is emphasized here; why?