Tor Browser 4.5 displays signature validation error during update
I suspect this is due to the fact that we allow an update to proceed if it is signed with either my mar signing key or gk's mar signing key, but nonetheless TBB 4.5 displays two error messages while updating on Linux: "ERROR: Error verifying signature" "ERROR: Not all signatures were verified".
We should ensure the signature validation behavior is actually correct, and if so remove these error messages for the stable release.
Activity
Replying to arma:
I got this too. Does this error block the upgrade or just chatter in the log window?
"Just" the latter. IIRC the code is trying the first key first (which Mike has) and is complaining with said error message if the MAR files are not signed with it.
The difficulty in fixing this bug is that Mozilla tends to consider stderr as something that most users will ignore. In this specific case, a function named mar_verify_signatures() that is in modules/libmar/verify/mar_verify.c is called twice by the updater code. The first call, with the cert that was not used to sign the 4.5a5 MAR files, causes two error lines to be sent to stderr.
Unfortunately, the code in mar_verify.c contains 32 fprintf(stderr, "ERROR: ...") statements. The information that is output could be very useful if there is a real problem with MAR file signature verification, and mar_verify.c is also used by the signmar command line program (where it makes sense to have the stderr output).
Maybe we can add a bool parameter to mar_verify_signatures() that controls whether messages are written to stderr. Then we could enable them for the signmar command line program and disable them for the updater. I think the patch to do this will be somewhat large but fairly simple. We might even be able to make it so that the messages are logged to stderr when the app.update.log pref. is true, although that is a little tricky because the updater program itself cannot read prefs. But we could use an environment variable like Mozilla does for other things; look for PR_SetEnv() calls inside toolkit/xre/nsUpdateDriver.cpp.
The other thing to consider is that stdout+stderr is now hidden from the user on all platforms. Linux was the last one, and that was fixed with Tor Browser Script can't be launched from the L... (#13375 - moved)#12468 (moved). So this should be way less visible now than it was in 4.5a4... If this is an involved change, it may not be worth it for 4.5-stable?
Perhaps we can revisit it after dealing with the ff38-esr bitrot?
-
Replying to mikeperry:
The other thing to consider is that stdout+stderr is now hidden from the user on all platforms. Linux was the last one, and that was fixed with Tor Browser Script can't be launched from the L... (#13375 - moved)#12468 (moved). So this should be way less visible now than it was in 4.5a4... If this is an involved change, it may not be worth it for 4.5-stable?
Kathy and I agree that it makes sense to postpone this.
Trac:
Keywords: tbb-4.5-alpha, TorBrowserTeam201504 deleted, ff38-esr, tbb-firefox-patch added I had the same error message with Tor Messenger's updater and I fixed it by copying the DER to both release_primary and release_secondary. (I actually did think there was an error in verifying the update and spent quite a while trying to figure out why.)
IMO this would be the easiest way to fix it unless I am missing something else.
-
As I said on IRC you don't want to have just one key baked in. Think about losing the key/having it compromised. How are you updating your users? You can't sign the MAR files with the new key you are about to bake in. Even if that would still work (because you just want to rotate to a new key) every user would need to update to that particular version. Let's assume you need to get a chemspill release out the week afterwards if you used your new key to sign the MAR files a considerable amount of users will have a broken update experience as they won't have updated to the version with the new signing key baked in yet.
mentioned in issue #21319 (closed)
