Deuxfleurs/garage
48
56
Fork 73
Code Issues 100 Pull requests 13 Releases 30 Wiki Activity

Support STREAMING-AWS4-HMAC-SHA256-PAYLOAD (#64) #156

Merged
lx merged 11 commits from KokaKiwi/garage:aws4-payload-signing into main 2022-01-17 09:55:31 +00:00
Conversation 0 Commits 11 Files changed 13 +19 -11
1 changed files with 19 additions and 11 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit 847b5ad407 - Show all commits

30
src/api/signature/streaming.rs
View file

@ -192,35 +192,42 @@ where
let mut this = self.project(); let mut this = self.project();
macro_rules! try_parse { macro_rules! try_parse {
($expr:expr) => { ($eof:expr, $expr:expr) => {
match $expr { match $expr {
Ok(value) => Ok(value), Ok(value) => Ok(value),
Err(nom::Err::Incomplete(_)) => continue, Err(nom::Err::Incomplete(_)) => {
if $eof {
return Poll::Ready(Some(Err(SignedPayloadStreamError::message(
"Unexpected EOF",
))));
}
continue;
}
Err(nom::Err::Error(e)) | Err(nom::Err::Failure(e)) => Err(e), Err(nom::Err::Error(e)) | Err(nom::Err::Failure(e)) => Err(e),
}? }?
}; };
} }
loop { loop {
match futures::ready!(this.stream.as_mut().poll_next(cx)) { let eof = match futures::ready!(this.stream.as_mut().poll_next(cx)) {
Some(Ok(bytes)) => { Some(Ok(bytes)) => {
log::debug!("Received: {:?}", bytes);
this.buf.extend(bytes); this.buf.extend(bytes);
false
} }
Some(Err(e)) => return Poll::Ready(Some(Err(SignedPayloadStreamError::Stream(e)))), Some(Err(e)) => return Poll::Ready(Some(Err(SignedPayloadStreamError::Stream(e)))),
None => { None => {
log::debug!("Buf: {:?}", this.buf);
KokaKiwi marked this conversation as resolved Outdated
lx commented 2022-01-04 16:54:18 +00:00
Outdated
Review
Copy link

What if the payload input stream is interrupted in the middle? Do we take care of exiting the infinite loop and return an error?

What if the payload input stream is interrupted in the middle? Do we take care of exiting the infinite loop and return an error?
if this.buf.is_empty() { if this.buf.is_empty() {
return Poll::Ready(None); return Poll::Ready(None);
} else {
return Poll::Ready(Some(Err(SignedPayloadStreamError::message(
"Unexpected EOF",
))));
}
} }
true
} }
};
let input: &[u8] = this.buf; let input: &[u8] = this.buf;
let (input, header) = try_parse!(payload::Header::parse(input)); let (input, header) = try_parse!(eof, payload::Header::parse(input));
// 0-sized chunk is the last // 0-sized chunk is the last
if header.size == 0 { if header.size == 0 {
@ -228,8 +235,9 @@ where
return Poll::Ready(None); return Poll::Ready(None);
} }
let (input, data) = try_parse!(take::<_, _, nom::error::Error<_>>(header.size)(input)); let (input, data) =
KokaKiwi marked this conversation as resolved Outdated
trinity-1686a commented 2022-01-12 18:28:18 +00:00
Outdated
Review
Copy link

this bit has an invalid edge case : by cutting the stream just before a new chunk header, an attacker can truncate the file without it being rejected. Getting here (inner stream returns None and this.buf is empy) is either such a truncation, or a call to SignedPayloadStream::poll_next after it returned Ok(Ready(None)) once, which is a contract error (Ok(Ready(None)) means that the stream has terminated, and poll_next should not be invoked again), so this check and return can be safely removed

this bit has an invalid edge case : by cutting the stream just before a new chunk header, an attacker can truncate the file without it being rejected. Getting here (inner stream returns None and this.buf is empy) is either such a truncation, or a call to SignedPayloadStream::poll_next after it returned Ok(Ready(None)) once, which is a contract error ([`Ok(Ready(None)) means that the stream has terminated, and poll_next should not be invoked again`](https://docs.rs/futures/0.2.0/futures/stream/trait.Stream.html#return-value)), so this check and return can be safely removed
let (input, _) = try_parse!(tag::<_, _, nom::error::Error<_>>("\r\n")(input)); try_parse!(eof, take::<_, _, nom::error::Error<_>>(header.size)(input));
let (input, _) = try_parse!(eof, tag::<_, _, nom::error::Error<_>>("\r\n")(input));
let data = Bytes::from(data.to_vec()); let data = Bytes::from(data.to_vec());
let data_sha256sum = sha256sum(&data); let data_sha256sum = sha256sum(&data);