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

Specify the minimum CSP for media #1600

Merged
merged 2 commits into from Aug 31, 2018

Conversation

2 participants
@turt2live
Member

turt2live commented Aug 29, 2018

Rendered: see 'docs' status check


Fixes #1066

@turt2live turt2live requested a review from matrix-org/spec-core-team Aug 29, 2018

@turt2live turt2live added this to In review (just the PRs) in August 2018 r0 via automation Aug 29, 2018

@erikjohnston

This comment has been minimized.

Member

erikjohnston commented Aug 30, 2018

I'm not sure we want to be so restrictive, all of this would be unnecessary for media hosted on other domains

@turt2live

This comment has been minimized.

Member

turt2live commented Aug 30, 2018

We can't guarantee that people respect that recommendation though. We can however try to tell servers what to do.

Fwiw, this is the csp used in synapse.

@erikjohnston

This comment has been minimized.

Member

erikjohnston commented Aug 30, 2018

We can't guarantee that people respect any recommendation at all, so I'm not sure I really follow your point.

There's two issues here:

  1. I'm not at all convinced that that CSP is either necessary or sufficient security going forward.
  2. That CSP is not at all user friendly, as it blocks a lot of stuff that end users may want to use (which is why we whitelist PDFs).

I'm not keen on mandating it given those concerns. Having a MUST saying that the server must protect clients against such attacks with recommendations would make me feel much happier

@turt2live

This comment has been minimized.

Member

turt2live commented Aug 30, 2018

@erikjohnston I've dialed back the language a bit. Please take a look.

@erikjohnston

This comment has been minimized.

Member

erikjohnston commented Aug 31, 2018

Thanks. I'd be sorely tempted to mandate the server does something to protect web clients, but its fine for now

August 2018 r0 automation moved this from In review (just the PRs) to Reviewer approved Aug 31, 2018

@turt2live turt2live merged commit c127eed into matrix-org:master Aug 31, 2018

7 checks passed

ci/circleci: build-dev-scripts Your tests passed on CircleCI!
Details
ci/circleci: build-docs Your tests passed on CircleCI!
Details
ci/circleci: build-swagger Your tests passed on CircleCI!
Details
ci/circleci: check-docs Your tests passed on CircleCI!
Details
ci/circleci: validate-docs Your tests passed on CircleCI!
Details
docs Click details to preview the HTML documentation.
Details
swagger Click to preview the swagger build.
Details

August 2018 r0 automation moved this from Reviewer approved to Done (this list will be incomplete) Aug 31, 2018

@turt2live turt2live deleted the turt2live:travis/c2s/media-csp branch Aug 31, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment