I fixed some ActivityPub bugs and security issues

Development
, ,
1

Hi folks,
I recently switched over from Ghost to WF and decided to also use it for a private community that we set up to replace Facebook Groups. Some context about me and ActivityPub. I built one of the first ActivityPub plugins for Ghost back in 2023 which became obsolete once they finally added support in Ghost 6 last year. I ended up writing my own AP implementation because the NodeJS libraries at the time were overkill and brought in too many unnecessary couplings or were not conformant. (Go is so much nicer than NodeJS).

At any rate, I prefer how minimal WF is and I don’t need or want the ActivityPub “inbox” that Ghost provides.

While tweaking WF’s source code for our community’s needs, I discovered and fixed some bugs and security issues regarding ActivityPub behavior and wanted to submit them upstream. A lot of these issues were discovered while federating with Mastodon, Mbin, and GoToSocial.

My bug fixes are a number of small things so I would be happy to open separate threads for each. Let me know how you’d like me to proceed:

WriteAs Web Core:

  • Federated activities reused their object’s id → Each activity gets a distinct id (per AP 2.0 spec).

    • There’s an existing source code comment/change regarding Pleroma but only for Delete activities, it needs the id for all activities.

  • ActivityPub key generation relies on OpenSSL binary → Now uses crypto/rsa directly in the source code instead.

    • I use Docker and didn’t configure openssl correctly. WF silently failed to generate keys (required by GoToSocial and “strict” Mastodon) breaking federation.

  • NewRemoteActor issues a plain unsigned GET response → Now signs actor requests

    • Mastodon/GtS authorized-fetch instances answer 401 and I could not follow my blog from hachyderm’s instance.

  • DecodePrivateKey checks whether pem.Decode returned a nil block and then dereferences that same nil block to format the error causing a panic. → Fixed by checking for nil before formatting.

WriteFreely:

  • log.Error("Target POST URL is empty! Person: %+v", p) — %+v prints WriteFreely’s ActivityPub signing key which gets written to logs. → Now only logs p.ID for Person objects.

  • WF fails to respect unfollow requests (sad path) → acceptAndPersistFollow returned before DELETE FROM remotefollows, so a delivery failure continues to post to someone who unfollowed. → Fixed by deleting the follow from the DB before the response is sent back so that remote failures don’t interrupt the unfollow logic on WF.

  • Similar to above, the “Unfollow” request was incorrectly mapped to the “unfollow” object instead of the “unfollow”'s actor. → Fixed by resolving the actor from the Follow object.

  • handleFetchCollectionInbox would write the follow only after the Accept’s serialize-and-deliver block, meaning failed Accept requests were never written to the database (the remote software assumed it was following when it wasn’t). → Fixed by moving the isFollow transaction above the serialize-and-deliver block so that harmless errors don’t break follows.

  • Fixed a webfinger cache poisoning bug. GetProfileURLFromHandle INSERTed a failed webfinger result as a remoteusers row with an empty actor_id. → RemoteLookup returns an error now instead of writing an empty actor_id string to the database. Particularly impactful due to once the signed actor get request bug above (but not mitigated entirely after fixing the signed GET bug)

    • Affects the rel=me verification as well (Eg: Put a fediverse handle instead of a URL in the blog’s verification links and it’s supposed to normalize into a profile URL but doesn’t)

Btw, the GitHub repo’s README link to the contribution guidelines are broken (404): writefreely/CONTRIBUTING.md at develop · writefreely/writefreely · GitHub

And the contributor agreement also 404s: http://todo.musing.studio/L1