Commit graph

6736 commits

Author SHA1 Message Date
floatingghost b1c6621e66 Merge pull request 'Read image description from EXIF data' (#744) from timorl/akkoma:elseinspe into develop
Reviewed-on: https://akkoma.dev/AkkomaGang/akkoma/pulls/744
2024-04-25 12:52:31 +00:00
floatingghost 764dbeded4 Merge pull request 'Accept all standard actor types' (#751) from Oneric/akkoma:all-actor-types into develop
Reviewed-on: https://akkoma.dev/AkkomaGang/akkoma/pulls/751
2024-04-24 17:09:02 +00:00
floatingghost 80e1c094c7 Merge pull request 'Don't strip newlines in pre' (#709) from snan/akkoma:pre into develop
Reviewed-on: https://akkoma.dev/AkkomaGang/akkoma/pulls/709
2024-04-24 17:00:34 +00:00
Oneric 83f75c3e93 Accept all standard actor types 2024-04-23 18:14:34 +02:00
Floatingghost 92168fa5a1 Merge remote-tracking branch 'origin/develop' into who-wants-to-yeet-c2s-i-want-to-yeet-c2s 2024-04-23 14:37:05 +01:00
Floatingghost 3e199242b0 remove upload_media from AP representation 2024-04-23 14:35:52 +01:00
timorl 9da0fe930e
Format, but this time with a non-ancient version of elixir 2024-04-19 18:07:50 +02:00
timorl 2a9db73b4c
Merge branch 'develop' into elseinspe 2024-04-19 17:11:55 +02:00
Floatingghost 1ed975636b Keep READ endpoints, purge WRITE 2024-04-19 11:06:01 +01:00
Floatingghost ddb8a5ef73 yeet AP C2S support
literally nothing uses C2S AP, and it's another route into core
systems which requires analysis and maintenance. A second API
is just extra surface for potentially bad things so let's take
it out back and obliterate it
2024-04-16 13:55:03 +01:00
Floatingghost 123db1abc4 Merge branch 'develop' into failed-fetch-processing 2024-04-16 12:35:54 +01:00
Floatingghost b2c29527fb make xmerl shut up about markup 2024-04-16 10:19:30 +01:00
timorl 59d32c10d9
Formatting 2024-04-16 08:02:13 +02:00
Floatingghost 2fc25980d1 fix pattern matching in fetch errors 2024-04-13 23:55:26 +01:00
floatingghost c1f0b6b875 Merge pull request 'Accept body parameters for /api/pleroma/notification_settings' (#738) from Oneric/akkoma:notif-setting-parameters into develop
Reviewed-on: https://akkoma.dev/AkkomaGang/akkoma/pulls/738
2024-04-13 22:55:02 +00:00
Floatingghost 49ed27cd96 require logger 2024-04-13 22:25:31 +01:00
Mark Felder d69cba1b93 Remove duplicate log messages from Transmogrifier
Object fetch errors are logged in the fetcher module
2024-04-12 20:31:31 +01:00
Mark Felder eeed051a0f Fix detection of user follower collection being private
We were overzealous with matching on a raw error from the object fetch that should have never been relied on like this. If we can't fetch successfully we should assume that the collection is private.

Building a more expressive and universal error struct to match on may be something to consider.
2024-04-12 20:29:11 +01:00
floatingghost 6f3c955aa0 Merge pull request 'elixir1.16 testing' (#742) from elixir1.16 into develop
Reviewed-on: https://akkoma.dev/AkkomaGang/akkoma/pulls/742
2024-04-12 18:49:33 +00:00
floatingghost 024ffadd80 Merge pull request 'Don't list old accounts as aliases in WebFinger' (#713) from erincandescent/akkoma:no-old-account-alias into develop
Reviewed-on: https://akkoma.dev/AkkomaGang/akkoma/pulls/713
2024-04-12 18:34:14 +00:00
Floatingghost df25d86999 Cleaned up FEP-fffd commits a bit 2024-04-12 18:50:57 +01:00
floatingghost 4887df12d7 Merge pull request 'Allow for url to be a list' (#718) from helge/akkoma:develop into develop
Reviewed-on: https://akkoma.dev/AkkomaGang/akkoma/pulls/718
2024-04-12 17:39:38 +00:00
floatingghost e6ca2b4d2a Merge pull request 'Fix array-less EmojiReacts' (#739) from Oneric/akkoma:tag-arrayless into develop
Reviewed-on: https://akkoma.dev/AkkomaGang/akkoma/pulls/739
2024-04-12 17:26:07 +00:00
floatingghost 6ba80aaff5 Merge pull request 'Check if data is visible before embedding it in OG tags' (#741) from ograph-restrictions into develop
Reviewed-on: https://akkoma.dev/AkkomaGang/akkoma/pulls/741
2024-04-12 17:22:59 +00:00
floatingghost 8e60177466 Merge pull request 'MRF.InlineQuotePolicy: Add link to post URL, not ID' (#733) from erincandescent/akkoma:quote-url into develop
Reviewed-on: https://akkoma.dev/AkkomaGang/akkoma/pulls/733
2024-04-12 17:02:52 +00:00
Erin Shepherd 75d9e2b375 MRF.InlineQuotePolicy: Add link to post URL, not ID
"id" is used for the canonical link to the AS2 representation of an object.
"url" is typically used for the canonical link to the HTTP representation.
It is what we use, for example, when following the "external source" link
in the frontend. However, it's not the link we include in the post contents
for quote posts.

Using URL instead means we include a more user-friendly URL for Mastodon,
and a working (in the browser) URL for Threads
2024-04-12 13:23:50 +02:00
Floatingghost 05f8179d08 check if data is visible before embedding it in OG tags
previously we would uncritically take data and format it into
tags for static-fe and the like - however, instances can be
configured to disallow unauthenticated access to these resources.

this means that OG tags as a vector for information leakage.

_technically_ this should only occur if you have both
restrict_unauthenticated *AND* you run static-fe, which makes no
sense since static-fe is for unauthenticated people in particular,
but hey ho.
2024-04-12 05:16:47 +01:00
Oneric bd74ad9ce4 Accept body parameters for /api/pleroma/notification_settings
This brings it in line with its documentation and akkoma-fe’s
expectations. For backwards compatibility URL parameters are still
accept with lower priority. Unfortunately this means duplicating
parameters and descriptions in the API spec.

Usually Plug already pre-merges parameters from different sources into
the plain 'params' parameter which then gets forwarded by Phoenix.
However, OpenApiSpex 3.x prevents this; 4.x is set to change this
  https://github.com/open-api-spex/open_api_spex/issues/334
  https://github.com/open-api-spex/open_api_spex/issues/92

Fixes: https://akkoma.dev/AkkomaGang/akkoma/issues/691
Fixes: https://akkoma.dev/AkkomaGang/akkoma/issues/722
2024-04-09 04:11:28 +02:00
Oneric 462225880a Accept EmojiReacts with non-array tag
JSON-LD compaction strips the array since it’s just one object

Fixes: https://akkoma.dev/AkkomaGang/akkoma/issues/720
2024-04-09 04:04:16 +02:00
Oneric 9598137d32 Drop base_url special casing in test env
61621ebdbc already explicitly added
the uploader base url to config/test.exs and it reduces differences
from prod.
2024-04-07 00:20:12 +02:00
floatingghost 554f19a9ed Merge pull request 'Refresh Users much more aggressively when processing Move activities' (#714) from erincandescent/akkoma:move-bust-cache into develop
Reviewed-on: https://akkoma.dev/AkkomaGang/akkoma/pulls/714
2024-04-03 10:03:14 +00:00
Erin Shepherd 464db9ea0b Don't list old accounts as aliases in WebFinger
Per the XRD specification:

> 2.4. Element <Alias>
>
> The <Alias> element contains a URI value that is an additional
> identifier for the resource described by the XRD. This value
> MUST be an absolute URI. The <Alias> element does not identify
> additional resources the XRD is describing, **but rather provides
> additional identifiers for the same resource.**

(http://docs.oasis-open.org/xri/xrd/v1.0/os/xrd-1.0-os.html#element.alias, emphasis mine)

In other words, the alias list is expected to link to things which are
not just semantically the same, but exactly the same. Old user accounts
don't do that

This change should not pose a compatibility issue: Mastodon does not
list old accounts here (See e1fcb02867/app/serializers/webfinger_serializer.rb (L12))

The use of as:alsoKnownAs is also not quite semantically right here
(see https://www.w3.org/TR/did-core/#dfn-alsoknownas, which defines
it to be used to refer to identifiers which are interchangable) but
that's what DID get for reusing a property definition that Mastodon
already squatted long before they got to it
2024-04-01 13:34:58 +02:00
Oneric 48b3a35793 Update user reference after fetch
Since we always followed redirects (and until recently allowed fuzzy id
matches), the ap_id of the received object might differ from the iniital
fetch url. This lead to us mistakenly trying to insert a new user with
the same nickname, ap_id, etc as an existing user (which will fail due
to uniqueness constraints) instead of updating the existing one.
2024-03-25 14:05:05 -01:00
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 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 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 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 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 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 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 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
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