Age | Commit message (Collapse) | Author |
|
- We use this in our API, and re-exporting it here makes it easy to
use the correct version of the crate in downstream code without
having to explicitly depend on it.
|
|
|
|
- Not only was the heap allocation superfluous, it also leaked
secrets into the heap.
|
|
- The PacketHeaderParser returns erased BufferedReaders anyway, so
we might as well do it early. This avoids any accidental
specialization and hence code duplication.
|
|
- For each packet type, add a private function
`from_buffered_reader`.
- Implement `Parse::from_reader` and `Parse::from_bytes` in terms of
`from_buffered_reader`. For `Parse::from_bytes`, this means that
we can wrap the input in a `buffered_reader::Memory`, which is
much faster than a `buffered_reader::Generic`, which we use now.
- Note: `PacketParserBuilder` and by extension `Cert` already
implement this optimimzation.
|
|
|
|
- If the `PacketParser` encounters junk (i.e., corruption) and is
able to find a valid packet within `RECOVERY_THRESHOLD` bytes of the
end of the last valid packet, it recovers by converting the junk to
an `Unknown` packet, and continuing to parse.
- Extend this recovery mechanism to junk at the end of the file. If
the `PacketParser` encounters up to `RECOVERY_THRESHOLD` bytes of
junk at the end of the file, convert that data into an `Unknown`
packet instead of immediately returning an error.
- By returning an `Unknown` packet instead of an error, we also
return the last buffered packet, which was otherwise lost.
- When converting `RECOVERY_THRESHOLD` bytes of junk into an
`Unknown` packet, queue an error (in `PacketParserState`) so that
the next call to `PacketParser::next` will not continue trying to
parse the input, but return an unrecoverable error.
- Fixes #967.
|
|
- When tracing the execution of a `PacketParser`, don't emit the
`BufferedReader`, as this can result in a huge amount of unreadable
output.
|
|
- Make `PacketParser::plausible_cert` generic over the cookie so
that it is usable with generic `BufferedReader`s.
|
|
|
|
- Make `hash_update_text` a method on `HashingMode<Digest>`,
`HashingMode<Digest>::update`.
|
|
- `HashingMode` is mostly used by `HashedReader`.
- Move the `HashingMode` declaration and implementation from
`parse.rs` to `parse/hashed_reader.rs`.
|
|
|
|
- While we correctly ignored marker packets in the CertParser, we
did not ignore them in the CertValidator. This made sq inspect
complain about marker packets in certrings.
|
|
- RFC 4880 explicitly allows the use of v3 signatures, but adds:
> Implementations SHOULD accept V3 signatures. Implementations
> SHOULD generate V4 signatures.
- In practice, rpm-based distributions are generating v3 signatures,
and it will be awhile before we can actually stop supporting them.
https://bugzilla.redhat.com/show_bug.cgi?id=2141686#c20
- Add support for parsing, verifying, and serializing v3
signatures (but not v3 certificates, and not generating v3
signatures!).
|
|
- Convert `encrypted` to `processed`.
- Since `set_encrypted` is internal API it was directly renamed without
forwarder stub.
- `encrypted()` is public API thus the old function is converted to a
forwarder of the negation of `processed()`.
- `unprocessed()` marked as deprecated.
- Update docs and NEWS file.
- Fixes #845.
|
|
- Rename `iv_size` to `nonce_size`.
- Introduce `iv_size` that forwards to `nonce_size` for compatibility
reasons.
- Change all calls to `iv_size` to `nonce_size`.
|
|
|
|
- Even though the documentation warns that this function returns
rich errors that must not be returned to the user, and the mid-level
streaming decryption's API prevents leaking rich errors, including
decrypted data in the error message seems dicey.
|
|
|
|
- Currently, if we don't understand a compression algorithm, parsing
a compressed data packet fails and it is turned into an Unknown
packet. This is rather unfortunate, and deviates from what we do
for the encryption containers.
- Encryption containers are either not decrypted, in which case they
have a Body::Unprocessed, decrypted with Body::Processed, or
decrypted and parsed Body::Structured.
- Likewise, if we don't understand a compression algorithm, we
should simply return a compressed data packet with an unprocessed
body. This change does exactly that.
- Fixes #830.
|
|
|
|
- This goes back a long way, to
e304deb0fc7a92801cf3ba58aafeb14ce2301aed where the flag was called
`decrypted`, and every packet but SEIP had decrypted set to
`true`. At some point, we inverted the flag, but for some reason
decided to mark Unknown packets as encrypted, which makes no
sense, and changing it doesn't seem to break
documented (i.e. tested) behavior.
|
|
- In order to deal with version 2 SEIP packets, we first need to be
explicit about the packet version in the message parser.
- Rename the token and grammar rules, pass in a version to
MessageParser::push.
|
|
- Do this using a deprecation so that anyone using it will get a
compiler warning.
- Revert this change once message::Token is private.
- See #836.
|
|
- There can be other reasons why parsing the packet fails. Notably,
any use of php_try! may return an unknown packet with an arbitrary
error type and value.
|
|
- Add S2K::parse_common that optionally takes an S2K octet count
parameter. We'll use that for v5 packet parsing where we now the
size of the S2K object we're parsing.
|
|
|
|
- We implement the cleartext signature framework by transforming the
message on the fly to a signed message, then using our parsing
framework as usual. However, we need to tweak the behavior
slightly.
- Notably, our CSF transformation yields just one OPS packet per
encountered 'Hash' algorithm header, and it cannot know how many
signatures are in fact following. Therefore, the message will not
be well-formed according to the grammar. But, since we created the
message structure during the transformation, we know it is good,
even if it is a little out of spec.
- This patch tweaks the streaming verifier's behavior to accommodate
this.
|
|
- Introduce a trait that schedules nonce and additional
authenticated data for each AEAD chunk.
- Factoring that out allows us to support different schemes, and
decouple memory encryption from the OpenPGP schedules.
|
|
- Previously, during parsing and serialization, OpenPGP's unsigned
32-bit timestamps were converted to Rust's SystemTime, which uses
time_t. On platforms where that is a signed 32-bit value, the time
was truncated. See #668.
- One way to fix that is to make Rust's SystemTime independent of
time_t. See https://github.com/rust-lang/rust/issues/44394.
- The other way is not to convert to SystemTime at the API
boundary. See
https://gitlab.com/sequoia-pgp/sequoia/-/issues/806.
- This fixes handling during parsing and serialization, but doesn't
address the API issue.
- Fixes #802.
|
|
|
|
- Replace manual implementations of Default where the derive(Default)
is identical.
- Suggested by clippy,
https://rust-lang.github.io/rust-clippy/master/index.html#derivable_impl
|
|
- Continuation of e6a335b93a10620bcb7cbfa32e232949758f0c99.
|
|
- Instead of creating a default struct and immediately afterwards
changing a field, use this type of initialization syntax:
Struct { field: value, ..Default::default() }.
- Suggested by clippy::field_reassign_with_default.
|
|
- Found by clippy::redundant_slicing.
|
|
- Use range syntac instad of manual comparisons. This is arguably
better to read.
- Found by clippy::manual_range_contains.
|
|
- In CamelCase, acronyms count as one word. Apply this rule where
API and lalrpop are not impacted.
- Found by clippy::upper_case_acronyms.
|
|
- Fixed with the help of clippy::needless_borrow.
|
|
- When we stream packet bodies, we hash their contents so that we
can compare them later on, even if we no longer have the data.
Previously, we used the fasted hash from the SHA2 family, either
SHA256 or SHA512 depending on the architecture.
- That, however, turned out to be a major performance problem. When
decrypting a non-compressed, binary file on amd64, we spent
roughly a third of the time just to compute the hash.
- Using the non-cryptographic hash function XXH3, we can greatly
improve the performance. On my system, it is 30x as fast as SHA3,
and reduces the overhead of computing the body hash considerably:
% time ./sq-sha512 decrypt --recipient-key juliet.key.pgp 3g-for-juliet.binary.pgp >/dev/null 2>&1
13.931 total
% time ./sq-xxh3 decrypt --recipient-key juliet.key.pgp 3g-for-juliet.binary.pgp >/dev/null 2>&1
9.264 total
- See #771.
|
|
An if condition is an expression and can be as complex as the
programmer wants. However, the more complex a condition is, the harder
it tends to be to understand. I marked functions with such if
conditions so that clippy won't complain about the code. I probably
should have simplified the code, perhaps by extracting the condition
to its own function, but it would have been much harder to do, so I
didn't.
Found by clippy lint blocks_in_if_conditions:
https://rust-lang.github.io/rust-clippy/master/index.html#blocks_in_if_conditions
|
|
It seems clearer to write this with "if let".
Found by clippy lint option_map_unit_fn:
https://rust-lang.github.io/rust-clippy/master/index.html#option_map_unit_fn
|
|
Instead of this:
if foo {
if bar {
...
}
}
do this:
if foo && bar {
...
}
Nesting statements implies a more complicated code structure than it
really is. Thus it's arguably simpler to write a combined condition by
joining the two conditions with a logical and operation.
Found by clippy lint collapsible_if:
https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_if
|
|
Instead of:
foo().and_then(|x| Ok(x.transmogrify())
use this:
foo().map(|x| x.transmogrify)
because it's shorter and simpler to understand.
Suggested by clippy lint bind_instead_of_map:
https://rust-lang.github.io/rust-clippy/master/index.html#bind_instead_of_map
|
|
When creating a struct with a field foo, using a variable also named
foo, it's not necessary to name the field explicitly. Thus, instead
of:
Self { foo: foo }
use this:
Self { foo }
The shorter form is more idiomatic and thus less confusing to
experienced Rust programmers.
This was found by the clippy lint redundant_field_names:
https://rust-lang.github.io/rust-clippy/master/index.html#redundant_field_names
Sponsored-by: author
|
|
Reduce this:
foo(&mut bar):
into
foo(&bar);
when the function does not expect a mutable reference, just a
read-only reference. The extra mut serves no function, but can confuse
the reader.
Found by clippy lint unnecessary_mut_passed:
https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_mut_passed
|
|
Suggested by Neal Walfield.
Found by clippy lint useless_format:
https://rust-lang.github.io/rust-clippy/master/index.html#useless_format
|
|
Rewrite:
i = i + 1
as
i += 1
For a simple variable this is shorter, and a little bit clearer. For
more complex expressions it avoids making the reader having to
visually check that the left and right hand side of the assignment
really do have the same expression and that nothing tricky is going
on.
This was found by the clippy lint assign_op_pattern:
https://rust-lang.github.io/rust-clippy/master/index.html#assign_op_pattern
|
|
The extra & in a pattern (match arm or if let) is unnecessary and only
makes the code harder to read. In most places it's enough to just
remove the & from the pattern, but in a few places a dereference (*)
needs to be added where the value captured in the pattern is used, as
removing the & changes the type of the captured value to be a
reference.
Overall, the changes are almost mechanical. Although the diff is huge,
it should be easy to read.
The clippy lint match_ref_pats warns about this. See:
https://rust-lang.github.io/rust-clippy/master/index.html#match_ref_pats
|
|
- Make sure that chunk sizes are between 64B and 4MiB.
- Fixes a DoS resulting from unconstrained, attacker-controlled heap
allocations.
- Fixes #738.
|