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.

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

ARI: Implement ARI POST with a new Order object field #7038

Closed
beautifulentropy opened this issue Aug 11, 2023 · 0 comments
Closed

ARI: Implement ARI POST with a new Order object field #7038

beautifulentropy opened this issue Aug 11, 2023 · 0 comments
Assignees

Comments

@beautifulentropy
Copy link
Member

beautifulentropy commented Aug 11, 2023

This change is specified in aarongable/draft-acme-ari#51.

  • Keep existing POST-as-GET to renewalInfo endpoint implementation intact.
  • Extend the existing newOrder request with a "replaces" field that is expected to be a "CertID" as detailed in https://datatracker.ietf.org/doc/html/draft-ietf-acme-ari-02#section-4.1
  • If the order is a replacement order, then we need to check that:
    • the certificate being replaced exists,
    • the requesting account owns that certificate, and
    • the names in this new order match the names in the certificate being replaced.
  • Once the above conditions are satisfied:
    • Indicate in the NewOrder request dispatched to the RA that this order should bypass rate limits.
    • (future) Mark this certificate as "replaced" in any existing incidents tables.

WFE implementation should look something like:

	var newOrderRequest struct {
		Identifiers                   []identifier.ACMEIdentifier `json:"identifiers"`
		NotBefore, NotAfter, Replaces string
	}

	...

	if newOrderRequest.Replaces != "" {
		// If the order is a replacement order, then we need to check that:
		//   - the certificate being replaced exists,
		//   - the requesting account owns that certificate, and
		//   - the names in this new order match the names in the certificate
		//     being replaced.
		certID, prob, err := newCertID(newOrderRequest.Replaces, wfe.issuerCertificates)
		if err != nil {
			wfe.sendError(response, logEvent, prob, err)
		}
		cert, err := wfe.sa.GetCertificate(ctx, &sapb.Serial{Serial: certID.Serial()})
		if err != nil {
			if errors.Is(err, berrors.NotFound) {
				wfe.sendError(response, logEvent, probs.NotFound("Certificate replaced by this order does not exist"), nil)
			} else {
				wfe.sendError(response, logEvent, web.ProblemDetailsForError(err, "Failed to retrieve certificate  replaced by this order"), err)
			}
			return
		}
		if cert.RegistrationID != acct.ID {
			wfe.sendError(response, logEvent, probs.Unauthorized("Account does not own certificate replaced by this order"), nil)
			return
		}
		var namesMatch bool
		parsedCert, err := x509.ParseCertificate(cert.Der)
		if err != nil {
			wfe.sendError(response, logEvent, probs.ServerInternal("Error parsing certificate replaced by this order"), err)
			return
		}
		for _, name := range core.UniqueLowerNames(names) {
			if !slices.Contains(parsedCert.DNSNames, name) {
				namesMatch = false
				break
			}
			namesMatch = true
		}
		if !namesMatch {
			wfe.sendError(response, logEvent, probs.Malformed("Certificate replaced by this order does not have matching identifiers"), nil)
			return
		}
		bypassLimits = true
	}

	order, err := wfe.ra.NewOrder(ctx, &rapb.NewOrderRequest{
		RegistrationID: acct.ID,
		Names:          names,
		BypassLimits:   bypassLimits,
	})
@beautifulentropy beautifulentropy self-assigned this Dec 5, 2023
@beautifulentropy beautifulentropy added this to the Sprint 2023-12-05 milestone Dec 5, 2023
beautifulentropy added a commit that referenced this issue Feb 26, 2024
Implement draft-ietf-acme-ari-02 changes in WFE newOrder:
- Add a `replaces` field to the newOrder request object
- Ensure that `replaces` values provided by subscribers are vetted
according to the requirements set out in the draft specification
- When a NewOrder request falls inside the suggested RenewalWindow,
exempt from rate limits in the WFE and indicate exemption in the RA
NewOrder request

Part of #7038
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants