Sign our MAR files

added TorBrowserTeam201412 in Legacy / Trac TorBrowserTeam201412R in Legacy / Trac component::applications/tor browser in Legacy / Trac owner::mcs in Legacy / Trac priority::high in Legacy / Trac resolution::fixed in Legacy / Trac severity::normal in Legacy / Trac status::closed in Legacy / Trac tbb-no-uplift-60 in Legacy / Trac tbb-security in Legacy / Trac type::defect in Legacy / Trac labels

This has been on my and brade's list of things to do for a while. I think Mozilla only uses signatures on Windows, e.g., see:

http://mxr.mozilla.org/mozilla-central/source/browser/confvars.sh#10

So it is possible this will require code changes as well (if code paths are Windows only or untested).

Trac:
Cc: N/A to brade, mcs

Sounds like a duplicate of legacy/trac#10392 (moved), no?

Trac:
Cc: brade, mcs to brade, mcs, gk

Trac:
Cc: brade, mcs, gk to brade, mcs, gk, boklm

As it turns out, Mozilla has recently made some progress towards adding signature support on all platforms. Here is their tracking bug:

https://bugzilla.mozilla.org/show_bug.cgi?id=973933

Most of the dependent bugs have patches. One thing that is messy is that they want to keep the updater executable as small and standalone as possible, so they are doing things like using OS APIs to verify signatures (on Windows they use CryptoAPI and on Mac OS they use Apple's Security framework). I think they plan to use NSS on Linux, and we may want to use NSS everywhere for trust and auditability reasons.

While thinking about comment:10:ticket:13407 and that it probably is wise to "just" have a role signing key due to just one key for verifying our MARs I was wondering whether it would be feasible to take advantage of reproducibly built MAR files given that no human interaction is interfering here. This is definitely worth a new bug if it is worth one at all (and I am volunteering for coding this actually). Given your knowledge of the MAR signing code Mozilla provides do you think there are general obstacles to extend that to add support for a verification method relying on more than one key?

Replying to gk:

Given your knowledge of the MAR signing code Mozilla provides do you think there are general obstacles to extend that to add support for a verification method relying on more than one key?

I am not sure exactly what you are asking. Mozilla currently supports embedding zero or more signatures in a MAR file. The signatures are added using a program named signmar which is really just a more capable variant of the mar program. signmar requires an NSS certificate database that contains a private key plus a self-signed certificate.

Then, if you configure the Firefox build with --enable-verify-mar, one or two certificates are embedded in the updater program and signatures contained within any MAR file that is downloaded are checked against those certificates. All signatures must be verified using one or the other cert or the MAR file will be rejected; that is, if the MAR file contains two signatures both must be verifiable. And at least one signature must be present when --enable-verify-mar is turned on.

Oh, I wasn't aware that multiple signatures were already supported in this way. If that is the case, we may want to consider having two or three keys: one held by Georg, one by myself, and one on a dist server. Though this has downsides in that it would require Georg and I to always be available to sign builds.. I suppose we could instead share a builders key, and then have the second key live on a signing machine that other people can get access to in an emergency?

Might be too much to deal with for the first rollout, though.

Replying to mcs:

Replying to gk:

Given your knowledge of the MAR signing code Mozilla provides do you think there are general obstacles to extend that to add support for a verification method relying on more than one key?

I am not sure exactly what you are asking. Mozilla currently supports embedding zero or more signatures in a MAR file. The signatures are added using a program named signmar which is really just a more capable variant of the mar program. signmar requires an NSS certificate database that contains a private key plus a self-signed certificate.

Then, if you configure the Firefox build with --enable-verify-mar, one or two certificates are embedded in the updater program and signatures contained within any MAR file that is downloaded are checked against those certificates. All signatures must be verified using one or the other cert or the MAR file will be rejected; that is, if the MAR file contains two signatures both must be verifiable. And at least one signature must be present when --enable-verify-mar is turned on.

Thanks. I was basically asking whether it is easily possible to avoid the bottleneck of just having one signing key. Originally, I was thinking in order to avoid that we somehow need to bolt the verification of the signing and hashing we do for the reproducible builds onto the MAR signing as a kind of additional assurance that everything is okay (like we have it now with a signature for each package and an "advanced verification" via the sah256sums and a couple of builder who sign that file with their own key). But, great that Mozilla supports having multiple signing keys as we may be able to leverage that work to get the same results or at least comparable ones (security-wise).

Replying to mikeperry:

Oh, I wasn't aware that multiple signatures were already supported in this way. If that is the case, we may want to consider having two or three keys: one held by Georg, one by myself, and one on a dist server. Though this has downsides in that it would require Georg and I to always be available to sign builds.. I suppose we could instead share a builders key, and then have the second key live on a signing machine that other people can get access to in an emergency?

Might be too much to deal with for the first rollout, though.

Yes, I think that should be in a new ticket. I've created legacy/trac#13730 (moved) for it. I am especially interested in thinking about needing just a threshold of valid signatures which might release the burden on us to be always available for signing purposes.

I have a design question. The new updater binary has a dependency on various shared libraries that are bundled with the browser (libnss3.so, libnspr4.so, etc.) On Windows, these libraries are found by the OS when the updater is started because of the fix we made for legacy/trac#13594 (moved).

On Mac OS and Linux, the libraries won't be found. Possible solutions:

(1) Modify the browser to set LD_LIBRARY_PATH before launching the updater. This means that while it is running, the updater would use libraries that are possibly going to be updated. I think that is OK because both Linux and Mac OS allow rename and unlink on an open file.

(2) Modify the browser to copy all of the required shared libraries when it makes a copy of the updater binary itself (i.e., we would extend the code here to do more: https://gitweb.torproject.org/tor-browser.git/blob/2822ccdb6d00b563413a285fe63488ab2ca7b460:/toolkit/xre/nsUpdateDriver.cpp#l385 ). To do this, we would need to embed a list of shared libraries inside the browser (which we would then have to maintain).

Kathy and I prefer solution (1) unless someone sees a problem with that approach. Comments?

I think whichever mechanism we use should be the same on all three platforms. I have a quick question about approach 2: would we be copying the old libraries into the update directory, or the new ones from the included mar?

I'm assuming old because the updater hasn't run at this point, and if this is true, it seems to me that option 1 is the winner (simply adding the libraries to the search path is better than trying to maintain a list and copy them, since its the same code either way).

Replying to mikeperry:

I think whichever mechanism we use should be the same on all three platforms. I have a quick question about approach 2: would we be copying the old libraries into the update directory, or the new ones from the included mar?

I'm assuming old because the updater hasn't run at this point, and if this is true, it seems to me that option 1 is the winner (simply adding the libraries to the search path is better than trying to maintain a list and copy them, since its the same code either way).

Correct, we would be copying old libraries. If we can avoid copying them, it seems like a win.

Trac:
Owner: tbb-team to mcs
Cc: brade, mcs, gk, boklm to brade, mcs, gk, boklm, mikeperry
Status: new to assigned

The tor-browser changes are ready for review. The patches may be found on a branch named bug13379-01 in user/brade/tor-browser.git; that is, here:

https://gitweb.torproject.org/user/brade/tor-browser.git/shortlog/refs/heads/bug13379-01

There are two "Brian R. Bondy" commits (from mozilla-central, where the fix for 902761 has landed), one commit to backport some Mozilla patches that have r+ on the Mozilla side but have not yet been committed, and one commit that contains our changes to always use NSS and fix up various things.

The most recent commit (68a488187fde8a1f50e1e85e45b0f0beac15446c) will need to be discarded; it embeds a certificate that brade and I used for testing our own 4.5-alpha-1-ish builds.

Also, these changes cause the NSS certutil command to be built as well as signmar, which is a variant of the mar program that supports signing, verifying signatures, etc. (signmar uses NSS key and cert databases). We have some uncommitted builders/tor-browser-bundle changes that cause certutil and signmar to be included in the mar-tools-linux*.zip archives along with the dependent NSS and NSPR libraries. We will publish those patches soon along with other changes we plan to make to automate the signing process as much as possible (e.g., only prompt once for the NSS password).

Trac:
Keywords: N/A deleted, MikePerry201411R added
Status: assigned to needs_review

Here are a few more details on the signing key and cert.

The certificates files that get embedded in the updater are contained in these files within the tor-browser tree:

toolkit/mozapps/update/updater/dep1.der
toolkit/mozapps/update/updater/dep2.der
toolkit/mozapps/update/updater/nightly_aurora_level3_primary.der
toolkit/mozapps/update/updater/nightly_aurora_level3_secondary.der
toolkit/mozapps/update/updater/release_primary.der
toolkit/mozapps/update/updater/release_secondary.der
toolkit/mozapps/update/updater/xpcshellCertificate.der

dep1.der and dep2.der are no longer used; Mozilla used to use them for "depend" builds (maybe for their try server?).

The nightly_aurora_level3*.der files will be embedded in nightly builds. We need to decide what to do about those, if anything (at the moment, people who run our nightly builds do not expect to receive automated updates).

The release_*.der files will be embedded in our alpha, beta, and release builds. These are the most important ones.

The xpcshellCertificate.der is used by Mozilla for testing; it is embedded in all other builds, e.g., developer builds that lack an update channel.

I generated a test certificate by running these two commands:

./certutil -d .nss -N
./certutil -d .nss -S -x -g 3072 -n marsigner -s "CN=Tor Browser MAR signing key" -t,,

I exported it to a .der file via:

./certutil -d .nss -L -r -n marsigner -o marsigner.der

I then replaced both release_primary.der and release_secondary.der with the contents of marsigner.der (currently, a signature is considered "good" if the key associated with either the primary or the secondary certificates was used to create the signature; other policies could be implemented).

The build changes are here: https://gitweb.torproject.org/user/brade/tor-browser-bundle.git/commit/58acbf5b8d6df5a5e9f8f4760cd8c21836be4cef Please review.

I believe signing should be a reproducible process, but we will need to confirm.

The change to add the --createIncrementalMARs command line to update_responses looks good.

The other changes introduce a single makefile rule to generate the incremental mar files and sign them. I am wondering if we should separate the incremental mar files generation, and the signature, to allow a process like this:

build tor-browser
generate incremental mars
upload sha256sums.incrementals.txt of unsigned mar files
check that sha256sums.txt and sha256sums.incrementals.txt are matching
sign the mar files, update responses xml files and upload

Trac:
Keywords: MikePerry201411R deleted, TorBrowserTeam201411R added

Replying to boklm:

The change to add the --createIncrementalMARs command line to update_responses looks good.

The other changes introduce a single makefile rule to generate the incremental mar files and sign them. I am wondering if we should separate the incremental mar files generation, and the signature, to allow a process like this:

build tor-browser

generate incremental mars

upload sha256sums.incrementals.txt of unsigned mar files

check that sha256sums.txt and sha256sums.incrementals.txt are matching

sign the mar files, update responses xml files and upload

It would be simple to keep 'incrementals' as a separate Make target. The reason I put everything in one script was to make it less likely that things would happen in the wrong order.

gk or mikeperry: What do you think? What will the release process look like once we need to sign the MAR files?

Replying to mcs:

Replying to boklm:

The change to add the --createIncrementalMARs command line to update_responses looks good.

The other changes introduce a single makefile rule to generate the incremental mar files and sign them. I am wondering if we should separate the incremental mar files generation, and the signature, to allow a process like this:

build tor-browser

generate incremental mars

upload sha256sums.incrementals.txt of unsigned mar files

check that sha256sums.txt and sha256sums.incrementals.txt are matching

sign the mar files, update responses xml files and upload

It would be simple to keep 'incrementals' as a separate Make target. The reason I put everything in one script was to make it less likely that things would happen in the wrong order.

gk or mikeperry: What do you think? What will the release process look like once we need to sign the MAR files?

I think we should use a process that allows independent builders to check whether they got the same results as we easily. And this means, I think, we should follow boklm's idea: building everything including the incremental MAR files and uploading everything and then in a separate step doing the signing and all the things needed for getting the updates delivered. I see at least two important reasons why we want to do it this way:

We want to have many builders to make it less likely our builds are compromised. Building with gitian is already tedious and we should not make it even more difficult to get matching builds which we we would if we included the signing before the SHA sum creation.
There may be people that trust our reproducible build system but not the complex signing process/code and fetching some update from some server. Following boklm's idea they could pretty well get the benefits of building Tor Browser themselves and applying the MAR update manually (which users are already doing).

OK, a revised patch for the build-related changes may be found here:

https://gitweb.torproject.org/user/brade/tor-browser-bundle.git/commit/b4f5af4f8645e9c5e36c8169766ebc7f68428aa6

We decided to keep all three of the MAR-related steps separate for maximum flexibility:

make incrementals
make signmars-alpha
make update_responses

Replying to mcs:

OK, a revised patch for the build-related changes may be found here:

https://gitweb.torproject.org/user/brade/tor-browser-bundle.git/commit/b4f5af4f8645e9c5e36c8169766ebc7f68428aa6

That one looks good. Nit: s/so we have can/so we can/ in signmar.sh.

Regarding the incremental updates: I think we want to create the incremental .mar files during the usual build process just before we call the hash-bundles script. But that belongs to a different bug as we need some build process changes then, too (like having the config.yml already updated when starting the release builds etc.).

Replying to mcs:

The tor-browser changes are ready for review. The patches may be found on a branch named bug13379-01 in user/brade/tor-browser.git; that is, here:

https://gitweb.torproject.org/user/brade/tor-browser.git/shortlog/refs/heads/bug13379-01

Looks good to me. Any reason for

-  nss/cmd/certutil \

in Makefile.in in security/build? I'd be happy if others would have a look at https://gitweb.torproject.org/user/brade/tor-browser.git/commit/2d7b84ca53c75686daebf1e203eb5369c24f965e, too.

Gonna test that shortly but it seems to me that the signing can be done offline, right? Like, I could download the MAR files, get offline, enter my passphrase, get all MAR files signed, get online again and upload them?

Oh, I forgot one nit:

  // On Windows we rely on CyrptoAPI to do verifications so we don't need to
   // initialize NSS at all there.

This (in updater.cpp) is a bit misleading as we rely on NSS on all platforms. Might be confusing for people reading our code.

Replying to mcs:

I think they plan to use NSS on Linux, and we may want to use NSS everywhere for trust and auditability reasons.

On the other hand there are the arguments in https://bugzilla.mozilla.org/show_bug.cgi?id=699700#c25 https://bugzilla.mozilla.org/show_bug.cgi?id=699700#c28

I guess we'll see how it goes :)

Replying to gk:

Replying to mcs:

The tor-browser changes are ready for review. The patches may be found on a branch named bug13379-01 in user/brade/tor-browser.git; that is, here:

https://gitweb.torproject.org/user/brade/tor-browser.git/shortlog/refs/heads/bug13379-01

Looks good to me. Any reason for {{{

nss/cmd/certutil
}}} in Makefile.in in security/build? I'd be happy if others would have a look at https://gitweb.torproject.org/user/brade/tor-browser.git/commit/2d7b84ca53c75686daebf1e203eb5369c24f965e, too.

We removed nss/cmd/certutil within the ifdef ENABLE_TESTS block and moved it outside so that nss/cmd/certutil is always included in NSS_DIRS (and therefore always compiled).

Gonna test that shortly but it seems to me that the signing can be done offline, right? Like, I could download the MAR files, get offline, enter my passphrase, get all MAR files signed, get online again and upload them?

Yes, I believe that should work.

Replying to gk:

Replying to mcs:

I think they plan to use NSS on Linux, and we may want to use NSS everywhere for trust and auditability reasons.

On the other hand there are the arguments in https://bugzilla.mozilla.org/show_bug.cgi?id=699700#c25 https://bugzilla.mozilla.org/show_bug.cgi?id=699700#c28

I guess we'll see how it goes :)

I missed those comments -- thanks for pointing them out. It is kind of amusing that NSS is viewed as less reliable than the OS-provided APIs. but the arguments in those comments make sense. But since we are stuck with NSS on Linux, I am still inclined to use it on Windows and Mac as well.

Replying to gk's [comment:22 comment:22]:

That one looks good. Nit: s/so we have can/so we can/ in signmar.sh.

Fixed. Please use this new commit: https://gitweb.torproject.org/user/brade/tor-browser-bundle.git/commit/14dc9d97f52daee6f3fed33735b89f13977a6215

Replying to gk's [comment:24 comment:24]:

Oh, I forgot one nit: {{{ // On Windows we rely on CyrptoAPI to do verifications so we don't need to // initialize NSS at all there. }}} This (in updater.cpp) is a bit misleading as we rely on NSS on all platforms. Might be confusing for people reading our code.

To be safe, I rewrote that comment. Please use this (new) branch: https://gitweb.torproject.org/user/brade/tor-browser.git/shortlog/refs/heads/bug13379-02 I did not commit any test certs. there, so you can just merge the whole thing, i.e., take the most recent 4 commits.

Replying to mcs:

We removed nss/cmd/certutil within the ifdef ENABLE_TESTS block and moved it outside so that nss/cmd/certutil is always included in NSS_DIRS (and therefore always compiled).

Oh, yes, sorry. Seems to have been kind of code-blindness on my side. :(

There are some wrinkles here when generating certificates:

We are stuck with SHA1 for the moment which is not optimal to say the least. I've opened https://bugzilla.mozilla.org/show_bug.cgi?id=1105689 to get that fixed upstream. Not sure how easy it would be to loosen that constraint ourselves. Maybe we'd just need to get rid of that check in https://mxr.mozilla.org/mozilla-central/source/modules/libmar/verify/mar_verify.c#330.
Newer certuils versions use SHA256 by default. This got implemented by https://bugzilla.mozilla.org/show_bug.cgi?id=1058933. So be sure to check the resulting cert with something like openssl x509 -in marsigner2.der -inform der -text | grep sha1WithRSAEncryption.
If you happen to have such a newer certutils you may change the default hash algorithm with the -Z option which is basically undocumented (this is https://bugzilla.mozilla.org/show_bug.cgi?id=1058870).
It is not possible to store two certs with the same CN in the database (even if the nicknames are different).

Replying to gk:

There are some wrinkles here when generating certificates:

We are stuck with SHA1 for the moment which is not optimal to say the least. I've opened https://bugzilla.mozilla.org/show_bug.cgi?id=1105689 to get that fixed upstream. Not sure how easy it would be to loosen that constraint ourselves. Maybe we'd just need to get rid of that check in https://mxr.mozilla.org/mozilla-central/source/modules/libmar/verify/mar_verify.c#330.

This seems important to fix before we ship a version of the browser that verifies MAR signatures. I do not fully understand all of the NSS and libmar code, but it looks to me like a signature algorithm ID of 1 is arbitrarily assigned to the only signature algorithm that is supported by the libmar code, SEC_OID_ISO_SHA1_WITH_RSA_SIGNATURE. What would be the best algorithm to use? I guess the signature algorithms that NSS supports can be seen by reading the sec_DecodeSigAlg() code here:

http://mxr.mozilla.org/mozilla-esr31/source/security/nss/lib/cryptohi/secvfy.c#213

It seems fine to me if we want to hold off until 4.5-alpha-3 for this for stability and logistical reasons (key management, release delay), but that said I think a SHA1-based sig is still better than no sig.

Still, to pick from the ones listed in secvfy.c, probably either: SEC_OID_ANSIX962_ECDSA_SHA256_SIGNATURE or SEC_OID_NIST_DSA_SIGNATURE_WITH_SHA256_DIGEST.

Replying to mikeperry:

It seems fine to me if we want to hold off until 4.5-alpha-3 for this for stability and logistical reasons (key management, release delay), but that said I think a SHA1-based sig is still better than no sig.

That's true but if we start signing with the current code then we get the additional problem of how to transition the users with a 4.5-alpha-2 to a later version that has additional signature algorithm support. Might be not a big deal but I think I'd prefer having the key creation/management issues properly sorted out (we don't even have them sorted out for the bundle signatures yet hint hint) and give the signed updates a bit more testing.

Still, to pick from the ones listed in secvfy.c, probably either: SEC_OID_ANSIX962_ECDSA_SHA256_SIGNATURE or SEC_OID_NIST_DSA_SIGNATURE_WITH_SHA256_DIGEST.

I have not looked at those algorithms yet but just wanted to add that we are probably going to use RSA 4096/SHA512 for the packages. Might make sense to use a comparable security level if it does not cost much.

Trac:
Keywords: TorBrowserTeam201411R deleted, TorBrowserTeam201412R added

Trac:
Status: needs_review to needs_revision
Keywords: TorBrowserTeam201412R deleted, TorBrowserTeam201412 added

Kathy and I made changes to use a SHA512-based signature. Please review.

https://gitweb.torproject.org/user/brade/tor-browser.git/commit/?h=bug13379-02&id=14447aca2f31c56ccadc289cef5f756e97d1f3a9

I created a test certificate and exported it to a .der file by using these commands:

./certutil -d .nss -N
./certutil -d .nss -S -x -g 4096 -Z SHA512 -n marsigner -s "CN=Tor Browser MAR signing key" -t,, 
./certutil -d .nss -L -r -n marsigner -o marsigner.der

Trac:
Status: needs_revision to needs_review

On the build side of things, boklm contributed some changes to update_responses to select the action based on the program name. Kathy and I incorporated those changes into a new commit:

https://gitweb.torproject.org/user/brade/tor-browser-bundle.git/commit/?h=bug13379-04&id=186d339c394a7083faed064a218280fe52500f1b

Please review this as well. The changes to gitian/descriptors/linux/gitian-firefox.yml and the contents of gitian/signmars.sh are the same as in the previous build-related patch that we posted.

Trac:
Keywords: N/A deleted, TorBrowserTeam201412R added

Replying to mcs:

Kathy and I made changes to use a SHA512-based signature. Please review.

https://gitweb.torproject.org/user/brade/tor-browser.git/commit/?h=bug13379-02&id=14447aca2f31c56ccadc289cef5f756e97d1f3a9

I created a test certificate and exported it to a .der file by using these commands: {{{ ./certutil -d .nss -N ./certutil -d .nss -S -x -g 4096 -Z SHA512 -n marsigner -s "CN=Tor Browser MAR signing key" -t,, ./certutil -d .nss -L -r -n marsigner -o marsigner.der }}}

This one looks good to me. Just one question: Why do we need the changes in cryptox.h? I was under the impression we have MAR_NSS defined anyway and thus there is no risk we would enter the #elif XP_MACOSX and #elif defined(XP_WIN) blocks.

I think I am going to test the MAR signing a bit. What scenarios did your testing already cover?

Replying to mcs:

On the build side of things, boklm contributed some changes to update_responses to select the action based on the program name. Kathy and I incorporated those changes into a new commit:

https://gitweb.torproject.org/user/brade/tor-browser-bundle.git/commit/?h=bug13379-04&id=186d339c394a7083faed064a218280fe52500f1b

Please review this as well. The changes to gitian/descriptors/linux/gitian-firefox.yml and the contents of gitian/signmars.sh are the same as in the previous build-related patch that we posted.

Looks good to me.

Replying to gk:

This one looks good to me. Just one question: Why do we need the changes in cryptox.h? I was under the impression we have MAR_NSS defined anyway and thus there is no risk we would enter the #elif XP_MACOSX and #elif defined(XP_WIN) blocks.

Just paranoia. Indeed, we should not be using any code in those blocks.

I think I am going to test the MAR signing a bit. What scenarios did your testing already cover?

We tested a variety of scenarios as we worked on signing. With the most recent code, we built a 4.5-alpha-2-ish build using the gitian-based process (embedding the certificate I described in comment:36) and ran the resulting builds on Mac OS 10.8.5, an old Fedora Linux32 system, and Win7.

To force an update, we modified the .htaccess file on a test update server of ours. We tested that unsigned MARs were rejected. We tested that signed MARs were accepted.

Okay, here is what I've got so far:

signmar.sh is not executable
I don't get the update working it seems. I get

ERROR: Unsupported signature algorithm (SHA1 with RSA).
ERROR: Unsupported signature algorithm (SHA1 with RSA).

How do I debug this? Any ideas? I did the following:

I created two certificates and added them atop of your tor-browser changes (commit 14447aca2f31c56ccadc289cef5f756e97d1f3a9) and tagged that (I just checked that I really included the 4k-bit certs with SHA-512)
I checked out your tor-browser-bundle branch (commit 186d339c394a7083faed064a218280fe52500f1b) and built a 4.5-alpha-2 with the tag mentioned in 1)
I bumped the version to 4.5-alpha-3 and excluded HTTPS-Everywhere from the bundling step and built that version, too.
I modified the config.yml to allow an incremental update from 4.5-alpha-2 to 4.5-alpha-3
I built that incremental update.
I downloaded the .mar files and the 4.5-alpha-2*tar.xz on my local computer and signed the .mar files (verifying the signature with signmar gives me no error and there is indeed a signature available; marsigner on my local computer is indeed the cert I added to tor-browser)
I extracted 4.5-alpha-2*tar.xz
I followed https://wiki.mozilla.org/Software_Update:Manually_Installing_a_MAR_file (Steps for Linux)
update.log shows basically "failed: 19" and the above error messages are shown

Replying to gk:

Okay, here is what I've got so far:

signmar.sh is not executable

Ugh. Kathy and I messed up the file mode when we created a new branch (where we merged in boklm's changes and applies other small fixes). We will fix it.

I don't get the update working it seems. I get {{{ ERROR: Unsupported signature algorithm (SHA1 with RSA). ERROR: Unsupported signature algorithm (SHA1 with RSA). }}} How do I debug this? Any ideas? I did the following:

I created two certificates and added them atop of your tor-browser changes (commit 14447aca2f31c56ccadc289cef5f756e97d1f3a9) and tagged that (I just checked that I really included the 4k-bit certs with SHA-512) ...

update.log shows basically "failed: 19" and the above error messages are shown

Based on the info you provided, I think the MAR file has been signed using the older (now wrong) algorithm. Kathy and I added the "Unsupported signature algorithm (SHA1 with RSA)" log message to make it easier to detect this situation. But it sounds like you did everything correctly. Is there any chance you used an older signmar program (from mar-tools)? If you used the signmars-alpha make target the correct signmar should have been used though.

Replying to mcs:

Replying to gk:

update.log shows basically "failed: 19" and the above error messages are shown

Based on the info you provided, I think the MAR file has been signed using the older (now wrong) algorithm. Kathy and I added the "Unsupported signature algorithm (SHA1 with RSA)" log message to make it easier to detect this situation. But it sounds like you did everything correctly. Is there any chance you used an older signmar program (from mar-tools)? If you used the signmars-alpha make target the correct signmar should have been used though.

Yes, you guessed correctly. I am not signing on my build server as I don't put the private keys there and had forgotten to update my local signmar copy. Interesting that it signed the .mar at all with the new key... Anyway, I found a new problem: signature verification works but for some reason my incremental update is broken now. In the update.log I get:

SOURCE DIRECTORY /home/firefox64/signtest/tor-browser_en-US/Browser/updates
DESTINATION DIRECTORY /home/firefox64/signtest/tor-browser_en-US/Browser
failed: 23
calling QuitProgressUI

The full update is working fine, though. I was curious and tested a vanilla 4.5-alpha-2 and made exactly the same changes as I did when testing your patch and it turned out that incremental update is working. Thus, I suspect there is something in the new code that is causing this. Any ideas?

And one request: Could you make the path to the nssdb configurable by an environment variable (e.g. NSSDBPATH)? For security reasons I plan to keep my signing keys offline using them offline directly from the storage device and hard-coding the path to the database does not work so well under that scenario.

Replying to gk:

And one request: Could you make the path to the nssdb configurable by an environment variable (e.g. NSSDBPATH)? For security reasons I plan to keep my signing keys offline using them offline directly from the storage device and hard-coding the path to the database does not work so well under that scenario.

Oh, and please make the name of the cert configurable as well (I think via environment variable might be enough for now, too) in order to avoid bottlenecks from the beginning.

If I sign the .mar files with the key embedded in the second certificate I get

ERROR: Error verifying signature.
ERROR: Not all signatures were verified.

But the update with the full .mar file works and the one with the incremental .mar file is broken as described above. I guess these errors occur as the verifier is first trying the first key which results in an error and then falling back to the second one. I am not sure whether users get to see these errors during a "real" update. They might get confused and thus it might be better to show errors only if the signature verification fails. On the other hand it might be helpful later on when we embed more than one signature to log verification failures even if the signature verification succeeds (for instance if we have two signatures but require only one to succeed). So, maybe we should leave that for now as-is?

Replying to gk:

The full update is working fine, though. I was curious and tested a vanilla 4.5-alpha-2 and made exactly the same changes as I did when testing your patch and it turned out that incremental update is working. Thus, I suspect there is something in the new code that is causing this. Any ideas?

Got that working although I don't know how. I re-did the whole procedure until it worked. :)

One final thing: I needed to copy libnss3.dylib and libmozglue.dylib to the directory where updater was on my OS X machine. Otherwise the update failed.

Replying to gk:

Yes, you guessed correctly. I am not signing on my build server as I don't put the private keys there and had forgotten to update my local signmar copy. Interesting that it signed the .mar at all with the new key... Anyway, I found a new problem: signature verification works but for some reason my incremental update is broken now. In the update.log I get: {{{ SOURCE DIRECTORY /home/firefox64/signtest/tor-browser_en-US/Browser/updates DESTINATION DIRECTORY /home/firefox64/signtest/tor-browser_en-US/Browser failed: 23 calling QuitProgressUI }}} The full update is working fine, though. I was curious and tested a vanilla 4.5-alpha-2 and made exactly the same changes as I did when testing your patch and it turned out that incremental update is working. Thus, I suspect there is something in the new code that is causing this. Any ideas?

Error 23 is "VERSION_DOWNGRADE_ERROR". The error codes are here: https://gitweb.torproject.org/tor-browser.git/tree/toolkit/mozapps/update/common/errors.h

I am not sure exactly what happened, but the product information block for the incremental MAR file must have contained the wrong version number. Unfortunately, the mar and signmar programs have a default version number embedded in them at build time, which is used to set the version within the Product Information Block of created MAR files. So we need to be really careful which mar or signmar program is used when the MAR files are created or we will need to modify Mozilla's make_incremental_update.sh and make_full_update.sh scripts to let us pass in the product version when we create a MAR file.

You can use the -T option with mar and signmar to see the version number that is embedded within the product info block. Kathy and I were hoping that using the default version number would not be a problem, but it may be depending on our signing procedure. Also, the mar and signmar programs support a -i option that can be used to "refresh" the product info that is embedded within a MAR file (including setting a new version number). The refresh can only be done on an unsigned MAR file. But if we need to, we could do that before signing the files. But I would like to know where the process went wrong for you (if you can figure that out).

And one request: Could you make the path to the nssdb configurable by an environment variable (e.g. NSSDBPATH)? For security reasons I plan to keep my signing keys offline using them offline directly from the storage device and hard-coding the path to the database does not work so well under that scenario.

Yes, will do.

Replying to gk:

Oh, and please make the name of the cert configurable as well (I think via environment variable might be enough for now, too) in order to avoid bottlenecks from the beginning.

Sure. We will use NSS_DB_DIR and NSS_CERTNAME to match the variable names used inside signmar.sh.

Replying to gk:

If I sign the .mar files with the key embedded in the second certificate I get {{{ ERROR: Error verifying signature. ERROR: Not all signatures were verified. }}} But the update with the full .mar file works and the one with the incremental .mar file is broken as described above. I guess these errors occur as the verifier is first trying the first key which results in an error and then falling back to the second one.

libmar writes those error messages to stderr. I don't think users will see them except at the terminal or if they capture stderr. Certainly they will have to go look for them. I think we should keep as possibly useful diagnostic messages (although "ERROR:" is misleading in this case).

Replying to gk:

One final thing: I needed to copy libnss3.dylib and libmozglue.dylib to the directory where updater was on my OS X machine. Otherwise the update failed.

You are running the updater program manually, right? Before it is started by the browser, DYLD_LIBRARY_PATH is set on Mac OS. See: https://gitweb.torproject.org/user/brade/tor-browser.git/diff/toolkit/xre/nsUpdateDriver.cpp?h=bug13379-02&id=81cefb5e64fb5af95ba86ffb8e4eb17e6b190252

If you are seeing a problem when the updater is launched by the browser, then there is a bug.

Okay, good. I feared we had a Windows like problem visible on my machine and had forgotten about the DYLD_LIBRARY_PATH workaround, sorry.

I am happy now. :) Feel free to merge your patches squashed to the repos or let me know if I should do it. My current plan is to generate the signing keys tomorrow and land them atop of your tor-browser patch(es).

I made a series of 4 tor-browser commits to the tor-browser-31.3.0esr-4.5-1 branch: 7d87311b58709d8ab2a8ec052e0613b574b19a7b 9c7ea1fb1df0545990a85aabcef8180ea287305f 40b509e6d7408a73c5a81a3d971dfc6daf5f7510 7612aeba131f3eca1c80a369aab97da6fc249565

And I made one builder/tor-browser-bundle commit to master: d852329ac0979d8005e9e8bdd9b3f8a049fc2db4

I am leaving this bug open until gk commits the der files for the signing certificates.

Okay, pushed. One final thing: Given that Mozilla's certificates were only valid in a three month period several years ago it seems the related cert attributes are not checked during signature verification and our certificates are essentially never invalid, right?

Replying to gk:

Okay, pushed. One final thing: Given that Mozilla's certificates were only valid in a three month period several years ago it seems the related cert attributes are not checked during signature verification and our certificates are essentially never invalid, right?

Yes. I am sorry we forgot to mention this sooner. Looking at the code in libmar, the public key is extracted from the cert data (that is compiled into the updater) via a couple of NSS calls: CERT_NewTempCertificate() and CERT_ExtractPublicKey(). I don't think those calls to do cert validity checks, and I don't think the signature verifications calls do either, e.g., NSS_VerifySignature().

On the one hand, this is good because it means that old browsers can verify the MAR signatures even after the signing key expires. On the other hand, there does not seem to be a way to revoke a certificate.

Do we need to fix this?

Replying to mcs:

On the one hand, this is good because it means that old browsers can verify the MAR signatures even after the signing key expires. On the other hand, there does not seem to be a way to revoke a certificate.

Do we need to fix this?

Definitely not in this ticket if at all. Having the certificate only valid for a certain amount of time would not help much as the procedure in all cases of key exchange (be it due to compromise, be it due to key expiry, be it due to a lost private key, ...) would be the same: exchanging the key in question with a new one, baking it into Tor Browser and signing the MAR files from now on with the new key (too).

Trac:
Status: needs_review to closed
Resolution: N/A to fixed

Trac:
Severity: N/A to Blocker
Sponsor: N/A to N/A
Keywords: N/A deleted, tbb-no-uplift added
Reviewer: N/A to N/A