Skip to content

Add HTTP signatures to all outgoing ActivityPub GET requests#11284

Merged
Gargron merged 1 commit intomasterfrom
feature-sign-all-requests
Jul 11, 2019
Merged

Add HTTP signatures to all outgoing ActivityPub GET requests#11284
Gargron merged 1 commit intomasterfrom
feature-sign-all-requests

Conversation

@Gargron
Copy link
Copy Markdown
Member

@Gargron Gargron commented Jul 10, 2019

This was a lot simpler than I thought it would be. All ActivityPub fetches are using fetch_resource so that's the only place where we need to add a signing account, besides FetchResourceService which works with HTML pages as well.

I'm removing the code for attempting to fetch without signature if fetch with signature fails because if #11269 will be enabled in the long-term it will be a waste of time.

Change default keyId format from acct to uri

@Gargron Gargron added the activitypub Protocol-related changes, federation label Jul 10, 2019
@kaniini
Copy link
Copy Markdown
Contributor

kaniini commented Jul 10, 2019

Picking a random account to sign the requests with is poor metadata hygeine. It would be desirable to ensure that Account.representative is a special account that represents the instance actor.

@Gargron
Copy link
Copy Markdown
Member Author

Gargron commented Jul 10, 2019

Instance actor is in the works, but this will do for now. It's only a random account when the "contact account" is not configured. But in either case it's the same account every time. The only risk/downside to this approach vs dedicated instance actor account is that personal accounts may be suspended on the remote end for personal reasons--or, in the case of reports, the remote admin may misinterpret the actions as personal rather than automated.

@Gargron Gargron force-pushed the feature-sign-all-requests branch 2 times, most recently from 099b8c2 to 4a4ca1f Compare July 10, 2019 19:33
@Gargron Gargron force-pushed the feature-sign-all-requests branch from 4a4ca1f to 878cdd6 Compare July 10, 2019 20:11
Copy link
Copy Markdown
Contributor

@ClearlyClaire ClearlyClaire left a comment

Choose a reason for hiding this comment

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

Looks ok to me. Even though I agree an instance actor would be way cleaner, for this particular use case it seems like a good interim solution.

@Gargron Gargron merged commit 4e8dcc5 into master Jul 11, 2019
@Gargron Gargron deleted the feature-sign-all-requests branch July 21, 2019 01:48
hiyuki2578 pushed a commit to ProjectMyosotis/mastodon that referenced this pull request Oct 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

activitypub Protocol-related changes, federation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants