Closed Bug 1267012 Opened 8 years ago Closed 8 years ago

Timebomb failure in test_signed_dir.js

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox46 --- disabled
firefox47 --- disabled
firefox48 --- disabled
firefox-esr38 --- unaffected
firefox-esr45 --- disabled
firefox52 --- fixed

People

(Reporter: philor, Assigned: Cykesiopka)

References

Details

(Whiteboard: [psm-assigned])

Attachments

(1 file)

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.
Depends on: armagadd-on
Assignee: nobody → cykesiopka.bmo
Status: NEW → ASSIGNED
Priority: -- → P1
Whiteboard: [psm-assigned]
Component: Security → Security: PSM
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"
Attachment #8801761 - Flags: review?(dkeeler) → review+
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.
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.
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/debc3dfbc36a
Update and re-enable test_signed_dir.js. r=keeler
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/debc3dfbc36a
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.