Last Comment Bug 704285 - Include certificates inside updater.exe and use them to verify MARs
: Include certificates inside updater.exe and use them to verify MARs
Status: RESOLVED FIXED
[sg:want P1][qa-]
:
Product: Toolkit
Classification: Components
Component: Application Update (show other bugs)
: unspecified
: x86_64 Windows 7
: -- normal (vote)
: mozilla12
Assigned To: Brian R. Bondy [:bbondy]
:
Mentors:
Depends on: 699700 701087 728935
Blocks: 711054 711139
  Show dependency treegraph
 
Reported: 2011-11-21 13:52 PST by Brian R. Bondy [:bbondy]
Modified: 2012-04-19 11:47 PDT (History)
16 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected
unaffected
fixed
fixed
unaffected


Attachments
Intermediate patch not ready for review. v1. (2.67 KB, patch)
2011-11-21 18:16 PST, Brian R. Bondy [:bbondy]
no flags Details | Diff | Splinter Review
Intermediate patch, done but not yet testesd. v2. (8.85 KB, patch)
2011-11-23 14:03 PST, Brian R. Bondy [:bbondy]
no flags Details | Diff | Splinter Review
Verifying signed MARs from updater.exe (9.93 KB, patch)
2011-11-24 10:24 PST, Brian R. Bondy [:bbondy]
ian.melven: feedback+
Details | Diff | Splinter Review
Verifying signed MARs from updater.exe. v2 (9.77 KB, patch)
2011-11-30 09:08 PST, Brian R. Bondy [:bbondy]
robert.strong.bugs: review+
Details | Diff | Splinter Review
Verifying signed MARs from updater.exe. v3. (10.85 KB, patch)
2011-12-15 08:58 PST, Brian R. Bondy [:bbondy]
netzen: review+
Details | Diff | Splinter Review
Certificates currently being used by RelEng for signing MARs (819 bytes, application/zip)
2011-12-17 07:34 PST, Brian R. Bondy [:bbondy]
no flags Details
Verifying signed MARs from updater.exe. v4. (10.84 KB, patch)
2011-12-19 12:00 PST, Brian R. Bondy [:bbondy]
netzen: review+
Details | Diff | Splinter Review
Verifying signed MARs from updater.exe. v5. (10.89 KB, patch)
2012-01-05 19:25 PST, Brian R. Bondy [:bbondy]
no flags Details | Diff | Splinter Review
try/dep and nightly certs (2.47 KB, application/x-compressed-tar)
2012-01-10 11:22 PST, Chris AtLee [:catlee]
no flags Details
Verifying signed MARs from updater.exe. v6. (15.70 KB, patch)
2012-01-27 07:34 PST, Brian R. Bondy [:bbondy]
no flags Details | Diff | Splinter Review
Verifying signed MARs from updater.exe. v6'. (9.14 KB, patch)
2012-01-31 07:58 PST, Brian R. Bondy [:bbondy]
robert.strong.bugs: review+
Details | Diff | Splinter Review
DER files. Patch v1. (6.71 KB, patch)
2012-01-31 08:00 PST, Brian R. Bondy [:bbondy]
robert.strong.bugs: review+
Details | Diff | Splinter Review
Verifying signed MARs from updater.exe. v7. (9.04 KB, patch)
2012-02-09 07:03 PST, Brian R. Bondy [:bbondy]
netzen: review+
Details | Diff | Splinter Review
Verifying signed MARs from updater.exe. v8. (9.05 KB, patch)
2012-02-19 09:41 PST, Brian R. Bondy [:bbondy]
netzen: review+
Details | Diff | Splinter Review
Verifying signed MARs from updater.exe. v9. (9.08 KB, patch)
2012-02-21 14:14 PST, Brian R. Bondy [:bbondy]
netzen: review+
bhearsum: feedback+
Details | Diff | Splinter Review

Description Brian R. Bondy [:bbondy] 2011-11-21 13:52:22 PST
As of bug 699700 MAR files will be signed on Windows.  This bug is to include the certificates we get from RelEng for MAR checking on Windows as part of updates and installers.

The code for updater.exe using the new verify function will also be done in this task.

I don't think this bug needs to be marked as Security, but I am marking it that way just in case.
Comment 1 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2011-11-21 14:04:40 PST
Where can I find the certificates and the procedures for generating the certificates/private keys? I would like to review them too.
Comment 2 Brian R. Bondy [:bbondy] 2011-11-21 14:06:40 PST
I did my testing with the certutil commands you gave me. But RelEng will be the ones who will be providing these.  I'll add catlee to the CC list.
Comment 3 Brian R. Bondy [:bbondy] 2011-11-21 18:16:11 PST
Created attachment 576054 [details] [diff] [review]
Intermediate patch not ready for review. v1.

Done:
- verify call added into archivereader, and used form udpater.cpp
- assumes cert is in mar file directory (this will change, see below).

TODO:
- Cert should be loaded from a trusted location (e.g. program files)
- Need to check both main cert and backup cert
- Installer needs to install the main cert and backup cert into program files.

Parts of this task are waiting the main cert and backup cert from RelEng.
Comment 4 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2011-11-21 18:21:54 PST
(In reply to Brian R. Bondy [:bbondy] from comment #3)
> TODO:
> - Cert should be loaded from a trusted location (e.g. program files)
> - Installer needs to install the main cert and backup cert into program
> files.

If we are going to do this, then we should have the certs signed by a CA cert that is embedded in the updater, and verify those signatures. Is there any way we could avoid doing this (at least for right now), and just embed the certs in the updater?

Otherwise, what happens when another program overwrites our certs with its own?

Also, remember the recent bug where *.jar files didn't survive system restore? We must make sure that doesn't happen with these certs.
Comment 5 Brian R. Bondy [:bbondy] 2011-11-21 18:44:10 PST
> Otherwise, what happens when another program overwrites our certs with its own?

It's my understanding that if someone could overwrite Program files then they are already running as a high integrity process (elevated) and so the system is already compromised.

> Is there any way we could avoid doing this (at least for right now), and just embed the certs in the updater?

We could store the certs in the resources of updater.exe but this is a bit more work.  I'm fine with doing it though if you feel it's best.  This would be a Windows only solution and the code wouldn't be able to be used on other platforms.

By the way there are Win32 APIs for updating resources on files as well, so like I mentioned earlier, if someone had permission to write inside Program Files, they could make a simple program that replaced the embedded certs in the updater.exe program.
Comment 6 Brian R. Bondy [:bbondy] 2011-11-21 18:48:49 PST
I see, the embedded udpater.exe solution also gets a cert check, so it should fail the verify of the signature if someone called UpdateResource on it.
Comment 7 Chris AtLee [:catlee] 2011-11-21 18:51:24 PST
(In reply to Brian Smith (:bsmith) from comment #1)
> Where can I find the certificates and the procedures for generating the
> certificates/private keys? I would like to review them too.

What's the correct process for generating these certificates? Should we include valid issuer/common name/expiry dates? My understanding this far is that they're only used as a container for public/private rsa keys and so the certificate metadata isn't that important.
Comment 8 Brian R. Bondy [:bbondy] 2011-11-21 18:57:48 PST
I'd like to recommend that we don't have expiry dates.
I don't think the metadata is important either, but I'll let bsmith chime in.
Comment 9 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2011-11-21 23:38:13 PST
(In reply to Brian R. Bondy [:bbondy] from comment #6)
> I see, the embedded udpater.exe solution also gets a cert check, so it
> should fail the verify of the signature if someone called UpdateResource on
> it.

I am not sure if Windows actually checks the signature on each execution in the normal case. I believe you have to do something special like [1][2], which we probably should do for updater.exe and the Windows service.

[1] http://blog.didierstevens.com/2011/10/27/using-dllcharacteristics-force_integrity-flag/
[2] http://social.technet.microsoft.com/wiki/contents/articles/255.aspx
Comment 10 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2011-11-22 00:24:45 PST
Nelson, Bob, Kai: One thing I am not sure about, regarding certutil: Why does certutil require us to bash on the keyboard for key generation? Is this input used in addition to /dev/random? Assuming I don't have a HSM for whatever reason, what is the best way to generate a key using certutil?

(In reply to Chris AtLee [:catlee] from comment #7)
> What's the correct process for generating these certificates? Should we
> include valid issuer/common name/expiry dates? My understanding this far is
> that they're only used as a container for public/private rsa keys and so the
> certificate metadata isn't that important.

* Install NSS (the certutil utility and all of the NSS shared libraries) onto the system that will do the MAR signing.

* mkdir mar-signing-database-do-not-copy-from-this-machine
* cd    mar-signing-database-do-not-copy-from-this-machine

These steps assume that you don't have any hardware security module or smartcard or anything else like that available for you to use. The commands for using a hardware security module or smartcard are similar, but you need to configure NSS to use your HSM/smartcard. Let me know if/when you have it available.

* certutil -d . -N

This will prompt you for the password that will be used to protect the key database. I believe this password is not as useful as a security measure as (1) NSS does not do a sufficient number of PBKDF2 iterations, (2) you are going to write automated scripts that pass these passwords around anyway.

* certutil -S -d . -s "CN=mar-nightly-1" -n mar-nightly-1 -x -t ",," -g 2048
* certutil -S -d . -s "CN=mar-nightly-2" -n mar-nightly-2 -x -t ",," -g 2048

Parameters are explained at [1]:
-S           : create a new certificate
-d .         : in the current directory
-s "CN=...." : Subject DN (shown in any cert viewer)
-n ...       : name of the certificate (used for finding cert in the DB)
               this is what you pass as the name of the cert to the
               mar command.
-x           : self-signed certificate
-t ",,"      : do not trust this certificate for anything
-g           : (RSA) 2048-bit key

You can see the certificates in the database:

certutil -L -d .

You can export the certificates (NOT the private keys):

certutil -L -d . -n mar-nightly-1 -r > mar-nightly-1.der
certutil -L -d . -n mar-nightly-2 -r > mar-nightly-2.der

The file "key3.db" contains the private keys. This is the file you need to protect the most. When you have a HSM/smartcard, then the private keys will be stored in the HSM. FWIW, cert8.db+key3.db+softokn3.so+freebl.so is basically a software emulation of an HSM.

Of course, when you acquire the HSM/smartcard, you will not be able to use the old private keys and you will need to generate new certs for the keys stored in the HSM/smartcard. That's why it is really a good idea to have the HSM/smartcard to start with, if we're ever going to use one.

This advice should be verified by rrelyea, Nelson, and Kai. They have a lot more experience using certutil and other parts of NSS than I do.

Note that this is assuming that you will use different keys/certs to sign each channel. I think, at a minimum, you should use different keys/certs for the pre-release builds than you use for final release builds, and that you should use different (stronger) security measures to protect the keys for the production builds than what you are planning for the nightly builds. 

You then need to pass the path to mar-signing-database-do-not-copy-from-this-machine as the "-d" parameter to the mar signing tool, and the name of the cert as the -n parameter to the mar signing tool (e.g. "-n mar-nightly-1").

You also need to backup the mar-signing-database-do-not-copy-from-this-machine in a very secure way (secure meaning in a way that doesn't leak the contents of the files, and also in meaning that we will never lose the backups). I have no experience with doing this so I cannot offer useful advice here.

[1] http://www.mozilla.org/projects/security/pki/nss/tools/certutil.html
Comment 11 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2011-11-22 00:31:20 PST
(In reply to Brian R. Bondy [:bbondy] from comment #8)
> I'd like to recommend that we don't have expiry dates.
> I don't think the metadata is important either, but I'll let bsmith chime in.

There is going to be an expiry date regardless. But, since we don't check the expiry date in the current implementation, we should just make sure the expiry date is always in the past. That way, we aren't surprised by any difference in behavior between expired and not-yet-expired certificates. To do this, add "-v 3" to the command lines above:

   certutil -S -d . -s "CN=mar-nightly-2" -n mar-nightly-2 \
                    -x -t ",," -g 2048 -v -3

It seems that certutil has "right now" as the notBefore time and "right now + 3 months" as the notAfter time. "-v -3" means "add -3 to 3 months == 0 months" i.e. "make notBefore and notAfter equal."

After you have exported the certs, export them and open them in Firefox (I think you can just drag and drop the file onto Firefox) and in Windows (maybe you have to name the files with a "crt" or "cer" extension instead of "der" to get the double-click-to-view behavior on Windows). Then, verify the metadata is as you please.

BTW, I would avoid including "Mozilla" or "Firefox" or anything official-looking in the cert metadata. And, I would include as little metadata in the certs as possible, since we aren't validating any of it.
Comment 12 Brian R. Bondy [:bbondy] 2011-11-22 06:31:10 PST
> bbondy:
> I see, the embedded udpater.exe solution also gets a cert check, so it
> should fail the verify of the signature if someone called UpdateResource on
> it.
> bsmith:
> I am not sure if Windows actually checks the signature on each execution

We check it though in bug 481815.
Comment 13 Chris AtLee [:catlee] 2011-11-22 12:00:44 PST
(In reply to Brian Smith (:bsmith) from comment #11)
> (In reply to Brian R. Bondy [:bbondy] from comment #8)
> > I'd like to recommend that we don't have expiry dates.
> > I don't think the metadata is important either, but I'll let bsmith chime in.
> 
> There is going to be an expiry date regardless. But, since we don't check
> the expiry date in the current implementation, we should just make sure the
> expiry date is always in the past. That way, we aren't surprised by any
> difference in behavior between expired and not-yet-expired certificates. To
> do this, add "-v 3" to the command lines above:
> 
>    certutil -S -d . -s "CN=mar-nightly-2" -n mar-nightly-2 \
>                     -x -t ",," -g 2048 -v -3
> 
> It seems that certutil has "right now" as the notBefore time and "right now
> + 3 months" as the notAfter time. "-v -3" means "add -3 to 3 months == 0
> months" i.e. "make notBefore and notAfter equal."

What version of nss do I need to use this? I have 3.12 on this machine currently, and when I try "-v -3" it generates an error:
certutil -v: incorrect validity period: "-3"
Comment 14 Brian R. Bondy [:bbondy] 2011-11-23 14:03:28 PST
Created attachment 576615 [details] [diff] [review]
Intermediate patch, done but not yet testesd. v2.

- Implemented loading primary cert and backup cert from updater.exe on Windows.
- Implemented extracting those certs to disk, but having the file locked during their check from the time of writing.
- Backup cert handling is only done if primary cert handling fails.
- Implemented checking the certs via libmar.
Comment 15 Brian R. Bondy [:bbondy] 2011-11-24 10:24:38 PST
Created attachment 576800 [details] [diff] [review]
Verifying signed MARs from updater.exe

Ready for review.

I tested this bug, bug 699700, and bug 481815 and I have successfully done a full silent update from a signed mar that was first verified.
Comment 16 Ian Melven :imelven 2011-11-29 12:27:24 PST
Comment on attachment 576800 [details] [diff] [review]
Verifying signed MARs from updater.exe

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

looks good to me, a couple of very minor nits

::: toolkit/mozapps/update/updater/archivereader.cpp
@@ +86,5 @@
> +
> +BOOL PathGetSiblingFilePath(LPWSTR destinationBuffer, 
> +                            LPCWSTR siblingFilePath, 
> +                            LPCWSTR newFileName);
> +

don't see this used anywhere in this patch, maybe it is in another bit of the code though

::: toolkit/mozapps/update/updater/updater.cpp
@@ +1879,5 @@
>                                   NULL, OPEN_EXISTING, 0, NULL);
>        if (callbackFile != INVALID_HANDLE_VALUE)
>          break;
>  
> +      DWORD lastErorr = GetLastError();

nit : typo
Comment 17 Brian R. Bondy [:bbondy] 2011-11-30 09:08:11 PST
Created attachment 577992 [details] [diff] [review]
Verifying signed MARs from updater.exe. v2

Implemented review nits, thanks for the feedback Ian.
Comment 18 Brian R. Bondy [:bbondy] 2011-12-09 07:42:00 PST
I talked to catlee about which certs will be in which channels:

beta+release will share one set of authenticode/mar certs, nightly+aurora will share another set, everything else will have a different set.
Comment 19 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2011-12-09 09:27:28 PST
(In reply to Brian R. Bondy [:bbondy] from comment #18)
> I talked to catlee about which certs will be in which channels:
> 
> beta+release will share one set of authenticode/mar certs, nightly+aurora
> will share another set, everything else will have a different set.

And every cert will have its own distinct keypair, right?
Comment 20 Brian R. Bondy [:bbondy] 2011-12-09 09:44:21 PST
Correct.  Actually each of those will also accept a matching fallback key. If either the primary key or fallback key can verify the MAR then we allow the update.
Comment 21 Brian R. Bondy [:bbondy] 2011-12-11 10:18:49 PST
This is pushed to elm but the actual verify check is disabled until I get the real certs from catlee. I will update elm as I make changes to this.
Comment 22 Robert Strong [:rstrong] (use needinfo to contact me) 2011-12-15 02:36:40 PST
Comment on attachment 577992 [details] [diff] [review]
Verifying signed MARs from updater.exe. v2

>diff --git a/toolkit/mozapps/readstrings/errors.h b/toolkit/mozapps/readstrings/errors.h
>--- a/toolkit/mozapps/readstrings/errors.h
>+++ b/toolkit/mozapps/readstrings/errors.h
>@@ -44,10 +44,13 @@
> // #define IO_ERROR 2  // Use READ_ERROR or WRITE_ERROR instead
> #define USAGE_ERROR 3
> #define CRC_ERROR 4
> #define PARSE_ERROR 5
> #define READ_ERROR 6
> #define WRITE_ERROR 7
> #define UNEXPECTED_ERROR 8
> #define ELEVATION_CANCELED 9
>+#define CERT_LOAD_ERROR 10
>+#define CERT_HANDLING_ERROR 11
>+#define CERT_VERIFY_ERROR 12
Not for this bug but I really dislike the updater overloading readstrings errors. This is an artifact of the time we needed readstrings available for the WinCE / WinMo installer. Please file a bug to separate them.

>diff --git a/toolkit/mozapps/update/updater/archivereader.cpp b/toolkit/mozapps/update/updater/archivereader.cpp
>--- a/toolkit/mozapps/update/updater/archivereader.cpp
>+++ b/toolkit/mozapps/update/updater/archivereader.cpp
>@@ -49,16 +49,77 @@
> # include <io.h>
> #endif
> 
> static int inbuf_size  = 262144;
> static int outbuf_size = 262144;
> static char *inbuf  = NULL;
> static char *outbuf = NULL;
> 
>+#ifdef XP_WIN
>+#include "resource.h"
>+
>+bool LoadFileInResource(int name, int type, DWORD& size, 
>+                        const char *&data)
bool
LoadFileInResource(...

>+{
>+  HMODULE handle = ::GetModuleHandle(NULL);
>+  if (!handle) {
>+    return false;
>+  }
>+
>+  HRSRC resourceInfoBlockHandle = ::FindResource(handle, 
>+                                                 MAKEINTRESOURCE(name),
>+                                                 MAKEINTRESOURCE(type));
>+  if (!resourceInfoBlockHandle) {
>+    FreeLibrary(handle);
>+    return false;
>+  }
>+
>+  HGLOBAL resourceHandle = ::LoadResource(handle, resourceInfoBlockHandle);
>+  if (!resourceHandle) {
>+    FreeLibrary(handle);
>+    return false;
>+  }
>+
>+  size = ::SizeofResource(handle, resourceInfoBlockHandle);
>+  data = static_cast<const char*>(::LockResource(resourceHandle));
>+  FreeLibrary(handle);
No return value

>+}
>+
>+int VerifyLoadedCert(const NS_tchar *pathToMAR, int name, int type)
int
VerifyLoadedCert(...

>+{
>+  DWORD size = 0;
>+  const char *data = NULL;
>+  if (!LoadFileInResource(name, type, size, data) || !data || !size) {
>+    return CERT_LOAD_ERROR;
>+  }
>+
>+  if (mar_verify_signatureW(pathToMAR, data, size, NULL, NULL)) {
I'd just use mar_verify_signature since we don't bother doing this elsewhere and we only support wide chars on Windows anyways.

>+    return CERT_VERIFY_ERROR;
>+  }
>+
>+  return OK;
>+}
>+#endif
>+
>+
>+int
>+ArchiveReader::VerifySignature(const NS_tchar *pathToMAR)
>+{
>+#ifdef XP_WIN
>+  int rv = VerifyLoadedCert(pathToMAR, IDR_PRIMARY_CERT, TYPE_CERT);
>+  if (rv != OK) {
>+    rv = VerifyLoadedCert(pathToMAR, IDR_BACKUP_CERT, TYPE_CERT);
>+  }
>+  return rv;
>+#else
>+  return 0;
This should be return OK

>diff --git a/toolkit/mozapps/update/updater/updater.rc b/toolkit/mozapps/update/updater/updater.rc
>--- a/toolkit/mozapps/update/updater/updater.rc
>+++ b/toolkit/mozapps/update/updater/updater.rc
>@@ -32,16 +32,24 @@ 1                       RT_MANIFEST     
> //
> // Icon
> //
> 
> IDI_DIALOG ICON "updater.ico"
> 
> /////////////////////////////////////////////////////////////////////////////
> //
>+// Embedded certificates for allowed MARs
>+//
>+
>+IDR_PRIMARY_CERT TYPE_CERT "primaryCert.der"
>+IDR_BACKUP_CERT TYPE_CERT  "backupCert.der"
note: we are going to need a way for other apps to specify the certs to use

r=me with the above addressed
Comment 23 Brian R. Bondy [:bbondy] 2011-12-15 07:30:47 PST
Will update a patch will the above addressed bu 2 things that won't be addressed in the patch are:
- mar_verify_signatureW is that way because mar_verify_signature is already used in the libmar C code.  I think we can eventually change that to C++ code in another bug to clenaup various things?  In C code you can't overload functions.
- The certs having a better way to specify the certs will be worked out with another patch in this bug once I get the needed Certs from RelEng.
Comment 24 Brian R. Bondy [:bbondy] 2011-12-15 07:33:52 PST
I'm going to carry forward the r+ on the newly submitted patch since only nits were addressed in this patch.  I also added javadoc to the new functions in this patch.
Comment 25 Brian R. Bondy [:bbondy] 2011-12-15 08:58:38 PST
Created attachment 581994 [details] [diff] [review]
Verifying signed MARs from updater.exe. v3.

Implemented review nits + javadoc.
Comment 26 Brian R. Bondy [:bbondy] 2011-12-17 07:34:27 PST
Created attachment 582530 [details]
Certificates currently being used by RelEng for signing MARs

This cert was sent to me by catlee and is the one we should use to verify the current signing.  I think this is subject to change though and there may be different certs per channel.
Comment 27 Brian R. Bondy [:bbondy] 2011-12-19 12:00:34 PST
Created attachment 582910 [details] [diff] [review]
Verifying signed MARs from updater.exe. v4.

Rebased for changes to libmar patch. Carrying forward r+ since there are no changes otherwise.
Comment 28 Brian R. Bondy [:bbondy] 2012-01-05 19:25:03 PST
Created attachment 586325 [details] [diff] [review]
Verifying signed MARs from updater.exe. v5.

Will wait for another review from bsmith on the dependent libmar patch and the certs from catlee before requesting another review on this patch.  Just updating with latest changes in the meantime.
Comment 29 Chris AtLee [:catlee] 2012-01-10 11:22:27 PST
Created attachment 587404 [details]
try/dep and nightly certs

we'll use dep1 for try and depend builds on all branches. we'll use nightly1 for elm, mozilla-central and mozilla-aurora nightlies.
dep2 and nightly2 are our backup keys.
Comment 30 Kai Engert (:kaie) 2012-01-10 11:46:51 PST
(In reply to Brian Smith (:bsmith) from comment #10)
> Nelson, Bob, Kai: One thing I am not sure about, regarding certutil: Why
> does certutil require us to bash on the keyboard for key generation? Is this
> input used in addition to /dev/random?


Each platform has different sources of randomness, /dev/random doesn't exist on Windows.
I suspect the keyboard trick was used as an easy solution that works cross platform.


> Assuming I don't have a HSM for
> whatever reason, what is the best way to generate a key using certutil?


If you only create a certificate rarely, manually, then you should comply with certutil's request and just type these keys.


Only if you are looking for a solution for test environments, or if you have a cross platform solution that can save true random data into a file, you can use the following parameter:
   -z noisefile      Specify the noise file to be used
Comment 31 Brian R. Bondy [:bbondy] 2012-01-27 07:34:32 PST
Created attachment 592132 [details] [diff] [review]
Verifying signed MARs from updater.exe. v6.

Added in Makefile support for determining which cert should be used based on the channel.
Comment 32 Brian R. Bondy [:bbondy] 2012-01-31 07:58:55 PST
Created attachment 593100 [details] [diff] [review]
Verifying signed MARs from updater.exe. v6'.

Same patch as before but without the binary DER files.
Comment 33 Brian R. Bondy [:bbondy] 2012-01-31 08:00:19 PST
Created attachment 593101 [details] [diff] [review]
DER files. Patch v1.

Updated DER file with secondary Release MAR signing certificate from RelEng.
Comment 34 Robert Strong [:rstrong] (use needinfo to contact me) 2012-02-08 23:35:19 PST
Comment on attachment 593100 [details] [diff] [review]
Verifying signed MARs from updater.exe. v6'.

>diff --git a/toolkit/mozapps/update/updater/archivereader.cpp b/toolkit/mozapps/update/updater/archivereader.cpp
>--- a/toolkit/mozapps/update/updater/archivereader.cpp
>+++ b/toolkit/mozapps/update/updater/archivereader.cpp
>@@ -49,16 +49,107 @@
>...
>+/**
>+ * Obtains the data of the specified resource name and type.
>+ *
>+ * @param  name The name ID of the resource
>+ * @param  type The type ID of the resource
>+ * @param  size Out parameter which sets the size of the returned data buffer 
>+ * @param  data Out parameter which sets the pointer to a buffer containing
>+ *                 the needed data.
>+ * @return TRUE on success
>+*/
>+BOOL
>+LoadFileInResource(int name, int type, DWORD& size, 
>+                   const char *&data)
nit: I typically put data before the size of the data but I'm fine if you prefer this way.
Comment 35 Robert Strong [:rstrong] (use needinfo to contact me) 2012-02-08 23:35:42 PST
Comment on attachment 593101 [details] [diff] [review]
DER files. Patch v1.

rs=rs
Comment 36 Brian R. Bondy [:bbondy] 2012-02-09 07:03:32 PST
Created attachment 595721 [details] [diff] [review]
Verifying signed MARs from updater.exe. v7.

> nit: I typically put data before the size of the data but I'm fine 
> if you prefer this way.

Me too, not sure why I did it that way, fixed.
Carrying forward r+.
Comment 37 Brian R. Bondy [:bbondy] 2012-02-19 09:41:56 PST
Created attachment 598668 [details] [diff] [review]
Verifying signed MARs from updater.exe. v8.

Now use aurora-nightly cert for elm since elm Nightly builds use that MAR signing cert instead.  Carrying forward r+.
Comment 38 Justin Wood (:Callek) 2012-02-20 12:22:21 PST
Comment on attachment 598668 [details] [diff] [review]
Verifying signed MARs from updater.exe. v8.

># HG changeset patch
># Parent 6f7d7c4c2d57976d180bf04cda0446875418200e
># User Brian R. Bondy <netzen@gmail.com>
>Bug 704285 - Include certificates for MAR sign checks on Windows as part of updates and installers
>diff --git a/toolkit/mozapps/update/updater/updater.rc b/toolkit/mozapps/update/updater/updater.rc
>--- a/toolkit/mozapps/update/updater/updater.rc
>+++ b/toolkit/mozapps/update/updater/updater.rc
>@@ -33,16 +33,33 @@ 1                       RT_MANIFEST     
> // Icon
> //
> 
> IDI_DIALOG ICON "updater.ico"
> 
> 
> /////////////////////////////////////////////////////////////////////////////
> //
>+// Embedded certificates for allowed MARs
>+//
>+
>+#if defined(MAR_SIGNING_RELEASE_BETA)
>+IDR_PRIMARY_CERT TYPE_CERT "release_primary.der"
>+IDR_BACKUP_CERT TYPE_CERT  "release_secondary.der"
>+#elif defined(MAR_SIGNING_AURORA_NIGHTLY)
>+IDR_PRIMARY_CERT TYPE_CERT "nightly_aurora_level3_primary.der"
>+IDR_BACKUP_CERT TYPE_CERT  "nightly_aurora_level3_secondary.der"
>+#else
>+IDR_PRIMARY_CERT TYPE_CERT "dep1.der"
>+IDR_BACKUP_CERT TYPE_CERT  "dep2.der"
>+#endif
>+
>+
>+/////////////////////////////////////////////////////////////////////////////
>+//
> // Embedded an identifier to uniquely identiy this as a Mozilla updater.
> //
> 
> STRINGTABLE
> {
>   IDS_UPDATER_IDENTITY, "moz-updater.exe-4cdccec4-5ee0-4a06-9817-4cd899a9db49"
> } 
> 

Am I reading this part right, and that it would break any applications doing a windows update without actually signing mars yet? In that the else is stuffing dep1.der/etc. into the resource? And these would be better defined in confvars.sh or something like that (I admittedly only skimmed this code after seeing Bug 728935 being filed)
Comment 39 Brian R. Bondy [:bbondy] 2012-02-20 12:27:50 PST
> Am I reading this part right, and that it would break any applications doing a
>  windows update without actually signing mars yet? 

Updater code will only do the signature check conditionally.
You need to have MOZ_VERIFY_MAR_SIGNATURE=1 in your confvars.sh or --enable-verify-mar in your .mozconfig
Comment 40 Justin Wood (:Callek) 2012-02-20 12:33:25 PST
(In reply to Brian R. Bondy [:bbondy] from comment #39)
> > Am I reading this part right, and that it would break any applications doing a
> >  windows update without actually signing mars yet? 
> 
> Updater code will only do the signature check conditionally.
> You need to have MOZ_VERIFY_MAR_SIGNATURE=1 in your confvars.sh or
> --enable-verify-mar in your .mozconfig

Ok, so it won't break us by default -- but then when say SeaMonkey or Thunderbird do start signing mars we'll be broken by this, since it explicitly does Firefox [MoCo] based cert verification with no way to change.
Comment 41 Brian R. Bondy [:bbondy] 2012-02-20 12:34:52 PST
Ya once you want to start signing we'll need another bug posted with some tweaks.  I can help with that when the time comes.
Comment 42 Brian R. Bondy [:bbondy] 2012-02-21 14:14:41 PST
Created attachment 599361 [details] [diff] [review]
Verifying signed MARs from updater.exe. v9.

Updated which channels get which DER file as per bug

Carrying forward r+ on the patch.
Requesting feedback review from bhearsum just for the changes in: 
toolkit/mozapps/update/updater/Makefile.in
Comment 43 Ben Hearsum (:bhearsum) 2012-02-22 06:00:19 PST
Comment on attachment 599361 [details] [diff] [review]
Verifying signed MARs from updater.exe. v9.

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

::: toolkit/mozapps/update/updater/Makefile.in
@@ +149,5 @@
> +
> +ifneq (,$(filter beta release esr,$(MOZ_UPDATE_CHANNEL)))
> +RCFLAGS += -DMAR_SIGNING_RELEASE_BETA=1
> +else
> +ifneq (,$(filter nightly aurora nightly-elm nightly-profiling nightly-oak,$(MOZ_UPDATE_CHANNEL)))

Ugh. This is going to be such a PITA to manage.
Comment 44 Brian R. Bondy [:bbondy] 2012-02-22 06:10:51 PST
I think using the dep cert or some new cert for all nightlies except for aurora and m-c would help that.
Comment 45 Ben Hearsum (:bhearsum) 2012-02-22 06:19:43 PST
(In reply to Brian R. Bondy [:bbondy] from comment #44)
> I think using the dep cert or some new cert for all nightlies except for
> aurora and m-c would help that.

Yes, but then folks on those would get UAC prompts, no?
Comment 46 Brian R. Bondy [:bbondy] 2012-02-22 06:25:25 PST
> Yes, but then folks on those would get UAC prompts, no?

Nope, the MAR signing is an updater security enhancement and is not really related to the work in the service to get rid of the UAC dialog.  The patches could have been applied independent or even before the service landed.
Comment 48 Brian R. Bondy [:bbondy] 2012-02-29 18:18:47 PST
http://hg.mozilla.org/releases/mozilla-aurora/rev/18840d884c97

Note You need to log in before you can comment on or make changes to this bug.