Closed Bug 1256495 Opened 8 years ago Closed 8 years ago

temporarily check built-time-generated certificates into the tree

Categories

(Core :: Security: PSM, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: keeler, Assigned: keeler)

References

Details

Attachments

(1 file)

Last year, we unified the mechanism by which PSM xpcshell test certificates were generated. Optimistically, we figured the new system would be fast enough to generate the certificates at build time. This turned out to not really be the case - even after removing extraneous certificates accounting for about 60-70% of the total time, we're still getting complaints that it just isn't fast enough. In the future it would be nice to generate the certificates in a just-in-time fashion (that is, if the certificates aren't available, the test harness should be able to generate them). That way, if a developer doesn't run those tests, they won't be affected by the time it takes to create them. As a temporary solution in the meantime, however, we can generate the certificates once and check them in to the tree. Note that this won't work for more than a year, because the certificates will expire (obviously, we can regenerate them, but it would be best to move to the just-in-time solution before then).
Assignee: nobody → dkeeler
Comment on attachment 8730867 [details]
MozReview Request: bug 1256495 - temporarily check build-time-generated PSM xpcshell test certificates in to the tree r?Cykesiopka

https://reviewboard.mozilla.org/r/40203/#review36759

:( I wish we didn't have to do this, but oh well.
Looks good anyways.

::: security/manager/ssl/tests/unit/xpcshell.ini:6
(Diff revision 1)
> -  ocsp_common/**
> +  bad_certs/**
> +  ocsp_certs/**

It looks like we technically don't need these lines because we only ever read the certs in these folders via TLSServer.cpp and friends. On the other hand, there's no real harm in having these lines here either. Up to you.
Attachment #8730867 - Flags: review?(cykesiopka.bmo) → review+
https://reviewboard.mozilla.org/r/40203/#review36759

Yeah, it's unfortunate. In any case, the new framework is an improvement over what we had before, so overall this is still a net gain.
Thanks for the quick review.
Try looked good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=dea05f5a2630

> It looks like we technically don't need these lines because we only ever read the certs in these folders via TLSServer.cpp and friends. On the other hand, there's no real harm in having these lines here either. Up to you.

I think there's some build step where the files now need to get copied (or symlinked) to the test directory hierarchy, since they're not generated anymore (in any case, the tests failed without these lines and they pass with them ¯\_(ツ)_/¯ )
https://hg.mozilla.org/mozilla-central/rev/e21f11e5e598
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Depends on: 1435644
You need to log in before you can comment on or make changes to this bug.