Skip to content

Update to the latest version of the crypto refresh#3

Open
lubux wants to merge 47 commits intotwiss:update-packet-versionsfrom
lubux:update-packet-versions
Open

Update to the latest version of the crypto refresh#3
lubux wants to merge 47 commits intotwiss:update-packet-versionsfrom
lubux:update-packet-versions

Conversation

@lubux
Copy link

@lubux lubux commented Jun 1, 2023

Includes the updates for the latest merged pull requests in the crypto refresh draft:

Further changes:

  • The PKESK constructor now sets the version to null
  • createSignaturePackets has an additional optional argument signatureSalts that allows to pass previously generated salts. This argument is used for signature creation with ops packets.
  • createSignaturePacket has an additional optional argument salt that allows to pass a previously generated salt.

twiss and others added 30 commits April 3, 2023 15:56
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`;
Compared to v5 keys, v6 keys contain additional length fields to aid in
parsing the key, but omit the secret key material length field.

Additionally, unencrypted v6 secret key packets don't include the count
of the optional fields, as per the updated crypto refresh. Since they
are always absent, the count is not needed.
Compared to v5 signatures, v6 signatures include a salt, and the
subpacket lengths are increased from 2 to 4 bytes.
Store key flags, features and preferences in a direct-key signature
instead of user ID signatures, for V6 keys.
The AEAD Encrypted Data packet has been removed from the draft
in favor of version 2 of the Sym. Encrypted Integrity Protected
Data packet. It also has a new feature flag to match.
This flag has been removed from the draft specification.
Also, set it as the preferred AEAD algorithm.
This subpacket replaces both symmetric algorithm preferences and
AEAD algorithm preferences when AEAD is supported, by providing
sets of preferred symmetric and AEAD algorithm pairs.

We still keep the symmetric algorithm preferences in case AEAD is
not supported.
Chrome's Web Crypto implementation doesn't support it, and it
seems unnecessary to list it when AES-256 is available.
This has been changed in the crypto refresh.
Key flags, expiration time, algorithm preferences, et cetera, are now
read from the direct-key signature instead of the primary User ID
binding signature for v6 keys.

This also requires a direct-key signature to be present for v6 keys.
The crypto refresh says that we MUST NOT reject messages where the
CRC24 checksum is incorrect. So, we remove the check for it.

Also, remove the checksumRequired config.
The latest crypto refresh specifies an HKDF step to be used for
deriving the key to encrypt the session key with.

It also specifies two additional length fields.
Instead of calling getPreferredAlgo('symmetric') and
getPreferredAlgo('aead'), we define and call getPreferredCipherSuite()
to determine the preferred symmetric and AEAD algorithm.

Additionally, we remove isAEADSupported(), instead we return
aeadAlgorithm = 0 from getPreferredCipherSuite() if AEAD is not
supported.

And finally, we define getPreferredCompressionAlgo() to replace
getPreferredAlgo('compression').
Rather than using the config to determine which algorithms to try
to decrypt session keys for, try the algorithm we know the message
was encrypted with.
Rather than padding in the PKESK packet, add padding inside the
public-key algorithms, so that we can later add algorithms without
any padding.

Additionally, simplify the constant-time decryption logic, by always
using the assumed algorithm, rather than doing a constant-time select
based on whether the decrypted algorithm identifier was correct.
@lubux lubux force-pushed the update-packet-versions branch 2 times, most recently from 7eccf32 to 0e7f20c Compare June 5, 2023 13:45
Introduces v6 one-pass signature packets required for v6 signatures.
Includes the changes from !305 of the crypto refresh:
https://gitlab.com/openpgp-wg/rfc4880bis/-/merge_requests/305
@lubux lubux force-pushed the update-packet-versions branch from 0e7f20c to 09f5d0f Compare June 6, 2023 06:13
lubux added 5 commits June 7, 2023 10:22
Implement the changes from !304 of the crypto refresh:
https://gitlab.com/openpgp-wg/rfc4880bis/-/merge_requests/304

"Introduces an explicit length octet for the fingerprint.
This length also covers the key version number.  This has the
advantage of nicely covering the anonymous recipient case, where a
zero length indicates not having a fingerprint in the packet without
having to resort to a fictitious key version zero."
The latest version of the crypto refresh (i.e., !313, !314) specifies that
the  "Hash" header is depricated. An implementation that is verifying
a cleartext signed message MUST ignore this header.
Thus, the header check can be removed.
This commit does not change the writing part of cleartext messages.
@lubux lubux force-pushed the update-packet-versions branch from 09f5d0f to c261097 Compare June 7, 2023 08:23
The latest version of the crypto refresh (i.e., !313, !314) specifies that
the "Hash" header is depricated. This commit changes that the Hash header
is only generated if a cleartext message contains a non-V6 signature.
@larabr larabr force-pushed the update-packet-versions branch 3 times, most recently from 891d3e7 to 7723b60 Compare July 31, 2023 13:49
@larabr larabr force-pushed the update-packet-versions branch 3 times, most recently from 7eceef2 to d051167 Compare August 3, 2023 15:32
const sizeFields = bytes[offset++];
if (sizeFields !== 0) {
this.publicKeyVersion = bytes[offset++];
const fingerprintLength = this.publicKeyVersion ? (this.publicKeyVersion >= 5 ? 32 : 20) : 0;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const fingerprintLength = this.publicKeyVersion ? (this.publicKeyVersion >= 5 ? 32 : 20) : 0;
const fingerprintLength = this.publicKeyVersion >= 5 ? 32 : 20;

public key version of 0 has been removed in favour of the explicit size

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and we could actually use the sizeFields - 1 value instead of hardcoding the fingerprint length

];

if (this.version === 6) {
if (this.publicKeyFingerprint !== null) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might be easier to follow what's going on by referencing the wildcard keyID?

Suggested change
if (this.publicKeyFingerprint !== null) {
if (!this.publicKeyID.isWildcard()) {


if (this.version === 6) {
const saltLength = saltLengthForHash(this.hashAlgorithm);
if (this.salt === null) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This conditional regeneration seems risky; to double check if needed.

return true;
};

function verifyHeaders(headers) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can now inline this

@larabr larabr force-pushed the update-packet-versions branch from d051167 to 79e45df Compare August 7, 2023 18:38
@larabr
Copy link

larabr commented Aug 7, 2023

I've cherry picked the changes to the target branch manually 👍 We'll also continue the review on the original PR since I cannot push to lubux's branch

@larabr larabr force-pushed the update-packet-versions branch 2 times, most recently from e1a4816 to 2231abd Compare August 7, 2023 20:08
@larabr larabr force-pushed the update-packet-versions branch 4 times, most recently from 7498223 to 2a67f02 Compare September 1, 2023 14:18
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.

3 participants