Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

V6 #1629

Draft
wants to merge 129 commits into
base: main
Choose a base branch
from
Draft

V6 #1629

wants to merge 129 commits into from

Conversation

twiss
Copy link
Member

@twiss twiss commented Apr 3, 2023

Breaking changes:

  • Remove embedded Web Streams ponyfill, since it's now supported in all browsers (applications can load a polyfill themselves instead, if they need to support older browser versions).
  • Drop support for Node 14 (EOL end of April '23)
  • ...

(Further changes to be added via PRs to this branch.)

TODO:

Includes changes from sub-PRs:

twiss and others added 18 commits October 25, 2023 12:53
Require the application to load a polyfill instead.
In terms of API, this feature is backwards compatible, no breaking changes.
However, since a Wasm module is loaded for the Argon2 computation, browser apps
might need to make changes to their CSP policy in order to use the feature.

Newly introduced config fields:
- `config.s2kType` (defaulting to `enums.s2k.iterated`): s2k to use on
password-based encryption as well as private key encryption;
- `config.s2kArgon2Params` (defaulting to "uniformly safe settings" from Argon
RFC): parameters to use on encryption when `config.s2kType` is set to
`enums.s2k.argon2`;
…s` to package.json

Mocha v10 requires the lib to be esm compliant.
ESM mandates the use of file extensions in imports, so to minimize the
changes (for now), we rely on the flag `experimental-specifier-resolution=node`
and on `ts-node` (needed only for Node 20).

Breaking changes:
downstream bundlers might be affected by the package.json changes depending on
how they load the library.
NB: legacy package.json entrypoints are still available.
Also, init config before any code is run in tests
…e.js

Unclear why the Node tests fails, but we're planning to drop support
fflate already supports ESM and is actively maintained
…to asmcrypto only if key size is not supported

CFB decryption is too slow using WebCrypto (CBC mode), since every block needs to be decrypted separately
WebCrypto performance is now on-par or better than non-native libs even for small messages
larabr and others added 6 commits February 27, 2024 14:43
Fallback needed for AES192, due to missing Chromium support.
The module was barely used, and its presence confusing, since
WebCrypto or asmcrypto are often directly used and usable instead.
Also, use AES_CBC instead of AES_ECB for single-block encryption,
so that we can drop support for the latter in the asmcrypto lib.
asn1.js is a fairly large lib and was simply needed to handle DER encodings in
some NodeCrypto operations.
This change replaces the dependency by moving to:

- JWT encoding for RSA (support added in Node v15)
- a much lighter dependency (eckey-utils) for ECDSA, where JWT cannot be used
for now, as Node has yet to add decoding support for Brainpool curves.

The change also allows us to drop BN.js as a direct dependency, optimising the
BigInteger-related chunking in the lightweight build.
…esh tests, and compare with gopenpgp v3

sop-openpgpjs did not correctly apply the `OPENPGPJS_PATH` env variable; as a result,
it did not actually test the code from either the PR and base branch, but always from
the hardcoded version bundled with it.
GPG v2 fails to parse detached signatures without the checksum
larabr and others added 5 commits March 22, 2024 17:10
The cleartext session key symmetric algorithm was accidentally included in the packet.
As a result, the generated messages may fail to parse and/or decrypt in other implementations.
The messages would still decrypt successfully in OpenPGP.js, due to an overly permissive parsing procedure,
which simply discarded the unused additional byte.

We know also throw on unexpected cleartext symmetric algo in PKESK v6.
…instead (#1736)

Unclear motivation for adding the original config option; if an expiration is there, it should
be honoured.

Breaking change:
the option used to default to `false`, and ignore revocation expirations. We now honour
those expirations, namely match the behaviour resulting from setting the option to `true`.
…terministicSignaturesViaNotation` to disable feature (#1737)

EdDSA is known to be vulnerable to fault attacks which can lead to secret key
extraction if two signatures over the same data can be collected. Randomly
occurring bitflips in specific parts of the computation might in principle
result in vulnerable faulty signatures being generated.
To protect signatures generated using v4 and v5 keys from this possibility, we
randomise each signature by adding a custom notation with a random value,
functioning as a salt. 
For simplicity, we add the salt to all algos, not just EdDSA, as it may also
serve as protection in case of weaknesses in the hash algo, potentially
hindering e.g. some chosen-prefix attacks.
v6 signatures do not need to rely on this, as they are non-deterministic by
design.

While this notation solution is interoperable, it will reveal that the
signature has been generated using OpenPGP.js, which may not be desirable in
some cases.
For this reason, the option `config.nonDeterministicSignaturesViaNotation`
(defaulting to true) has been added to turn off the feature.
The implemented profiles do not work on v5, hence for now they need to be manually
disabled in the config of 'sop-openpgpjs-main'.
larabr and others added 8 commits April 9, 2024 17:12
We need to include the checksum to work around a GnuPG bug where data fails to
be decoded if the base64 ends with no padding chars (=) (see https://dev.gnupg.org/T7071).
Pure v6 artifacts are unaffected and won't include the checksum, as mandated by
the spec.

Breaking change:
`openpgp.armor` takes an additional `emitChecksum` argument (defaults to
false).
NB: some types of data must not include the checksum, but compliance is left as
responsibility of the caller: this function does not carry out any checks.
Refer to the crypto-refresh RFC for more details.

---------

Co-authored-by: Daniel Huigens <d.huigens@protonmail.com>
The `config` input was not passed down to the armor function due to an oversight.
…urther relax constraints (#1739)

We relaxed constraints in a previous commit, but excluded unicode chars, which are however allowed in v5.

We now drop almost all email address constraints, by primarily rejecting
control and spaces char classes.
Library users are strongly encouraged to implement additional checks as needed,
based on their supported email address format.

NB: the validity checks in question affect the userID inputs accepted by e.g.
`generateKey` and `reformatKey`, not the values parsed from existing entities,
e.g. using `readKey` (where almost no validation is performed).
…Protect`

The logic was updated in github.com//pull/1678 .
The tests worked anyway thanks to the config option matching the (monkey patched)
keys' feature flags, which are the deciding factor for whether to use AEAD.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants