Media-links now force download instead of just showing content inline in browser

[dennisse]

Description

PR #15680 just broke my ability to quickly open and view attachements.

The PR makes the media api to send the header content-disposition: attachment when answering requests for attachements. This again forces the browser to try to download the content, instead of just viewing it directly.

I, and many others, use less fancy clients, like weechat-matrix, and attachements are often just presented as a link to the content. I used to be able to just open the link and view the content directly in my browser. Now I have to download the content first, and then open it.

Anyone getting a link to a matrix-attachement outside of a fancy matrix-client will probably be affected by this. This includes:

• IRC-users on the other side of a bridge (who I hear already aren't too pleased with matrix these days)
• Anyone getting a link to content from a matrix-server in general. It's not unheard of to share links to media from a matrix-chat outside matrix.

A way to turn this off, or the mimetype whitelist the PR hints at, would be greatly appreciated.

Steps to reproduce

• Try to view https://matrix.dnns.no/_matrix/media/r0/download/dnns.no/cgFcEHtGykrmYbRjbzoxOUtl
• It will present as a download, instead of just opening as a picture in your browser

Homeserver

matrix.dnns.no

Synapse Version

1.87.0

Installation Method

Docker (matrixdotorg/synapse)

Database

PostgreSQL

Workers

Single process

Platform

Podman container on a VM running Debian, on host running OpenBSD

Configuration

No response

Relevant log output

There are no relevant logs here

Anything else that would be useful to know?

Possible workaround until this gets fixed: Strip the Content-Disposition header from the response in your reverse proxy. I.e. proxy_hide_header Content-Disposition; in nginx (this goes in the same place as proxy_pass).

[FlyveHest]

I am using the Matrix/IRC appservice, and when I click media links on the IRC side, I too am forced to download instead of just being able to watch them in my browser.

This worked in 1.85.x

[joshqou]

I’d suggest against blindly stripping Content-Disposition as the change was a security fix.

[dennisse]

Here's a new workaround (for nginx) that takes content-type into consideration:

map $upstream_http_content_type $matrix_media {
    "text/plain"            "inline";
    "text/css"              "inline";
    
    "application/pdf"       "inline";
    
    "image/gif"             "inline";
    "image/jpeg"            "inline";
    "image/avif"            "inline";
    "image/png"             "inline";
    "image/tiff"            "inline";
    "image/vnd.wap.wbmp"    "inline";
    "image/webp"            "inline";
    "image/x-icon"          "inline";
    "image/x-jng"           "inline";
    "image/x-ms-bmp"        "inline";
    
    "audio/basic"           "inline";
    "audio/midi"            "inline";
    "audio/mpeg"            "inline";
    "audio/ogg"             "inline";
    "audio/x-m4a"           "inline";
    "audio/x-realaudio"     "inline";
    
    "video/3gpp"            "inline";
    "video/mp2t"            "inline";
    "video/mp4"             "inline";
    "video/mpeg"            "inline";
    "video/quicktime"       "inline";
    "video/webm"            "inline";
    "video/x-flv"           "inline";
    "video/x-m4v"           "inline";
    "video/x-matroska"      "inline";
    "video/x-mng"           "inline";
    "video/x-ms-asf"        "inline";
    "video/x-ms-wmv"        "inline";
    "video/x-msvideo"       "inline";
    
    default                 "attachement";
}

The above goes in the http-context in nginx (above/outside your server-block).

Then this goes in your location/server-context (probably right beneath proxy_pass):

proxy_hide_header Content-Disposition;
add_header Content-Disposition $matrix_media;

This will strip the original filename when dealing with other filetypes than those we mapped, but I think this is preferable to having to download images.

@joshqou: Do you have any more details on the security issue? I'm not clear on what "NVT#1548992" is. If you can at least provide a list of content-types that should not be inlined, I can ammend this workaround.

[FlyveHest]

As I am using Apache as my reverse proxy, this little piece of configuration should be equivalent to the NGinx config above

  # Temporarily fix the attachment / inline content-disposition issue introduced in Synapse 1.87
  <Location /_matrix/media/v3/download/>
    Header edit Content-Disposition "\battachment\b" "inline" "expr=resp('Content-Type') in { \
      'image/gif','image/jpeg','image/avif','image/png', 'image/svg+xml','image/tiff', \
      'image/vnd.wap.wbmp','image/webp','image/x-icon','image/x-jng','image/x-ms-bmp', \
      'audio/basic','audio/midi','audio/mpeg','audio/ogg','audio/x-m4a','audio/x-realaudio','video/3gpp', \
      'video/mp2t','video/mp4','video/mpeg','video/quicktime','video/webm','video/x-flv','video/x-m4v', \
      'video/x-matroska','video/x-mng','video/x-ms-asf','video/x-ms-wmv','video/x-msvideo'}"
  </Location>

Edit: Removed non-media types (HTML, CSS etc)

If you do not care about specific mime-types (which I think was the behaviour prior), you can skip the last part, like this

  # Temporarily fix the attachment / inline content-disposition issue introduced in Synapse 1.87
  <Location /_matrix/media/v3/download/>
    Header edit Content-Disposition "\battachment\b" "inline"
  </Location>

[CyberShadow]

@dennisse @FlyveHest I don't know about the actual vulnerability but I don't think it's safe to serve text/html (and maybe image/svg+xml) with the inline disposition.

[FlyveHest]

I don't know about the actual vulnerability but I don't think it's safe to serve text/html (and maybe image/svg+xml) with the inline disposition.

@CyberShadow I'm guessing this is what led to the change, if you send inline html, it might lead to XSS issues in a web based client.

I can see the original PR that implemented this, #15680, originally intended to only change this behaviour for non-media type data, but due to complications ended up implementing it for everything served, which in turn broke many "dumb" clients.

Personally I think this should be rolled back until it can be done properly, it severely impedes on one of the core tenets of Matrix, being able to use it across an abundance of clients.

[FlyveHest]

Can't say that I agree on neither the occasional or minor tags here, but if this is not something that is going to be fixed I think it would be a good idea to add information about this in the documentation, so admins that are using bridges on their homeservers does not have to find information in an issue that will be buried soon.

Also, the workaround completely counters the change to begin with, and if this really is a security issue (which I can't really see it being), will leave homeservers wanting to use bridges for "dumber" clients just as vulnerable as before.

This affects all my users on the IRC bridge at all times, and i'm guessing that all other non-web based clients are affected as well.

[FlyveHest]

I’d suggest against blindly stripping Content-Disposition as the change was a security fix.

@joshqou Can you possibly link to the CVE that necessitated this change?

[t3chguy]

This affects all my users on the IRC bridge at all times

Arguably that's a bug of the IRC bridge in abusing the media repo as its way of sharing media. The same would happen in the future when the media repo becomes authenticated. So the IRC bridge needs a way to expose media itself anyhow.

[FlyveHest]

I'm not sure that I would call it a bug, how else would any bridge serve any media, unless it is expected that a bridge will run its own proxy for media purposes? (And in turn, completely defeating the idea of authenticating media to begin with)

It's not as black and white as that I think, but I can see that at least the issue of authenticating the media repos is an old discussion, matrix-org/matrix-spec#870, which haven't found a solution yet.

[CyberShadow]

I agree with @t3chguy here.

it is expected that a bridge will run its own proxy for media purposes?

I think that makes sense.

In encrypted rooms, the media download URLs will serve encrypted content (see e.g. hifi/heisenbridge#227), so it already does not work there. I think it does make sense that bridges run their own HTTP server or bridge encrypted content in their own way.

After all, the /media/v3/download/ endpoint is meant for Matrix client-to-server communication; bridges telling other non-Matrix software (or users!) to just go ahead and use that URL directly isn't right.

And in turn, completely defeating the idea of authenticating media to begin with

Not necessarily. For example, when bridging to a protocol that has E2E messages but not E2E attachments, the bridge could host a web page which downloads and decrypts the media using a key specified in the URL hash; that way, the Matrix sender, bridge, and recipient will see the attachment cleartext, but the Matrix homeserver and the bridged service will not.

[erikjohnston]

Hi all.

This was a bit unfortunate that it landed without much warning and has caused issues for folks. Though the part of the reason we landed this change was due to security concerns, so I would caution against fully reverting the change without careful thought.

As people have pointed out, there are two things that changes the situation here a bit:

• this is already an issue for media in encrypted rooms; and
• we'll be adding authentication to the media download endpoints (Matrix was never meant to provide an open file host, which can cause a lot of issues for people hosting Matrix)

As such, we as the core team are not planning to spend time trying to mitigate these changes, given it'll soon become moot. However, if people want to send a PR over that changes the header back to inline for some known safe mime types (e.g. jpg, png, etc) then we'd be happy to accept it.

[FlyveHest]

Hi @erikjohnston

Though the part of the reason we landed this change was due to security concerns, so I would caution against fully reverting the change without careful thought.

Is there a publicly available discussion on this subject? I'm somewhat curious as to what the security implications of serving data inline would be.

• we'll be adding authentication to the media download endpoints (Matrix was never meant to provide an open file host, which can cause a lot of issues for people hosting Matrix)

While it is definitely a valid reason, I think it would be nice to consider allowing homeservers to decide if this should be possible or not on their own.

I think it would be a waste of resources having a bridge running a media cache itself, and just relaying the information through the bridges credentials seems as more work than needed, of course depending on what the security issue that forced the change is.

As such, we as the core team are not planning to spend time trying to mitigate these changes, given it'll soon become moot. However, if people want to send a PR over that changes the header back to inline for some known safe mime types (e.g. jpg, png, etc) then we'd be happy to accept it.

Seems fair, i've "solved" the problem on my end via my reverse proxy, so it will continue to work until the media store becomes protected.

And that will most likely need to, if not handled, then adressed in bridges that need to bridge media as well.

[itsrachelfish]

I use matrix as a gateway to my IRC network with https://github.com/matrix-org/matrix-appservice-irc. Essentially matrix is just a fancy mobile client for IRC. None of the rooms on my homeserver are end-to-end encrypted except for private messages.

Please do not remove the ability for the IRC appservice bridge to share uploaded files. Removing the ability for users to share media via matrix will mean that we will need to come up with a different solution for our network and stop using matrix entirely.

In the mean time we will be updating our nginx config to strip the content-disposition headers.

[poVoq]

@dennisse any idea why your Nginx work around still doesn't work for the plain/text pastbins that appservice-irc links on overly long messages? Maybe the charset=utf-8 also needs to be specified somehow?

[clokep]

Fixed by #15988.

[nekopsykose]

#15988 describes Render plain, CSS, CSV, JSON, .. as part of what should be 'inline', and if media_type.lower() in INLINE_CONTENT_TYPES: contains things like text/plain, but matrix multiline messages or code blocks still seem to have an attachment disposition when sent over a matrix<->irc bridge. in my case, specifically from a matrix user on OFTC. image uploads do display normally in a browser now (as intended), but before the original change that 'broke' this, multiline messages / code blocks also displayed in a browser, and now they seem to still not be able to even after the fix.

this might be because in

    if media_type.lower() in INLINE_CONTENT_TYPES:
        disposition = "inline"

the INLINE_CONTENT_TYPES contains

    "text/plain",

but the actual header of a text-based response has (curl -v >/dev/null to view in response on a media url):

< content-type: text/plain; charset=utf-8

text-based content-types contain something more on the string (e.g. "text/plain; charset=utf-8" in ["text/plain"] will fail). i'm not sure if this is the reason it doesn't work, but it might be..

[DMRobertson]

https://www.rfc-editor.org/rfc/rfc9110#field.content-type, https://www.rfc-editor.org/rfc/rfc9110#name-media-types and https://www.rfc-editor.org/rfc/rfc9110#parameter describe the grammar of content-type headers. Looks like we need to ignore parameters by reading only up to the first semicolon?