1267012 - Timebomb failure in test_signed_dir.js

Status: RESOLVED FIXED

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

Description

[Phil Ringnalda (:philor)]

Apparently despite bug 842064 making us think that the -v 480 was giving us signing certs that would expire in 40 years, something used by test_signed_dir.js expired after one year, so I'm disabling it on every branch.

Comment 1

[Pulsebot]

https://hg.mozilla.org/mozilla-central/rev/ab8a76ac7b34

Comment 2

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/2b7291cc565421eebd5328f09dede6ff40ce0b42
https://hg.mozilla.org/releases/mozilla-release/rev/0f1d5ce151c41b82219404dbb9f3bc9c46e0c0d0
https://hg.mozilla.org/releases/mozilla-esr45/rev/6a695027eb1c8c9aa2a39321273932d04348cf6c

Comment 3

[:Cykesiopka]

Attachment: Bug 1267012 - Update and re-enable test_signed_dir.js.

test_signed_dir.js was previously disabled for perma-failing after a year or so.
The reason was that the signature for sslcontrol.xpi (the test extension used in
the test) expired. Bug 1267318 fixed this expiry problem in general by simply
ignoring expired signatures.

This patch does the following:
1. Replaces sslcontrol.xpi with a newer extension.
Lightbeam is now used for this purpose, since it is still hosted on AMO, is
a Mozilla extension, and is MPL2 code.
2. Moves the test extension outside of test_signed_apps/.
The extension and the test itself have nothing to do with the rest of the files
in that directory.
3. Makes misc improvements to the test file.

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

Comment 4

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

Comment on attachment 8801761 [details]
Bug 1267012 - Update and re-enable test_signed_dir.js.

https://reviewboard.mozilla.org/r/86418/#review85172

Great - r=me. Just some comments.

::: security/manager/ssl/tests/unit/test_signed_dir.js:1
(Diff revision 1)
>  "use strict";

Should we add a license block?

::: security/manager/ssl/tests/unit/test_signed_dir.js:15
(Diff revision 1)
> -const certdb = Cc["@mozilla.org/security/x509certdb;1"].getService(Ci.nsIX509CertDB);
> +const certdb = Cc["@mozilla.org/security/x509certdb;1"]
> +                 .getService(Ci.nsIX509CertDB);
>  
> -var gSignedXPI = do_get_file("test_signed_apps/sslcontrol.xpi", false);
> +/**
> + * Signed test extension. This is any arbitrary Mozilla signed XPI that
> + * preferably has recently been signed.

Might add a comment that it actually doesn't matter, since we ignore expired certificates when checking signing.

::: security/manager/ssl/tests/unit/test_signed_dir.js:47
(Diff revision 1)
> +/**
> + * Extracts the signed XPI into a directory, and tampers the files in that
> + * directory if instructed.
> + *
> + * @param {TamperInstructions} tamper
> + *        Instructions on whether to tamper any files, and is so, how.

typo: "if so"

Comment 5

[:Cykesiopka]

Comment on attachment 8801761 [details]
Bug 1267012 - Update and re-enable test_signed_dir.js.

https://reviewboard.mozilla.org/r/86418/#review85172

Thanks!

> Should we add a license block?

Sure. PD seems the most appropriate for this file.

> Might add a comment that it actually doesn't matter, since we ignore expired certificates when checking signing.

Done.

> typo: "if so"

Fixed.

Comment 6

[:Cykesiopka]

Comment on attachment 8801761 [details]
Bug 1267012 - Update and re-enable test_signed_dir.js.

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

Comment 7

[:Cykesiopka]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bac1754d8d226130072087c9581b61755acb896a
The SM(arm) and SM(p) failures look unrelated - probably already present in the base commit I used.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/debc3dfbc36a
Update and re-enable test_signed_dir.js. r=keeler

Comment 9

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/debc3dfbc36a