Closed Bug 1893950 Opened 1 year ago Closed 1 month ago

[RNP] Update to next major RNP release, v0.18.1

Categories

(MailNews Core :: Security: OpenPGP, enhancement)

Product:
MailNews Core
Component:
Security: OpenPGP
Type:
enhancement

Tracking

(thunderbird_esr140 wontfix, thunderbird147 wontfix)

Status:
RESOLVED FIXED
Milestone:
148 Branch
Tracking Flags:
Tracking Status
thunderbird_esr140 --- wontfix
thunderbird147 --- wontfix

People

(Reporter: KaiE, Assigned: KaiE)

References

(Blocks 3 open bugs)

Details

Attachments

(7 files, 1 obsolete file)

Bug 1893950 - Update cmake_define_files.py to properly handle #define lines. r=#thunderbuild
48 bytes, text/x-phabricator-request
Details | Review
Bug 1893950 - json-c build config updates for RNP 0.18.0. r=#thunderbuild
48 bytes, text/x-phabricator-request
Details | Review
Bug 1893950 - Exclude PQC and CRYPTO_REFRESH functions in librnp's symbols file. r=#thunderbuild
48 bytes, text/x-phabricator-request
Details | Review
Bug 1893950 - Update to RNP 0.18.0. r=#thunderbuild
48 bytes, text/x-phabricator-request
Details | Review
Bug 1893950 - Build changes for RNP v0.18.0. r=#thunderbuild
48 bytes, text/x-phabricator-request
Details | Review
rnp-public-api-new-in-v0.18.0.patch
142.84 KB, patch
Details | Diff | Splinter Review
Bug 1893950 - Incremental update to RNP 0.18.1. r=#thunderbuild
48 bytes, text/x-phabricator-request
Details | Review

Tracking bug for updating Thunderbird to the next major version after 0.17.x - probably that will be v0.18.0 ?
Let's adjust the subject of this bug accordingly, once more information about the version number is known.

Blocks: 1884337

This is also necessary to build with the latest clang (18+) since it includes the following PR: https://github.com/rnpgp/rnp/pull/2242

(In reply to Ihar Hrachyshka from comment #1)

This is also necessary to build with the latest clang (18+) since it includes the following PR: https://github.com/rnpgp/rnp/pull/2242

Could you please specify why it would require the latest clang? It's not what we are aimed for, and actually our CI runs on lower versions.

The need for newer clang support is because Nixpkgs lately bumped LLVM to 19: https://github.com/NixOS/nixpkgs/pull/357750/files for their unstable branch, which broke thunderbird build for the repo. I understand that upstream may still target an older version, but this bump will be needed for downstream consumers.

Nickolay, are there plans for the timing of a new major release version of RNP?

Flags: needinfo?(o.nickolay)

@Kai I was thinking about the late May-June, however if you have a need this could be adjusted, moving unfinished features from https://github.com/rnpgp/rnp/milestone/13 to the next milestone.
Main thing left for implementation is https://github.com/rnpgp/rnp/issues/2157 which should be helpful for Thunderbird as well.

Flags: needinfo?(o.nickolay)

Thanks for the update. It isn't urgent, but an update would be interesting.

RNP v0.18.0 was release in June.
https://github.com/rnpgp/rnp/releases/tag/v0.18.0

Now that we have Botan 3 upgrade available, I'd like to start experimenting with newer RNP locally.

I tried to upgrade locally, but there are several build failures.

Summary: [RNP] Update to next major RNP release, v0.18.0 ? → [RNP] Update to next major RNP release, v0.18.0

Hi Rob, it's great to see you around, thanks for your help with Botan 3 !
If you could help with an uplift for RNP too, that would be very much appreciated!

Flags: needinfo?(rob)

Our existing RNP vendor script still does a lot of things correctly.

In rnp.py I had to add two entries to the defines dict:
CRYPTO_BACKEND_LOWERCASE="botan",
CRYPTO_BACKEND_VERSION="3.8.1",

New file config.h.in contains statements for FALLTHROUGH_STATEMENT
and that didn't work right.

I had to set that manually to
#define FALLTHROUGH_STATEMENT [[fallthrough]]
to get past some build errors related to that.

Next, I see lots of undefined symbols (probably need to add new source files).

If there was a line like:
#define FALLTHROUGH_STATEMENT [[fallthrough]]
that cmake_define_files.py was processing, the output was wrong. These
lines need to be left as they are.

Assignee: nobody → rob
Status: NEW → ASSIGNED

The updated cmake_define_files.py processing allows setting json_c_strtoll and
json_c_strtoull correctly.

Runs RNP API header (rnp.h) through Clang's preprocessor to filter out
the PQC and CRYPTO_REFRESH functions.

HHG: changed python/thirdroc/thirdroc/rnp_symbols.py

(In reply to Kai Engert [:KaiE:] from comment #9)

Our existing RNP vendor script still does a lot of things correctly.

In rnp.py I had to add two entries to the defines dict:
CRYPTO_BACKEND_LOWERCASE="botan",
CRYPTO_BACKEND_VERSION="3.8.1",

New file config.h.in contains statements for FALLTHROUGH_STATEMENT
and that didn't work right.

I had to set that manually to
#define FALLTHROUGH_STATEMENT [[fallthrough]]
to get past some build errors related to that.

Next, I see lots of undefined symbols (probably need to add new source files).

The preprocessor that I hacked up to handle #cmakedefine lines didn't properly handle those FALLTHROUGH_STATEMENT #defines. That's the first patch. I fixed that, which helps with json-c's header file as well.

I didn't enable the new PQC and CRYPTO_REFRESH functionality in RNP 0.18.0. It shouldn't be a problem to add them later.

The RNP_VERSION_STRING_FULL now includes the crypto backend version: 0.18.0.MZLA.145.0a1.botan.3.8.1

Flags: needinfo?(rob)

Rob, I want to thank you very much, and you were able to do this so quickly!
This is really helping me a lot.

Before we consider landing, I need to check for behavior changes,
see my comment on the whiteboard.

Whiteboard: [Do not land yet. Needs review for behavior changes.]

It might be useful to experiment with the PQC and crypto-refresh functionality in Thunderbird, for learning purposes, to understand the scope of the implementation.

I'll attach some build changes that allowed me to build RNP v0.18.0 with it. They are an initial hack, and it probably should be done more cleanly in the future.

It is noteworthy that the code uses the word "experimental" for the #defines that enable the PQC and crypto-refresh code.

I assume that means the code is not yet ready for production releases.

Before Thunderbird could consider to use that code, we'd have to wait for an RNP release in which that functionality is enabled by default, and is no longer described as experimental.

(In reply to Kai Engert [:KaiE:] from comment #18)

It might be useful to experiment with the PQC and crypto-refresh functionality in Thunderbird, for learning purposes, to understand the scope of the implementation.

I'd like to clarify this part about PQC/crypto-refresh:
"experiment with local/developer builds, only"

(In reply to Kai Engert [:KaiE:] from comment #19)

It is noteworthy that the code uses the word "experimental" for the #defines that enable the PQC and crypto-refresh code.

I assume that means the code is not yet ready for production releases.

Yes, that is exactly why the word "experimental" is used. I further want to add that the PQC and "Crypto Refresh"/RFC9580 code is much more mature in several open pullrequests where the newest PQC draft version is implemented. The code in the main branch is based on an older version of the PQC draft, so you won't have compatibility with other PQC implementations.

This patch illustrates the changes/improvements in RNP's public API in version v0.18.0

Thanks Johannes. Based on your comment 22, it seems it is too early to experiment with the PQC code in v0.18.0

(In reply to Kai Engert [:KaiE:] from comment #17)

Before we consider landing, I need to check for behavior changes,
see my comment on the whiteboard.

The question I would like to answer:
Does v0.18.0 introduce any new behavior (by default) that is specified in LibrePGP, only?

If yes, I would need to find ways to disable it prior to upgrading to v0.18.0

(I'm already aware that RNP supports the LibrePGP OCB mode, that's not new in 18.
Thunderbird already disables the use of that mode.)

If the answer is no, then we can go ahead immediately with upgrading to v0.18.0

(I assume, when building with both RNP_EXPERIMENTAL_CRYPTO_REFRESH and RNP_EXPERIMENTAL_PQC undefined,
no functionality/behavior from RFC 9580 is enabled yet.)

Blocks: 1848103
Blocks: 1991329
No longer blocks: 1848103
See Also: → 1848103
Blocks: 1666599

I suggest that we upgrade Thunderbird Daily on November 11, at the beginning of the version 147 development cycle, and right after we do, we should ask the community to test and give feedback.

There is a bug in v0.18.0 that we shouldn't ship.

We will wait until after the v0.18.1 release.

Summary: [RNP] Update to next major RNP release, v0.18.0 → [RNP] Update to next major RNP release, v0.18.1

When the v0.18.1 is ready, I will be posting incremental patches on top of the existing ones, so we don't have to redo those.

(In reply to Kai Engert [:KaiE:] from comment #26)

I suggest that we upgrade Thunderbird Daily on November 11, at the beginning of the version 147 development cycle, and right after we do, we should ask the community to test and give feedback.

The v0.18.1 release is available.

Let's upgrade on December 9 or 10, at the beginning of the version 148 cycle.

The patch to update to 0.18.1 was created by executing
../../../mach vendor -r v0.18.1 ./moz.yaml

I saw an error message regarding rnp_export.h
I don't know what's wrong. I ignored it, because at this time there are no changed exports.

Then I created a diff between the release archives of 0.18.0 and 0.18.1 and compared that with the results of our upgrade script.
I confirmed both approaches produce the same code changes.

Assignee: rob → kaie
See Also: → 2001993

@Kai JFYI: I updated Botan version to 3.10.0 just because there were obsolete 2.x in the file. Any 3.x should be suitable (or 2.x if you need it), and at least 3.9.0 was run through CI on OpenSuse Tumbleweed.

Thanks Nickolay for the explanation.
We are already on version 3.8.1 in the latest TB releases.
We'll do bug 2001993 with a low priority.

FYI, I'm running this page here which is automatically updated and which shows the RNP and Botan versions we have on the TB branches:
https://kuix.de/mozilla/versions/

Attachment #9514639 - Attachment is obsolete: true

Pushed by kaie@kuix.de:
https://hg.mozilla.org/comm-central/rev/31361358d6be
Update cmake_define_files.py to properly handle #define lines. r=dandarnell
https://hg.mozilla.org/comm-central/rev/4cf6a88bd4cd
json-c build config updates for RNP 0.18.0. r=dandarnell
https://hg.mozilla.org/comm-central/rev/c9ffb1a3b7b3
Exclude PQC and CRYPTO_REFRESH functions in librnp's symbols file. r=dandarnell
https://hg.mozilla.org/comm-central/rev/7558b2162b7f
Update to RNP 0.18.0. r=dandarnell,kaie
https://hg.mozilla.org/comm-central/rev/43ec02dcbfb4
Build changes for RNP v0.18.0. r=dandarnell,kaie
https://hg.mozilla.org/comm-central/rev/022d025ff73b
Incremental update to RNP 0.18.1. r=dandarnell

Status: ASSIGNED → RESOLVED
Closed: 1 month ago
Resolution: --- → FIXED
Whiteboard: [Do not land yet. Needs review for behavior changes.]
status-thunderbird147: --- → wontfix
status-thunderbird_esr140: --- → wontfix
Target Milestone: --- → 148 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: