Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

MSC2461: Proposal for Authenticated Content Repository API #2461

Closed
wants to merge 4 commits into from
Closed

MSC2461: Proposal for Authenticated Content Repository API #2461

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
74dea68
Proposal for Authenticated Content Repository API
SyrupThinker Mar 15, 2020
d10e27b
Clarify intent in MSC2461
SyrupThinker Mar 15, 2020
c036c71
MSC2461 Revision 3
SyrupThinker Mar 28, 2020
c49b2f6
MSC2461 Revision 4
SyrupThinker Apr 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 106 additions & 0 deletions proposals/2461-authenticated-content-repository-api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
# Authenticated Content Repository API
Currently anyone can fetch resources from content repositories.
This can be undesired behaviour for several reasons as discussed
in [synapse#2133](https://github.com/matrix-org/synapse/issues/2133) and
somewhat in [synapse#2150](https://github.com/matrix-org/synapse/issues/2150).

Homeserver administrators might want to be able to
restrict access to the content they serve.

This is unrelated to controlling access to content on a per-file basis,
which is something a user might desire.

## Proposal
Homeservers may reject unauthenticated access to media endpoints.

When an unauthenticated client accesses an endpoint, the homeserver
may reject the request like it would with an authenticated endpoint.

Thus it returns status code 401 and an error
with an errcode of M_MISSING_TOKEN or M_UNKNOWN_TOKEN as apropriate.

Example response:
```json
{
"errcode": "M_MISSING_TOKEN",
"error": "Media access is restricted to authenticated clients"
}
```

### Configuration
To allow clients to predetermine whether authentication is required,
the field m.media.unauthenticated is added to
Copy link
Member

Choose a reason for hiding this comment

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

generally we should be naming things in the positive:

Suggested change
the field m.media.unauthenticated is added to
the field m.media.authenticated is added to

Copy link
Author

Choose a reason for hiding this comment

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

This would lead to the inverted enum values m.non-local and m.uncached
which are in my opinion worse than having the field name in the negative.

I'm open to suggestions for better enum names

Copy link
Member

Choose a reason for hiding this comment

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

tbh when I was leaving this comment I was expecting it to be a boolean, not an enum. Not sure there's a benefit for having it be an enum at all, or even really needing this flag is needed.

Copy link
Author

Choose a reason for hiding this comment

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

Well, that depends on whether servers can actually make use of that value (according to you they should not) and whether its considered useful enough for clients (as noted in the MSC).

If those are not considered useful then all that stuff can be thrown out.

`GET /_matrix/media/r0/config`.
It specifies what content can be accessed unauthenticated.

The following behaviours are defined,
when a client encounters an unknown enum value it should be treated like `m.unspecified` :

| Enum value | Description |
| ---------- | ----------- |
| null / missing | All content can be accessed unauthenticated |
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm seeing m.none mentioned below, should that one be added here? Or what's the status on that?

Copy link
Author

Choose a reason for hiding this comment

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

null should actually be m.none here, thanks for pointing that out.

| m.local | Only content with an authority the server is responsible for can be accessed unauthenticated |
| m.cached | Cached content can be accessed unauthenticated |
| m.unspecified | Authenticated access might be required in some cases |
| m.all | No content can be accessed unauthenticated |

Example response:
```json
{
"m.media.unauthenticated": "m.local"
}
```

Clients can decide based on this
Copy link
Contributor

Choose a reason for hiding this comment

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

why not add the authentification type to m.file etc.? So like

{
  "url": "blah",
  "info": { ... },
  "authenticated": "m.local"
}

Copy link
Author

Choose a reason for hiding this comment

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

I might have miscommunicated the intent of this proposal by adding synapse#2150 considering it is only tagentally related. I've read the conversation some time ago and added it from memory. Only checking whether I cought the issues I was thinking of.

I have clarified the intent in a new revision.

if sharing a download link to a non Matrix user is possible.

### Server to server
To reduce the amount of server to server communication,
when one homeserver tries to fetch content from another homeserver,
the configuration should first be retrieved and cached.
Copy link
Member

Choose a reason for hiding this comment

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

There is no federation API for this.

Copy link
Author

@SyrupThinker SyrupThinker Mar 28, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I figured that because servers use the client API for media downloads they could also use it for the media config endpoint

Copy link
Member

Choose a reason for hiding this comment

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

They cannot. Also they aren't supposed to be using the client endpoints, they just haven't been defined correctly in the server-side endpoints.

Copy link
Author

@SyrupThinker SyrupThinker Mar 28, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Then this proposal is the time to specify that I'd say. Sharing media will be hard.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe the "federation api" could be an extra query parameter on existing download/thumbnail endpoints, which can be as simple as authed=yes/no? Of course, servers would need to also authenticate that it is another server that is requesting such media, and not someone else.

When the value is m.none the server should not attempt to fetch the
Copy link
Contributor

@ShadowJonathan ShadowJonathan Apr 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Does m.none effectively mean "All content uploaded on here is non-accessible to other servers"? that breaks quite a lot of (core) assumptions in matrix, where f.e. someone setting a room avatar in a federated room can suddenly "not display" that avatar simply because the server does not allow federated propagation of that file, per server-wide rules.

Copy link
Author

Choose a reason for hiding this comment

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

I assume my other comment clears the first part up, but m.none actually means no restrictions, as in the current behaviour. m.all would actually be the behaviour you describe, as in everything is restricted.

For the latter, yes, that is an expected consequence of this proposal.
I'd expect a user to be made aware by server owners if such media restrictions are in place.

Copy link
Contributor

Choose a reason for hiding this comment

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

"restricted" as in "can only be accessed once authenticated"? And that any server over federation can request such media in an authenticated user's stead?

content from the remote server
and return status code 404 and an error with an errcode of M_NOT_FOUND.

Example response:
```json
{
"errcode": "M_NOT_FOUND",
"error": "Remote homeserver rejected access"
}
```

## Potential issues
Copy link
Member

Choose a reason for hiding this comment

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

Also mentioned in MSC701 (I think, I'm going off memory) is a way to solve the authentication issue without potentially leaking your access token. Clients which have 'download this file' or 'open in new tab' buttons will need to pass along the access token via the query string. In doing so, when someone copies the link and pastes it to someone else they've exposed their account.

Copy link
Author

@SyrupThinker SyrupThinker Mar 28, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I really try to stay away from crypto, so this is just a naive simplification of MSC701:

The authentication token is created through HMAC(access_token, media_id).
This way only the media download can be replayed.
I'm not sure how the HMAC would be verified, but that'd need to be solved for MSC701 as well.

- Once homeservers enable this behaviour with a m.media.unauthenticated
value other than null, older clients will not be able to access some content.
This is desired for the server operator and undesired for the user.

- Older clients and servers might encounter an unexpected error code
which may lead to unknown behaviour.

- GUI toolkits used by client authors might not support modifying request headers
for image widgets. This change would thus increase client complexity in these cases.

## Alternatives and comparisons
- All media endpoints could always require authentication,
but then the server to server exchange would still need
to be extended to allow access for remote homeservers.

- MSC701
This MSC proposes a way to authenticate content repo access on a per file basis.
With it uploads that are not marked public are inaccessible to clients without a
content token.

MSC701 provides control to the user as to who sees content.
Which is unrelated to this proposal which tries to give control to server admins.

For the following reasons can MSC701 not provide the functionality of this MSC:
- When the server allows the public flag its admin has no control as
to who can access content
- When the server forbids the public flag (which is not specified in MSC701),
its admin can still not block non-local users who share rooms with local users
- The federation API of this proposal allows access to non-Matrix users if they can
get access to a content token (unless the HMAC is enforced)

## Security considerations
- Download links for content might leak the users access token when shared