1730711 - ship rnp and rnpkeys tools

Status: RESOLVED FIXED

(MailNews Core :: Security: OpenPGP, enhancement)

Description

[Kai Engert [:KaiE:]]

We want to enable users to do more self diagnosis of problems.
The RNP command line tools might be helpful.

We could consider to distribute the tools.

However, even if we decide not to ship the tools - for example to keep distribution size small - we should have a way to build compatible versions of the tools as part of the Thunderbird build. This can allow us to use "try" builds to obtain the tools for all platforms.

I experimented on Linux, and have initial build rules. I don't know yet if they work on other platforms.

On my local Linux system, the distribution size increased by 1.5 MB.

If we don't want to ship the tools by default, then we'll have to introduce a build parameter to conditionally enabling building of the tools.

Comment 1

[Kai Engert [:KaiE:]]

Attachment: Bug 1730711 - Build and ship the rnp and keys tools. r=rjl

Comment 2

[Kai Engert [:KaiE:]]

Rob, we probably don't want a final review yet, but please provide feedback, if you have. Note that I wasn't very careful by creating the new moz.build files - I have copied from the rnp library moz.build, and simply tweaked to make it build. I didn't check if I have unnecessary sections.

We should get the numbers of the size increase on all platforms, and then Magnus probably should make a call regarding "ship or don't ship".

Comment 3

[Kai Engert [:KaiE:]]

try build fails
https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=b80c9d5d41f4389134d9641c469f72d37397de20

Comment 4

[Rob Lemley [:rjl]]

I prefer a separate download that would be hosted with the shipped builds on the Mozilla FTP server. Easy enough to point users at them when needed.

The config option for building them can be disabled by default, and Taskcluster builds can automatically enable.

Comment 5

[Rob Lemley [:rjl]]

Attachment: Bug 1730711 - Include a real getopt implementation for Windows. r=kaie

Copied from ipc/chromium/src/third_party/libevent/WIN32-Code.

Comment 6

[Rob Lemley [:rjl]]

Attachment: Bug 1730711 - Build and include RNP commandline tools with distribution. r=kaie

Depends on D126029

Comment 7

[Rob Lemley [:rjl]]

I fixed things up a bit.

Some issues with Windows. Both utilities use getopt_long() which doesn't exist on Windows. As the shared library doesn't use any of the getopt functions (just includes the header) we just had an empty getopt.h until now. Fortunately, there is a MSVC compatible version included with the Chromium IPC code that is vendored in mozilla-central, so I copied that.

Then there is the issue of missing #includes. fficli.cpp is missing a #include <shellapi.h> for the GetCommandLineW function. It needs to be included after #include <windows.h> apparently. rnpcfg.cpp is missing #include <cctype> for std::tolower.
I kind of cheated and stuck all of those missing includes into getopt.h so I didn't have to patch the RNP code. I'll send pull requests upstream for those.

Try build running: https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=82d2e321a472f5b1f3ae9fc0b7ec0b4bbb7f09ab

I'm not 100% sure that Linux will work. We might run into problems with too-new libstdc++ symbols again.

Regarding the size increase. Both of these tools are pretty small, and will be dynamically linked to the librnp shared library. It looks like it will be <1MB. That's not so bad.

Comment 8

[Nickolay Olshevsky]

JFYI: RNP can be built with MSVC, we have CI runner for Windows as well. It uses getopt.dll installed via vcpkg.
Did you consider building RNP lib and tools using our CMake scripts, or do you need some custom build phases/processing?

Comment 9

[Rob Lemley [:rjl]]

I've avoided using CMake since it would be an additional build requirement for local developers. Same with installing vcpkg. I'll come back to this tomorrow, the try build failed on Windows, even though it worked for my local build. macOS at least builds, it runs into a packaging error.

Comment 10

[Rob Lemley [:rjl]]

Okay, I have mostly figured it out. See D126029 and D126030. Working try build at https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=5efad374b7002d2c68b668854d507024c7283f7d

GetCommandLineW comes from shellapi.h, which is automatically included if WIN32_LEAN_AND_MEAN is not defined. That seems to be the case with the RNP CMake builds, but not with ours. I've been able to unset it just for building these two binaries.

I cannot figure out the issue with std::tolower. The compiler is throwing error: no matching function for call to 'tolower' at switch (std::tolower(mult)) {. Including <cctype> makes it work for me. I created a force-include file for rnpcfg.cpp that includes cctype to get it to build.

For getopt, I went with the copy already in the tree at ipc/chromium/src/third_party/libevent/WIN32-Code. I did that as it's BSD licensed, whereas the version that vcpkg would pull in is LGPL (and would need to be dynamically linked).

I did a quick smoke test of the produced Windows binaries, and they execute and return help information.

Comment 11

[Kai Engert [:KaiE:]]

Regarding getopt.c: How would we comply with the BSD 3-clause license's requirement, clause 2

"2. Redistributions in binary form must reproduce the above copyright
notice, this list of conditions and the following disclaimer in the
documentation and/or other materials provided with the distribution."

Would the RNP binary we build be required to print this license statement at runtime?

Comment 12

[Kai Engert [:KaiE:]]

Ok, no it wouldn't be needed at runtime, it says "In the documentation".

Will our existing LICENSE documentation sufficiently cover that copied getopt.c file? Would we need to mention that file explicitly anywhere? For example, Thunderbird's interactive license display (from about:license) lists the directories/files that a specific license refers to.

Comment 13

[Rob Lemley [:rjl]]

Probably best to update about:license. I'll update the patch.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/7915cdc68570
Include a real getopt implementation for Windows. r=kaie
https://hg.mozilla.org/comm-central/rev/dfd512c51382
Build and include RNP commandline tools with distribution. r=kaie

Comment 15

[Geoff Lankow (:darktrojan)]

Backed out dfd512c51382:
https://hg.mozilla.org/comm-central/rev/ee04eaa268433ace0ebb1ce12f81be972addf34f

Comment 16

[Rob Lemley [:rjl]]

Updated D126030, try build at: https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=da00051677c6119b3016accb81fdf8e7cc7aa08f. I specifically checked Linux and macOS to make sure the binaries linked to the correct librnp and executed.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/ba558f135828
Build and include RNP commandline tools with distribution. r=kaie

Comment 18

[Magnus Melin [:mkmelin]]

We should document somewhere (the wiki?) how users can use this for debugging.