Skip to content

Commit

Permalink
age: Don't exit peeking state if entire identity file fits in the buffer
Browse files Browse the repository at this point in the history
This ensures we can call `PeekableReader::reset` when the file is a
single line without a trailing newline character, which rage-keygen does
not generate but users can.

Closes #484.
  • Loading branch information
str4d committed Aug 28, 2024
1 parent 303fa6e commit 548ee65
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 10 deletions.
5 changes: 5 additions & 0 deletions age/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ to 1.0.0 are beta releases.
constrain how the stanzas may be combined with those from other recipients.
- `age::plugin::RecipientPluginV1` now supports the labels extension.

### Fixed
- `age::cli_common::read_identities` once again correctly parses identity files
that are a single line without a trailing newline. This broke in 0.10.0 due to
an unrelated refactor.

### Removed
- `age::decryptor::PassphraseDecryptor` (use `age::Decryptor` with
`age::scrypt::Identity` instead).
Expand Down
53 changes: 43 additions & 10 deletions age/src/cli_common/identities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,17 @@ pub(super) fn parse_identity_files<Ctx, E: From<ReadError> + From<io::Error>>(
) -> Result<(), E> {
for filename in filenames {
#[cfg_attr(not(any(feature = "armor", feature = "ssh")), allow(unused_mut))]
let mut reader = PeekableReader::new(BufReader::new(
stdin_guard.open(filename.clone()).map_err(|e| match e {
let mut reader =
PeekableReader::new(stdin_guard.open(filename.clone()).map_err(|e| match e {
ReadError::Io(e) if matches!(e.kind(), io::ErrorKind::NotFound) => {
ReadError::IdentityNotFound(filename.clone())
}
_ => e,
})?,
));
})?);

// Note to future self: the order in which we try parsing formats here is critical
// to the correct behaviour of `PeekableReader::fill_buf`. See the comments in
// that method.

#[cfg(feature = "armor")]
// Try parsing as an encrypted age identity.
Expand Down Expand Up @@ -144,20 +147,28 @@ pub(super) fn parse_identity_files<Ctx, E: From<ReadError> + From<io::Error>>(
Ok(())
}

/// Same as default buffer size for `BufReader`, but hard-coded so we know exactly what
/// the buffer size is, and therefore can detect if the entire file fits into a single
/// buffer.
///
/// This must be at least 71 bytes to ensure the correct behaviour of
/// [`PeekableReader::fill_buf`]. See the comments in that method.
const PEEKABLE_SIZE: usize = 8 * 1024;

enum PeekState {
Peeking { consumed: usize },
Reading,
}

struct PeekableReader<R: io::BufRead> {
inner: R,
struct PeekableReader<R: io::Read> {
inner: BufReader<R>,
state: PeekState,
}

impl<R: io::BufRead> PeekableReader<R> {
impl<R: io::Read> PeekableReader<R> {
fn new(inner: R) -> Self {
Self {
inner,
inner: BufReader::with_capacity(PEEKABLE_SIZE, inner),
state: PeekState::Peeking { consumed: 0 },
}
}
Expand All @@ -177,7 +188,7 @@ impl<R: io::BufRead> PeekableReader<R> {
}
}

impl<R: io::BufRead> io::Read for PeekableReader<R> {
impl<R: io::Read> io::Read for PeekableReader<R> {
fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
match self.state {
PeekState::Peeking { .. } => {
Expand All @@ -195,7 +206,7 @@ impl<R: io::BufRead> io::Read for PeekableReader<R> {
}
}

impl<R: io::BufRead> io::BufRead for PeekableReader<R> {
impl<R: io::Read> io::BufRead for PeekableReader<R> {
fn fill_buf(&mut self) -> io::Result<&[u8]> {
match self.state {
PeekState::Peeking { consumed } => {
Expand All @@ -211,6 +222,28 @@ impl<R: io::BufRead> io::BufRead for PeekableReader<R> {
// on `self.inner` to outside the conditional, which would prevent us
// from performing other mutable operations on the other side.
Ok(&self.inner.fill_buf()?[consumed..])
} else if inner_len < PEEKABLE_SIZE {
// We have read the entire file into a single buffer and consumed all
// of it. Don't fall through to change the state to `Reading`, because
// we can always reset a single-buffer stream.
//
// Note that we cannot distinguish between the file being the exact
// same size as our buffer, and the file being larger than it. But
// this only becomes relevant if we cannot distinguish between the
// kinds of identity files we support parsing, within a single buffer.
// We should always be able to distinguish before then, because we
// parse in the following order:
//
// - Encrypted identities, which are parsed incrementally as age
// ciphertexts with optional armor, and can be detected in at most
// 70 bytes.
// - SSH identities, which are parsed as a PEM encoding and can be
// detected in at most 36 bytes.
// - Identity files, which have one identity per line and therefore
// can have arbitrarily long lines. We intentionally try this format
// last.
assert_eq!(consumed, inner_len);
Ok(&[])
} else {
// We're done peeking.
self.inner.consume(consumed);
Expand Down

0 comments on commit 548ee65

Please sign in to comment.