Build certutil for Windows
Right now, certutil is built for all platforms in Tor's rbm-based build scripts, but the certutil binary is only copied to the output directory on Linux targets. Mozilla doesn't currently distribute official certutil binaries for Windows, and there's a significant demand for reproducibly built certutil Windows binaries. Would Tor accept a patch that copies certutil to the output directory on Windows?
(I have a draft version of such a patch, and I can confirm that the resulting certutil works properly on Windows.)
Activity
Hm, how would we distribute that? Would that be another .exe in our dist directory next to the actual bundles, the debug symbol archives, the expert bundle, the sha265sum files etc.?
@gk I was leaning toward adding it to the mar-tools zip archive, since that's where it's distributed for the Linux targets. But if you think there's a better place to put it, I'm open to suggestions.
As an aside, the certutil Windows binary created by Tor's build scripts has an incorrect PE header; it's set to GUI mode rather than console mode. This causes its stdout and stderr streams to not show up in a terminal. This is easy to fix by editing the PE header with a short Python script or something similar.
Adding it to the mar-tools zip sounds like a good idea. We'd take a patch for that. FWIW the GUI mode is epxected as we compile Firefox with the
-mwindowsflag. For the expert bundle (i.e. just a tor binary with necessary dlls) we explicitly omit that flag during compilation to get the console mode.Trac:
Status: needs_information to newIt seems like it would be better to patch the certutils makefile to filter out mwindows rather than add another script and build step...
Indeed, that probably is a better approach. I wasn't sure where the flag was being set, so I initially went with the Python script in my testing, but if it's just something in CFLAGS and LDFLAGS, that shouldn't be too hard for me to do. Would it make sense to filter out -mwindows for all of the stuff in the security/nss/cmd directory of Firefox? I assume all of the stuff in that directory is intended to run in console mode always, and that way if other NSS command-line tools get added to the mar-tools zip in the future, messing with the Makefiles again won't be needed.
So when I initially wrote my draft patch, it was against the ESR52 branch. I've just rebased against latest master branch, and it looks like ESR60 actually fixes the PE header already (certutil.exe is created as a console application when building nightly with master branch of tor-browser-build), so no changes are needed to force the Windows command-line tools to run in console mode. That's a pleasant surprise.
If you're going to add the binary to Windows, and the mar tools zip exists on macOS, please consider adding it to macOS as well.
AFAIK the mar-tools zip is indeed created for macOS, and I agree that it makes sense to add certutil to macOS as well. That said, I don't have a macOS machine available, so I won't have any way to verify on my end that the resulting binary actually works properly. I don't want to cause you guys undue work on this, so let me know if that's a problem.
While I'm fiddling with this, there are a few other potential changes in this area of the build script that seem relevant:
- Currently, libnssckbi.so / nssckbi.dll isn't copied to mar-tools. This library is only needed for a subset of certutil's functionality (specifically, the ability for certutil to change the trust settings of built-in certificates), and if the library is missing, such operations fail silently rather than giving a missing library error, which I assume is why Tor didn't realize that that library was relevant. Is it okay if I add that library to the mar-tools zip on all 3 OS's?
- There are 3 other NSS command-line tools already being built by Tor's build scripts and then discarded. These are modutil, pk12util, and shlibsign. modutil and pk12util are, like certutil, tools for interacting with NSS databases, and are regularly used in combination with certutil. I'm not directly familiar with shlibsign, but some quick Googling suggests that it's a utility that's required in order to enable the FIPS-compliant mode of the other NSS command-line tools. My inclination is to add these binaries to mar-tools (on all 3 OS's) since users who want to use certutil are likely to be following a workflow that needs one or more of those other tools too. Is that okay?
- signmar is currently, like certutil, added to mar-tools on Linux but not other OS's. For consistency's sake I'm inclined to add it to Windows and macOS's mar-tools as well. Is that okay?
For review, my current draft is at https://notabug.org/JeremyRand/tor-browser-build/src/certutil (current commit hash is b345e6128419493ef8051f2e68bd1863716f072a ). Please don't merge until the above questions are figured out, but certainly feel free to review what I have so far. This draft adds signmar, but does not yet add macOS binaries, the nssckbi library, or the other NSS command-line tools.
-
Thanks for working on this, really appreciated. Putting this in
needs_reviewfor now so we don't lose track of it. It might take a while for someone from us to look at it, though, as we are currently busy dealing with Tor Browser 8 issues.Trac:
Keywords: N/A deleted, TorBrowserTeam201809R added
Status: new to needs_review It might take a while for someone from us to look at it, though, as we are currently busy dealing with Tor Browser 8 issues.
No worries. I glanced at the issue tracker, and indeed, sounds like you guys are dealing with a bunch of things far more important than my patch. Feel free to prioritize accordingly. :)
In the meantime, I pushed a few more commits to https://notabug.org/JeremyRand/tor-browser-build/src/certutil , which add macOS output, nssckbi, and modutil/pk12/shlibsign. Current Git commit hash is 3f99b9190e056ae5f46d97d51b77ab428ffefc41 .
-
Replying to JeremyRand:
It might take a while for someone from us to look at it, though, as we are currently busy dealing with Tor Browser 8 issues.
No worries. I glanced at the issue tracker, and indeed, sounds like you guys are dealing with a bunch of things far more important than my patch. Feel free to prioritize accordingly. :)
In the meantime, I pushed a few more commits to https://notabug.org/JeremyRand/tor-browser-build/src/certutil , which add macOS output, nssckbi, and modutil/pk12/shlibsign. Current Git commit hash is 3f99b9190e056ae5f46d97d51b77ab428ffefc41 .
Sorry for the delay. I think the commits look mostly good. I am not convinced yet that we need to include
libssp-0.dllyet. If you check the included binaries in the mar-tools directory there is non that hasstack_chk* in it. (see: comment:11:ticket:15138) Compare that to the output you get from checking the binaries in tor-browser_en-US\Browser\TorBrowser\Tor. (I think that's due to us usingLDFLAGS="$LDFLAGS -static"for our mingw-w64 builds for the browser part)Could you rebase your branch on
master, too?Trac:
Status: needs_review to needs_revision
Keywords: TorBrowserTeam201810R deleted, N/A added I am not convinced yet that we need to include libssp-0.dll yet. If you check the included binaries in the mar-tools directory there is non that has stack_chk* in it. (see: comment:11:ticket:15138) Compare that to the output you get from checking the binaries in tor-browser_en-US\Browser\TorBrowser\Tor. (I think that's due to us using LDFLAGS="$LDFLAGS -static" for our mingw-w64 builds for the browser part)
You are correct; I just tested the binaries I built, and certutil.exe works fine with libssp-0.dll removed (I tested creation of a database and listing the certs from nssckbi.dll). I haven't tested the other programs, but I'll take your word for it that they don't need it either. I'll make that change and then rebase against master, hopefully later today. Thanks for catching that!
-
Thanks. Merged to
master(commit 6ed7eacc5b6b75fe888c67a7810c6bddad815f09, b40b18fc4b0e87b8536be04383258b2cd4e4c69b, bec987dfaae5e683a15e09a1e722b0de15161c52, and 9a0a161ab2732102d89d2397a032abc6ea86131a).Trac:
Keywords: N/A deleted, TorBrowserTeam201810R added
Resolution: N/A to fixed
Status: needs_revision to closed
