Skip to content

Implement Content Security Policy (CSP). #4577

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

Open
Ms2ger opened this issue Jan 8, 2015 · 7 comments
Open

Implement Content Security Policy (CSP). #4577

Ms2ger opened this issue Jan 8, 2015 · 7 comments
Labels
A-network I-enhancement No impact; the issue is a missing or proposed feature. I-safety Some piece of code violates memory safety guarantees.

Comments

@Ms2ger
Copy link
Contributor

Ms2ger commented Jan 8, 2015

Depends on #4576.

Estimated 2 months work assuming the right hooks.

@Ms2ger Ms2ger added I-enhancement No impact; the issue is a missing or proposed feature. A-network E-more-complex Variable effort required; may require a mentor. Recommended solution is clearly described in the iss labels Jan 8, 2015
@Ms2ger Ms2ger removed the E-more-complex Variable effort required; may require a mentor. Recommended solution is clearly described in the iss label Jan 20, 2015
@kmcallister kmcallister added the I-safety Some piece of code violates memory safety guarantees. label May 17, 2015
@Ms2ger Ms2ger changed the title Implement Content Security Policy. Implement Content Security Policy (CSP). Jan 7, 2016
@nox nox self-assigned this Apr 8, 2017
@notriddle
Copy link
Contributor

notriddle commented Oct 4, 2017

https://github.com/notriddle/rust-content-security-policy

I started working on it for Ammonia's purposes, but after looking around for an existing implementation, decided that if Servo needs it then here it is.

notriddle added a commit to notriddle/servo that referenced this issue Sep 28, 2019
This needs a lot more hooks before it'll actually be a good
implementation, but for a start it can help get some feedback on if this
is the right way to go about it.

Part of servo#4577
notriddle added a commit to notriddle/servo that referenced this issue Sep 28, 2019
This needs a lot more hooks before it'll actually be a good
implementation, but for a start it can help get some feedback on if this
is the right way to go about it.

Part of servo#4577
notriddle added a commit to notriddle/servo that referenced this issue Sep 28, 2019
This needs a lot more hooks before it'll actually be a good
implementation, but for a start it can help get some feedback on if this
is the right way to go about it.

Part of servo#4577
notriddle added a commit to notriddle/servo that referenced this issue Oct 3, 2019
This needs a lot more hooks before it'll actually be a good
implementation, but for a start it can help get some feedback on if this
is the right way to go about it.

Part of servo#4577
bors-servo pushed a commit that referenced this issue Oct 3, 2019
[WIP] Add simple implementation of content-security-policy on scripts / styles

This needs a lot more hooks before it'll actually be a good implementation, but for a start it can help get some feedback on if this is the right way to go about it.

Part of #4577 but we should probably track the rest of the implementation somewhere.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] There are tests for these changes (before merging, this PR should fix at least some of the WPT tests for CSP)

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/24315)
<!-- Reviewable:end -->
bors-servo pushed a commit that referenced this issue Oct 4, 2019
[WIP] Add simple implementation of content-security-policy on scripts / styles

This needs a lot more hooks before it'll actually be a good implementation, but for a start it can help get some feedback on if this is the right way to go about it.

Part of #4577 but we should probably track the rest of the implementation somewhere.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] There are tests for these changes (before merging, this PR should fix at least some of the WPT tests for CSP)

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/24315)
<!-- Reviewable:end -->
notriddle added a commit to notriddle/servo that referenced this issue Oct 4, 2019
This needs a lot more hooks before it'll actually be a good
implementation, but for a start it can help get some feedback on if this
is the right way to go about it.

Part of servo#4577
notriddle added a commit to notriddle/servo that referenced this issue Oct 4, 2019
This needs a lot more hooks before it'll actually be a good
implementation, but for a start it can help get some feedback on if this
is the right way to go about it.

Part of servo#4577
notriddle added a commit to notriddle/servo that referenced this issue Oct 4, 2019
This needs a lot more hooks before it'll actually be a good
implementation, but for a start it can help get some feedback on if this
is the right way to go about it.

Part of servo#4577
bors-servo pushed a commit that referenced this issue Oct 5, 2019
[WIP] Add simple implementation of content-security-policy on scripts / styles

This needs a lot more hooks before it'll actually be a good implementation, but for a start it can help get some feedback on if this is the right way to go about it.

Part of #4577 but we should probably track the rest of the implementation somewhere.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] There are tests for these changes (before merging, this PR should fix at least some of the WPT tests for CSP)

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/24315)
<!-- Reviewable:end -->
bors-servo pushed a commit that referenced this issue Oct 5, 2019
[WIP] Add simple implementation of content-security-policy on scripts / styles

This needs a lot more hooks before it'll actually be a good implementation, but for a start it can help get some feedback on if this is the right way to go about it.

Part of #4577 but we should probably track the rest of the implementation somewhere.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] There are tests for these changes (before merging, this PR should fix at least some of the WPT tests for CSP)

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/24315)
<!-- Reviewable:end -->
notriddle added a commit to notriddle/servo that referenced this issue Oct 10, 2019
This needs a lot more hooks before it'll actually be a good
implementation, but for a start it can help get some feedback on if this
is the right way to go about it.

Part of servo#4577
notriddle added a commit to notriddle/servo that referenced this issue Oct 10, 2019
This needs a lot more hooks before it'll actually be a good
implementation, but for a start it can help get some feedback on if this
is the right way to go about it.

Part of servo#4577
notriddle added a commit to notriddle/servo that referenced this issue Oct 16, 2019
This needs a lot more hooks before it'll actually be a good
implementation, but for a start it can help get some feedback on if this
is the right way to go about it.

Part of servo#4577
bors-servo pushed a commit that referenced this issue Oct 17, 2019
Add simple implementation of content-security-policy on network requests

This needs a lot more hooks before it'll actually be a good implementation, but for a start it can help get some feedback on if this is the right way to go about it.

Part of #4577 but we should probably track the rest of the implementation somewhere.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] There are tests for these changes (before merging, this PR should fix at least some of the WPT tests for CSP)

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/24315)
<!-- Reviewable:end -->
bors-servo pushed a commit that referenced this issue Oct 17, 2019
Add simple implementation of content-security-policy on network requests

This needs a lot more hooks before it'll actually be a good implementation, but for a start it can help get some feedback on if this is the right way to go about it.

Part of #4577 but we should probably track the rest of the implementation somewhere.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] There are tests for these changes (before merging, this PR should fix at least some of the WPT tests for CSP)

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/24315)
<!-- Reviewable:end -->
TimvdLippe added a commit to TimvdLippe/servo that referenced this issue May 14, 2025
Also update the `html/dom/reflection-metadata.html` test
to handle the case where `nonce` does not reflect back
to the attribute after an IDL change.

Part of servo#4577
Fixes web-platform-tests/wpt#43286

Signed-off-by: Tim van der Lippe <[email protected]>
github-merge-queue bot pushed a commit that referenced this issue May 14, 2025
Also update the `html/dom/reflection-metadata.html` test
to handle the case where `nonce` does not reflect back
to the attribute after an IDL change.

Part of #4577
Fixes web-platform-tests/wpt#43286

Signed-off-by: Tim van der Lippe <[email protected]>
@jdm
Copy link
Member

jdm commented May 14, 2025

No I don't think that's what's missing here. https://github.com/servo/servo/tree/main/tests/wpt/tests/content-security-policy/child-src are tests that are supposed to run when Destination::Frame is set: (https://github.com/servo/rust-content-security-policy/blob/334bfcbf0a3f503b21c90aee6aee30d4b8c9558a/src/lib.rs#L1478 coming from https://github.com/servo/rust-content-security-policy/blob/334bfcbf0a3f503b21c90aee6aee30d4b8c9558a/src/lib.rs#L1499). That's set in

servo/components/net/fetch/methods.rs

Line 177 in fdb9c0a
destination: request.destination,
which comes from

servo/components/shared/net/request.rs

Line 396 in fdb9c0a
pub fn destination(mut self, destination: Destination) -> RequestBuilder {

When I look into the callsides of that builder, I never see any codepath set it to Destination::Frame. I also am missing the codepath from the iframe code into this. Can you point me to the location where the iframe code eventually flows into the RequestBuilder code and where I would need to set the destination for it?

I remember looking for a frame destination in the specification, and I think.it might be missing. Our iframe navigation code creates a LoadData in htmliframeelement.rs and passes it to the constellation, where a pipeline is spawned in an appropriate script thread:

.initiate_fetch(&self.resource_threads.core_thread, None);

.initiate_fetch(&self.resource_threads.core_thread, response_init);
is another caller that handles redirects.

@jdm
Copy link
Member

jdm commented May 14, 2025

Looks like it's currently always the document destination: https://github.com/servo/servo/blob/main/components/script/navigation.rs#L205

@jdm
Copy link
Member

jdm commented May 15, 2025

I filed whatwg/html#11306.

TimvdLippe added a commit to TimvdLippe/servo that referenced this issue May 15, 2025
This way, we don't always set the destination to Document
(which is as the spec is written today). Instead, we set
it it in the load_data, depending on which context we
load it from.

Doing so allows us to set the `Destination::IFrame` for
navigations in iframes, enabling all frame-related CSP
checks.

Part of servo#4577

Signed-off-by: Tim van der Lippe <[email protected]>
TimvdLippe added a commit to TimvdLippe/servo that referenced this issue May 15, 2025
This way, we don't always set the destination to Document
(which is as the spec is written today). Instead, we set
it it in the load_data, depending on which context we
load it from.

Doing so allows us to set the `Destination::IFrame` for
navigations in iframes, enabling all frame-related CSP
checks.

Part of servo#4577

Signed-off-by: Tim van der Lippe <[email protected]>
TimvdLippe added a commit to TimvdLippe/servo that referenced this issue May 15, 2025
This way, we don't always set the destination to Document
(which is as the spec is written today). Instead, we set
it it in the load_data, depending on which context we
load it from.

Doing so allows us to set the `Destination::IFrame` for
navigations in iframes, enabling all frame-related CSP
checks.

While we currently block iframes when `frame-src` or
`child-src` is set, their respective tests don't pass
yet. That's because we don't yet handle the cases
where we fire the correct `load` event.

Part of servo#4577

Signed-off-by: Tim van der Lippe <[email protected]>
TimvdLippe added a commit to TimvdLippe/servo that referenced this issue May 15, 2025
This way, we don't always set the destination to Document
(which is as the spec is written today). Instead, we set
it it in the load_data, depending on which context we
load it from.

Doing so allows us to set the `Destination::IFrame` for
navigations in iframes, enabling all frame-related CSP
checks.

While we currently block iframes when `frame-src` or
`child-src` is set, their respective tests don't pass
yet. That's because we don't yet handle the cases
where we fire the correct `load` event.

Also update one WPT test to correctly fail, rather than
erroring. That's because it was using the wrong JS
test variable.

Part of servo#4577

Signed-off-by: Tim van der Lippe <[email protected]>
TimvdLippe added a commit to TimvdLippe/servo that referenced this issue May 15, 2025
This way, we don't always set the destination to Document
(which is as the spec is written today). Instead, we set
it it in the load_data, depending on which context we
load it from.

Doing so allows us to set the `Destination::IFrame` for
navigations in iframes, enabling all frame-related CSP
checks.

While we currently block iframes when `frame-src` or
`child-src` is set, their respective tests don't pass
yet. That's because we don't yet handle the cases
where we fire the correct `load` event.

Also update one WPT test to correctly fail, rather than
erroring. That's because it was using the wrong JS
test variable.

Part of servo#4577

Signed-off-by: Tim van der Lippe <[email protected]>
TimvdLippe added a commit to TimvdLippe/servo that referenced this issue May 15, 2025
This way, we don't always set the destination to Document
(which is as the spec is written today). Instead, we set
it it in the load_data, depending on which context we
load it from.

Doing so allows us to set the `Destination::IFrame` for
navigations in iframes, enabling all frame-related CSP
checks.

While we currently block iframes when `frame-src` or
`child-src` is set, their respective tests don't pass
yet. That's because we don't yet handle the cases
where we fire the correct `load` event.

Also update one WPT test to correctly fail, rather than
erroring. That's because it was using the wrong JS
test variable.

Part of servo#4577

Signed-off-by: Tim van der Lippe <[email protected]>
TimvdLippe added a commit to TimvdLippe/servo that referenced this issue May 16, 2025
This way, we don't always set the destination to Document
(which is as the spec is written today). Instead, we set
it it in the load_data, depending on which context we
load it from.

Doing so allows us to set the `Destination::IFrame` for
navigations in iframes, enabling all frame-related CSP
checks.

While we currently block iframes when `frame-src` or
`child-src` is set, their respective tests don't pass
yet. That's because we don't yet handle the cases
where we fire the correct `load` event.

Also update one WPT test to correctly fail, rather than
erroring. That's because it was using the wrong JS
test variable.

Part of servo#4577

Signed-off-by: Tim van der Lippe <[email protected]>
TimvdLippe added a commit to TimvdLippe/servo that referenced this issue May 16, 2025
This way, we don't always set the destination to Document
(which is as the spec is written today). Instead, we set
it it in the load_data, depending on which context we
load it from.

Doing so allows us to set the `Destination::IFrame` for
navigations in iframes, enabling all frame-related CSP
checks.

While we currently block iframes when `frame-src` or
`child-src` is set, their respective tests don't pass
yet. That's because we don't yet handle the cases
where we fire the correct `load` event.

Also update one WPT test to correctly fail, rather than
erroring. That's because it was using the wrong JS
test variable.

Part of servo#4577

Signed-off-by: Tim van der Lippe <[email protected]>
TimvdLippe added a commit to TimvdLippe/servo that referenced this issue May 16, 2025
These changes allow a minimal set of checks for font-src
CSP checks to pass.

Part of servo#4577
Part of servo#35035

Signed-off-by: Tim van der Lippe <[email protected]>
TimvdLippe added a commit to TimvdLippe/servo that referenced this issue May 16, 2025
These changes allow a minimal set of checks for font-src
CSP checks to pass.

Part of servo#4577
Part of servo#35035

Signed-off-by: Tim van der Lippe <[email protected]>
TimvdLippe added a commit to TimvdLippe/servo that referenced this issue May 16, 2025
These changes allow a minimal set of checks for font-src
CSP checks to pass.

Part of servo#4577
Part of servo#35035

Signed-off-by: Tim van der Lippe <[email protected]>
TimvdLippe added a commit to TimvdLippe/servo that referenced this issue May 17, 2025
These changes allow a minimal set of checks for font-src
CSP checks to pass.

Part of servo#4577
Part of servo#35035

Signed-off-by: Tim van der Lippe <[email protected]>
github-merge-queue bot pushed a commit that referenced this issue May 17, 2025
This way, we don't always set the destination to Document (which is as
the spec is written today). Instead, we set it it in the load_data,
depending on which context we load it from.

Doing so allows us to set the `Destination::IFrame` for navigations in
iframes, enabling all frame-related CSP checks.

While we currently block iframes when `frame-src` or `child-src` is set,
their respective tests don't pass yet. That's because we don't yet
handle the cases
where we fire the correct `load` event.

Also update one WPT test to correctly fail, rather than erroring. That's
because it was using the wrong JS test variable.

Part of #4577

Signed-off-by: Tim van der Lippe <[email protected]>
Co-authored-by: Josh Matthews <[email protected]>
TimvdLippe added a commit to TimvdLippe/servo that referenced this issue May 17, 2025
These changes allow a minimal set of checks for font-src
CSP checks to pass.

Part of servo#4577
Part of servo#35035

Signed-off-by: Tim van der Lippe <[email protected]>
TimvdLippe added a commit to TimvdLippe/servo that referenced this issue May 18, 2025
These changes allow a minimal set of checks for font-src
CSP checks to pass.

Part of servo#4577
Part of servo#35035

Signed-off-by: Tim van der Lippe <[email protected]>
TimvdLippe added a commit to TimvdLippe/servo that referenced this issue May 18, 2025
These changes allow a minimal set of checks for font-src
CSP checks to pass.

Part of servo#4577
Part of servo#35035

Signed-off-by: Tim van der Lippe <[email protected]>
TimvdLippe added a commit to TimvdLippe/servo that referenced this issue May 18, 2025
These changes allow a minimal set of checks for font-src
CSP checks to pass.

Part of servo#4577
Part of servo#35035

Signed-off-by: Tim van der Lippe <[email protected]>
TimvdLippe added a commit to TimvdLippe/servo that referenced this issue May 18, 2025
These changes allow a minimal set of checks for font-src
CSP checks to pass.

Part of servo#4577
Part of servo#35035

Signed-off-by: Tim van der Lippe <[email protected]>
TimvdLippe added a commit to TimvdLippe/servo that referenced this issue May 18, 2025
These changes allow a minimal set of checks for font-src
CSP checks to pass.

Part of servo#4577
Part of servo#35035

Signed-off-by: Tim van der Lippe <[email protected]>
TimvdLippe added a commit to TimvdLippe/servo that referenced this issue May 20, 2025
These changes allow a minimal set of checks for font-src
CSP checks to pass.

Part of servo#4577
Part of servo#35035

Signed-off-by: Tim van der Lippe <[email protected]>
TimvdLippe added a commit to TimvdLippe/servo that referenced this issue May 20, 2025
These changes allow a minimal set of checks for font-src
CSP checks to pass.

Part of servo#4577
Part of servo#35035

Signed-off-by: Tim van der Lippe <[email protected]>
TimvdLippe added a commit to TimvdLippe/servo that referenced this issue May 20, 2025
These changes allow a minimal set of checks for font-src
CSP checks to pass.

Part of servo#4577
Part of servo#35035

Signed-off-by: Tim van der Lippe <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-network I-enhancement No impact; the issue is a missing or proposed feature. I-safety Some piece of code violates memory safety guarantees.
Projects
None yet
Development

No branches or pull requests

6 participants