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

age: Don't exit peeking state if entire identity file fits in the buffer #519

Merged
merged 2 commits into from
Aug 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
-----BEGIN AGE ENCRYPTED FILE-----
YWdlLWVuY3J5cHRpb24ub3JnL3YxCi0+IFgyNTUxOSBHUGc3Zlhpekp0K012aXdu
T1VZN0lmWlRmNjdLYVB4RldkTFVLTkNDUXlBCmJjRUcrM3E0a0U0N3IyK1JsTitG
dHVTd0N6TVFRTWgzdG5uSzJmNm9YMTgKLT4gQXQ1WWAtZ3JlYXNlIDxodGFSVHJg
IFg0cWYsO0ogZ2Fzc1EKZGtPSTB3Ci0tLSBKazRIaHJxdnNJcHpyclRkQjg3QW5r
SVE2MHdtWkErYTNrNWJibWd1bmNBCkK9FoOkiLB93gD79vNed8L3LM9rhKm5qma2
lSiwRx/aM1DKaZO0CMmYQkoM2tPReA==
-----END AGE ENCRYPTED FILE-----
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
bin.name = "rage"
args = "-d -i - file.age.txt"
stdin = "AGE-SECRET-KEY-1SRQGS50G584HFA5JG9D6D9S6639VVHJUE5XHHKJET9DRU76HK4RQP0X5Q3"
stdout = """
Test plaintext.
"""
stderr = ""
Loading