replace CSP with new default, with start/end markers for auto replacement

[labkey-willm]

Rationale

https://github.com/LabKey/kanban/issues/362

we're going to use the server repo's Content Security Policy as the source going forward. Updates there will be automatically sent to syseng-chef-server and Dockerfile repos.

Both report and enforce policies are copied in full to syseng-chef-server, with minor changes performed by the action. The 'enforce' policy is copied and uncommented to LabKey/Dockerfile/application.properties.

more on the action:

• Upon merge of a PR in which 'server/configs/application.properties' was updated, it copies the Content Security Policy blocks to other repos, as marked by:
• start: "## START OF CSP COPY BLOCK" or "## START OF CSP ENFORCE BLOCK" (to only copy and uncomment the csp.enforce section)
• end: "## END OF CSP COPY BLOCK" or "## START OF CSP ENFORCE BLOCK"
• note: if the contents between the start/end have not changed, it's a no-op, and no PRs are pushed to the target repos

Related Pull Requests

• https://github.com/LabKey/terraform_module_ecs_service_lkapps/pull/30
• https://github.com/LabKey/syseng-chef-server/pull/608
• add default enforced Content Security Policy Dockerfile#135
• https://github.com/LabKey/terraform_cloud/pull/2073

Changes

[labkey-tchad]

This file is for TeamCity and dev deployments, not cloud deployments. You've removed the bit that enables the enforce CSP on dev enlistments (#useLocalBuild#) and made it so the CSP reports will be invisible on the test instance. TeamCity tests can't see the CSP warnings if they get sent to labkey.org.

[labkey-willm]

This file is for TeamCity and dev deployments, not cloud deployments. You've removed the bit that enables the enforce CSP on dev enlistments (#useLocalBuild#) and made it so the CSP reports will be invisible on the test instance. TeamCity tests can't see the CSP warnings if they get sent to labkey.org.

Ah, I see. I didn't realize what all that was doing. Do teamcity or devs need the report block at all? If not, I could leave that out of what I'm copying in from the chef repo (while retaining the #useLocalBuild# comments).

Or better/simpler, if we only copy in an uncommented enforce block, but strip "https://www.labkey.org" out of the report-uri, would that work for both dev and teamcity?

[labkey-tchad]

Ah, I see. I didn't realize what all that was doing. Do teamcity or devs need the report block at all? If not, I could leave that out of what I'm copying in from the chef repo (while retaining the #useLocalBuild# comments).

Or better/simpler, if we only copy in an uncommented enforce block, but strip "https://www.labkey.org" out of the report-uri, would that work for both dev and teamcity?

Not all of our TeamCity suites can handle the enforce policy. RStudio still throws several warnings.
We want the report policy on TeamCity because the test suites will reliably check for CSP alerts in the log and alert us to problems without interrupting the rest of the test. It also lets us selectively ignore violations without disabling the CSP check entirely (LabKey/testAutomation#1908).
For developers we want the enforce policy because it can make the violations more in-your-face since most folks don't monitor the csp log. (#956)

[labkey-willm]

Ah, I see. I didn't realize what all that was doing. Do teamcity or devs need the report block at all? If not, I could leave that out of what I'm copying in from the chef repo (while retaining the #useLocalBuild# comments).
Or better/simpler, if we only copy in an uncommented enforce block, but strip "https://www.labkey.org" out of the report-uri, would that work for both dev and teamcity?

Not all of our TeamCity suites can handle the enforce policy. RStudio still throws several warnings. We want the report policy on TeamCity because the test suites will reliably check for CSP alerts in the log and alert us to problems without interrupting the rest of the test. It also lets us selectively ignore violations without disabling the CSP check entirely (LabKey/testAutomation#1908). For developers we want the enforce policy because it can make the violations more in-your-face since most folks don't monitor the csp log. (#956)

ok, I've got it all fixed up to keep report enabled, enforce hidden by the #useLocalBuild#, the trimmed report-uri for both, and the github action in chef wired up to maintain all of that: https://github.com/LabKey/syseng-chef-server/pull/608

[labkey-tchad]

Not ready to remove https yet

[labkey-jeckels]

Not ready to remove https yet

Can tests and labkey.org add YouTube as an allowed external connection? I don't think we want to add that everywhere.

And should we support nonces on style elements too?

[labkey-adam]

Not ready to remove https yet

Can tests and labkey.org add YouTube as an allowed external connection? I don't think we want to add that everywhere.

And should we support nonces on style elements too?

Yes, I was going to suggest adding https://www.youtube.com as well. I don't know about the style elements.

[labkey-tchad]

And should we support nonces on style elements too?

The styles triggering the CSP aren't actually in a style element: "<link type="text/css" rel="stylesheet" href="https://cdn.datatables.net/1.13.6/css/jquery.dataTables.min.css">"

[labkey-tchad]

Not ready to remove https yet

Can tests and labkey.org add YouTube as an allowed external connection? I don't think we want to add that everywhere.

It would take a gradle plugin update to modify the CSP in this file when deploying dev and test environments. Probably not too tricky but not a freebie.

[labkey-jeckels]

Not ready to remove https yet

Can tests and labkey.org add YouTube as an allowed external connection? I don't think we want to add that everywhere.

It would take a gradle plugin update to modify the CSP in this file when deploying dev and test environments. Probably not too tricky but not a freebie.

I meant, can tests add allowed servers through the Admin Console's External Allowed Sources link?

[labkey-tchad]

Not ready to remove https yet

Can tests and labkey.org add YouTube as an allowed external connection? I don't think we want to add that everywhere.

It would take a gradle plugin update to modify the CSP in this file when deploying dev and test environments. Probably not too tricky but not a freebie.

I meant, can tests add allowed servers through the Admin Console's External Allowed Sources link?

Unfortunately, no.
connect-src doesn't cover iframes. We'd need a frame-src or child-src directive to allow embedding external videos.

[labkey-willm]

Trey has made the excellent point that the CSP source should be in server repo, then flow on to chef/dockerfile from there. I had dismissed it early on because I thought that copy was only for local dev... didn't realize that's what all the teamcity tests were keyed from.

So now the server repo is the canonical source, with a github action to copy it with a few changes to the others.

[labkey-tchad]

Could you swap the order of the report and enforce block back to how it was? Just to make the diff a bit clearer for future history tracking. (Not a huge deal if it would have other side-effects I'm unaware of)

We'll deal with youtube embedding later. There's just one self-contained test that will trigger a CSP warning.

[labkey-willm]

Could you swap the order of the report and enforce block back to how it was? Just to make the diff a bit clearer for future history tracking. (Not a huge deal if it would have other side-effects I'm unaware of)

We'll deal with youtube embedding later. There's just one self-contained test that will trigger a CSP warning.

when I tested swapping the order back to how it was, I found and fixed a problem with the push to chef repo, in which the chef recipe version bump was happening even if the application.properties.erb was unchanged. Lucky!