Skip to content

Commit

Permalink
Authenticate ciphertext length when seeking from end of StreamReader
Browse files Browse the repository at this point in the history
`StreamReader::seek(SeekFrom::End(offset))` did not previously authenticate
the ciphertext length; if the ciphertext had been truncated or extended by
`adversary_offset`, it would instead seek to `offset + adversary_offset`.
This allowed an adversary with temporary control of an encrypted age file
to control the location of a plaintext read following a seek-from-end.

`age` now returns an error if the last chunk is invalid.

Fixes #195.
  • Loading branch information
str4d committed Jan 24, 2021
1 parent fe8fed6 commit b2ec527
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 3 deletions.
11 changes: 11 additions & 0 deletions age/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,17 @@ and this project adheres to Rust's notion of
to 1.0.0 are beta releases.

## [Unreleased]
### Security
- `StreamReader::seek(SeekFrom::End(offset))` did not previously authenticate
the ciphertext length; if the ciphertext had been truncated or extended by
`adversary_offset`, it would instead seek to `offset + adversary_offset`. This
allowed an adversary with temporary control of an encrypted age file to
control the location of a plaintext read following a seek-from-end. `age` now
returns an error if the last chunk is invalid.
- `rage` was not affected by this security issue, as it does not use `Seek`.
- `rage-mount` may have been affected; it does not use `SeekFrom::End`
directly, but the `tar` or `zip` crates might do so.

### Added
- Plugin support, enabled by the `plugin` feature flag:
- `age::plugin::{Identity, Recipient}` structs for parsing plugin recipients
Expand Down
24 changes: 21 additions & 3 deletions age/src/primitives/stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -515,9 +515,10 @@ impl<R: Read + Seek> StreamReader<R> {
fn len(&mut self) -> io::Result<u64> {
match self.plaintext_len {
None => {
// Cache the current position, and then grab the start and end ciphertext
// positions.
// Cache the current position and nonce, and then grab the start and end
// ciphertext positions.
let cur_pos = self.inner.seek(SeekFrom::Current(0))?;
let cur_nonce = self.stream.nonce.0;
let ct_start = self.start()?;
let ct_end = self.inner.seek(SeekFrom::End(0))?;
let ct_len = ct_end - ct_start;
Expand All @@ -526,11 +527,28 @@ impl<R: Read + Seek> StreamReader<R> {
let num_chunks =
(ct_len + (ENCRYPTED_CHUNK_SIZE as u64 - 1)) / ENCRYPTED_CHUNK_SIZE as u64;

// Authenticate the ciphertext length by checking that we can successfully
// decrypt the last chunk _as_ a last chunk.
let last_chunk_start = ct_start + ((num_chunks - 1) * ENCRYPTED_CHUNK_SIZE as u64);
let mut last_chunk = Vec::with_capacity((ct_end - last_chunk_start) as usize);
self.inner.seek(SeekFrom::Start(last_chunk_start))?;
self.inner.read_to_end(&mut last_chunk)?;
self.stream.nonce.set_counter(num_chunks - 1);
self.stream.decrypt_chunk(&last_chunk, true).map_err(|_| {
io::Error::new(
io::ErrorKind::InvalidData,
"Last chunk is invalid, stream might be truncated",
)
})?;

// Now that we have authenticated the ciphertext length, we can use it to
// calculate the plaintext length.
let total_tag_size = num_chunks * TAG_SIZE as u64;
let pt_len = ct_len - total_tag_size;

// Return to the original position.
// Return to the original position and restore the nonce.
self.inner.seek(SeekFrom::Start(cur_pos))?;
self.stream.nonce = Nonce(cur_nonce);

// Cache the length for future calls.
self.plaintext_len = Some(pt_len);
Expand Down

0 comments on commit b2ec527

Please sign in to comment.