1350868 - Make HSTS preload script preload test domains for use in tests

Status: RESOLVED FIXED

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

Description

[:Cykesiopka]

Several of our HSTS tests require preloaded domain with specific characteristics.

In some of these tests, our solution was to pick some real preloaded domains that we assumed would always have the same characteristics. Bug 1350599 shows that this was not the best idea.

Instead, a better solution is to modify the HSTS preload script so that it always inject test domains into the preload list. We can then use these domains in our tests and be 100% certain the domains will have the desired characteristics.

Comment 1

[:Cykesiopka]

Attachment: Bug 1350868 - Make HSTS preload script preload test domains for use in tests.

This lets us migrate off depending on real preloaded domains and onto
domains that are guaranteed to have the correct characteristics.

Review commit: https://reviewboard.mozilla.org/r/123878/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/123878/

Comment 2

[:Cykesiopka]

Attachment: Bug 1350868 - Semi-manually update nsSTSPreloadList.inc to include test domains.

Periodic updates on m-c are currently broken due to Bug 1350619, so this change
inserts the test domains into the preload list semi-manually.

Review commit: https://reviewboard.mozilla.org/r/123880/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/123880/

Comment 3

[:Cykesiopka]

Attachment: getHSTSPreloadList-modified.js

This was the modified HSTS preload script I used to generate attachment 8851612 [details] / the nsSTSPreloadList.inc update.

Comment 4

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Comment on attachment 8851611 [details]
Bug 1350868 - Make HSTS preload script preload test domains for use in tests.

https://reviewboard.mozilla.org/r/123878/#review126490

Sounds good. Thanks for jumping on this.

::: security/manager/tools/getHSTSPreloadList.js:454
(Diff revision 1)
> +  for (let testEntry of TEST_ENTRIES) {
> +    hstsStatuses.push({
> +      name: testEntry.name,
> +      maxAge: MINIMUM_REQUIRED_MAX_AGE,
> +      includeSubdomains: testEntry.includeSubdomains,
> +      error: ERROR_NONE,

Might add a comment that this deliberately doesn't have a value for `retries` (because we should never attempt to connect to this host)

Comment 5

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Comment on attachment 8851612 [details]
Bug 1350868 - Semi-manually update nsSTSPreloadList.inc to include test domains.

https://reviewboard.mozilla.org/r/123880/#review126492

Comment 6

[:Cykesiopka]

Comment on attachment 8851611 [details]
Bug 1350868 - Make HSTS preload script preload test domains for use in tests.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/123878/diff/1-2/

Comment 7

[:Cykesiopka]

Comment on attachment 8851612 [details]
Bug 1350868 - Semi-manually update nsSTSPreloadList.inc to include test domains.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/123880/diff/1-2/

Comment 8

[:Cykesiopka]

Thanks!

https://treeherder.mozilla.org/#/jobs?repo=try&revision=340c6e05b684cdbf967c3cf2110a931d7c1b9ea0

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/73d180896413
Make HSTS preload script preload test domains for use in tests. r=keeler
https://hg.mozilla.org/integration/autoland/rev/f2fc9e7b66db
Semi-manually update nsSTSPreloadList.inc to include test domains. r=keeler

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/73d180896413
https://hg.mozilla.org/mozilla-central/rev/f2fc9e7b66db

Comment 11

[:Cykesiopka]

Comment on attachment 8851611 [details]
Bug 1350868 - Make HSTS preload script preload test domains for use in tests.

If necessary, see https://wiki.mozilla.org/SecurityEngineering/HTTP_Strict_Transport_Security_(HSTS)_Preload_List for background on the preload script.

Approval Request Comment
[Feature/Bug causing the regression]: None. Long standing issue.
[User impact if declined]: Some tests for the HSTS feature will stay disabled, which means we have less confidence in fixes if we ever have to uplift.
[Is this code covered by automated tests?]: Not directly, but we have tests in m-c that depend on the functionality provided here, and those tests still pass.
[Has the fix been verified in Nightly?]: Yes. At least two periodic updates have happened on m-c with the changes here included.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: Change itself is NPOTB. The code itself has already been verified to work for the m-c periodic update cycle.
[String changes made/needed]: None.

Comment 12

[Julien Cristau [:jcristau]]

Comment on attachment 8851611 [details]
Bug 1350868 - Make HSTS preload script preload test domains for use in tests.

add test domains to hsts preload script, esr52+, aurora54+

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/ddd6c44790c0

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/18015fe9a21f

Comment 15

[Andrei Vaida [:avaida]]

Setting qe-verify- based on Cykesiopka's assessment on manual testing needs (see Comment 11).

David, Cykesiopka, if you think manual testing would in fact be of value here, feel free to flip the flag or ni? me directly.