Skip to content

Commit

Permalink
age: Add Decryptor::new_buffered
Browse files Browse the repository at this point in the history
This is significantly more efficient than `Decryptor::new` at parsing
headers, due to avoiding repeated short reads.
  • Loading branch information
str4d committed Mar 25, 2023
1 parent 1538294 commit 7ef416a
Show file tree
Hide file tree
Showing 8 changed files with 226 additions and 4 deletions.
2 changes: 2 additions & 0 deletions age/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ to 1.0.0 are beta releases.

## [Unreleased]
### Added
- `age::Decryptor::new_buffered`, which is more efficient for types implementing
`std::io::BufRead` (which includes `&[u8]` slices).
- `impl std::io::BufRead for age::armor::ArmoredReader`

## [0.9.1] - 2022-03-24
Expand Down
2 changes: 1 addition & 1 deletion age/benches/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ fn bench(c: &mut Criterion) {
output.write_all(&[]).unwrap();
output.finish().unwrap();

b.iter(|| Decryptor::new(&encrypted[..]))
b.iter(|| Decryptor::new_buffered(&encrypted[..]))
});
}

Expand Down
2 changes: 1 addition & 1 deletion age/benches/throughput.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ fn bench(c: &mut Criterion_) {
output.finish().unwrap();

b.iter(|| {
let decryptor = match Decryptor::new(&ct_buf[..]).unwrap() {
let decryptor = match Decryptor::new_buffered(&ct_buf[..]).unwrap() {
Decryptor::Recipients(decryptor) => decryptor,
_ => panic!(),
};
Expand Down
30 changes: 29 additions & 1 deletion age/src/format.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! The age file format.

use age_core::format::Stanza;
use std::io::{self, Read, Write};
use std::io::{self, BufRead, Read, Write};

use crate::{
error::DecryptError,
Expand Down Expand Up @@ -88,6 +88,34 @@ impl Header {
}
}

pub(crate) fn read_buffered<R: BufRead>(mut input: R) -> Result<Self, DecryptError> {
let mut data = vec![];
loop {
match read::header(&data) {
Ok((_, mut header)) => {
if let Header::V1(h) = &mut header {
h.encoded_bytes = Some(data);
}
break Ok(header);
}
Err(nom::Err::Incomplete(nom::Needed::Size(_))) => {
// As we have a buffered reader, we can leverage the fact that the
// currently-defined header formats are newline-separated, to more
// efficiently read data for the parser to consume.
if input.read_until(b'\n', &mut data)? == 0 {
break Err(DecryptError::Io(io::Error::new(
io::ErrorKind::UnexpectedEof,
"Incomplete header",
)));
}
}
Err(_) => {
break Err(DecryptError::InvalidHeader);
}
}
}
}

#[cfg(feature = "async")]
#[cfg_attr(docsrs, doc(cfg(feature = "async")))]
pub(crate) async fn read_async<R: AsyncRead + Unpin>(
Expand Down
31 changes: 30 additions & 1 deletion age/src/protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

use age_core::{format::grease_the_joint, secrecy::SecretString};
use rand::{rngs::OsRng, RngCore};
use std::io::{self, Read, Write};
use std::io::{self, BufRead, Read, Write};

use crate::{
error::{DecryptError, EncryptError},
Expand Down Expand Up @@ -181,6 +181,13 @@ impl<R: Read> Decryptor<R> {
/// Attempts to create a decryptor for an age file.
///
/// Returns an error if the input does not contain a valid age file.
///
/// # Performance
///
/// This constructor will work with any type implementing [`io::Read`], and uses a
/// slower parser and internal buffering to ensure no overreading occurs. Consider
/// using [`Decryptor::new_buffered`] for types implementing `std::io::BufRead`, which
/// includes `&[u8]` slices.
pub fn new(mut input: R) -> Result<Self, DecryptError> {
let header = Header::read(&mut input)?;

Expand All @@ -194,6 +201,28 @@ impl<R: Read> Decryptor<R> {
}
}

impl<R: BufRead> Decryptor<R> {
/// Attempts to create a decryptor for an age file.
///
/// Returns an error if the input does not contain a valid age file.
///
/// # Performance
///
/// This constructor is more performant than [`Decryptor::new`] for types implementing
/// [`io::BufRead`], which includes `&[u8]` slices.
pub fn new_buffered(mut input: R) -> Result<Self, DecryptError> {
let header = Header::read_buffered(&mut input)?;

match header {
Header::V1(v1_header) => {
let nonce = Nonce::read(&mut input)?;
Decryptor::from_v1_header(input, v1_header, nonce)
}
Header::Unknown(_) => Err(DecryptError::UnknownFormat),
}
}
}

#[cfg(feature = "async")]
#[cfg_attr(docsrs, doc(cfg(feature = "async")))]
impl<R: AsyncRead + Unpin> Decryptor<R> {
Expand Down
139 changes: 139 additions & 0 deletions age/tests/testkit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,145 @@ fn testkit(filename: &str) {
#[test_case("x25519_not_canonical_body")]
#[test_case("x25519_not_canonical_share")]
#[test_case("x25519_short_share")]
fn testkit_buffered(filename: &str) {
let testfile = TestFile::parse(filename);
let comment = format_testkit_comment(&testfile);

match Decryptor::new_buffered(ArmoredReader::new(&testfile.age_file[..])).and_then(
|d| match d {
Decryptor::Recipients(d) => {
let identities = get_testkit_identities(filename, &testfile);
d.decrypt(identities.iter().map(|i| i as &dyn Identity))
}
Decryptor::Passphrase(d) => {
let passphrase = get_testkit_passphrase(&testfile, &comment);
d.decrypt(&passphrase, Some(16))
}
},
) {
Ok(mut r) => {
let mut payload = vec![];
let res = io::Read::read_to_end(&mut r, &mut payload);
check_decrypt_success(filename, testfile, &comment, res, &payload);
}
Err(e) => check_decrypt_error(filename, testfile, e),
}
}

#[test_case("armor")]
#[test_case("armor_crlf")]
#[test_case("armor_empty_line_begin")]
#[test_case("armor_empty_line_end")]
#[test_case("armor_eol_between_padding")]
#[test_case("armor_full_last_line")]
#[test_case("armor_garbage_encoded")]
#[test_case("armor_garbage_leading")]
#[test_case("armor_garbage_trailing")]
#[test_case("armor_header_crlf")]
#[test_case("armor_headers")]
#[test_case("armor_invalid_character_header")]
#[test_case("armor_invalid_character_payload")]
#[test_case("armor_long_line")]
#[test_case("armor_lowercase")]
#[test_case("armor_no_end_line")]
#[test_case("armor_no_eol")]
#[test_case("armor_no_match")]
#[test_case("armor_no_padding")]
#[test_case("armor_not_canonical")]
#[test_case("armor_pgp_checksum")]
#[test_case("armor_short_line")]
#[test_case("armor_whitespace_begin")]
#[test_case("armor_whitespace_end")]
#[test_case("armor_whitespace_eol")]
#[test_case("armor_whitespace_last_line")]
#[test_case("armor_whitespace_line_start")]
#[test_case("armor_whitespace_outside")]
#[test_case("armor_wrong_type")]
#[test_case("header_crlf")]
#[test_case("hmac_bad")]
#[test_case("hmac_extra_space")]
#[test_case("hmac_garbage")]
#[test_case("hmac_missing")]
#[test_case("hmac_no_space")]
#[test_case("hmac_not_canonical")]
#[test_case("hmac_trailing_space")]
#[test_case("hmac_truncated")]
#[test_case("scrypt")]
#[test_case("scrypt_and_x25519")]
#[test_case("scrypt_bad_tag")]
#[test_case("scrypt_double")]
#[test_case("scrypt_extra_argument")]
#[test_case("scrypt_long_file_key")]
#[test_case("scrypt_no_match")]
#[test_case("scrypt_not_canonical_body")]
#[test_case("scrypt_not_canonical_salt")]
#[test_case("scrypt_salt_long")]
#[test_case("scrypt_salt_missing")]
#[test_case("scrypt_salt_short")]
#[test_case("scrypt_uppercase")]
#[test_case("scrypt_work_factor_23")]
#[test_case("scrypt_work_factor_hex")]
#[test_case("scrypt_work_factor_leading_garbage")]
#[test_case("scrypt_work_factor_leading_plus")]
#[test_case("scrypt_work_factor_leading_zero_decimal")]
#[test_case("scrypt_work_factor_leading_zero_octal")]
#[test_case("scrypt_work_factor_missing")]
#[test_case("scrypt_work_factor_negative")]
#[test_case("scrypt_work_factor_overflow")]
#[test_case("scrypt_work_factor_trailing_garbage")]
#[test_case("scrypt_work_factor_wrong")]
#[test_case("scrypt_work_factor_zero")]
#[test_case("stanza_bad_start")]
#[test_case("stanza_base64_padding")]
#[test_case("stanza_empty_argument")]
#[test_case("stanza_empty_body")]
#[test_case("stanza_empty_last_line")]
#[test_case("stanza_invalid_character")]
#[test_case("stanza_long_line")]
#[test_case("stanza_missing_body")]
#[test_case("stanza_missing_final_line")]
#[test_case("stanza_multiple_short_lines")]
#[test_case("stanza_no_arguments")]
#[test_case("stanza_not_canonical")]
#[test_case("stanza_spurious_cr")]
#[test_case("stanza_valid_characters")]
#[test_case("stream_bad_tag")]
#[test_case("stream_bad_tag_second_chunk")]
#[test_case("stream_bad_tag_second_chunk_full")]
#[test_case("stream_empty_payload")]
#[test_case("stream_last_chunk_empty")]
#[test_case("stream_last_chunk_full")]
#[test_case("stream_last_chunk_full_second")]
#[test_case("stream_missing_tag")]
#[test_case("stream_no_chunks")]
#[test_case("stream_no_final")]
#[test_case("stream_no_final_full")]
#[test_case("stream_no_final_two_chunks")]
#[test_case("stream_no_final_two_chunks_full")]
#[test_case("stream_no_nonce")]
#[test_case("stream_short_chunk")]
#[test_case("stream_short_nonce")]
#[test_case("stream_short_second_chunk")]
#[test_case("stream_three_chunks")]
#[test_case("stream_trailing_garbage_long")]
#[test_case("stream_trailing_garbage_short")]
#[test_case("stream_two_chunks")]
#[test_case("stream_two_final_chunks")]
#[test_case("version_unsupported")]
#[test_case("x25519")]
#[test_case("x25519_bad_tag")]
#[test_case("x25519_extra_argument")]
#[test_case("x25519_grease")]
#[test_case("x25519_identity")]
#[test_case("x25519_long_file_key")]
#[test_case("x25519_long_share")]
#[test_case("x25519_lowercase")]
#[test_case("x25519_low_order")]
#[test_case("x25519_multiple_recipients")]
#[test_case("x25519_no_match")]
#[test_case("x25519_not_canonical_body")]
#[test_case("x25519_not_canonical_share")]
#[test_case("x25519_short_share")]
#[tokio::test]
async fn testkit_async(filename: &str) {
let testfile = TestFile::parse(filename);
Expand Down
6 changes: 6 additions & 0 deletions fuzz/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,9 @@ path = "fuzz_targets/header.rs"
[[bin]]
name = "decrypt"
path = "fuzz_targets/decrypt.rs"

[[bin]]
name = "decrypt_buffered"
path = "fuzz_targets/decrypt_buffered.rs"
test = false
doc = false
18 changes: 18 additions & 0 deletions fuzz/fuzz_targets/decrypt_buffered.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#![no_main]
use libfuzzer_sys::fuzz_target;

use std::iter;

use age::Decryptor;

fuzz_target!(|data: &[u8]| {
if let Ok(decryptor) = Decryptor::new_buffered(data) {
match decryptor {
Decryptor::Recipients(d) => {
let _ = d.decrypt(iter::empty());
}
// Don't pay the cost of scrypt while fuzzing.
Decryptor::Passphrase(_) => (),
}
}
});

0 comments on commit 7ef416a

Please sign in to comment.