-
Notifications
You must be signed in to change notification settings - Fork 3
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
2238 document unsafe-inline exception #2941
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2941 +/- ##
========================================
Coverage 93.11% 93.11%
========================================
Files 272 272
Lines 7055 7055
Branches 591 591
========================================
Hits 6569 6569
Misses 392 392
Partials 94 94
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
|
docs/Technical-Documentation/Architecture-Decision-Record/016-zap-waiver.md
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to move the unsafe-inline header to location instead of server global to limit the security implication
changes needed:
|
UPDATE Apr 9th:
|
set $CSP "${CSP}script-src-attr 'self' 'unsafe-inline' http://{{env "KIBANA_BASE_URL"}}:5601;"; | ||
``` | ||
|
||
Kibana performs ajax requests from its frontend to scripts hosted on the server in order to provide styling and javascript functionality - in order to serve Kibana from behind a proxy, which is a requirement with cloud.gov, these `unsafe-inline` and `unsafe-eval` directives are required. The CSP exceptions are fairly low risk as they are limited to artifacts within the Kibana domain. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jtimpe please include the links in #2843 as references. a comment from cloud.gov staff is included in this issue
@@ -22,6 +22,21 @@ When investigating, we sought out information on what other, modern ACF systems | |||
|
|||
While we also investigated and developed technical solutions utilizing either React-app-rewired or Next.js, we decided that the departure from our existing stack was not a worthwhile investment and could lead to other complications down the road. | |||
|
|||
### 2024 Update |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jtimpe can we rename this markdown to 016-security-scan-waiver.md
?
@@ -22,6 +22,26 @@ When investigating, we sought out information on what other, modern ACF systems | |||
|
|||
While we also investigated and developed technical solutions utilizing either React-app-rewired or Next.js, we decided that the departure from our existing stack was not a worthwhile investment and could lead to other complications down the road. | |||
|
|||
### 2024 Update |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jtimpe @raftmsohani are any of the CSP related findings on the latest nightly scan related to what is described below? if so, we may want acknowledge that this is not just being found in webinspect but zap as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i do not believe any of those findings are related to this waiver
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ADPennington Jan is correct, they are not relevant.
On a separate note, I am creating a ticket to check for user auth, similar to what we do for Kibana auth, to prevent access to api without auth: https://tdp-frontend-develop.acf.hhs.gov/v1/users/id/
* [elastic/kibana #27047](https://github.com/elastic/kibana/issues/27047#issuecomment-799680263) in March 2021 | ||
* an [elastic discussion post](https://discuss.elastic.co/t/does-kibana-need-the-autorisation-of-unsafe-inline-or-unsafe-eval-to-work-properly/234390/2) in May 2020 | ||
|
||
The CSP exceptions are fairly low risk as they are limited to artifacts within the Kibana domain. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jtimpe can you provide a couple of examples of what we mean by artifacts here? might also be worth explicitly stating there is no risk to data assets.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
open to ideas on how we can convey that there's no risk to data assets - what assets are specifically of concern?
* Vega (when interpreting its visualization syntax) | ||
* Lodash (via the _.template function) | ||
|
||
Maliciously side-loading a script would require Kibana have an unresolved persistent-XSS vulnerability, as well as file-system access to our cloud.gov Kibana application to load a script from an allowed domain. The attack vector is further limited by our authentication implementation, which requires a user be approved and granted Kibana access before loading any Kibana-related pages at all. In these cases, an attacker would have gained access to the system already. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@elipe17 @raftmsohani i added the following note about mitigation. please double-check my logic here :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you @jtimpe 🚀
``` | ||
|
||
Kibana performs ajax requests from its frontend to scripts hosted on the server in order to provide styling and javascript functionality - in order to serve Kibana from behind a proxy, which is a requirement with cloud.gov, these `unsafe-inline` and `unsafe-eval` directives are required. | ||
We currently use Kibana version 7.4 [released October, 2019](https://www.elastic.co/blog/kibana-7-4-0-released). This requirement is present in our current version as mentioned by the Elastic team in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cloud.gov is actually running elastic/kibana 7.10 now!
Summary of Changes
Pull request closes #2238
Pull request closes #2843
Upon further investigation, the following low priority findings no longer show up in our latest Webinspect scans
I have removed the changes associated with those findings and instead only addressed the final one
Since our Kibana implementation requires being served behind a proxy, the
unsafe-eval
andunsafe-inline
directives mentioned by the scan are required. I've included some documentation in ADR 16 to reflect this.How to Test
List the steps to test the PR
These steps are generic, please adjust as necessary.
Deliverables
More details on how deliverables herein are assessed included here.
Deliverable 1: Accepted Features
Checklist of ACs:
lfrohlich
and/oradpennington
confirmed that ACs are met.Deliverable 2: Tested Code
CodeCov Report
comment in PR)CodeCov Report
comment in PR)Deliverable 3: Properly Styled Code
Deliverable 4: Accessible
iamjolly
andttran-hub
using Accessibility Insights reveal any errors introduced in this PR?Deliverable 5: Deployed
Deliverable 6: Documented
Deliverable 7: Secure
Deliverable 8: User Research
Research product(s) clearly articulate(s):