Skip to content

Commit

Permalink
core: bail on run if insecure ssl certification (#6300)
Browse files Browse the repository at this point in the history
  • Loading branch information
connorjclark authored and brendankenny committed Oct 19, 2018
1 parent ecedf5a commit 42091b2
Show file tree
Hide file tree
Showing 6 changed files with 107 additions and 2 deletions.
27 changes: 27 additions & 0 deletions lighthouse-core/gather/driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,13 @@ class Driver {
// properties. See https://github.com/Microsoft/TypeScript/pull/22348.
this._eventEmitter.emit(event.method, event.params);
});

/**
* Used for monitoring network status events during gotoURL.
* @type {?LH.Crdp.Security.SecurityStateChangedEvent}
* @private
*/
this._lastSecurityState = null;
}

static get traceCategories() {
Expand Down Expand Up @@ -876,6 +883,26 @@ class Driver {
});
}

async listenForSecurityStateChanges() {
this.on('Security.securityStateChanged', state => {
this._lastSecurityState = state;
});
await this.sendCommand('Security.enable');
}

/**
* @return {LH.Crdp.Security.SecurityStateChangedEvent}
*/
getSecurityState() {
if (!this._lastSecurityState) {
// happens if 'listenForSecurityStateChanges' is not called,
// or if some assumptions about the Security domain are wrong
throw new Error('Expected a security state.');
}

return this._lastSecurityState;
}

/**
* @param {string} name The name of API whose permission you wish to query
* @return {Promise<string>} The state of permissions, resolved in a promise.
Expand Down
19 changes: 19 additions & 0 deletions lighthouse-core/gather/gather-runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ class GatherRunner {
await driver.cacheNatives();
await driver.registerPerformanceObserver();
await driver.dismissJavaScriptDialogs();
await driver.listenForSecurityStateChanges();
if (resetStorage) await driver.clearDataForOrigin(options.requestedUrl);
}

Expand Down Expand Up @@ -172,6 +173,22 @@ class GatherRunner {
}
}

/**
* Throws an error if the security state is insecure.
* @param {LH.Crdp.Security.SecurityStateChangedEvent} securityState
* @throws {LHError}
*/
static assertNoSecurityIssues({securityState, explanations}) {
if (securityState === 'insecure') {
const errorDef = {...LHError.errors.INSECURE_DOCUMENT_REQUEST};
const insecureDescriptions = explanations
.filter(exp => exp.securityState === 'insecure')
.map(exp => exp.description);
errorDef.message += ` ${insecureDescriptions.join(' ')}`;
throw new LHError(errorDef);
}
}

/**
* Navigates to about:blank and calls beforePass() on gatherers before tracing
* has started and before navigation to the target page.
Expand Down Expand Up @@ -280,6 +297,8 @@ class GatherRunner {
passContext.LighthouseRunWarnings.push(pageLoadError.friendlyMessage);
}

this.assertNoSecurityIssues(driver.getSecurityState());

// Expose devtoolsLog, networkRecords, and trace (if present) to gatherers
/** @type {LH.Gatherer.LoadData} */
const passData = {
Expand Down
6 changes: 6 additions & 0 deletions lighthouse-core/lib/lh-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,12 @@ const ERRORS = {
message: strings.pageLoadFailed,
lhrRuntimeError: true,
},
/* Used when security error prevents page load. */
INSECURE_DOCUMENT_REQUEST: {
code: 'INSECURE_DOCUMENT_REQUEST',
message: strings.pageLoadFailedInsecure,
lhrRuntimeError: true,
},

// Protocol internal failures
TRACING_ALREADY_STARTED: {
Expand Down
1 change: 1 addition & 0 deletions lighthouse-core/lib/strings.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ module.exports = {
badTraceRecording: `Something went wrong with recording the trace over your page load. Please run Lighthouse again.`,
pageLoadTookTooLong: `Your page took too long to load. Please follow the opportunities in the report to reduce your page load time, and then try re-running Lighthouse.`,
pageLoadFailed: `Lighthouse was unable to reliably load the page you requested. Make sure you are testing the correct URL and that the server is properly responding to all requests.`,
pageLoadFailedInsecure: `The URL you have provided does not have valid security credentials.`,
internalChromeError: `An internal Chrome error occurred. Please restart Chrome and try re-running Lighthouse.`,
requestContentTimeout: 'Fetching resource content has exceeded the allotted time',
urlInvalid: `The URL you have provided appears to be invalid.`,
Expand Down
8 changes: 8 additions & 0 deletions lighthouse-core/test/gather/fake-driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,14 @@ const fakeDriver = {
setExtraHTTPHeaders() {
return Promise.resolve();
},
listenForSecurityStateChanges() {
return Promise.resolve();
},
getSecurityState() {
return Promise.resolve({
securityState: 'secure',
});
},
};

module.exports = fakeDriver;
Expand Down
48 changes: 46 additions & 2 deletions lighthouse-core/test/gather/gather-runner-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,7 @@ describe('GatherRunner', function() {
clearDataForOrigin: createCheck('calledClearStorage'),
blockUrlPatterns: asyncFunc,
setExtraHTTPHeaders: asyncFunc,
listenForSecurityStateChanges: asyncFunc,
};

return GatherRunner.setupDriver(driver, {settings: {}}).then(_ => {
Expand Down Expand Up @@ -379,6 +380,7 @@ describe('GatherRunner', function() {
clearDataForOrigin: createCheck('calledClearStorage'),
blockUrlPatterns: asyncFunc,
setExtraHTTPHeaders: asyncFunc,
listenForSecurityStateChanges: asyncFunc,
};

return GatherRunner.setupDriver(driver, {
Expand Down Expand Up @@ -543,9 +545,9 @@ describe('GatherRunner', function() {
],
};

return GatherRunner.afterPass({url, driver, passConfig}, {TestGatherer: []}).then(vals => {
return GatherRunner.afterPass({url, driver, passConfig}, {TestGatherer: []}).then(passData => {
assert.equal(calledDevtoolsLogCollect, true);
assert.strictEqual(vals.devtoolsLog[0], fakeDevtoolsMessage);
assert.strictEqual(passData.devtoolsLog[0], fakeDevtoolsMessage);
});
});

Expand Down Expand Up @@ -647,6 +649,7 @@ describe('GatherRunner', function() {
mainRecord.localizedFailDescription = 'foobar';
const error = GatherRunner.getPageLoadError(url, [mainRecord]);
assert.equal(error.message, 'FAILED_DOCUMENT_REQUEST');
assert.equal(error.code, 'FAILED_DOCUMENT_REQUEST');
assert.ok(/^Lighthouse was unable to reliably load/.test(error.friendlyMessage));
});

Expand All @@ -655,6 +658,7 @@ describe('GatherRunner', function() {
const records = [];
const error = GatherRunner.getPageLoadError(url, records);
assert.equal(error.message, 'NO_DOCUMENT_REQUEST');
assert.equal(error.code, 'NO_DOCUMENT_REQUEST');
assert.ok(/^Lighthouse was unable to reliably load/.test(error.friendlyMessage));
});

Expand All @@ -665,6 +669,7 @@ describe('GatherRunner', function() {
mainRecord.statusCode = 404;
const error = GatherRunner.getPageLoadError(url, [mainRecord]);
assert.equal(error.message, 'ERRORED_DOCUMENT_REQUEST');
assert.equal(error.code, 'ERRORED_DOCUMENT_REQUEST');
assert.ok(/^Lighthouse was unable to reliably load/.test(error.friendlyMessage));
});

Expand All @@ -675,10 +680,49 @@ describe('GatherRunner', function() {
mainRecord.statusCode = 500;
const error = GatherRunner.getPageLoadError(url, [mainRecord]);
assert.equal(error.message, 'ERRORED_DOCUMENT_REQUEST');
assert.equal(error.code, 'ERRORED_DOCUMENT_REQUEST');
assert.ok(/^Lighthouse was unable to reliably load/.test(error.friendlyMessage));
});
});

describe('#assertNoSecurityIssues', () => {
it('succeeds when page is secure', () => {
const secureSecurityState = {
securityState: 'secure',
};
GatherRunner.assertNoSecurityIssues(secureSecurityState);
});

it('fails when page is insecure', () => {
const insecureSecurityState = {
explanations: [
{
description: 'reason 1.',
securityState: 'insecure',
},
{
description: 'blah.',
securityState: 'info',
},
{
description: 'reason 2.',
securityState: 'insecure',
},
],
securityState: 'insecure',
};
try {
GatherRunner.assertNoSecurityIssues(insecureSecurityState);
assert.fail('expected INSECURE_DOCUMENT_REQUEST LHError');
} catch (err) {
assert.equal(err.message, 'INSECURE_DOCUMENT_REQUEST');
assert.equal(err.code, 'INSECURE_DOCUMENT_REQUEST');
/* eslint-disable-next-line max-len */
assert.equal(err.friendlyMessage, 'The URL you have provided does not have valid security credentials. reason 1. reason 2.');
}
});
});

describe('artifact collection', () => {
// Make sure our gatherers never execute in parallel
it('runs gatherer lifecycle methods strictly in sequence', async () => {
Expand Down

0 comments on commit 42091b2

Please sign in to comment.