dealing with forked/scoped dependencies (asmcrypto.js and elliptic in particular)

[dkg]

I've been looking at the dependencies OpenPGP,js as i'm trying to help people get OpenPGP.js into Debian for wider distribution. I would really like to make it easy for any Debian (or Ubuntu or other downstream) user to just use the system package manager to get OpenPGP.js tooling available locally, in particular for utility-style interfaces like sop-openpgpjs. I'm basing this report on OpenPGP.js v5.5.0 and the current main branch.

I'm concerned that OpenPGP.js's reliance on forked versions of other modules via scoped modules makes it more difficult to package and cleanly redistribute, and might hide some security problems. At a minimum, i'd hope that we could have useful/clear version numbering and tagging schemes, but i'm also wondering what work needs to be done to get any changes from scoped modules upstreamed to the original modules, so that OpenPGP.js can go back to relying on the standard global modules instead of scoped modules.

Some concrete suggestions that are hopefully useful:

• clarify version numbers and how they are are related between scoped and "global" modules
• differentiate git tag naming schemes so that scoped modules can have distinct tags from the global module they derive from, even if their version numbers happen to be identical.
• identify each outstanding difference between the scoped module and its "upstream" global module, and try to get that change upstream, either with a PR or an open github issue. If upstream rejects the change, fine, but at least there's a clear, public discussion about why.
• keep scoped modules up-to-date with any releases of upstream modules, especially with respect to security-related fixes.
• if all changes in a scoped module are adopted by the corresponding upstream global module, revert OpenPGP.js to depend on the global module and mothball the scoped module.

Looking at the devDependencies in package.json, i think that the following scoped modules are effectively forked by OpenPGP.js (if i'm wrong, please correct me, my understanding of the javascript ecosystem is limited):

    "@openpgp/asmcrypto.js": "^2.3.2",
    "@openpgp/elliptic": "^6.5.1",
    "@openpgp/jsdoc": "^3.6.4",
    "@openpgp/pako": "^1.0.12",
    "@openpgp/seek-bzip": "^1.0.5-git",
    "@openpgp/tweetnacl": "^1.0.3",
    "@openpgp/web-stream-tools": "^0.0.11",

I'll address the rest of my comments and examples to asmcrypto.js and elliptic, which i've looked into more, but similar concerns likely apply to the other scoped/forked modules.

asmcrypto.js

Looking concretely at @openpgp/asmcrypto.js, i think that's referring to https://github.com/openpgpjs/asmcrypto.js (i'll call that "forked asmcrypto"), which is a fork of the original asmcrypto.js, https://github.com/asmcrypto/asmcrypto.js (i'll call this one "upstream asmcrypto") -- but reviewing the git history of both projects, i surmise a few things:

• upstream asmcrypto's latest version is v2.3.2, but it was released back in 2018. Maybe it is abandoned? there are a handful of commits on upstream's main branch after v2.3.2 (including several from protonmail's own @danrr)
• forked asmcrypto's v2.3.2 (commit 62f097803ae0d3885ca011d5caf96adc66090388) is different from upstream's v2.3.2 (commit 62f097803ae0d3885ca011d5caf96adc66090388). there is no git tag for the forked v2.3.2
• forked asmcrypto has a newly-released version v2.3.3-0, released by @larabr, which includes a few changes on top of v2.3.2.

One simple concern here is the version numbering: v2.3.3-0 is odd and challenging for debian. In particular, Debian assumes that the upstream version number does not have a hyphen (-) in it, because Debian uses the hyphen (-) to distinguish the upstream version from the debian revision. Technically, debian can handle having a hyphen in the upstream version, but it makes the rest of the tooling quite awkward to have an upstream version number like this. Is there a reason to use 2.3.3-0 instead of just using 2.3.3 ?

Another concern is git tag names -- if the project tag naming scheme is identical (i.e. they both tag version x.y.z with vx.y.z, then any git repo that tracks both projects will have tag collisions, since there can't be two tags both named vx.y.z. if the forked project issued tags with a different prefix (e.g. openpgp/vx.y.z) that might make it easier to track the work. That said, i don't see any tags related to

elliptic

Looking at @openpgp/elliptic, i'm comparing the forked version with the upstream version. I note:

• there have been several releases of upstream elliptic that have had parallel releases of the forked elliptic: 6.4.1, 6.5.0, and 6.5.1
• the forked version of elliptic does not appear to have any git tags of its own.
• upstream elliptic has had several releases (v6.5.2, v6.5.3, and v6.5.4) which do not have a corresponding release in the scoped module, including what appear to be security-related fixes regarding signature malleability and point validation.

What kind of expectations should we have about the scoped module pulling in security fixes from upstream? There's no issue tracker at https://github.com/openpgpjs/elliptic so it's unclear how to report the lack of a new version with the corresponding fixes.

[twiss]

Hey dkg 👋 Thanks for the comments and questions. I'll try to answer some of the questions and then give some high-level thoughts at the end.

---

First of all, some of the upstream projects do have open PRs, that weren't merged for one reason or another: e.g. asmcrypto/asmcrypto.js#155, indutny/elliptic#144, and jsdoc/jsdoc#1770.

Then, some others have mostly build-time-related changes, that make them more suitable for use in OpenPGP.js, but don't have much value for upstreaming. For example, the changes to tweetnacl.js are just about making the resulting bundle smaller, by removing stuff we don't need. Some of these are also only important for the browser bundle, which might not be important for Debian, if the intended use case is to run OpenPGP.js on Node.js? Not that I'd necessarily recommend using the upstream version instead, but - some of these packages aren't used at runtime at all; for example, jsdoc is used to generate documentation. Perhaps it would be possible to get away without packaging some of these? (More on this below.)

Finally, @openpgp/web-stream-tools is an exception in that it's not a fork, it's a package that was created by us, intended to contain some generic stream-related helper functions.

---

Regarding the versioning scheme, please note that we consider the versions published on npm as "authoritative", rather than the git tags. On npm, the versions are specific to the package, while in git, the fork initially also contains the tags of upstream, so the namespace is shared to some extent, which could make looking at git tags confusing. Also, normally the git tags are the same as the npm versions (but in the forks we don't create them to avoid confusion) - of course we could do something different manually, but I would prefer to look at the (npm package, version) tuple rather than use git tags to specify that the version is the fork (since the repo and package name already indicate that).

Then, my philosophy for the versioning is: for any upstream package X@vY, the fork @openpgp/X@vY should be the same, plus any OpenPGP.js-specific patches. This means that if version X@vZ doesn't exist, we can't / shouldn't publish @openpgp/X@vZ, because we don't know what it will be yet. So, it should be a prerelease, which in semver has the syntax @openpgp/X@vZ-0 (or we could also use @openpgp/X@vZ-openpgp.0, but the former is the default).

For Debian, I'm not sure what the usual syntax for a prerelease would be, but maybe it can be converted to that? Or 2.3.3-0 could be converted to 2.3.3.0, since a semver will never have three dots. That might be confusing if 2.3.3 is ever released, though.

---

Regarding elliptic: we only use it for curves that aren't supported by the native crypto API nor by tweetnacl.js. On Node.js, that should be none of them. On the web platform, that's the brainpool curves and secp256k1, and since elliptic isn't constant-time, we warn against using those curves. But you're right that we should update the fork, regardless. If a vulnerability affects OpenPGP.js, you can open an issue here, but even if not (or we're not sure), it can be discussed in this repo.

---

As I alluded to, not all of the dependencies are actually used at runtime. That's especially true for the Node.js build, which can use Node.js Crypto API, which is more extensive than the Web Crypto API. We check at runtime whether certain algorithms are available, but if the Node.js runtime is known (as it might be in Debian?) then perhaps some of the dependencies aren't needed.

For the web build, we've done quite a lot of work to ensure that code that isn't needed, isn't included in the build. For the Node.js build, we haven't done that work, on the assumption that disk space is cheaper than bandwidth. But, if it's valuable for Debian, it might be worth looking into which dependencies are actually used, as part of the packaging process.

Another thought: is there any chance the dist build could be published as a binary package (either as-is, or after trimming the unused dependencies contained within it)? That one has very few external dependencies, basically only Node.js core libraries and asn1.js.

---

Finally - I've been thinking separately that we should at some point aim to reduce our number of dependencies a bit. In an ideal world, Web Crypto would provide all the cryptographic primitives we need, and it would be supported everywhere (Node.js does have an experimental implementation now) - simplifying our build and packaging story. We can probably also remove some of the polyfills, such as those for Web Streams and BigIntegers, at some point, and tell applications to polyfill them if they need to support old browsers. Perhaps we can push for that in v6, although - if you want to package OpenPGP.js now - that won't help you, of course.

Out of curiosity, can I ask what's the concrete impetus, at the moment? Is there an application using OpenPGP.js that you want to package?

[dkg]

Thanks for the prompt and thoughtful replies, @twiss! You've already cleared up a lot for me. A few replies here:

Regarding elliptic: we only use it for curves that aren't supported by the native crypto API nor by tweetnacl.js. On Node.js, that should be none of them.

My understanding is that packaging here for debian would be reliant on node.js. If that's right (hopefully @pravi can help clarify this). If that's right, i think what you're saying is that we could drop the dependency on elliptic for debian packaging entirely.

So, it should be a prerelease, which in semver has the syntax @openpgp/X@vZ-0 (or we could also use @openpgp/X@vZ-openpgp.0, but the former is the default).

Ah! I didn't know that semver used -0 for a prerelease, thanks! I see it now as item 9 on https://semver.org/ . For debian, prereleases use a ~ delimiter, so we'd probably want to translate your 2.3.3-0 to 2.3.3~0 as the "upstream version".

is there any chance the dist build could be published as a binary package (either as-is, or after trimming the unused dependencies contained within it)?

My understanding is that this would be against debian policy -- we want only free software in debian, that is buildable from source by the toolchains already in debian. To ensure that we meet that goal, we need to actually build from the preferred form of modification, which is the source code, not the dist build.

Web Crypto would provide all the cryptographic primitives we need, and it would be supported everywhere (Node.js does have an experimental implementation now) - simplifying our build and packaging story.

This would be great, from my perspective, though i don't know enough about how the different parts of the ecosystem are moving at this point to know how plausible it is for the near term. If the work we're doing to get OpenPGP.js into debian helps push that kind of simplifying transition along, i think that would be an overall win, both for Debian and for OpenPGP.js.

Out of curiosity, can I ask what's the concrete impetus, at the moment? Is there an application using OpenPGP.js that you want to package?

At the moment, i'm most interested in getting sop-openpgpjs into debian, because i'm in the process of trying to convert various tools that don't need anything more than SOP to using SOP instead of bits and pieces of GnuPG or arbitrary perl code. We already have three SOP implementations in debian (sqop, gosop, and pgpainless-cli), but having more compatible tooling from the leading implementations ensures a healthier ecosystem.

Long term, if we can land these cleanly in a maintainable way in debian and its derivatives, it'll make it possible to run things like the OpenPGP interoperability test suite against widely-distributed versions, which can help the ecosystem determine inflection or transition points (e.g., it is reasonable to generate SEIPDv2 encrypted data by default, regardless of the Feature flags subpacket in recipient keys). That kind of thing is probably more than a year off (if that), but we need to get the pieces in place now if we want it to be feasible to do in the future.

[twiss]

My understanding is that packaging here for debian would be reliant on node.js. If that's right (hopefully @pravi can help clarify this). If that's right, i think what you're saying is that we could drop the dependency on elliptic for debian packaging entirely.

Yes, I think that's right (if the version of Node.js in Debian supports all relevant curves, which I assume it does). It might not be entirely trivial to actually remove it, and it might be necessary to adapt the build system slightly if it's not there - but I think it should be possible.

My understanding is that this would be against debian policy -- we want only free software in debian, that is buildable from source by the toolchains already in debian. To ensure that we meet that goal, we need to actually build from the preferred form of modification, which is the source code, not the dist build.

I see, alright. I see that rollup is already in Debian, so I guess this might be more feasible than I thought. Still, I think this might be quite a lot of work. I say that not to discourage you, but to say that some work on the OpenPGP.js side might be needed to make it easier - while we have a goal of minimizing the dependencies that end up in the build (to save bandwidth), minimizing the build-time dependencies hasn't been an active goal, until now. I'm not sure if we have the bandwidth for this very soon, but we can keep it in mind for v6. Contributions to this would also be welcome, of course.

[dkg]

I'm looking at the divergence between pako and @openpgp/pako as well now, since debian is already shipping pako 2.0.4, and the scoped module is significantly older (based on 1.0.11) -- is this divergence something that is needed by OpenPGP.js?

The "modernize" changes look like they might already be handled upstream (e.g. 505e36d9f021a27f14e02d46a84db307a2f8150f, "Move to es6"), but i'm not sure why OpenPGP.js might need 7d389f04d6334d96dcd6cd2f6171864177039640 ("Export constants individually instead of as an object") or f38f7368a5fa511e54b95add2f04444c3a9d803f ("Switch back from class properties to instance properties").

[twiss]

I don't think you need pako at all, actually. On Node.js, we use the built-in zlib module.

(The changes you mentioned were to make the build smaller, and more compatible with certain browsers, respectively. We might be able to use upstream, but it seems they're still not using ES6 modules. Those initial changes weren't made by us, which is why we didn't make a PR.)

[pravi]

@dkg for command line usage we will be using nodejs and sid currently has nodejs 18.11.

@twiss providing rollup.config.js to build would help us a lot. Sticking to widely adopted build tools will be appreciated, instead of trying to use the latest tool in town. So if there is a rollup.node.config.js and rollup.browser.config.js that does node and browser builds separately, that would be awesome. Also if you document dependencies that is only needed for browser build, that saves us some time and effort too. May be down the road some apps might need the browser build too, but as a start I think we can focus on nodejs.

[twiss]

There is already a rollup.config.js :) And you can build the Node.js bundles only using npm run build --build-only=node, or you can specify exactly the build file you want.

[pravi]

Thanks, in general we prefer rollup.config.js over a custom build.js as we already know how to adapt a rollup.config.js but may need to adapt each build.js separately. For example in asmcrypto.js, it'd be great if we can have a rollup.config.js. Currently there is build.js but it needs esm module to be packaged. Even though nodejs 18.11 supports esm, we can't launch it from command line easily. We will probably have to transpile build.js to cjs first and then run it. Would it be possible to provide a rollup.config.js for asmcrypto.js so we can build it easily with rollup?

[pravi]

nodejs/node#41136 (comment) gives another idea

[twiss]

Ah, I see. Could you request this to upstream first? I assume you'll run into the same issue if someone uses upstream asmcrypto.js, and we can then adopt those changes in the fork, if they're made upstream.

[pravi]

Also we want corresponding source code to any generated files we ship, so tags matching the npmjs.com releases will help us a lot. Alternatively if you include the source in the npmjs dist tarballs, we can just remove the generated files and regenerated them during the deb creation. But that will increase size of your modules. Adding tags would be the efficient and convenient option here. or if you just provide source tar balls corresponding to npmjs releases somewhere else, that can work too.

[pravi]

If upstreams are inactive due to lack of time, they may be open to sharing access to openpgp contributers if we try contacting them?

[dkg]

I do think it could be useful to try to assist and lobby upstream for the scoped+forked modules, to help them adopt the changes that are useful for OpenPGP.js, as long as they don't break upstream's existing API.

If an OpenPGP.js change breaks upstream's existing API, i think that will be a lot harder to convince upstream of.

[twiss]

For OpenPGP.js, we already have tags matching the npm releases. We could add tags for the forks as well, yeah 👍

For asmcrypto.js, our changes are backwards compatible. For most of the others they should be too, but some of them there might be some differences in how to import the module, for example. Tweetnacl.js is a major exception as we mostly removed stuff we didn't need, so those changes are of course neither backwards compatible nor useful for upstream.

[dkg]

Where the changes require a different form of importing the module, it sounds like we might also need to float some patches in OpenPGP.js itself, to be able to import the upstream version of the module instead of importing the scoped+forked version. is that right?

[twiss]

Yeah, probably. The scoped name is also mentioned in imports in the code, so you might also have to revert some of this, for example.

[dkg]

reviewing other scoped modules :

jsdoc

jsdoc/jsdoc#1770 looks like something that we might not want or need in terms of debian packaging. To the extent that our packaging builds documentation with jsdoc, we probably want to ship debian-built documentation so that debian users have a local copy of the documentation that matches the version of openpgp.js that they're using.

The only other difference is openpgpjs/jsdoc@ff7cb39 (which just lists global functions first in the generated doc navigation), which doesn't appear to be submitted upstream at all, and also doesn't seem critical. Seems like maybe this should be submitted upstream as a configuration option.

So for debian, if we do plan to generate documentation from the package, we might prefer to just patch out the configuration bits that use externalSourceLinks, and use upstream jsdoc instead of the scoped version.

seek-bzip

The main differences here are:

• openpgpjs/seek-bzip@3aca608 (switch from Node Buffers to Uint8Arrays)
• openpgpjs/seek-bzip@4b89457 (Export all objects separately)

Neither of these changes appear to be in any PR upstream -- are they not relevant for upstream?

Assuming that we're working with Node, we probably don't need the first change; and the second change appears to be intended specifically to improve code minimization, which again isn't the top priority for debian packaging (though it would be nice if seek-bzip upstream were to adopt it). This suggests to me that we can also use upstream seek-bzip, and don't need the scoped version of the module.

Summary

For Debian packaging assuming a node-based deployment, it looks to me like:

• we do need to package the scoped+forked @openpgp/asmcrypto.js and the scoped (not-forked) @openpgp/web-stream-tools.
• We can outright skip pako and elliptic.
• We can use the upstream versions of tweetnacl (1.0.3 is already in debian) and seek-bzip.
• And we can either skip jsdoc (if we don't want to build the documentation at all) or we can use upstream jsdoc and patch the config to avoid externalSourceLinks.

[twiss]

Neither of these changes appear to be in any PR upstream -- are they not relevant for upstream?

cscott/seek-bzip is meant for use in Node.js, per the documentation, so we assumed not.

For Debian packaging assuming a node-based deployment, it looks to me like: (...)

I think that's right 👍

It might go without saying but please check, after building with upstream versions, that all the tests still pass (with npm test) - to make sure that we aren't missing anything.

[dkg]

yes, we will definitely run the test suites with whatever changes end up being made.

[Sanddrah]

Hello, I am trying to build asmcrypto.js package for Debian, but I am currently encountering an error after making modifications to leverage nodejs' esm support rather than use esm library.
Your help will be highly appreciated, find my build logs here: https://bin.disroot.org/?a0e6f1d83ede4d65#14Vb8QhATc2mjMjwpj3wLnW7WivtbL9NrfGZni13Gudi
My changes here: https://salsa.debian.org/sandra_uwah/node-openpgp-asmcrypto.js/-/blob/master/debian/patches/use-esm-modules.patch

Best case scenario, could you please add esm support to openpgpjs packages, particularly asmcrypto.js :)?

[Sanddrah]

Thanks for your reply. I've done that and it threw a new error for me. Don't know if it's something serious. Here's the log: https://bin.disroot.org/?b4669dc032146ff2#Er5c42kwYwdCQ8tSQhCcoSysBbFk1g9hahfi62HjRP9T

[twiss]

Cannot find name 'process'. Do you need to install type definitions for node? Try npm i --save-dev @types/node.

I think this means you're missing some package, possibly node-typescript-types?

[Sanddrah]

Thank you so much, I've been able to build the package and I didn't see any errors

[Sanddrah]

Hello @twiss .Thanks for your help so far. I'm trying to run the test for asmcrypto.js but some are failing. Any idea how I can fix this?. Here's my log : https://paste.debian.net/1263297/

[Sanddrah]

Hello @twiss . I am trying to build @openpgpjs/web-stream-tools for Debian but I'm getting an error of directory import. I've tried to import the full file path but I still gt the same error. Please any help on how I can resolve this?
Here are my logs: https://paste.debian.net/1264204/