Renew HTTP signatures when following redirects #973

Oneric · 2025-08-30T20:53:35Z

Oneric commented

2025-08-30 20:53:35 +00:00

Finally fixing our side of #731

This means we can now fetch posts from e.g. Mastodon and PeerTube’s remote post display links (since they redirect to the original) even if the actual author’s instance requires authorized fetch.

Unfortunately most other implementations remain buggy afaik, so we have to keep the awful "route alias" handling in our own verification logic for now (though, with lazy route-alias evaluation this is at least no longer as costly as it used to be)

Initial checks and mix test seem good, but let’s have it spend some time in prod testing before merge to be sure; more testers welcome though

Finally fixing our side of #731 This means we can now fetch posts from e.g. Mastodon and PeerTube’s remote post display links *(since they redirect to the original)* even if the actual author’s instance requires authorized fetch. Unfortunately most other implementations remain buggy afaik, so we have to keep the awful "route alias" handling in our own verification logic for now (though, with lazy route-alias evaluation this is at least no longer as costly as it used to be) Initial checks and `mix test` seem good, but let’s have it spend some time in prod testing before merge to be sure; more testers welcome though

Oneric force-pushed httpsig_redirect_resign from ccbf5ff204

ci/woodpecker/pr/test/2 Pipeline is pending

Details

ci/woodpecker/pr/test/1 Pipeline failed

Details

to e116558dcf

ci/woodpecker/pr/test/1 Pipeline was successful

Details

ci/woodpecker/pr/test/2 Pipeline was successful

Details

2025-08-30 21:02:22 +00:00

Compare

Oneric force-pushed httpsig_redirect_resign from e116558dcf

ci/woodpecker/pr/test/1 Pipeline was successful

Details

ci/woodpecker/pr/test/2 Pipeline was successful

Details

to e8d217f3c7

ci/woodpecker/pr/test/1 Pipeline was successful

Details

ci/woodpecker/pr/test/2 Pipeline was successful

Details

2025-09-06 14:13:20 +00:00

Compare

floatingghost reviewed

2025-09-06 16:48:08 +00:00

floatingghost left a comment

this seems fine, could do with an extra test for the condition though methinks

lib/pleroma/object/fetcher.ex Outdated

					
				@ -414,1 +380,3 @@

				      |> sign_fetch(id, date)

				    http_opts =

				      if Pleroma.Config.get([:activitypub, :sign_object_fetches]) do

part of me wonders why we even have this option

If we drop it a bunch of tests fail since they only mocked requests with only a content-type header and nothing more. I suspect this and lack of motivation to fix the mocks is the main reason for this and the (actually limited to :test only) option to not set the user-agent header existing

If we drop it a bunch of tests fail since they only mocked requests with _only_ a `content-type` header and nothing more. I suspect this and lack of motivation to fix the mocks is the main reason for this and the (actually limited to `:test` only) option to not set the user-agent header existing

Oneric force-pushed httpsig_redirect_resign from e8d217f3c7

ci/woodpecker/pr/test/1 Pipeline was successful

Details

ci/woodpecker/pr/test/2 Pipeline was successful

Details

to 8066db8e0c

ci/woodpecker/pr/test/1 Pipeline was successful

Details

ci/woodpecker/pr/test/2 Pipeline was successful

Details

2025-09-07 00:45:00 +00:00

Compare

Oneric commented

2025-09-07 01:06:13 +00:00

could do with an extra test for the condition though

if you mean the Pleroma.Config.get([:activitypub, :sign_object_fetches]) check, there’s already a test for it (and it’s updated in the diff)

As for the actual signature logic, I’m pretty confident it works well without unwatend drawbacks now.

But turns out HTTP was being sloppy and mixed and duplicated all Tesla options into adapter options so the key redaction for logs didn’t actually work. (It only redacted the top-level copy not the copy in adapter options).
To (probably) fix this I had to force adapter options into a subkey and go through all uses of Pleroma.HTTP functions. In the process I noticed a bunch of obsolete unused or unneeded modules and deleted them. This makes this quite a bigger change — but also a net removal of ~260 lines of codes

> could do with an extra test for the condition though if you mean the `Pleroma.Config.get([:activitypub, :sign_object_fetches])` check, there’s already a test for it (and it’s updated in the diff) --- As for the actual signature logic, I’m pretty confident it works well without unwatend drawbacks now. But turns out `HTTP` was being sloppy and mixed and duplicated all Tesla options into adapter options so the key redaction for logs didn’t actually work. (It only redacted the top-level copy not the copy in adapter options). To (probably) fix this I had to force adapter options into a subkey and go through all uses of `Pleroma.HTTP` functions. In the process I noticed a bunch of obsolete unused or unneeded modules and deleted them. This makes this quite a bigger change — but also a net removal of ~260 lines of codes

Oneric force-pushed httpsig_redirect_resign from 8066db8e0c

ci/woodpecker/pr/test/1 Pipeline was successful

Details

ci/woodpecker/pr/test/2 Pipeline was successful

Details

to a5b727ef81

ci/woodpecker/pr/test/2 Pipeline is pending

Details

ci/woodpecker/pr/test/1 Pipeline failed

Details

2025-09-07 09:25:35 +00:00

Compare