Commit graph

15596 commits

Author SHA1 Message Date
Oneric f07eb4cb55 Sanity check fetched user data
In order to properly process incoming notes we need
to be able to map the key id back to an actor.
Also, check collections actually belong to the same server.

Key ids of Hubzilla and Bridgy samples were updated to what
modern versions of those output. If anything still uses the
old format, we would not be able to verify their posts anyway.
2024-03-25 14:05:05 -01:00
Oneric 59a142e0b0 Never fetch resource from ourselves
If it’s not already in the database,
it must be counterfeit (or just not exists at all)

Changed test URLs were only ever used from "local: false" users anyway.
2024-03-25 14:05:05 -01:00
Oneric fee57eb376 Move actor check into fetch_and_contain_remote_object_from_id
This brings it in line with its name and closes an,
in practice harmless, verification hole.

This was/is the only user of contain_origin making it
safe to change the behaviour on actor-less objects.

Until now refetched objects did not ensure the new actor matches the
domain of the object. We refetch polls occasionally to retrieve
up-to-date vote counts. A malicious AP server could have switched out
the poll after initial posting with a completely different post
attribute to an actor from another server.
While we indeed fell for this spoof before the commit,
it fortunately seems to have had no ill effect in practice,
since the asociated Create activity is not changed. When exposing the
actor via our REST API, we read this info from the activity not the
object.

This at first thought still keeps one avenue for exploit open though:
the updated actor can be from our own domain and a third server be
instructed to fetch the object from us. However this is foiled by an
id mismatch. By necessity of being fetchable and our longstanding
same-domain check, the id must still be from the attacker’s server.
Even the most barebone authenticity check is able to sus this out.
2024-03-25 14:05:05 -01:00
Oneric c4cf4d7f0b Reject cross-domain redirects when fetching AP objects
Such redirects on AP queries seem most likely to be a spoofing attempt.
If the object is legit, the id should match the final domain anyway and
users can directly use the canonical URL.

The lack of such a check (and use of the initially queried domain’s
authority instead of the final domain) was enabling the current exploit
to even affect instances which already migrated away from a same-domain
upload/proxy setup in the past, but retained a redirect to not break old
attachments.

(In theory this redirect could, with some effort, have been limited to
 only old files, but common guides employed a catch-all redirect, which
 allows even future uploads to be reachable via an initial query to the
 main domain)

Same-domain redirects are valid and also used by ourselves,
e.g. for redirecting /notice/XXX to /objects/YYY.
2024-03-25 14:05:05 -01:00
Oneric baaeffdebc Update spoofed activity test
Turns out we already had a test for activities spoofed via upload due
to an exploit several years. Back then *oma did not verify content-type
at all and doing so was the only adopted countermeasure.
Even the added test sample though suffered from a mismatching id, yet
nobody seems to have thought it a good idea to tighten id checks, huh

Since we will add stricter id checks later, make id and URL match
and also add a testcase for no content type at all. The new section
will be expanded in subsequent commits.
2024-03-25 14:05:05 -01:00
Oneric 2bcf633dc2 Document Pleroma.Object.Fetcher 2024-03-25 14:05:05 -01:00
Oneric 93ab6a018e mix: fix docs task 2024-03-18 22:40:43 -01:00
Oneric c806adbfdb Refactor Fetcher.get_object for readability
Apart from slightly different error reasons wrt content-type,
this does not change functionality in any way.
2024-03-18 22:40:43 -01:00
Oneric ddd79ff22d Proactively harden emoji pack against path traversal
No new path traversal attacks are known. But given the many entrypoints
and code flow complexity inside pack.ex, it unfortunately seems
possible a future refactor or addition might reintroduce one.
Furthermore, some old packs might still contain traversing path entries
which could trigger undesireable actions on rename or delete.

To ensure this can never happen, assert safety during path construction.

Path.safe_relative was introduced in Elixir 1.14, but
fortunately, we already require at least 1.14 anyway.
2024-03-18 22:33:10 -01:00
Oneric d6d838cbe8 StealEmoji: check remote size before downloading
To save on bandwith and avoid OOMs with large files.
Ofc, this relies on the remote server
 (a) sending a content-length header and
 (b) being honest about the size.

Common fedi servers seem to provide the header and (b) at least raises
the required privilege of an malicious actor to a server infrastructure
admin of an explicitly allowed host.

A more complete defense which still works when faced with
a malicious server requires changes in upstream Finch;
see https://github.com/sneako/finch/issues/224
2024-03-18 22:33:10 -01:00
Oneric 6d003e1acd test/steal_emoji: consolidate configuration setup 2024-03-18 22:33:10 -01:00
Oneric d1ce5fd911 test/steal_emoji: reduce code duplication with mock macro 2024-03-18 22:33:10 -01:00
Oneric a4fa2ec9af StealEmoji: make final paths infeasible to predict
Certain attacks rely on predictable paths for their payloads.
If we weren’t so overly lax in our (id, URL) check, the current
counterfeit activity exploit would be one of those.
It seems plausible for future attacks to hinge on
or being made easier by predictable paths too.

In general, letting remote actors place arbitrary data at
a path within our domain of their choosing (sans prefix)
just doesn’t seem like a good idea.

Using fully random filenames would have worked as well, but this
is less friendly for admins checking emoji dirs.
The generated suffix should still be more than enough;
an attacker needs on average 140 trillion attempts to
correctly guess the final path.
2024-03-18 22:33:10 -01:00
Oneric ee5ce87825 test: use pack functions to check for emoji
The hardocded path and filenames assumptions
will be broken with the next commit.
2024-03-18 22:33:10 -01:00
Oneric d1c4d07404 Convert StealEmoji to pack.json
This will decouple filenames from shortcodes and
allow more image formats to work instead of only
those included in the auto-load glob. (Albeit we
still saved other formats to disk, wasting space)

Furthermore, this will allow us to make
final URL paths infeasible to predict.
2024-03-18 22:33:10 -01:00
Oneric fa98b44acf Fill out path for newly created packs
Before this was only filled on loading the pack again,
preventing the created pack from being used directly.
2024-03-18 22:33:10 -01:00
Oneric 5b126567bb StealEmoji: drop superfluous basename
Since 3 commits ago we restrict shortcodes to a subset of
the POSIX Portable Filename Character Set, therefore
this can never have a directory component.
2024-03-18 22:33:10 -01:00
Oneric a8c6c780b4 StealEmoji: use Content-Type and reject non-images
E.g. *key’s emoji URLs typically don’t have file extensions, but
until now we just slapped ".png" at its end hoping for the best.

Furthermore, this gives us a chance to actually reject non-images,
which before was not feasible exatly due to those extension-less URLs
2024-03-18 22:33:10 -01:00
Oneric 111cdb0d86 Split steal_emoji function for better readability 2024-03-18 22:33:10 -01:00
Norm af041db6dc Limit emoji stealer to alphanum, dash, or underscore characters
As suggested in b387f4a1c1, only steal
emoji with alphanumerc, dash, or underscore characters.

Also consolidate all validation logic into a single function.

===

Taken from akkoma#703 with cosmetic tweaks

This matches our existing validation logic from Pleroma.Emoji,
and apart from excluding the dot also POSIX’s Portable Filename
Character Set making it always safe for use in filenames.

Mastodon is even stricter also disallowing U+002D HYPEN-MINUS
and requiring at least two characters.

Given both we and Mastodon reject shortcodes excluded
by this anyway, this doesn’t seem like a loss.
2024-03-18 22:33:10 -01:00
Oneric fb54c47f0b Update example nginx config
To account for our subdomain recommendations
2024-03-18 22:33:10 -01:00
Oneric fc36b04016 Drop media proxy same-domain default for base_url
Even more than with user uploads, a same-domain proxy setup bears
significant security risks due to serving untrusted content under
the main domain space.

A risky setup like that should never be the default.
2024-03-18 22:33:10 -01:00
Oneric 11ae8344eb Sanitise Content-Type of media proxy URLs
Just as with uploads and emoji before, this can otherwise be used
to place counterfeit AP objects or other malicious payloads.
In this case, even if we never assign a priviliged type to content,
the remote server can and until now we just mimcked whatever it told us.

Preview URLs already handle only specific, safe content types
and redirect to the external host for all else; thus no additional
sanitisiation is needed for them.

Non-previews are all delegated to the modified ReverseProxy module.
It already has consolidated logic for building response headers
making it easy to slip in sanitisation.

Although proxy urls are prefixed by a MAC built from a server secret,
attackers can still achieve a perfect id match when they are able to
change the contents of the pointed to URL. After sending an posts
containing an attachment at a controlled destination, the proxy URL can
be read back and inserted into the payload. After injection of
counterfeits in the target server the content can again be changed
to something innocuous lessening chance of detection.
2024-03-18 22:33:10 -01:00
Oneric bcc528b2e2 Never automatically assign privileged content types
By mapping all extensions related to our custom privileged types
back to innocuous text/plain, our custom types will never automatically
be inserted which was one of the factors making impersonation possible.

Note, this does not invalidate the upload and emoji Content-Type
restrictions from previous commits. Apart from counterfeit AP objects
there are other payloads with standard types this protects against,
e.g. *.js Javascript payloads as used in prior frontend injections.
2024-03-18 22:33:10 -01:00
Oneric e88d0a2853 Fix Content-Type of our schema
Strict servers fail to process anything from us otherwise.

Fixes: akkoma#716
2024-03-18 22:33:10 -01:00
Oneric ba558c0c24 Limit instance emoji to image types
Else malicious emoji packs or our EmojiStealer MRF can
put payloads into the same domain as the instance itself.
Sanitising the content type should prevent proper clients
from acting on any potential payload.

Note, this does not affect the default emoji shipped with Akkoma
as they are handled by another plug. However, those are fully trusted
and thus not in needed of sanitisation.
2024-03-18 22:33:10 -01:00
Oneric 0ec62acb9d Always insert Dedupe upload filter
This actually was already intended before to eradict all future
path-traversal-style exploits and to fix issues with some
characters like akkoma#610 in 0b2ec0ccee. However, Dedupe and
AnonymizeFilename got mixed up. The latter only anonymises the name
in Content-Disposition headers GET parameters (with link_name),
_not_ the upload path.

Even without Dedupe, the upload path is prefixed by an UUID,
so it _should_ already be hard to guess for attackers. But now
we actually can be sure no path shenanigangs occur, uploads
reliably work and save some disk space.

While this makes the final path predictable, this prediction is
not exploitable. Insertion of a back-reference to the upload
itself requires pulling off a successfull preimage attack against
SHA-256, which is deemed infeasible for the foreseeable futures.

Dedupe was already included in the default list in config.exs
since 28cfb2c37a, but this will get overridde by whatever the
config generated by the "pleroma.instance gen" task chose.

Upload+delete tests running in parallel using Dedupe might be flaky, but
this was already true before and needs its own commit to fix eventually.
2024-03-18 22:33:10 -01:00
Oneric fef773ca35 Drop media base_url default and recommend different domain
Same-domain setups enabled now at least two exploits,
so they ought to be discouraged and definitely not be the default.
2024-03-18 22:33:10 -01:00
Oneric bdefbb8fd9 plug/upload_media: query config only once on init 2024-03-18 22:33:10 -01:00
Oneric f7c9793542 Sanitise Content-Type of uploads
The lack thereof enables spoofing ActivityPub objects.

A malicious user could upload fake activities as attachments
and (if having access to remote search) trick local and remote
fedi instances into fetching and processing it as a valid object.

If uploads are hosted on the same domain as the instance itself,
it is possible for anyone with upload access to impersonate(!)
other users of the same instance.
If uploads are exclusively hosted on a different domain, even the most
basic check of domain of the object id and fetch url matching should
prevent impersonation. However, it may still be possible to trick
servers into accepting bogus users on the upload (sub)domain and bogus
notes attributed to such users.
Instances which later migrated to a different domain and have a
permissive redirect rule in place can still be vulnerable.
If — like Akkoma — the fetching server is overly permissive with
redirects, impersonation still works.

This was possible because Plug.Static also uses our custom
MIME type mappings used for actually authentic AP objects.

Provided external storage providers don’t somehow return ActivityStream
Content-Types on their own, instances using those are also safe against
their users being spoofed via uploads.

Akkoma instances using the OnlyMedia upload filter
cannot be exploited as a vector in this way — IF the
fetching server validates the Content-Type of
fetched objects (Akkoma itself does this already).

However, restricting uploads to only multimedia files may be a bit too
heavy-handed. Instead this commit will restrict the returned
Content-Type headers for user uploaded files to a safe subset, falling
back to generic 'application/octet-stream' for anything else.
This will also protect against non-AP payloads as e.g. used in
past frontend code injection attacks.

It’s a slight regression in user comfort, if say PDFs are uploaded,
but this trade-off seems fairly acceptable.

(Note, just excluding our own custom types would offer no protection
 against non-AP payloads and bear a (perhaps small) risk of a silent
 regression should MIME ever decide to add a canonical extension for
 ActivityPub objects)

Now, one might expect there to be other defence mechanisms
besides Content-Type preventing counterfeits from being accepted,
like e.g. validation of the queried URL and AP ID matching.
Inserting a self-reference into our uploads is hard, but unfortunately
*oma does not verify the id in such a way and happily accepts _anything_
from the same domain (without even considering redirects).
E.g. Sharkey (and possibly other *keys) seem to attempt to guard
against this by immediately refetching the object from its ID, but
this is easily circumvented by just uploading two payloads with the
ID of one linking to the other.

Unfortunately *oma is thus _both_ a vector for spoofing and
vulnerable to those spoof payloads, resulting in an easy way
to impersonate our users.

Similar flaws exists for emoji and media proxy.

Subsequent commits will fix this by rigorously sanitising
content types in more areas, hardening our checks, improving
the default config and discouraging insecure config options.
2024-03-18 22:33:10 -01:00
Sandra Snan 6116f81546
Don't strip newlines in the Atom feed 2024-03-11 12:50:14 +01:00
Oneric 7ef93c0b6d Add set_content_type to Plug.StaticNoCT 2024-03-04 17:50:20 +01:00
Oneric dbb6091d01 Import copy of Plug.Static from Plug 1.15.3
The following commit will apply the needed patch
2024-03-04 17:50:20 +01:00
Oneric 5d467af6c5 Update notes on security exploit handling 2024-03-04 17:50:19 +01:00
Helge 5d89e0c917 Allow for url to be a list
This solves interoperability issues, see:
- https://git.pleroma.social/pleroma/pleroma/-/issues/3253
- https://socialhub.activitypub.rocks/t/fep-fffd-proxy-objects/3172/30?u=helge
- https://data.funfedi.dev/0.1.1/#url-parameter
2024-03-03 09:11:45 +01:00
Erin Shepherd f18e2ba42c Refresh Users much more aggressively when processing Move activities
The default refresh interval of 1 day is woefully inadequate here;
users expect to be able to add the alias to their new account and
press the move button on their old account and have it work.

This allows callers to specify a maximum age before a refetch is
triggered. We set that to 5s for the move code, as a nice compromise
between Making Things Work and ensuring that this can't be used
to hammer a remote server
2024-02-29 21:14:53 +01:00
Oneric fc95519dbf Allow fetching over IPv6
Mint/Finch disable IPv6 by default preventing us from
fetching anything from IPv6-only hosts without this.
2024-02-25 23:50:51 +01:00
FloatingGhost 889b57df82 2024.02 release 2024-02-24 13:54:21 +00:00
Weblate 34ffb92db4 Update translation files
Updated by "Squash Git commits" hook in Weblate.

Co-authored-by: Weblate <noreply@weblate.org>
Translate-URL: http://translate.akkoma.dev/projects/akkoma/akkoma-backend-posix-errors/
Translation: Pleroma fe/Akkoma Backend (Posix Errors)
2024-02-24 13:42:59 +00:00
Weblate c6dceb1802 Translated using Weblate (Polish)
Currently translated at 100.0% (47 of 47 strings)

Co-authored-by: Weblate <noreply@weblate.org>
Co-authored-by: subtype <subtype@hollow.capital>
Translate-URL: http://translate.akkoma.dev/projects/akkoma/akkoma-backend-posix-errors/pl/
Translation: Pleroma fe/Akkoma Backend (Posix Errors)
2024-02-24 13:42:59 +00:00
Weblate caaf2deb22 Translated using Weblate (Polish)
Currently translated at 18.1% (183 of 1006 strings)

Translated using Weblate (Polish)

Currently translated at 6.6% (67 of 1006 strings)

Co-authored-by: Weblate <noreply@weblate.org>
Co-authored-by: subtype <subtype@hollow.capital>
Translate-URL: http://translate.akkoma.dev/projects/akkoma/akkoma-backend-config-descriptions/pl/
Translation: Pleroma fe/Akkoma Backend (Config Descriptions)
2024-02-24 13:42:59 +00:00
floatingghost 7d61fb0906 Merge pull request 'Fix static-fe Twitter metadata / URL previews' (#700) from Oneric/akkoma:staticfe-metadata into develop
Reviewed-on: https://akkoma.dev/AkkomaGang/akkoma/pulls/700
2024-02-24 13:42:55 +00:00
floatingghost cdf73e0ac8 Merge pull request 'Better document database differences for Pleroma migrations' (#699) from Oneric/akkoma:doc_pleroma-migration-db into develop
Reviewed-on: https://akkoma.dev/AkkomaGang/akkoma/pulls/699
2024-02-24 04:33:43 +00:00
floatingghost 967e6b8ade Merge pull request 'Docs: Add description for mrf_reject_newly_created_account_notes' (#695) from YokaiRick/akkoma:doc_mrf_reject_acc_notes into develop
Reviewed-on: https://akkoma.dev/AkkomaGang/akkoma/pulls/695
2024-02-24 04:31:28 +00:00
Oneric d7c8e9df27 static-fe: don’t squeeze non-square avatars
This will crop them to a square matching behaviour of Husky and *key
and allowing us to never worry about consistent alignment.
Note, akkoma-fe instead displays the full image with inserted spacing.
2024-02-23 23:39:44 +00:00
Oneric a0daec6ea1 static-fe: don’t squeeze non-square emoji
Emoji and the navbar items want to let blend in with lines of text,
so fix their height and let the width adjust as needed.
2024-02-23 23:39:44 +00:00
Oneric bff2812a93 More prominently document db migrations in migrations from Pleroma
By now most instance will run a version past 2022-08 but the guide
only documented it for from source installs and Pleroma develop.
2024-02-23 23:54:14 +01:00
Oneric 7964272c98 Document how to avoid data loss on migration from Pleroma 2024-02-23 23:54:09 +01:00
Oneric c08f49d88e Add tests for static-fe metadata tags 2024-02-21 00:33:32 +00:00
FloatingGhost 3111181d3c mix format 2024-02-20 15:09:04 +00:00