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.

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

added "media-src: 'self'" to CSP for resources #3578

Merged
merged 6 commits into from Sep 25, 2018

Conversation

Projects
None yet
5 participants
@remjey
Contributor

remjey commented Jul 23, 2018

Synapse doesn’t allow for media resources to be played directly from
Chrome. It is a problem for users on other networks (e.g. IRC)
communicating with Matrix users through a gateway. The gateway sends
them the raw URL for the resource when a Matrix user uploads a video
and the video cannot be played directly in Chrome using that URL.

Chrome argues it is not authorized to play the video because of the
Content Security Policy. Chrome checks for the "media-src" policy which
is missing, and defauts to the "default-src" policy which is "none".

As Synapse already sends "object-src: 'self'" I thought it wouldn’t be
a problem to add "media-src: 'self'" to the CSP to fix this problem.

added "media-src: 'self'" to CSP for resources
Synapse doesn’t allow for media resources to be played directly from
Chrome. It is a problem for users on other networks (e.g. IRC)
communicating with Matrix users through a gateway. The gateway sends
them the raw URL for the resource when a Matrix user uploads a video
and the video cannot be played directly in Chrome using that URL.

Chrome argues it is not authorized to play the video because of the
Content Security Policy. Chrome checks for the "media-src" policy which
is missing, and defauts to the "default-src" policy which is "none".

As Synapse already sends "object-src: 'self'" I thought it wouldn’t be
a problem to add "media-src: 'self'" to the CSP to fix this problem.
@matrixbot

This comment has been minimized.

Member

matrixbot commented Jul 23, 2018

Can one of the admins verify this patch?

1 similar comment
@matrixbot

This comment has been minimized.

Member

matrixbot commented Jul 23, 2018

Can one of the admins verify this patch?

added changelog.d entry
Signed-off-by: Jérémy Farnaud <jf@almel.fr>
Show resolved Hide resolved changelog.d/3578.bugfix Outdated
@turt2live

This comment has been minimized.

Member

turt2live commented Sep 18, 2018

Note: Whatever happens here, the official suggestion in the spec should be updated alongside it: https://matrix.org/docs/spec/client_server/r0.4.0.html#id112

@richvdh

This comment has been minimized.

Member

richvdh commented Sep 19, 2018

Thanks. @dbkr can you comment on the sanity of the CSP change?

@richvdh richvdh requested a review from dbkr Sep 19, 2018

Show resolved Hide resolved changelog.d/3578.bugfix Outdated
@dbkr

This comment has been minimized.

Member

dbkr commented Sep 19, 2018

This looks fine: it'll allow you to upload an html page with a tag that has media from the same HS content repo & have it play, so would be able to trigger the browser to make a GET request to the HS, but no more than with object-src, so as @remjey says, since we allow object-src there's no reason not to allow this.

@richvdh

This comment has been minimized.

Member

richvdh commented Sep 19, 2018

@dbkr: thanks. Do you think we should also update the spec as travis suggests?

@dbkr

This comment has been minimized.

Member

dbkr commented Sep 25, 2018

Oh, yep, if it's in the spec too we should change it there.

@richvdh

This comment has been minimized.

Member

richvdh commented Sep 25, 2018

Oh, yep, if it's in the spec too we should change it there.

matrix-org/matrix-doc#1684

@richvdh

This comment has been minimized.

Member

richvdh commented Sep 25, 2018

ugh now the tests are failing because the branch is out-of-date relative to sytest. merging anyway because bb5d380 passed.

@richvdh richvdh merged commit 6cf2619 into matrix-org:develop Sep 25, 2018

4 of 9 checks passed

ci/circleci: sytestpy2postgres Your tests failed on CircleCI
Details
ci/circleci: sytestpy2 CircleCI is running your tests
Details
ci/circleci: sytestpy3 CircleCI is running your tests
Details
ci/circleci: sytestpy3merged CircleCI is running your tests
Details
ci/circleci: sytestpy3postgres CircleCI is running your tests
Details
ci/circleci: sytestpy2merged Your tests passed on CircleCI!
Details
ci/circleci: sytestpy2postgresmerged Your tests passed on CircleCI!
Details
ci/circleci: sytestpy3postgresmerged Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment