Closed Bug 1564221 Opened 6 years ago Closed 6 years ago

Make nsITransportSecurityInfo builtinclass

Categories

(Core :: Security: PSM, task, P1)

Product:
Core
Component:
Security: PSM
Type:
task

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla70
Tracking Flags:
Tracking Status
firefox70 --- fixed

People

(Reporter: beth, Assigned: beth)

References

Details

(Whiteboard: [psm-assigned])

Attachments

(9 files)

Bug 1564221 - Add a contract ID for nsITransportSecurityInfo r?keeler,jcj
47 bytes, text/x-phabricator-request
Details | Review
Bug 1564221 - Do not use FakeTransportSecurityInfo in test_forget_about_site_security_headers.js r?keeler,jcj
47 bytes, text/x-phabricator-request
Details | Review
Bug 1564221 - Do not use FakeTransportSecurityInfo in test_ocsp_must_staple.js r?keeler,jcj
47 bytes, text/x-phabricator-request
Details | Review
Bug 1564221 - Do not use FakeTransportSecurityInfo in test_pinning_header_parsing.js r?keeler,jcj
47 bytes, text/x-phabricator-request
Details | Review
Bug 1564221 - Do not use FakeTransportSecurityInfo in test_sss_enumerate.js r?keeler,jcj
47 bytes, text/x-phabricator-request
Details | Review
Bug 1564221 - Do not use FakeTransportSecurityInfo in test_sss_originAttributes.js r?keeler,jcj
47 bytes, text/x-phabricator-request
Details | Review
Bug 1564221 - Do not use FakeTransportSecurityInfo in test_sss_resetState.js r?keeler,jcj
47 bytes, text/x-phabricator-request
Details | Review
Bug 1564221 - Make nsITransportSecurityInfo builtinclass r?keeler,jcj
47 bytes, text/x-phabricator-request
Details | Review
Bug 1564221 - Remove QueryInterface parameter from MockSecurityInfo in devtools tests r?past
47 bytes, text/x-phabricator-request
Details | Review

nsITransportSecurityInfo is presently not a builtinclass interface since several tests create a JS implemented nsITransportSecurityInfo in order to manually modify the cert. This not being builtinclass blocks

After discussing this with :keeler, the way forward is to

  1. Add a contract ID for nsITransportSecurityInfo so that tests that only need the existence of this object can create one.
  2. Add support to the test TLS server for setting the headers that these tests are testing.
  3. Translate all the tests manually testing headers to use the machinery provided in (2).
Priority: -- → P1
Whiteboard: [psm-assigned]

After further discussion, we've decided to not do (2) since neither side of the connection actually speaks HTTP. Instead, the tests will be updated to use the callback provided by add_connection_test.

There is now a contract ID for nsITransportSecurityInfo, allowing
mozilla::psm::TransportSecurityInfo instances to be created from JS. Tests
using a JS-implemented nsITransportSecurityInfo that were not modifying,
e.g., the serverCert attribute have been updated to create a
mozilla::psm::TransportSecurityInfo via the contract.

As part of making nsITranportSecurityInfo builtinclass, we can no longer use
JS-implemented nsITransportSecurityInfo instances in test cases. This patch
migrates test_forget_about_site_security_headers.js to use add_connection_test()to get a validnsITransportSecurityInfo` instance for
the unit tests.

To make this work, we also need default-ee cert and keys, as well as an
alternate.key (required as the subject key for
a.pinning2.example.com-pinningroot.pem) in test_pinning_dynamic, or the
tests will fail due to certificate errors.

Depends on D40346

As part of making nsITranportSecurityInfo builtinclass, we can no longer use
JS-implemented nsITransportSecurityInfo instances in test cases. This patch
migrates test_ocsp_must_staple.js to use add_connection_test() to get a
valid nsITransportSecurityInfo instance for the unit tests.

Depends on D40347

As part of making nsITranportSecurityInfo builtinclass, we can no longer use
JS-implemented nsITransportSecurityInfo instances in test cases. This patch
migrates test_pinning_header_parsing.js to use add_connection_test() to get
a valid nsITransportSecurityInfo instance for the unit tests.

Depends on D40348

As part of making nsITranportSecurityInfo builtinclass, we can no longer
use JS-implemented nsITransportSecurityInfo instances in test cases.
This patch migrates test_sss_enumerate.js to use add_connection_test() to
get a valid nsITransportSecurityInfo instance for the unit tests.

Depends on D40349

As part of making nsITranportSecurityInfo builtinclass, we can no longer use
JS-implemented nsITransportSecurityInfo instances in test cases. This patch
migrates test_sss_originAttributes.js to use add_connection_test() to get a
valid nsITransportSecurityInfo instance for the unit tests.

Depends on D40350

As part of making nsITranportSecurityInfo builtinclass, we can no longer
use JS-implemented nsITransportSecurityInfo instances in test cases.
This patch migrates test_sss_resetState.js to use add_connection_test() to
get a valid nsITransportSecurityInfo instance for the unit tests.

Depends on D40351

There are no longer any consumers of the JS-implemented
FakeTransportSecurityInfo class, so it can be removed. That removes the last
JS-implemented nsITransportSecurityInfo instance and it therefore can be
marked builtinclass.

Depends on D40352

The MockSecurityInfo instances in the patched devtools tests are not actually
being used as nsITransportSecurityInfo instances; while QueryInterface
methods were generated for the them, these were never called. Additionally, the
methods they are being passed to are not XPCOM-defined and therefore do not
strictly require nsITransportSecurityInfo.

Pushed by brennie@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/bc933f236434 Add a contract ID for nsITransportSecurityInfo r=keeler https://hg.mozilla.org/integration/autoland/rev/dc76eeb3a74a Do not use FakeTransportSecurityInfo in test_forget_about_site_security_headers.js r=keeler https://hg.mozilla.org/integration/autoland/rev/02a324e713d6 Do not use FakeTransportSecurityInfo in test_ocsp_must_staple.js r=keeler https://hg.mozilla.org/integration/autoland/rev/a140da3467e0 Do not use FakeTransportSecurityInfo in test_pinning_header_parsing.js r=keeler https://hg.mozilla.org/integration/autoland/rev/31f730109760 Do not use FakeTransportSecurityInfo in test_sss_enumerate.js r=keeler https://hg.mozilla.org/integration/autoland/rev/ad7a644c5a8d Do not use FakeTransportSecurityInfo in test_sss_originAttributes.js r=keeler https://hg.mozilla.org/integration/autoland/rev/8c3157ad3ac9 Do not use FakeTransportSecurityInfo in test_sss_resetState.js r=keeler https://hg.mozilla.org/integration/autoland/rev/12d1607c1415 Make nsITransportSecurityInfo builtinclass r=keeler https://hg.mozilla.org/integration/autoland/rev/36e33a3b59f0 Remove QueryInterface parameter from MockSecurityInfo in devtools tests r=ochameau

Latest patch should address these failures.

Flags: needinfo?(brennie)
Pushed by brennie@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/4cd81a6d3b25 Add a contract ID for nsITransportSecurityInfo r=keeler https://hg.mozilla.org/integration/autoland/rev/e804c46a9541 Do not use FakeTransportSecurityInfo in test_forget_about_site_security_headers.js r=keeler https://hg.mozilla.org/integration/autoland/rev/63d578684d29 Do not use FakeTransportSecurityInfo in test_ocsp_must_staple.js r=keeler https://hg.mozilla.org/integration/autoland/rev/ede7219b9a9e Do not use FakeTransportSecurityInfo in test_pinning_header_parsing.js r=keeler https://hg.mozilla.org/integration/autoland/rev/6cd17e69d1c7 Do not use FakeTransportSecurityInfo in test_sss_enumerate.js r=keeler https://hg.mozilla.org/integration/autoland/rev/a1947eef7d86 Do not use FakeTransportSecurityInfo in test_sss_originAttributes.js r=keeler https://hg.mozilla.org/integration/autoland/rev/aaa8ffb687f2 Do not use FakeTransportSecurityInfo in test_sss_resetState.js r=keeler https://hg.mozilla.org/integration/autoland/rev/0efeb9b1f5fa Make nsITransportSecurityInfo builtinclass r=keeler https://hg.mozilla.org/integration/autoland/rev/bcae1e55fc27 Remove QueryInterface parameter from MockSecurityInfo in devtools tests r=ochameau,nchevobbe

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception&fromchange=bcae1e55fc27a2df59f94f233cf661d1c62fb87e&tochange=eb80ebd4ac1d91d04e842658f75860d859f4dd11&searchStr=devtools&selectedJob=262340222

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=262340222&repo=autoland&lineNumber=13477

Backout link: https://hg.mozilla.org/integration/autoland/rev/4270a51c13610f43de1bec53fa717a1620524cd5

task 2019-08-19T21:01:02.491Z] 21:01:02 INFO - TEST-PASS | devtools/client/netmonitor/test/browser_net_security-redirect.js | There were two requests due to redirect. -
[task 2019-08-19T21:01:02.491Z] 21:01:02 INFO - Buffered messages finished
[task 2019-08-19T21:01:02.493Z] 21:01:02 INFO - TEST-UNEXPECTED-FAIL | devtools/client/netmonitor/test/browser_net_security-redirect.js | Initial request was marked insecure for domain column. -
[task 2019-08-19T21:01:02.494Z] 21:01:02 INFO - Stack trace:
[task 2019-08-19T21:01:02.494Z] 21:01:02 INFO - chrome://mochikit/content/browser-test.js:test_ok:1576
[task 2019-08-19T21:01:02.494Z] 21:01:02 INFO - chrome://mochitests/content/browser/devtools/client/netmonitor/test/browser_net_security-redirect.js:null:39
[task 2019-08-19T21:01:02.495Z] 21:01:02 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1346
[task 2019-08-19T21:01:02.495Z] 21:01:02 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1381
[task 2019-08-19T21:01:02.496Z] 21:01:02 INFO - chrome://mochikit/content/browser-test.js:nextTest/<:1209
[task 2019-08-19T21:01:02.496Z] 21:01:02 INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:803
[task 2019-08-19T21:01:02.496Z] 21:01:02 INFO - TEST-PASS | devtools/client/netmonitor/test/browser_net_security-redirect.js | Redirected request was marked secure for domain column. -
[task 2019-08-19T21:01:02.497Z] 21:01:02 INFO - Not taking screenshot here: see the one that was previously logged
[task 2019-08-19T21:01:02.497Z] 21:01:02 INFO - TEST-UNEXPECTED-FAIL | devtools/client/netmonitor/test/browser_net_security-redirect.js | Initial request was marked insecure for URL column. -
[task 2019-08-19T21:01:02.498Z] 21:01:02 INFO - Stack trace:
[task 2019-08-19T21:01:02.498Z] 21:01:02 INFO - chrome://mochikit/content/browser-test.js:test_ok:1576
[task 2019-08-19T21:01:02.498Z] 21:01:02 INFO - chrome://mochitests/content/browser/devtools/client/netmonitor/test/browser_net_security-redirect.js:null:49
[task 2019-08-19T21:01:02.499Z] 21:01:02 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1346
[task 2019-08-19T21:01:02.499Z] 21:01:02 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1381
[task 2019-08-19T21:01:02.499Z] 21:01:02 INFO - chrome://mochikit/content/browser-test.js:nextTest/<:1209
[task 2019-08-19T21:01:02.500Z] 21:01:02 INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:803

Flags: needinfo?(brennie)
Pushed by brennie@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/15994e94ce79 Add a contract ID for nsITransportSecurityInfo r=keeler https://hg.mozilla.org/integration/autoland/rev/1877f9c9aeeb Do not use FakeTransportSecurityInfo in test_forget_about_site_security_headers.js r=keeler https://hg.mozilla.org/integration/autoland/rev/d01f8050aa3b Do not use FakeTransportSecurityInfo in test_ocsp_must_staple.js r=keeler https://hg.mozilla.org/integration/autoland/rev/09b75b688829 Do not use FakeTransportSecurityInfo in test_pinning_header_parsing.js r=keeler https://hg.mozilla.org/integration/autoland/rev/72e97b86ce0b Do not use FakeTransportSecurityInfo in test_sss_enumerate.js r=keeler https://hg.mozilla.org/integration/autoland/rev/efd936e4cafd Do not use FakeTransportSecurityInfo in test_sss_originAttributes.js r=keeler https://hg.mozilla.org/integration/autoland/rev/1023f2ecd9b5 Do not use FakeTransportSecurityInfo in test_sss_resetState.js r=keeler https://hg.mozilla.org/integration/autoland/rev/6cfbe7c8ad5f Make nsITransportSecurityInfo builtinclass r=keeler https://hg.mozilla.org/integration/autoland/rev/acd7b8cc02ab Remove QueryInterface parameter from MockSecurityInfo in devtools tests r=ochameau,nchevobbe

Trivial fix for this, rolled up into last commit.

Flags: needinfo?(brennie)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: