Skip to content

Commit

Permalink
Revert "feat: Do not strip CSP headers from HTTPResponse (#24760)"
Browse files Browse the repository at this point in the history
This reverts commit 0472bb9.
  • Loading branch information
AtofStryker committed Jan 12, 2023
1 parent f71c30a commit 7927488
Show file tree
Hide file tree
Showing 10 changed files with 50 additions and 559 deletions.
17 changes: 0 additions & 17 deletions packages/driver/cypress/e2e/e2e/security.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,4 @@ describe('security', () => {
cy.visit('/fixtures/security.html')
cy.get('div').should('not.exist')
})

it('works even with content-security-policy script-src', () => {
// create report URL
cy.intercept('/csp-report', (req) => {
throw new Error(`/csp-report should not be reached:${ req.body}`)
})

// inject script-src on visit
cy.intercept('/fixtures/empty.html', (req) => {
req.continue((res) => {
res.headers['content-security-policy'] = `script-src http://not-here.net; report-uri /csp-report`
})
})

cy.visit('/fixtures/empty.html')
.wait(1000)
})
})
37 changes: 3 additions & 34 deletions packages/proxy/lib/http/response-middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ import { URL } from 'url'
import { CookiesHelper } from './util/cookies'
import { doesTopNeedToBeSimulated } from './util/top-simulation'
import { toughCookieToAutomationCookie } from '@packages/server/lib/util/cookies'
import { generateCspDirectives, hasCspHeader, parseCspHeaders } from './util/csp-header'
import crypto from 'crypto'

interface ResponseMiddlewareProps {
/**
Expand Down Expand Up @@ -313,36 +311,6 @@ const SetInjectionLevel: ResponseMiddleware = function () {
// We set the header here only for proxied requests that have scripts injected that set the domain.
// Other proxied requests are ignored.
this.res.setHeader('Origin-Agent-Cluster', '?0')

// Only patch the headers that are being supplied by the response
const incomingCspHeaders = ['content-security-policy', 'content-security-policy-report-only']
.filter((headerName) => hasCspHeader(this.incomingRes.headers, headerName))

if (incomingCspHeaders.length) {
// In order to allow the injected script to run on sites with a CSP header
// we must add a generated `nonce` into the response headers
const nonce = crypto.randomBytes(16).toString('base64')

this.res.injectionNonce = nonce

// Since CSP headers are not cumulative, the nonce policy must be set on each CSP header individually
const mapPolicies = (policy: Map<string, string[]>) => {
const cspScriptSrc = policy.get('script-src') || []

policy.set('script-src', [...cspScriptSrc, `'nonce-${nonce}'`])

return generateCspDirectives(policy)
}

// Iterate through each CSP header
incomingCspHeaders.forEach((headerName) => {
// Map the nonce on each CSP header
const modifiedCspHeaders = parseCspHeaders(this.incomingRes.headers, headerName).map(mapPolicies)

// To replicate original response CSP headers, we must apply all header values as an array
this.res.setHeader(headerName, modifiedCspHeaders)
})
}
}

this.res.wantsSecurityRemoved = (this.config.modifyObstructiveCode || this.config.experimentalModifyObstructiveThirdPartyCode) &&
Expand Down Expand Up @@ -388,6 +356,8 @@ const OmitProblematicHeaders: ResponseMiddleware = function () {
'x-frame-options',
'content-length',
'transfer-encoding',
'content-security-policy',
'content-security-policy-report-only',
'connection',
])

Expand Down Expand Up @@ -570,7 +540,6 @@ const MaybeInjectHtml: ResponseMiddleware = function () {

const decodedBody = iconv.decode(body, nodeCharset)
const injectedBody = await rewriter.html(decodedBody, {
cspNonce: this.res.injectionNonce,
domainName: cors.getDomainNameFromUrl(this.req.proxiedUrl),
wantsInjection: this.res.wantsInjection,
wantsSecurityRemoved: this.res.wantsSecurityRemoved,
Expand Down Expand Up @@ -644,8 +613,8 @@ export default {
AttachPlainTextStreamFn,
InterceptResponse,
PatchExpressSetHeader,
OmitProblematicHeaders, // Since we might modify CSP headers, this middleware needs to come BEFORE SetInjectionLevel
SetInjectionLevel,
OmitProblematicHeaders,
MaybePreventCaching,
MaybeStripDocumentDomainFeaturePolicy,
MaybeCopyCookiesFromIncomingRes,
Expand Down
58 changes: 0 additions & 58 deletions packages/proxy/lib/http/util/csp-header.ts

This file was deleted.

19 changes: 4 additions & 15 deletions packages/proxy/lib/http/util/inject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { getRunnerInjectionContents, getRunnerCrossOriginInjectionContents } fro
import type { AutomationCookie } from '@packages/server/lib/automation/cookies'

interface InjectionOpts {
cspNonce?: string
shouldInjectDocumentDomain: boolean
}
interface FullCrossOriginOpts {
Expand All @@ -13,7 +12,6 @@ interface FullCrossOriginOpts {
}

export function partial (domain, options: InjectionOpts) {
const { cspNonce } = options
let documentDomainInjection = `document.domain = '${domain}';`

if (!options.shouldInjectDocumentDomain) {
Expand All @@ -23,17 +21,13 @@ export function partial (domain, options: InjectionOpts) {
// With useDefaultDocumentDomain=true we continue to inject an empty script tag in order to be consistent with our other forms of injection.
// This is also diagnostic in nature is it will allow us to debug easily to make sure injection is still occurring.
return oneLine`
<script type='text/javascript'${
cspNonce ? ` nonce="${cspNonce}"` : ''
}>
<script type='text/javascript'>
${documentDomainInjection}
</script>
`
}

export function full (domain, options: InjectionOpts) {
const { cspNonce } = options

return getRunnerInjectionContents().then((contents) => {
let documentDomainInjection = `document.domain = '${domain}';`

Expand All @@ -42,9 +36,7 @@ export function full (domain, options: InjectionOpts) {
}

return oneLine`
<script type='text/javascript'${
cspNonce ? ` nonce="${cspNonce}"` : ''
}>
<script type='text/javascript'>
${documentDomainInjection}
${contents}
Expand All @@ -55,7 +47,6 @@ export function full (domain, options: InjectionOpts) {

export async function fullCrossOrigin (domain, options: InjectionOpts & FullCrossOriginOpts) {
const contents = await getRunnerCrossOriginInjectionContents()
const { cspNonce, ...crossOriginOptions } = options

let documentDomainInjection = `document.domain = '${domain}';`

Expand All @@ -64,14 +55,12 @@ export async function fullCrossOrigin (domain, options: InjectionOpts & FullCros
}

return oneLine`
<script type='text/javascript'${
cspNonce ? ` nonce="${cspNonce}"` : ''
}>
<script type='text/javascript'>
${documentDomainInjection}
(function (cypressConfig) {
${contents}
}(${JSON.stringify(crossOriginOptions)}));
}(${JSON.stringify(options)}));
</script>
`
}
5 changes: 0 additions & 5 deletions packages/proxy/lib/http/util/rewriter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ export type SecurityOpts = {
}

export type InjectionOpts = {
cspNonce?: string
domainName: string
wantsInjection: CypressWantsInjection
wantsSecurityRemoved: any
Expand All @@ -33,7 +32,6 @@ function getRewriter (useAstSourceRewriting: boolean) {

function getHtmlToInject (opts: InjectionOpts & SecurityOpts) {
const {
cspNonce,
domainName,
wantsInjection,
modifyObstructiveThirdPartyCode,
Expand All @@ -46,11 +44,9 @@ function getHtmlToInject (opts: InjectionOpts & SecurityOpts) {
case 'full':
return inject.full(domainName, {
shouldInjectDocumentDomain,
cspNonce,
})
case 'fullCrossOrigin':
return inject.fullCrossOrigin(domainName, {
cspNonce,
modifyObstructiveThirdPartyCode,
modifyObstructiveCode,
simulatedCookies,
Expand All @@ -59,7 +55,6 @@ function getHtmlToInject (opts: InjectionOpts & SecurityOpts) {
case 'partial':
return inject.partial(domainName, {
shouldInjectDocumentDomain,
cspNonce,
})
default:
return
Expand Down
1 change: 0 additions & 1 deletion packages/proxy/lib/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ export type CypressWantsInjection = 'full' | 'fullCrossOrigin' | 'partial' | fal
* An outgoing response to an incoming request to the Cypress web server.
*/
export type CypressOutgoingResponse = Response & {
injectionNonce?: string
isInitial: null | boolean
wantsInjection: CypressWantsInjection
wantsSecurityRemoved: null | boolean
Expand Down
109 changes: 0 additions & 109 deletions packages/proxy/test/integration/net-stubbing.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,27 +72,9 @@ context('network stubbing', () => {

destinationApp.get('/', (req, res) => res.send('it worked'))

destinationApp.get('/csp-header', (req, res) => {
const headerName = req.query.headerName

res.setHeader('content-type', 'text/html')
res.setHeader(headerName, 'fake-directive fake-value')
res.send('<foo>bar</foo>')
})

destinationApp.get('/csp-header-multiple', (req, res) => {
const headerName = req.query.headerName

res.setHeader('content-type', 'text/html')
res.setHeader(headerName, ['default \'self\'', 'script-src \'self\' localhost'])
res.send('<foo>bar</foo>')
})

server = allowDestroy(destinationApp.listen(() => {
destinationPort = server.address().port
remoteStates.set(`http://localhost:${destinationPort}`)
remoteStates.set(`http://localhost:${destinationPort}/csp-header`)
remoteStates.set(`http://localhost:${destinationPort}/csp-header-multiple`)
done()
}))
})
Expand Down Expand Up @@ -303,95 +285,4 @@ context('network stubbing', () => {
expect(sendContentLength).to.eq(receivedContentLength)
expect(sendContentLength).to.eq(realContentLength)
})

describe('CSP Headers', () => {
// Loop through valid CSP header names can verify that we handle them
[
'content-security-policy',
'Content-Security-Policy',
'content-security-policy-report-only',
'Content-Security-Policy-Report-Only',
].forEach((headerName) => {
describe(`${headerName}`, () => {
it('does not add CSP header if injecting JS and original response had no CSP header', () => {
netStubbingState.routes.push({
id: '1',
routeMatcher: {
url: '*',
},
hasInterceptor: false,
staticResponse: {
body: '<foo>bar</foo>',
},
getFixture: async () => {},
matches: 1,
})

return supertest(app)
.get(`/http://localhost:${destinationPort}`)
.set('Accept', 'text/html,application/xhtml+xml')
.then((res) => {
expect(res.headers[headerName]).to.be.undefined
expect(res.headers[headerName.toLowerCase()]).to.be.undefined
})
})

it('does not modify CSP header if not injecting JS and original response had CSP header', () => {
return supertest(app)
.get(`/http://localhost:${destinationPort}/csp-header?headerName=${headerName}`)
.then((res) => {
expect(res.headers[headerName.toLowerCase()]).to.equal('fake-directive fake-value')
})
})

it('modifies CSP header if injecting JS and original response had CSP header', () => {
return supertest(app)
.get(`/http://localhost:${destinationPort}/csp-header?headerName=${headerName}`)
.set('Accept', 'text/html,application/xhtml+xml')
.then((res) => {
expect(res.headers[headerName.toLowerCase()]).to.match(/^fake-directive fake-value; script-src 'nonce-[^-A-Za-z0-9+/=]|=[^=]|={3,}'/)
})
})

it('modifies CSP header if injecting JS and original response had multiple CSP headers', () => {
return supertest(app)
.get(`/http://localhost:${destinationPort}/csp-header-multiple?headerName=${headerName}`)
.set('Accept', 'text/html,application/xhtml+xml')
.then((res) => {
expect(res.headers[headerName.toLowerCase()]).to.match(/default 'self'; script-src 'nonce-[^-A-Za-z0-9+/=]|=[^=]|={3,}'/)
expect(res.headers[headerName.toLowerCase()]).to.match(/script-src 'self' localhost 'nonce-[^-A-Za-z0-9+/=]|=[^=]|={3,}'/)
})
})

if (headerName !== headerName.toLowerCase()) {
// Do not add a non-lowercase version of a CSP header, because most-restrictive is used
it('removes non-lowercase CSP header to avoid conflicts on unmodified CSP headers', () => {
return supertest(app)
.get(`/http://localhost:${destinationPort}/csp-header?headerName=${headerName}`)
.then((res) => {
expect(res.headers[headerName]).to.be.undefined
})
})

it('removes non-lowercase CSP header to avoid conflicts on modified CSP headers', () => {
return supertest(app)
.get(`/http://localhost:${destinationPort}/csp-header?headerName=${headerName}`)
.set('Accept', 'text/html,application/xhtml+xml')
.then((res) => {
expect(res.headers[headerName]).to.be.undefined
})
})

it('removes non-lowercase CSP header to avoid conflicts on multiple CSP headers', () => {
return supertest(app)
.get(`/http://localhost:${destinationPort}/csp-header-multiple?headerName=${headerName}`)
.set('Accept', 'text/html,application/xhtml+xml')
.then((res) => {
expect(res.headers[headerName]).to.be.undefined
})
})
}
})
})
})
})
Loading

0 comments on commit 7927488

Please sign in to comment.