superseriousbusiness / gotosocial

Fast, fun, small ActivityPub server.
https://docs.gotosocial.org
GNU Affero General Public License v3.0
3.68k stars 311 forks source link

[bug] Federation issues with Bookwyrm #1676

Closed WesleyAC closed 1 year ago

WesleyAC commented 1 year ago

Describe the bug with a clear and concise description of what the bug is.

As reported by @kvibber in https://github.com/bookwyrm-social/bookwyrm/issues/2794, searching for a GTS user from a Bookwyrm instance results in a error, stemming from GTS giving a 406 error in response to Bookwyrm's signed request. It's fairly likely the error is on Bookwyrm's side, since signed requests are fairly new for us, but I'm opening this issue here to get some feedback, in case it's obvious what's going wrong.

Our signatures for this request have a keyId that looks like https://bookwyrm.social/user/bookwyrm.instance.actor#main-key, and a algorithm of rsa-sha256, you can see the code we use to generate them in bookwyrm/signatures.py.

What's your GoToSocial Version?

0.8.0-rc1 git-e46323c

GoToSocial Arch

No response

What happened?

No response

What you expected to happen?

No response

How to reproduce it?

Search for a GTS user from any Bookwyrm instance running Bookwyrm v0.5.5 or later.

Anything else we need to know?

No response

tsmethurst commented 1 year ago

Hmmm, 406 means that content negotiation is failing and GtS can't serve anything in response. What are you setting for your content-type on GET requests to AP resources?

edit: sorry, I mean what content type are you setting in your Accept header

WesleyAC commented 1 year ago

Looks like we have Accept: application/json; charset=utf-8, which I guess should probably be ld+json instead?

tsmethurst commented 1 year ago

Aha! Yeah, I think it should be something like what's described here: https://www.w3.org/TR/activitypub/#retrieving-objects

So application/ld+json; profile="https://www.w3.org/ns/activitystreams"

EDIT just double checked and we also serve AP when Accept is application/activity+json

WesleyAC commented 1 year ago

Great, thanks for the help! Just put up a PR in Bookwyrm to fix this, will close this issue here :)

tsmethurst commented 1 year ago

You're welcome! Lemme know if there's anything else, would be happy to help :)

WesleyAC commented 1 year ago

Reopening this, since after fixing the Accept header, we're seeing a 401 when trying to make requests to GtS. How can we get more info on what GtS doesn't like about the request? See https://github.com/bookwyrm-social/bookwyrm/pull/2800 for details.

Any pointers on figuring this out?

cc @hughrun

hughrun commented 1 year ago

I've done some more testing of this and it appears to be something to do with Bookwyrm signatures not being accepted by GtS. If I send a message from GtS to Bookwyrm, Bookwyrm tries to access the GtS user's account info (to check the GtS user's signature in order to verify the signed digest) but GtS is returning a 401 to this request. This in turn means Bookwyrm can't authenticate the original signed request so it returns its own 401. Unfortunately the GtS logs aren't giving me much to go on, and I'm not familiar with either the GtS codebase nor with Go, so any pointers we can get on this would be great.

tsmethurst commented 1 year ago

I've been poking at it myself this morning and indeed you seem to be right about GtS not accepting Bookwyrm's signature. I looked in here and this all looks OK to me: https://github.com/bookwyrm-social/bookwyrm/blob/main/bookwyrm/signatures.py#L25. So I'm not sure yet what the issue is. I'll try to gather some trace logs and see what's happening.

Did you already get Bookwyrm working with secure mode mastodon btw? I tried following some bookwyrm accounts from ondergrond.org (which uses secure mode), and that also didn't seem to work, as far as I can tell. So there could be a common issue there, unless I'm mistaken (which is v. possible!).

hughrun commented 1 year ago

Did you already get Bookwyrm working with secure mode mastodon btw?

The latest Bookwyrm definitely works with secure Mastodon, but this is a fairly recent update so some servers may not be up to date.

tsmethurst commented 1 year ago

OK, thanks for the confirmation! We (GtS) are going on holiday soonish until the end of this month, so this is likely something that will have to wait til start of May. Keen to get GtS and Bookwyrm playing nicely together.

kvibber commented 1 year ago

I got a bit further on the Bookwyrm side of this, and I think I found the reason GTS isn't validating the signatures from Bookwyrm.

With that patch applied, I get the following on my test instance when trying to follow:

gotosocial | timestamp="09/04/2023 18:27:46.014" func=federation.(*federator).AuthenticateFederatedRequest level=DEBUG requestID=p0r4csw704000wywd66g msg="error parsing public key http://bw.builder.hyperborea.org/user/bookwyrm.instance.actor#main-key: cannot find publicKey with id: http://bw.builder.hyperborea.org/user/bookwyrm.instance.actor#main-key"

And when I hit that endpoint on the bookwyrm instance, I get the following JSON:

{"id": "http://bw.builder.hyperborea.org/user/bookwyrm.instance.actor", "type": "Person", "preferredUsername": "bookwyrm.instance.actor", "inbox": "http://bw.builder.hyperborea.org/user/bookwyrm.instance.actor/inbox", "publicKey": {"id": "http://bw.builder.hyperborea.org/user/bookwyrm.instance.actor/#main-key", "owner": "http://bw.builder.hyperborea.org/user/bookwyrm.instance.actor", "publicKeyPem": "-----BEGIN PUBLIC KEY-----\nMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA13GWgPpnbQUH+jT5dy3G\nQxUGuMgcL7GzkYZRtT6RIX++H/7fMhxRX2nxqqEWC/bc/tioZbrABtjRD4otVqLe\nghEkHCakwOVzYnyBs6Be7Nw7DYGsuBVmirgcVG8SK0bfoVowv069q+vei3VZlqeZ\nJobhFs+sLf0HAFWJbtLk1gvH3DMz2GJKDBSoFAK5hYsVLq4NpybynhvSi09ZlQEc\nRPCCe0YaDvecScQbRHIsOVseHVn5M3Zi9AviTwx2qgkEw7nWeDNMURFDd4QWquAZ\nI7nEOLO43fkxu8ohiY+3EddXW39VY61c7KQAx5ITRil8JMa+jLeE0IIGXSmN0kPj\nuQIDAQAB\n-----END PUBLIC KEY-----"}, "followers": "http://bw.builder.hyperborea.org/user/bookwyrm.instance.actor/followers", "following": "http://bw.builder.hyperborea.org/user/bookwyrm.instance.actor/following", "outbox": "http://bw.builder.hyperborea.org/user/bookwyrm.instance.actor/outbox", "endpoints": {"sharedInbox": "http://bw.builder.hyperborea.org/inbox"}, "icon": {}, "bookwyrmUser": true, "manuallyApprovesFollowers": false, "discoverable": false, "hideFollows": false, "@context": ["https://www.w3.org/ns/activitystreams", "https://w3id.org/security/v1", {"manuallyApprovesFollowers": "as:manuallyApprovesFollowers", "schema": "http://schema.org#", "PropertyValue": "schema:PropertyValue", "value": "schema:value"}]}

http://bw.builder.hyperborea.org/user/bookwyrm.instance.actor#main-key - What GTS is looking fow http://bw.builder.hyperborea.org/user/bookwyrm.instance.actor/#main-key - What the JSON identifies the key as.

If GTS is looking for the exact ID in the JSON, that's probably where the communication is breaking down.

I'm not familiar enough with the spec to say which side should be normalizing this, though I'd tend to go with the robustness principle and say that both should.

hughrun commented 1 year ago

Ok I have almost fixed this!

  1. @kvibber is right, our key ids don't match properly and they should. however that doesn't seem to be causing the signature verification problems in my testing - it doesn't seem to make any difference;
  2. The actual cause of the main issue is that Bookwyrm erroneously assumes all key ids are URL fragments when checking to see that the sending user is using their own key. See this Issue, which conveniently was posted just recently: https://github.com/bookwyrm-social/bookwyrm/issues/2801
  3. I can fix that, and this allows mutual follows and Bookwyrm generated statuses to appear in GtS timelines, however there is a problem in both directions when it comes to mentions. I'm not understanding much on the GtS side even with debug logging, but the problem on Bookwyrm's end is that GtS seems to be sending the tag value as a single object rather than a list if there is only one user tagged (mentioned). Whilst the spec is slightly ambiguous, given the example they provide is a single tag object wrapped in a list, it seems to be that GtS is non-compliant in this regard. On the other hand, I suppose the Bookwyrm team could just suck it up and build in some logic to account for the tag value being either a list/array or a single dict/object. In any case, there still seems to be an issue where GtS isn't understanding the tag user value Bookwyrm is sending.
tsmethurst commented 1 year ago

Ahhh thank you for the investigation and the fixes! Awesome :heart_eyes:

I think in the case of sending tag as a single object when there's only one, GtS is in-line with ActivityStreams (see example 157 here -- https://www.w3.org/TR/activitystreams-vocabulary/#microsyntaxes). We recently fixed an issue where we were wrongly serializing orderedItems as an object when there was only one of them (https://github.com/superseriousbusiness/gotosocial/pull/1673#event-8944845917), but that seems to have been an exception because orderedItems is a special case. I could be wrong though, I struggle to follow json-ld :grimacing:

hughrun commented 1 year ago

@tsmethurst all good, I think I have dealt with that at our end - it's a bit unclear to me which is the correct behaviour but both make sense.

The last piece of the puzzle (since every time I fix something I find a new, exciting bug) is that there are two problems which I suspect have the same root cause:

  1. As I noted above, mentions don't create notifications. The message does flow through to the user's timeline if they are following the original poster.
  2. Direct Messages are dropped - they simply don't appear anywhere for either Bookwyrm users or GtS users if the DM is sent in either direction.

As far as I can tell, these messages are received and accepted both by GtS and Bookwyrm, but both systems then just don't notify mentioned users and don't deliver DMs to user inboxes. This is quite weird, and I'd appreciate any tips you might have (noting that you've told us you'll be on leave soon, so if we have to wait, we have to wait 😄 ).

hughrun commented 1 year ago

Oh in case you're interested, my fixes so far are at https://github.com/bookwyrm-social/bookwyrm/pull/2812

tsmethurst commented 1 year ago

As far as I can tell, these messages are received and accepted both by GtS and Bookwyrm, but both systems then just don't notify mentioned users and don't deliver DMs to user inboxes. This is quite weird

that is super weird! could you post an example of a bookwyrm post with a mention in it? then (after break) we can add it to our GtS tests and see if we can figure out what's happening...

hughrun commented 1 year ago

I figured it out!!!

GtS expects the name field in a mention tag to be formatted like @user@example.tld but Bookwyrm is sending them as user@example.tld. When I change that, everything works.

I think GtS should accept either format, but we can make changes at the Bookwyrm end too.

There's a small remaining problem but that has nothing to do with GtS. Thanks for helping talk this through.

tsmethurst commented 1 year ago

I think GtS should accept either format

agreed!

Thanks for helping talk this through.

My pleasure! :heart_eyes:

tsmethurst commented 1 year ago

Gonna close this from our side since it's fixed in https://github.com/bookwyrm-social/bookwyrm/pull/2812 <3