feat: add reject unauthorized to api server calls and standardize CA usage (#24493)

.circleci/config.yml

@@ -27,7 +27,7 @@ mainBuildFilters: &mainBuildFilters
     branches:
       only:
         - develop
-        - 'feature/v8-snapshots'
+        - 'ryanm/feat/certificates'
 
 # usually we don't build Mac app - it takes a long time
 # but sometimes we want to really confirm we are doing the right thing
@@ -36,7 +36,7 @@ macWorkflowFilters: &darwin-workflow-filters
   when:
     or:
     - equal: [ develop, << pipeline.git.branch >> ]
-    - equal: [ 'feature/v8-snapshots', << pipeline.git.branch >> ]
+    - equal: [ 'ryanm/feat/certificates', << pipeline.git.branch >> ]
     - matches:
           pattern: "-release$"
           value: << pipeline.git.branch >>
@@ -130,7 +130,7 @@ commands:
       - run:
           name: Check current branch to persist artifacts
           command: |
-            if [[ "$CIRCLE_BRANCH" != "develop" && "$CIRCLE_BRANCH" != "feature/v8-snapshots" ]]; then
+            if [[ "$CIRCLE_BRANCH" != "develop" && "$CIRCLE_BRANCH" != "ryanm/feat/certificates" ]]; then
               echo "Not uploading artifacts or posting install comment for this branch."
               circleci-agent step halt
             fi

cli/lib/cli.js

@@ -405,7 +405,21 @@ module.exports = {
       args = process.argv
     }
 
-    const { CYPRESS_INTERNAL_ENV } = process.env
+    const { CYPRESS_INTERNAL_ENV, CYPRESS_DOWNLOAD_USE_CA } = process.env
+
+    if (process.env.CYPRESS_DOWNLOAD_USE_CA) {
+      let msg = `
+        ${logSymbols.warning} Warning: It looks like you're setting CYPRESS_DOWNLOAD_USE_CA=${CYPRESS_DOWNLOAD_USE_CA}
+
+        The environment variable "CYPRESS_DOWNLOAD_USE_CA" is no longer required to be set.
+        
+        You can safely unset this environment variable.
+      `
+
+      logger.log()
+      logger.warn(stripIndent(msg))
+      logger.log()
+    }
 
     if (!util.isValidCypressInternalEnvValue(CYPRESS_INTERNAL_ENV)) {
       debug('invalid CYPRESS_INTERNAL_ENV value', CYPRESS_INTERNAL_ENV)

cli/lib/tasks/download.js

@@ -40,13 +40,7 @@ const getBaseUrl = () => {
 
 const getCA = () => {
   return new Promise((resolve) => {
-    if (!util.getEnv('CYPRESS_DOWNLOAD_USE_CA')) {
-      resolve()
-    }
-
-    if (process.env.npm_config_ca) {
-      resolve(process.env.npm_config_ca)
-    } else if (process.env.npm_config_cafile) {
+    if (process.env.npm_config_cafile) {
       fs.readFile(process.env.npm_config_cafile, 'utf8')
       .then((cafileContent) => {
         resolve(cafileContent)
@@ -54,6 +48,8 @@ const getCA = () => {
       .catch(() => {
         resolve()
       })
+    } else if (process.env.npm_config_ca) {
+      resolve(process.env.npm_config_ca)
     } else {
       resolve()
     }

cli/test/lib/tasks/download_spec.js

@@ -607,7 +607,7 @@ describe('lib/tasks/download', function () {
       // prevent ambient environment masking of environment variables referenced in this test
 
       ;([
-        'CYPRESS_DOWNLOAD_USE_CA', 'NO_PROXY', 'http_proxy',
+        'NO_PROXY', 'http_proxy',
         'https_proxy', 'npm_config_ca', 'npm_config_cafile',
         'npm_config_https_proxy', 'npm_config_proxy',
       ]).forEach((e) => {
@@ -683,7 +683,6 @@ describe('lib/tasks/download', function () {
     })
 
     it('returns CA from npm_config_ca', () => {
-      process.env.CYPRESS_DOWNLOAD_USE_CA = 'true'
       process.env.npm_config_ca = 'foo'
 
       return download.getCA().then((ca) => {
@@ -692,7 +691,6 @@ describe('lib/tasks/download', function () {
     })
 
     it('returns CA from npm_config_cafile', () => {
-      process.env.CYPRESS_DOWNLOAD_USE_CA = 'true'
       process.env.npm_config_cafile = 'test/fixture/cafile.pem'
 
       return download.getCA().then((ca) => {
@@ -701,7 +699,6 @@ describe('lib/tasks/download', function () {
     })
 
     it('returns undefined if failed reading npm_config_cafile', () => {
-      process.env.CYPRESS_DOWNLOAD_USE_CA = 'true'
       process.env.npm_config_cafile = 'test/fixture/not-exists.pem'
 
       return download.getCA().then((ca) => {

packages/network/lib/agent.ts

@@ -8,11 +8,53 @@ import url from 'url'
 import { createRetryingSocket, getAddress } from './connect'
 import { lenientOptions } from './http-utils'
 import { ClientCertificateStore } from './client-certificates'
+import { CaOptions, getCaOptions } from './ca'
 
 const debug = debugModule('cypress:network:agent')
 const CRLF = '\r\n'
 const statusCodeRe = /^HTTP\/1.[01] (\d*)/
 
+let baseCaOptions: CaOptions | undefined
+const getCaOptionsPromise = (): Promise<CaOptions> => {
+  return getCaOptions().then((options: CaOptions) => {
+    baseCaOptions = options
+
+    return options
+  }).catch(() => {
+    // Errors reading the config are treated as warnings by npm and node and handled by those processes separately
+    // from what we're doing here.
+    return {}
+  })
+}
+let baseCaOptionsPromise: Promise<CaOptions> = getCaOptionsPromise()
+
+// This is for testing purposes only
+export const _resetBaseCaOptionsPromise = () => {
+  baseCaOptions = undefined
+  baseCaOptionsPromise = getCaOptionsPromise()
+}
+
+const mergeCAOptions = (options: https.RequestOptions, caOptions: CaOptions): https.RequestOptions => {
+  if (!caOptions.ca) {
+    return options
+  }
+
+  if (!options.ca) {
+    return {
+      ...options,
+      ca: caOptions.ca,
+    }
+  }
+
+  // First, normalize the options.ca option. It can be a string, a Buffer, an array of strings, or an array of Buffers
+  const caArray = _.castArray(options.ca).map((caOption) => caOption.toString())
+
+  return {
+    ...options,
+    ca: [...caArray, ...caOptions.ca],
+  }
+}
+
 export const clientCertificateStore = new ClientCertificateStore()
 
 type WithProxyOpts<RequestOptions> = RequestOptions & {
@@ -195,7 +237,7 @@ export class CombinedAgent {
       if (isHttps) {
         _.assign(options, clientCertificateStore.getClientCertificateAgentOptionsForUrl(options.uri))
 
-        return this.httpsAgent.addRequest(req, options)
+        return this.httpsAgent.addRequest(req, options as https.RequestOptions)
       }
 
       this.httpAgent.addRequest(req, options)
@@ -283,7 +325,7 @@ class HttpsAgent extends https.Agent {
     super(opts)
   }
 
-  addRequest (req: http.ClientRequest, options: http.RequestOptions) {
+  addRequest (req: http.ClientRequest, options: https.RequestOptions) {
     // Ensure we have a proper port defined otherwise node has assumed we are port 80
     // (https://github.com/nodejs/node/blob/master/lib/_http_client.js#L164) since we are a combined agent
     // rather than an http or https agent. This will cause issues with fetch requests (@cypress/request already handles it:
@@ -293,7 +335,13 @@ class HttpsAgent extends https.Agent {
       options.port = 443
     }
 
-    super.addRequest(req, options)
+    if (baseCaOptions) {
+      super.addRequest(req, mergeCAOptions(options, baseCaOptions))
+    } else {
+      baseCaOptionsPromise.then((caOptions) => {
+        super.addRequest(req, mergeCAOptions(options, caOptions))
+      })
+    }
   }
 
   createConnection (options: HttpsRequestOptions, cb: http.SocketCallback) {

packages/network/lib/ca.ts

@@ -0,0 +1,54 @@
+import { promises as fs } from 'fs'
+import tls from 'tls'
+
+export type CaOptions = { ca?: string[] }
+
+const getNpmConfigCAFileValue = (): Promise<string | undefined> => {
+  if (process.env.npm_config_cafile) {
+    return fs.readFile(process.env.npm_config_cafile, 'utf8').then((ca) => {
+      return ca
+    }).catch(() => {
+      return undefined
+    })
+  }
+
+  return Promise.resolve(undefined)
+}
+const getNodeExtraCACertsFileValue = (): Promise<string | undefined> => {
+  if (process.env.NODE_EXTRA_CA_CERTS) {
+    return fs.readFile(process.env.NODE_EXTRA_CA_CERTS, 'utf8').then((ca) => {
+      return ca
+    }).catch(() => {
+      return undefined
+    })
+  }
+
+  return Promise.resolve(undefined)
+}
+
+const getCaOptions = (): Promise<CaOptions> => {
+  // Load the contents of process.env.npm_config_cafile and process.env.NODE_EXTRA_CA_CERTS
+  // They will be cached so we don't have to actually read them on every call
+  return Promise.all([getNpmConfigCAFileValue(), getNodeExtraCACertsFileValue()]).then(([npm_config_cafile, NODE_EXTRA_CA_CERTS]) => {
+    // Merge the contents of ca with the npm config options. These options are meant to be replacements, but we want to keep the tls client certificate
+    // config that our consumers provide
+    if (npm_config_cafile) {
+      return { ca: [npm_config_cafile] }
+    }
+
+    if (process.env.npm_config_ca) {
+      return { ca: [process.env.npm_config_ca] }
+    }
+
+    // Merge the contents of ca with the NODE_EXTRA_CA_CERTS options. This option is additive to the tls root certificates
+    if (NODE_EXTRA_CA_CERTS) {
+      return { ca: tls.rootCertificates.concat(NODE_EXTRA_CA_CERTS) }
+    }
+
+    return {}
+  })
+}
+
+export {
+  getCaOptions,
+}

packages/network/test/support/servers.ts

@@ -27,9 +27,11 @@ function createExpressApp () {
   return app
 }
 
-function getLocalhostCertKeys () {
+function getCAInformation () {
   return CA.create()
-  .then((ca) => ca.generateServerCertificateKeys('localhost'))
+  .then((ca) => {
+    return Promise.all([ca.generateServerCertificateKeys('localhost'), ca.getCACertPath()])
+  })
 }
 
 function onWsConnection (socket) {
@@ -42,20 +44,22 @@ export class Servers {
   httpsServer: https.Server & AsyncServer
   wsServer: any
   wssServer: any
+  caCertificatePath: string
 
   start (httpPort: number, httpsPort: number) {
     return Promise.join(
       createExpressApp(),
-      getLocalhostCertKeys(),
+      getCAInformation(),
     )
-    .spread((app: Express.Application, [cert, key]: string[]) => {
+    .spread((app: Express.Application, [serverCertificateKeys, caCertificatePath]: [serverCertificateKeys: string[], caCertificatePath: string]) => {
       this.httpServer = Promise.promisifyAll(
         allowDestroy(http.createServer(app)),
       ) as http.Server & AsyncServer
 
       this.wsServer = new SocketIOServer(this.httpServer)
 
-      this.https = { cert, key }
+      this.caCertificatePath = caCertificatePath
+      this.https = { cert: serverCertificateKeys[0], key: serverCertificateKeys[1] }
       this.httpsServer = Promise.promisifyAll(
         allowDestroy(https.createServer(this.https, <http.RequestListener>app)),
       ) as https.Server & AsyncServer

packages/network/test/unit/agent_spec.ts

@@ -18,14 +18,16 @@ import {
   regenerateRequestHead,
   CombinedAgent,
   clientCertificateStore,
+  _resetBaseCaOptionsPromise,
 } from '../../lib/agent'
 import { allowDestroy } from '../../lib/allow-destroy'
 import { AsyncServer, Servers } from '../support/servers'
 import { UrlClientCertificates, ClientCertificates, PemKey } from '../../lib/client-certificates'
-import Forge from 'node-forge'
+import { pki } from 'node-forge'
 import fetch from 'cross-fetch'
 import os from 'os'
-const { pki } = Forge
+import path from 'path'
+import fs from 'fs'
 
 const expect = chai.expect
 
@@ -35,7 +37,14 @@ const PROXY_PORT = 31000
 const HTTP_PORT = 31080
 const HTTPS_PORT = 443
 
-function createCertAndKey (): [object, object] {
+const tempDirName = 'ca-config-tests'
+const tempDirPath = path.join(os.tmpdir(), tempDirName)
+
+if (!fs.existsSync(tempDirPath)) {
+  fs.mkdirSync(tempDirPath)
+}
+
+function createCertAndKey (): [pki.Certificate, pki.rsa.PrivateKey] {
   let keys = pki.rsa.generateKeyPair(2048)
   let cert = pki.createCertificate()
 
@@ -491,6 +500,143 @@ describe('lib/agent', function () {
     })
   })
 
+  context('CombinedAgent with CA overrides', function () {
+    const proxyUrl = `https://localhost:${PROXY_PORT}`
+
+    before(function () {
+      this.servers = new Servers()
+
+      return this.servers.start(HTTP_PORT, HTTPS_PORT)
+    })
+
+    after(function () {
+      return this.servers.stop()
+    })
+
+    ;[
+      {
+        name: 'should use the npm_config_cafile override',
+        option: 'npm_config_cafile',
+      },
+      {
+        name: 'should use the npm_config_ca override',
+        option: 'npm_config_ca',
+      },
+      {
+        name: 'should use the NODE_EXTRA_CA_CERTS override',
+        option: 'NODE_EXTRA_CA_CERTS',
+      },
+    ].slice().map((testCase) => {
+      context(testCase.name, function () {
+        beforeEach(function () {
+          // PROXY vars should override npm_config vars, so set them to cause failures if they are used
+          // @see https://github.com/cypress-io/cypress/pull/8295
+          process.env.npm_config_proxy = process.env.npm_config_https_proxy = 'http://erroneously-used-npm-proxy.invalid'
+          process.env.npm_config_noproxy = 'just,some,nonsense'
+
+          process.env.HTTP_PROXY = process.env.HTTPS_PROXY = proxyUrl
+          process.env.NO_PROXY = ''
+
+          this.agent = new CombinedAgent()
+
+          this.request = request.defaults({
+            proxy: null,
+            agent: this.agent,
+          })
+
+          let options: any = {
+            keepRequests: true,
+            https: {
+              ...this.servers.https,
+              ca: this.caContents,
+            },
+            auth: false,
+          }
+
+          if (testCase.option === 'npm_config_cafile') {
+            process.env.npm_config_cafile = this.servers.caCertificatePath
+            this.caContents = [fs.readFileSync(this.servers.caCertificatePath, 'utf-8')]
+
+            // Ensure the priority picks cafile over the next two options
+            process.env.npm_config_ca = 'a'
+            process.env.NODE_EXTRA_CA_CERTS = 'b'
+          }
+
+          if (testCase.option === 'npm_config_ca') {
+            this.caContents = [fs.readFileSync(this.servers.caCertificatePath, 'utf-8')]
+            process.env.npm_config_ca = this.caContents[0]
+
+            // Ensure the priority picks cafile over the next option
+            process.env.NODE_EXTRA_CA_CERTS = 'b'
+          }
+
+          if (testCase.option === 'NODE_EXTRA_CA_CERTS') {
+            process.env.NODE_EXTRA_CA_CERTS = this.servers.caCertificatePath
+            this.caContents = [fs.readFileSync(this.servers.caCertificatePath, 'utf-8'), ...tls.rootCertificates]
+          }
+
+          _resetBaseCaOptionsPromise()
+
+          this.debugProxy = new DebuggingProxy(options)
+
+          return this.debugProxy.start(PROXY_PORT)
+        })
+
+        afterEach(function () {
+          delete process.env.npm_config_cafile
+          delete process.env.npm_config_ca
+          delete process.env.NODE_EXTRA_CA_CERTS
+          this.debugProxy.stop()
+        })
+
+        it(`CA from ${testCase.option} presented for https request`, function () {
+          return this.request({
+            url: `https://localhost:${HTTPS_PORT}/get`,
+            rejectUnauthorized: true,
+          }).then((body) => {
+            // Test the CA options the first time through
+            expect(body).to.eq('It worked!')
+            if (this.debugProxy) {
+              expect(this.debugProxy.requests[0]).to.include({
+                https: true,
+                url: `localhost:${HTTPS_PORT}`,
+              })
+            }
+
+            const socketKey = Object.keys(this.agent.httpsAgent.sockets).filter((key) => key.includes(`localhost:${HTTPS_PORT}`))
+
+            expect(socketKey.length).to.eq(1, 'There should only be a single localhost TLS Socket')
+
+            for (const ca of this.caContents) {
+              expect(socketKey[0]).to.contain(ca, `${testCase.option} should be used for the TLS Socket`)
+            }
+
+            return this.request({
+              url: `https://localhost:${HTTPS_PORT}/get`,
+            })
+          }).then((body) => {
+            // Test that the caching of the ca options works
+            expect(body).to.eq('It worked!')
+            if (this.debugProxy) {
+              expect(this.debugProxy.requests[0]).to.include({
+                https: true,
+                url: `localhost:${HTTPS_PORT}`,
+              })
+            }
+
+            const socketKey = Object.keys(this.agent.httpsAgent.sockets).filter((key) => key.includes(`localhost:${HTTPS_PORT}`))
+
+            expect(socketKey.length).to.eq(1, 'There should only be a single localhost TLS Socket')
+
+            for (const ca of this.caContents) {
+              expect(socketKey[0]).to.contain(ca, `${testCase.option} should be used for the TLS Socket`)
+            }
+          })
+        })
+      })
+    })
+  })
+
   context('CombinedAgent with client certificates', function () {
     const proxyUrl = `https://localhost:${PROXY_PORT}`
 
@@ -509,6 +655,21 @@ describe('lib/agent', function () {
         name: 'should present a client certificate',
         presentClientCertificate: true,
       },
+      {
+        name: 'should present a client certificate with npm_config_cafile',
+        option: 'npm_config_cafile',
+        presentClientCertificate: true,
+      },
+      {
+        name: 'should present a client certificate with npm_config_ca',
+        option: 'npm_config_ca',
+        presentClientCertificate: true,
+      },
+      {
+        name: 'should present a client certificate with NODE_EXTRA_CA_CERTS',
+        option: 'NODE_EXTRA_CA_CERTS',
+        presentClientCertificate: true,
+      },
       {
         name: 'should not present a client certificate',
         presentClientCertificate: false,
@@ -546,21 +707,48 @@ describe('lib/agent', function () {
             const testCerts = new UrlClientCertificates(`https://localhost`)
 
             testCerts.clientCertificates = new ClientCertificates()
-            testCerts.clientCertificates.cert.push(pemCert)
-            testCerts.clientCertificates.key.push(new PemKey(pki.privateKeyToPem(certAndKey[1]), undefined))
+            testCerts.clientCertificates.cert.push(Buffer.from(pemCert, 'utf-8'))
+            testCerts.clientCertificates.key.push(new PemKey(Buffer.from(pki.privateKeyToPem(certAndKey[1]), 'utf-8'), undefined))
             clientCertificateStore.addClientCertificatesForUrl(testCerts)
           }
 
+          if (testCase.option === 'npm_config_cafile') {
+            process.env.npm_config_cafile = this.servers.caCertificatePath
+            this.caContents = [fs.readFileSync(this.servers.caCertificatePath, 'utf-8')]
+
+            // Ensure the priority picks cafile over the next two options
+            process.env.npm_config_ca = 'a'
+            process.env.NODE_EXTRA_CA_CERTS = 'b'
+          }
+
+          if (testCase.option === 'npm_config_ca') {
+            this.caContents = [fs.readFileSync(this.servers.caCertificatePath, 'utf-8')]
+            process.env.npm_config_ca = this.caContents[0]
+
+            // Ensure the priority picks cafile over the next option
+            process.env.NODE_EXTRA_CA_CERTS = 'b'
+          }
+
+          if (testCase.option === 'NODE_EXTRA_CA_CERTS') {
+            process.env.NODE_EXTRA_CA_CERTS = this.servers.caCertificatePath
+            this.caContents = [fs.readFileSync(this.servers.caCertificatePath, 'utf-8'), ...tls.rootCertificates]
+          }
+
+          _resetBaseCaOptionsPromise()
+
           this.debugProxy = new DebuggingProxy(options)
 
           return this.debugProxy.start(PROXY_PORT)
         })
 
         afterEach(function () {
+          delete process.env.npm_config_cafile
+          delete process.env.npm_config_ca
+          delete process.env.NODE_EXTRA_CA_CERTS
           this.debugProxy.stop()
         })
 
-        it(`Client certificate${testCase.presentClientCertificate ? ' ' : ' not '}presented for https request`, function () {
+        it(`Client certificate${testCase.presentClientCertificate ? ' ' : ' not '}presented for https request${testCase.option ? ` with config option ${testCase.option}` : '' }`, function () {
           return this.request({
             url: `https://localhost:${HTTPS_PORT}/get`,
           }).then((body) => {
@@ -583,6 +771,12 @@ describe('lib/agent', function () {
             } else {
               expect(socketKey[0]).not.to.contain(this.clientCert, 'A client cert should not be used for the TLS Socket')
             }
+
+            if (this.caContents) {
+              for (const ca of this.caContents) {
+                expect(socketKey[0]).to.contain(ca, `${testCase.option} should be used for the TLS Socket`)
+              }
+            }
           })
         })
 

packages/server/lib/cloud/api.ts

@@ -46,6 +46,7 @@ const rp = request.defaults((params, callback) => {
     proxy: null,
     gzip: true,
     cacheable: false,
+    rejectUnauthorized: true,
   })
 
   const headers = params.headers != null ? params.headers : (params.headers = {})

packages/server/test/unit/cloud/api_spec.js

@@ -69,6 +69,20 @@ describe('lib/cloud/api', () => {
       })
     })
 
+    it('sets rejectUnauthorized on the request', () => {
+      nock.cleanAll()
+
+      return api.ping()
+      .thenThrow()
+      .catch(() => {
+        expect(agent.addRequest).to.be.calledOnce
+
+        expect(agent.addRequest).to.be.calledWithMatch(sinon.match.any, {
+          rejectUnauthorized: true,
+        })
+      })
+    })
+
     context('with a proxy defined', () => {
       beforeEach(function () {
         nock.cleanAll()

tooling/v8-snapshot/cache/dev-darwin/snapshot-meta.cache.json

@@ -1101,7 +1101,6 @@
     "./node_modules/ansi_up/ansi_up.js",
     "./node_modules/any-base/index.js",
     "./node_modules/any-base/src/converter.js",
-    "./node_modules/anymatch/index.js",
     "./node_modules/archiver-utils/file.js",
     "./node_modules/archiver-utils/index.js",
     "./node_modules/archiver-utils/node_modules/glob/common.js",
@@ -3285,6 +3284,7 @@
     "./node_modules/yn/lenient.js",
     "./packages/data-context/node_modules/@babel/code-frame/lib/index.js",
     "./packages/data-context/node_modules/@babel/parser/lib/index.js",
+    "./packages/data-context/node_modules/anymatch/index.js",
     "./packages/data-context/node_modules/cross-spawn/index.js",
     "./packages/data-context/node_modules/cross-spawn/lib/enoent.js",
     "./packages/data-context/node_modules/cross-spawn/lib/parse.js",
@@ -3528,5 +3528,5 @@
     "./tooling/v8-snapshot/cache/dev-darwin/snapshot-entry.js"
   ],
   "deferredHashFile": "yarn.lock",
-  "deferredHash": "66156a5424fbd0d70c750ef2b16c22b0084a30220c67b632cff9212a7de7a226"
+  "deferredHash": "9fe6e763921f20d9fa851fee03e5b5e7c334c2bd85a88bcdf0ed49cde252e095"
 }