Skip to content

Require signatures on HEAD requests to peer-only endpoints#3235

Merged
pfefferle merged 4 commits intotrunkfrom
harden/head-mandatory-auth
Apr 27, 2026
Merged

Require signatures on HEAD requests to peer-only endpoints#3235
pfefferle merged 4 commits intotrunkfrom
harden/head-mandatory-auth

Conversation

@pfefferle
Copy link
Copy Markdown
Member

Proposed changes:

  • verify_signature() short-circuits on HEAD so caches and link-checkers can probe public endpoints without needing to sign — that's the right default. But the bypass also applied to peer-only routes that pass $force_signature = true (today only FEP-8fcf's /followers/sync). Result: an unauthenticated HEAD on /followers/sync reached the handler, which then rejected with 403 (authority mismatch) — same outcome as a malformed authority, instead of the spec-correct 401 (signature required).
  • No data leak (HEAD always strips the body), but the status-code semantics violated the FEP-8fcf "authentication required" contract: a peer-only route shouldn't accept any unsigned method. RFC 9110 also treats HEAD identically to GET for authentication; FEP-8fcf doesn't carve out a method-specific allowance.
  • Tighten the bypass to apply only when $force_signature is false. Mandatory-signing endpoints now reject unsigned HEAD with 401, matching how unsigned GET on the same endpoint is rejected.

Browser-app compatibility verified:

  • Browser CORS preflight uses OPTIONS, not HEAD. WP core's rest_handle_options_request handles OPTIONS via the rest_pre_dispatch filter and never reaches verify_signature.
  • The plugin's own OPTIONS preflight bypass in class-router.php:92 is on the WordPress main-query path (outbox permalinks), not REST routes — also untouched.
  • The only REST route currently passing $force_signature = true is /followers/sync, which is server-to-server (FEP-8fcf), not browser-facing.

So this fix narrows the HEAD bypass to non-mandatory-auth endpoints while leaving every browser flow untouched.

Other information:

  • Have you written new tests for your changes, if applicable?

Testing instructions:

  • Sign a /followers/sync request with a valid keyId and round-trip it as a GET. Should respond as before.
  • Send the same request as HEAD without a Signature header. Should now return 401 with activitypub_signature_verification (was 403 activitypub_authority_mismatch).
  • HEAD on any other endpoint (/inbox, /actors/{id}, /oauth/authorization-server-metadata, etc.) still returns whatever it would have returned on GET, body stripped. No regression for caches/link-checkers.
  • Browser CORS preflight on the C2S API continues to work (OPTIONS, handled by WP core before verify_signature runs).

Changelog entry

  • Automatically create a changelog entry from the details below.
Changelog Entry Details

Significance

  • Patch

Type

  • Security

Message

Required signatures on HEAD requests to peer-only endpoints.

verify_signature() short-circuits on HEAD so caches and link-checkers
can probe public endpoints without needing to sign. That bypass is
the right default, but it also applied to peer-only routes that pass
$force_signature = true (e.g. FEP-8fcf's /followers/sync). The result:
an unauthenticated HEAD on /followers/sync returned 200 — disclosing
that the actor exists and the endpoint is reachable, which is exactly
what FEP-8fcf's mandatory-signing requirement is meant to prevent.

The response body was empty, so no follower data ever leaked, but the
existence-probe still violates the contract. Tighten the bypass to
only apply when $force_signature is false; mandatory-signing endpoints
now require the signature on HEAD too.
Copilot AI review requested due to automatic review settings April 27, 2026 10:47
@pfefferle pfefferle self-assigned this Apr 27, 2026
@pfefferle pfefferle requested a review from a team April 27, 2026 10:47
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR tightens REST signature verification behavior so that peer-only endpoints (those calling verify_signature(..., true)) no longer bypass authentication for HEAD requests, aligning HEAD with GET for mandatory-signing routes (notably FEP-8fcf /followers/sync).

Changes:

  • Restricts the HEAD short-circuit in verify_signature() to only apply when $force_signature is false.
  • Adds a PHPUnit test covering unsigned HEAD requests to /followers/sync expecting a 401.
  • Adds a security changelog entry documenting the behavior change.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
includes/rest/trait-verification.php Updates verify_signature() so unsigned HEAD is only bypassed for non-forced endpoints.
tests/phpunit/tests/includes/rest/class-test-followers-controller.php Adds test coverage for rejecting unsigned HEAD to the peer-only followers sync route.
.github/changelog/harden-head-mandatory-auth Documents the security hardening in the changelog system.

Comment thread tests/phpunit/tests/includes/rest/class-test-followers-controller.php Outdated
The previous assertion only checked the 401 status. Tighten it to
also verify the response body's error code is
`activitypub_signature_verification`, so a future refactor that
returns a different 401 (e.g. authority mismatch reshaped) doesn't
silently pass the test.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comment thread .github/changelog/harden-head-mandatory-auth
The test mutates activitypub_actor_mode and called delete_option()
only at the end. If an assertion failed mid-test the option would
leak into other tests in the suite. Move cleanup into a finally
block so it always runs.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@pfefferle pfefferle merged commit f59d285 into trunk Apr 27, 2026
15 checks passed
@pfefferle pfefferle deleted the harden/head-mandatory-auth branch April 27, 2026 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants