Skip to content

mod_md: Honour the Retry-After header of ACME responses (BZ 70282) - #868

Open
arturobernalg wants to merge 1 commit into
apache:trunkfrom
arturobernalg:fix/httpd-70282-retry-after
Open

arturobernalg wants to merge 1 commit into
apache:trunkfrom
arturobernalg:fix/httpd-70282-retry-after

Conversation

@arturobernalg

Copy link
Copy Markdown
Member

Bugzilla: https://bz.apache.org/bugzilla/show_bug.cgi?id=70282

mod_md ignores the Retry-After header of ACME responses. RFC 8555 recommends honouring it, and ignoring it makes mod_md race against pebble in CI.

This adds ap_parse_retry_after() to the core. It accepts delay-seconds or an HTTP-date in any of the three formats of RFC 9110, section 5.6.7, and rejects everything else, such as time zones other than GMT, text after the date, or lists. An HTTP-date is read against the Date header of the response, so a skewed server clock does not matter. The day name is checked for its syntax only, as RFC 9110 does not require rejecting an inconsistent one.

mod_md uses it when polling orders and authorizations, after the challenge POST and the finalize response, and on rate limited errors, where the job is rescheduled no earlier than the CA asked, capped at one day. A hint beyond the timeout is waited for only until the deadline, then one last poll is made. The wait ends early when the server stops or restarts, and such a cancelled run is not counted as a renewal error.

Validated on Linux:

  • libcheck unit tests: 177 ok.
  • The complete test/modules/md suite with pebble: 346 passed, 28 skipped (no OCSP responder, Sectigo and ZeroSSL environment variables not set, boulder-only cases).
  • End-to-end runs of a2md and of httpd with mod_md against a fake ACME server, with Retry-After as seconds, dates, invalid values, skewed clocks, rate limits and timeouts, including stop and restart while waiting.
  • The parser under UBSan and ASan with extreme clocks.

Limits of this validation:

  • Not tested on Windows, including the a2md build.
  • a2md does not link httpd, so it carries a copy of the parser in md_acme.c under MD_A2MD_BUILD. The two copies are identical and were checked against the same test cases, but they are still two copies. A private header shared by both would avoid that, if the maintainers prefer it.
  • The change touches the core and mod_md together and can be split if preferred.
  • MMN 20211221.34 is also used by other open PRs and will need renumbering if one of them is merged first.
  • This does not prove that the original CI flake is gone in every environment. The Apache CI run is the confirmation.

The new tests are libcheck unit tests in test/unit/util.c, not pyhttpd tests. The existing test/modules/md suite covers the mod_md side.


  • This PR does not report or address a security vulnerability
    (see SECURITY.md and
    https://www.apache.org/security/#reporting-a-vulnerability).
  • Code follows the httpd style guide.
  • New log messages (level debug or higher) use an empty APLOGNO() tag;
    numbers are assigned by a committer at merge time (see docs/log-message-tags/README).
  • If the change is user-visible, a changes-entries/*.txt file is included,
    following the template in README.CHANGES (not needed otherwise).
  • Test cases based on pyhttpd are included where appropriate, in the
    test/modules/xxx directory matching the modules/xxx module source,
    or test/modules/core for core server changes. On Unix, verify these
    pass locally with e.g. make check-pytest PYTEST_DIRS=test/modules/xxx.

Add ap_parse_retry_after() to parse the value of a Retry-After header, either
delay-seconds or an HTTP-date as defined in RFC 9110, section 5.6.7. mod_md
uses it as RFC 8555 recommends: when polling orders and authorizations, after
finalizing an order and when a request is rate limited. A hint beyond the
timeout is waited for only until the deadline, and the wait ends early on
shutdown. a2md does not link httpd and carries its own copy of the parser.

PR 70282.
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.

1 participant