Skip to content

Conversation

@pgporada
Copy link
Member

@pgporada pgporada commented May 2, 2024

The draft spec version at the time of this PR was draft-ietf-acme-ari-03, but failed replacement order handling is from the yet-to-be-released draft-ietf-acme-ari-04.

  • Add a renewalInfo entry to the directory object which provides the base URL for ARI requests.
  • Add a new WFE handlefunc which parses incoming requests and returns reasonable renewalInfo for determining when the client should attempt renewal of a certificate.
  • Add support for marking orders as replaced. Replacement orders can be chained, but there can be no duplicate replacement of orders, just like boulder.
  • Restructured the asynchronous finalization anonymous go func to handle storing replaced orders. To be replaced, an order must previously have been finalized and have an issued certificate.

Fixes #403

@pgporada
Copy link
Member Author

pgporada commented May 10, 2024

I've got a fork of eggsampler/acme that I've been working against. To test that against this branch:

# In the pebble dir
$ tmux

# Run challtestsrv, version doesn't particularly matter because we're not touching this code
$ docker rm challtestsrv 2>&1; docker run -p 5001:5001 -p 5002:5002 -p 5003:5003 -p 8053:8053 -p 8055:8055 -p 8443:8443 --name challtestsrv ghcr.io/letsencrypt/pebble-challtestsrv:latest

# Get the IP of that container
$ CHALLTESTSRV=$(docker inspect challtestsrv | jq -r '.[].NetworkSettings.Networks.bridge.IPAddress')

# Run pebble
$ go run cmd/pebble/main.go -config ./test/config/pebble-config.json -dnsserver ${CHALLTESTSRV}:8053

-----------------------------
# In the eggsampler repo
$ export PEBBLE_PATH=/path/to/pebble/on/your/computer
$ go test -test.run TestClient_IssueReplacementCert

@pgporada
Copy link
Member Author

pgporada commented May 14, 2024

Replacement orders are now supported

Pebble 2024/05/14 16:36:21 Order "6it4fspAVD-CNbSI06L4iPf9l29mNpL6rkQi-XnICBo" is not a replacement
Pebble 2024/05/14 16:36:21 There are now 1 orders in the db
Pebble 2024/05/14 16:36:28 Order "4gw9BYS8UnmQnymq8HnxSQA8hOs5le0SkB34Ux1cfDQ" is a replacement of "6it4fspAVD-CNbSI06L4iPf9l29mNpL6rkQi-XnICBo"
Pebble 2024/05/14 16:36:28 There are now 2 orders in the db
Pebble 2024/05/14 16:37:11 Order "vOKNgiDK8aoTvYWC8Q6GJp49UYK-iGPD34vBDEwhnTs" is not a replacement
Pebble 2024/05/14 16:37:11 There are now 3 orders in the db
Pebble 2024/05/14 16:37:20 Order "s7mJpikMeEy8OL57wlwB54_kakew2g6ns6nit8VWuKQ" is a replacement of "vOKNgiDK8aoTvYWC8Q6GJp49UYK-iGPD34vBDEwhnTs"
Pebble 2024/05/14 16:37:20 There are now 4 orders in the db

@pgporada
Copy link
Member Author

pgporada commented May 17, 2024

There's a race condition in wfe.FinalizeOrder at existingOrder := wfe.db.GetOrderByID(orderID) causing an error during manual testing with my eggsample/acme fork.

=== RUN   TestClient_IssueReplacementCert
    ari_test.go:66: Issuing initial order
    ari_test.go:73: Issuing first replacement order
    ari_test.go:80: Issuing second replacement order
    ari_test.go:83: unexpected error: acme: error code 403 "urn:ietf:params:acme:error:orderNotReady": Order's status ("pending") was not ready

vs

=== RUN   TestClient_IssueReplacementCert
    ari_test.go:66: Issuing initial order
    ari_test.go:73: Issuing first replacement order
    ari_test.go:80: Issuing second replacement order
    ari_test.go:87: Should not be able to create a duplicate replacement
--- PASS: TestClient_IssueReplacementCert (19.05s)
PASS

EDIT: This whole entire detour was actually an implementation bug in my eggsampler fork. 🤦🏼

@pgporada pgporada changed the title Begin implementing draft-ietf-acme-ari-03 Implement draft-ietf-acme-ari-03 May 17, 2024
@pgporada pgporada marked this pull request as ready for review May 17, 2024 18:39
@pgporada pgporada requested review from a team, aarongable, beautifulentropy and jsha and removed request for a team May 17, 2024 18:39
aarongable
aarongable previously approved these changes May 20, 2024
@pgporada pgporada changed the title Implement draft-ietf-acme-ari-03 Implement latest draft-ietf-acme-ari spec May 21, 2024
aarongable
aarongable previously approved these changes May 23, 2024
Copy link
Member

@beautifulentropy beautifulentropy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking the time to get this added to Pebble!

Some nits regarding contexts that we're dropping, but nothing that means we can't merge. I think these are just some contexts params from the Boulder implementation which aren't necessary in the Pebble implementation?

@pgporada pgporada merged commit db1f587 into main May 24, 2024
@pgporada pgporada deleted the pARtI branch May 24, 2024 16:06
@orangepizza
Copy link
Contributor

it's sad that this mission latest release by just a day 😢

@pgporada
Copy link
Member Author

pgporada commented May 25, 2024 via email

kwatson added a commit to kwatson/letsencrypt-pebble that referenced this pull request Jun 9, 2025
* 'main' of https://github.com/letsencrypt/pebble: (35 commits)
  add overriding of ARI response (letsencrypt#501)
  wfe: fix a race in `orderForDisplay` (letsencrypt#500)
  Bump golang.org/x/ dependencies (letsencrypt#499)
  currectly triggers BadSignatureAlgorithmProblem at JWS parse time (letsencrypt#492)
  use newer validation subdomain for dns-account-01 (fix CI eggsampler/acme error) (letsencrypt#498)
  Orders don't have a "deactivated" status. (letsencrypt#301)
  Update golangci-lint (letsencrypt#488)
  build(deps): bump github.com/go-jose/go-jose/v4 from 4.0.4 to 4.0.5 (letsencrypt#487)
  Truncate ARI timestamps to millisecond resolution (letsencrypt#485)
  return logical and compliant ARI windows for expiring certs (letsencrypt#484)
  Update dependencies (letsencrypt#481)
  docs: rm mention of subproblems being unimpl'd (letsencrypt#479)
  Fix(NOISSUE): Fix docker compose file example in README.md (letsencrypt#475)
  Add support for ACME Profiles (letsencrypt#473)
  Simplify KU, EKU, and SKID fields of issued certs (letsencrypt#472)
  Update golangci-lint to 1.60.2 (letsencrypt#474)
  Update /x/net for compatibility with go1.23 (letsencrypt#470)
  Reject extra command line args and fix README invocation (letsencrypt#467)
  Document exposing API and management ports when not using docker-compose.yaml (letsencrypt#465)
  Implement latest draft-ietf-acme-ari spec (letsencrypt#461)
  ...
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

Successfully merging this pull request may close these issues.

ACME Renewal Information (ARI)

5 participants