Rework HTTPSignatures and fix bridgy interop #874

Merged

Oneric merged 13 commits from Oneric/akkoma:httpsig_rework into develop

2025-06-07 19:00:02 +00:00

Oneric commented

2025-03-01 19:53:21 +00:00

Owner

Counterpart to AkkomaGang/http_signatures#2. Details in commit messages and changelog entries, but in brief summary it will:

replace the awful old API with something sane to use
get rid of a bunch of unnecessary duplication
placate non-bleeding edge Mastodon about Deletes of users we never knew so it stops resending it over and over
proper support for explicit signature expiry and implicit lifetime limits
ensures all headers required for safe signatures are set and used
ensure we don't produce signatures we’d reject ourselves
fix bridgy interop

Note for testing: if you can't pull the http_signatures lib with mix edit it to point at the latest commit of its PR; commits get garbage collected fast so even a cosmetic touchup over there might break it here and I don't want to (would probably forget anyway) to repush here every time too

Counterpart to https://akkoma.dev/AkkomaGang/http_signatures/pulls/2. Details in commit messages and changelog entries, but in brief summary it will: - replace the awful old API with something sane to use - get rid of a bunch of unnecessary duplication - placate non-bleeding edge Mastodon about Deletes of users we never knew so it stops resending it over and over - proper support for explicit signature expiry and implicit lifetime limits - ensures all headers required for safe signatures are set and used - ensure we don't produce signatures we’d reject ourselves - fix bridgy interop Note for testing: if you can't pull the http_signatures lib with mix edit it to point at the latest commit of its PR; commits get garbage collected fast so even a cosmetic touchup over there might break it here and I don't want to (would probably forget anyway) to repush here every time too

❤️ 1

Oneric added 12 commits 2025-03-01 19:53:22 +00:00

Drop EnsureUserPublicKey plug 3d58c0b624

It is not needed since fetch_public_keys will already
initiate remote lookup if necessary

Adapt to new http_signature API 21591decbd

instances: drop has_request_signatures 63887233dd

This property was introduced as a way to gauge whether and
how much enabling authfetch might break passive federation in
#312.

However, with the db field defaulting to false, there’s no distinction
between instances without valid signatures and those which just never
attempted to fetch anything from the local instance.
Furthermore, this was never exposed anywhere and required manually
checking the database or cachex state via a remote shell.

Given the above it appears this doesn't actually
provide anything useful, thus drop it.

federation: let http_signatures library handle request aliases a11c862ad7

This avoids spurious key refetches on each failing alias

federation: fake success on Deletes signed with an unknown gone key 6cb6e3886c

http_signatures: short-circuit gracefully on MRF rejects 3b9055f7d9

And adjust log details

http_signatures: only compute request-target aliases when needed 6aaa727533

Activity db queries are not cached
and most request will not actually need these aliases

http_signatures: ensure mandatory headers are set 2d5b0ba897

Most headers are automatically checked by the library after this
upgrade. But since digest is only required for requests with a body
and body processing is handled outside the lib atm, we need to
explicity pass the presence or absence along or not get feedback
about creating broken signatures.

This makes bugs in our signatures more apparent
allowing faster discovery and fixing

Distinguish targeted ratelimit and general unavailability in logs e5d154ebfe

ap: don't require explicit addressing of personal-inbox owners 8ab0a63e74

This requirement was originally added together with splicing the
inbox owner into the non b* addressing fields to make bcc transports
work in https://git.pleroma.social/pleroma/pleroma/-/merge_requests/390.
Later on this was relaxed to always allow deliveries devoid of any
addressing at all in f6cb963df2
and always allow deliveries from actors the owner is following in
750b369d04 to fix interop issues with
Mastodon and Honk respectively.

The justification for both the filtering and splicing comes from
one sentence in AP spec’s inbox section:
> In general, the owner of an inbox is likely
> to be able to access all of their inbox contents.

While this may provide plausible justification for splicing the owner
into cc, it is less clear how this requires or justifies the set of
filtering rules employed here.
Surveying a few other implementations no similar
filtering or splicing appears to be employed.

Furthermore, spec-compliant servers will strip bto/bcc _before_
delivery to remote servers, meaning any compliant bcc transport
out there will NOT contain any explicit addressing of the inbox owner.
Thus the addressing requirement directly opposes
the goal of the original patch.

Currently the requirement for the owner to be addressed once again
is causing interop issues. It turns out to be the root cause of
a long-standing (2+ years) bug preventing meaningful federation.
Bridgy sends e.g. Follow activities and Accepts for Follows directly
to the affected user’s personal inbox while solely addressing
the public scope in the to field. Notably follow relations never
getting established prevented the "accept if followed" allow rule
to ever come into effect.

To make matters worse non-addressed messages simply lead to a
vague "internal server error" response being sent back
which likely slowed down locating the issue.
Furthermore additional issues wrt to signatures cropped up after
the 500-response issues wa first reported, but they seem to have
already been fixed in the meantime, possibly with the signature
handling overhaul in Akkoma.

Given it repeatedly caused issues, does not appear to align with common
practice in the wider fedi ecosystem and apparently contradicts its
original intention, simply remove the requirement.

This is confirmed to fix bridgy interop.

The addressing splicing actually should also add the inbox owner to bto
or bcc instead of cc, but for now this is not changed and in practice
bto/bcc delivery appears to be basically unused anyway.

Lower load messages for individual emoji packs to debug f3fa5eafc6

The initial info message listing all found packs ought to be sufficient
and with many packs installed thiscan create multiple pages of log
messages on each emoji reload or server start.
Any errors or non-indexed packs are still logged to higher levels.

changelog: summarise user-visible parts of http signature rework

ci/woodpecker/pr/build-amd64 Pipeline is pending approval

Details

ci/woodpecker/pr/build-arm64 Pipeline is pending approval

Details

ci/woodpecker/pr/docs Pipeline is pending approval

Details

ci/woodpecker/pr/lint Pipeline is pending approval

Details

ci/woodpecker/pr/test Pipeline is pending approval

Details

3ad36e4c07