Stop storing certs used for MAR verification in EXE resource files

RESOLVED FIXED in mozilla36

Status

()

defect
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: bbondy, Assigned: bbondy)

Tracking

Trunk
mozilla36
x86_64
Windows 8
Points:
---
Dependency tree / graph
Bug Flags:
sec-review +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 9 obsolete attachments)

5.24 KB, patch
spohl
: review+
Details | Diff | Splinter Review
3.19 KB, patch
bbondy
: review+
glandium
: review-
Details | Diff | Splinter Review
(Assignee)

Description

6 years ago
In preparation for verifying MARs on all platforms, we should have a platform independent way to store the certificates used in MAR verification.
On Windows we currently stash them in the exe's resource section.

The idea is to instead create a python scrip that will run when building to generate a .h file.  The .h file will contain an array of hex char numbers. The array will hold the byte of the .der file.  It will simply be included into updater.cpp/archivereader.cpp and used directly.

This bug is only for making the cert loading platform independent.
Other bugs will be posted later for doing the rest of the work to get OSX and Linux verifying mar files.
(Assignee)

Updated

6 years ago
Flags: sec-review?
(Assignee)

Updated

6 years ago
See Also: → 903126
(Assignee)

Comment 2

6 years ago
Will be pushing to oak for testing, didn't test yet.
Attachment #787296 - Attachment is obsolete: true
Attachment #787759 - Flags: review?(robert.bugzilla)
(Assignee)

Updated

6 years ago
Attachment #787759 - Attachment description: bug902761_buildconfig.diff → Build config for turning DER files into .h files - rev1
(Assignee)

Comment 3

6 years ago
Attachment #787762 - Flags: review?(robert.bugzilla)
(Assignee)

Updated

6 years ago
See Also: → 903135
Flags: sec-review? → sec-review+
(Assignee)

Comment 4

6 years ago
Fix cert verification error
Attachment #787762 - Attachment is obsolete: true
Attachment #787762 - Flags: review?(robert.bugzilla)
Attachment #789339 - Flags: review?(robert.bugzilla)
(Assignee)

Comment 5

6 years ago
rstrong this is all green and tested now, and should be ready to go:
https://tbpl.mozilla.org/?tree=Try&rev=ae3c132f37dd
Comment on attachment 787759 [details] [diff] [review]
Build config for turning DER files into .h files - rev1

For the ifneq you can use the following format
http://mxr.mozilla.org/mozilla-central/source/layout/build/Makefile.in#78

Please convert to unix newlines
Attachment #787759 - Flags: review?(robert.bugzilla) → review+
(Assignee)

Comment 7

6 years ago
(In reply to Robert Strong [:rstrong] (do not email) from comment #6)
> Comment on attachment 787759 [details] [diff] [review]
> Build config for turning DER files into .h files - rev1
> 
> For the ifneq you can use the following format
> http://mxr.mozilla.org/mozilla-central/source/layout/build/Makefile.in#78
> 
> Please convert to unix newlines

It is already unix newlines
(Assignee)

Comment 8

6 years ago
(In reply to Brian R. Bondy [:bbondy] from comment #7)
> (In reply to Robert Strong [:rstrong] (do not email) from comment #6)
> > Comment on attachment 787759 [details] [diff] [review]
> > Build config for turning DER files into .h files - rev1
> > 
> > For the ifneq you can use the following format
> > http://mxr.mozilla.org/mozilla-central/source/layout/build/Makefile.in#78

The only difference I could see here was the else and ifneq on different lines. So uploading that change in a new patch. Let me know if I missed anything else.
(Assignee)

Comment 9

6 years ago
Implemented nit. Carrying forward r+.
Attachment #787759 - Attachment is obsolete: true
Attachment #808654 - Flags: review+
(Assignee)

Comment 10

6 years ago
Didn't realize combining the else and ifneq on the same line would reduce the need for the exra endif. Fixed.
Attachment #808654 - Attachment is obsolete: true
Attachment #808662 - Flags: review+
(Assignee)

Comment 11

6 years ago
qref'ed :)
Attachment #808662 - Attachment is obsolete: true
Attachment #808665 - Flags: review+
(In reply to Brian R. Bondy [:bbondy] from comment #7)
> (In reply to Robert Strong [:rstrong] (do not email) from comment #6)
> > Comment on attachment 787759 [details] [diff] [review]
> > Build config for turning DER files into .h files - rev1
> > 
> > For the ifneq you can use the following format
> > http://mxr.mozilla.org/mozilla-central/source/layout/build/Makefile.in#78
> > 
> > Please convert to unix newlines
> 
> It is already unix newlines
When I imported the patch a second time gen_cert_header.py it was created again with CRLF newlines
(Assignee)

Comment 13

6 years ago
(In reply to Robert Strong [:rstrong] (do not email) from comment #12)
> (In reply to Brian R. Bondy [:bbondy] from comment #7)
> > (In reply to Robert Strong [:rstrong] (do not email) from comment #6)
> > > Comment on attachment 787759 [details] [diff] [review]
> > > Build config for turning DER files into .h files - rev1
> > > 
> > > For the ifneq you can use the following format
> > > http://mxr.mozilla.org/mozilla-central/source/layout/build/Makefile.in#78
> > > 
> > > Please convert to unix newlines
> > 
> > It is already unix newlines
> When I imported the patch a second time gen_cert_header.py it was created
> again with CRLF newlines


Ah sorry, the comment appeared just under Makefile.in so I thought you were talking about that file. Should have checked both.  New patch coming.
(Assignee)

Comment 14

6 years ago
Stripped \r from the python file. Carrying forward r+.
Attachment #808665 - Attachment is obsolete: true
Attachment #808922 - Flags: review+
(Assignee)

Updated

5 years ago
Blocks: 973933
Updated for current trunk. Carrying over r+.
Attachment #8416007 - Flags: review+
Updated for current trunk. Carrying over r+.
Attachment #789339 - Attachment is obsolete: true
Attachment #808922 - Attachment is obsolete: true
Attachment #8416008 - Flags: review+
(Assignee)

Comment 17

5 years ago
Rebase to m-c tip and add nightly-ux (it was missed before)
Attachment #8416007 - Attachment is obsolete: true
Attachment #8505915 - Flags: review+
(Assignee)

Updated

5 years ago
Target Milestone: --- → mozilla36
Depends on: 1094521
Comment on attachment 8505915 [details] [diff] [review]
Part 1: Build config for turning DER files into .h files - rev5

Review of attachment 8505915 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/mozapps/update/updater/Makefile.in
@@ +35,5 @@
> +
> +export::
> +	$(PYTHON) $(srcdir)/gen_cert_header.py primaryCertData $(srcdir)/$(PRIMARY_CERT) > primaryCert.h
> +	$(PYTHON) $(srcdir)/gen_cert_header.py secondaryCertData $(srcdir)/$(SECONDARY_CERT) > secondaryCert.h
> +	$(PYTHON) $(srcdir)/gen_cert_header.py xpcshellCertData $(srcdir)/xpcshellCertificate.der > xpcshellCert.h

This should have been reviewed by a build peer.
Attachment #8505915 - Flags: review-
You need to log in before you can comment on or make changes to this bug.