fix(sync): bound response reads to the snapshot wire limit
This commit is contained in:
@@ -1,4 +1,7 @@
|
|||||||
use std::time::{Duration, SystemTime, UNIX_EPOCH};
|
use std::{
|
||||||
|
io::{self, Read},
|
||||||
|
time::{Duration, SystemTime, UNIX_EPOCH},
|
||||||
|
};
|
||||||
|
|
||||||
use serde::de::DeserializeOwned;
|
use serde::de::DeserializeOwned;
|
||||||
use ureq::{Agent, AgentBuilder};
|
use ureq::{Agent, AgentBuilder};
|
||||||
@@ -13,7 +16,7 @@ use crate::{
|
|||||||
},
|
},
|
||||||
device_revocation::{DeviceRevocationDocument, DeviceRevocationRequest},
|
device_revocation::{DeviceRevocationDocument, DeviceRevocationRequest},
|
||||||
error::SyncClientError,
|
error::SyncClientError,
|
||||||
snapshot::{SnapshotDownload, SnapshotUploadRequest},
|
snapshot::{MAX_SNAPSHOT_BYTES, SnapshotDownload, SnapshotUploadRequest},
|
||||||
vault::SyncVaultDocument,
|
vault::SyncVaultDocument,
|
||||||
vault_bootstrap::SyncVaultBootstrapRequest,
|
vault_bootstrap::SyncVaultBootstrapRequest,
|
||||||
};
|
};
|
||||||
@@ -421,6 +424,20 @@ pub struct SyncDeviceStatusDocument {
|
|||||||
pub current_device_approved: bool,
|
pub current_device_approved: bool,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// A snapshot download legally carries `MAX_SNAPSHOT_BYTES` of payload as
|
||||||
|
/// base64 (4/3 expansion) plus its JSON envelope, which exceeds ureq's
|
||||||
|
/// 10 MiB `into_string` cap. Anything above this bound fails closed.
|
||||||
|
const MAX_RESPONSE_BODY_BYTES: usize = MAX_SNAPSHOT_BYTES.div_ceil(3) * 4 + 64 * 1024;
|
||||||
|
|
||||||
|
fn read_response_body(response: ureq::Response) -> io::Result<String> {
|
||||||
|
let mut body = String::new();
|
||||||
|
response.into_reader().take(MAX_RESPONSE_BODY_BYTES as u64 + 1).read_to_string(&mut body)?;
|
||||||
|
if body.len() > MAX_RESPONSE_BODY_BYTES {
|
||||||
|
return Err(io::Error::other("response body exceeds the sync wire limit"));
|
||||||
|
}
|
||||||
|
Ok(body)
|
||||||
|
}
|
||||||
|
|
||||||
fn read_json_response<T: DeserializeOwned>(
|
fn read_json_response<T: DeserializeOwned>(
|
||||||
endpoint: &str,
|
endpoint: &str,
|
||||||
response: Result<ureq::Response, ureq::Error>,
|
response: Result<ureq::Response, ureq::Error>,
|
||||||
@@ -428,7 +445,7 @@ fn read_json_response<T: DeserializeOwned>(
|
|||||||
match response {
|
match response {
|
||||||
Ok(ok) => read_json_from_response(endpoint, ok),
|
Ok(ok) => read_json_from_response(endpoint, ok),
|
||||||
Err(ureq::Error::Status(status, raw)) => {
|
Err(ureq::Error::Status(status, raw)) => {
|
||||||
let body = raw.into_string().unwrap_or_default();
|
let body = read_response_body(raw).unwrap_or_default();
|
||||||
if session::response_ends_session(status, &body) {
|
if session::response_ends_session(status, &body) {
|
||||||
return Err(SyncClientError::SessionEnded);
|
return Err(SyncClientError::SessionEnded);
|
||||||
}
|
}
|
||||||
@@ -445,7 +462,7 @@ fn read_json_from_response<T: DeserializeOwned>(
|
|||||||
response: ureq::Response,
|
response: ureq::Response,
|
||||||
) -> Result<T, SyncClientError> {
|
) -> Result<T, SyncClientError> {
|
||||||
let status = response.status();
|
let status = response.status();
|
||||||
let body = response.into_string().map_err(|error| SyncClientError::HttpStatus {
|
let body = read_response_body(response).map_err(|error| SyncClientError::HttpStatus {
|
||||||
endpoint: endpoint.to_string(),
|
endpoint: endpoint.to_string(),
|
||||||
status,
|
status,
|
||||||
body: error.to_string(),
|
body: error.to_string(),
|
||||||
|
|||||||
@@ -44,7 +44,7 @@ impl SyncApiClient {
|
|||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
Err(ureq::Error::Status(status, response)) => {
|
Err(ureq::Error::Status(status, response)) => {
|
||||||
let body = response.into_string().unwrap_or_default();
|
let body = super::read_response_body(response).unwrap_or_default();
|
||||||
if response_ends_session(status, &body) {
|
if response_ends_session(status, &body) {
|
||||||
return Ok(());
|
return Ok(());
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -282,6 +282,52 @@ fn read_complete_request(stream: &mut std::net::TcpStream) -> std::io::Result<Ve
|
|||||||
Ok(request)
|
Ok(request)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn download_parses_snapshot_bodies_above_ureq_default_cap() -> Result<(), Box<dyn Error>> {
|
||||||
|
let data_base64 = "A".repeat(11 * 1024 * 1024);
|
||||||
|
let (base_url, server) = spawn_snapshot_download_server(data_base64.clone())?;
|
||||||
|
let client = SyncApiClient::new(
|
||||||
|
ApiClientConfig::custom(base_url, "auto"),
|
||||||
|
BearerToken::new("a".repeat(64))?,
|
||||||
|
)?;
|
||||||
|
let requested = SnapshotHeadRef::new(7, "snapshot-big", "ab".repeat(32))?;
|
||||||
|
|
||||||
|
let SnapshotDownloadResult::Downloaded(download) = client.download_snapshot(&requested)? else {
|
||||||
|
return Err("snapshot download body was not preserved".into());
|
||||||
|
};
|
||||||
|
assert_eq!(download.data_base64.len(), data_base64.len());
|
||||||
|
join_server(server)
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn oversized_response_bodies_fail_closed() -> Result<(), Box<dyn Error>> {
|
||||||
|
let (base_url, server) = spawn_snapshot_download_server("A".repeat(14 * 1024 * 1024))?;
|
||||||
|
let client = SyncApiClient::new(
|
||||||
|
ApiClientConfig::custom(base_url, "auto"),
|
||||||
|
BearerToken::new("a".repeat(64))?,
|
||||||
|
)?;
|
||||||
|
let requested = SnapshotHeadRef::new(7, "snapshot-big", "ab".repeat(32))?;
|
||||||
|
|
||||||
|
let result = client.download_snapshot(&requested);
|
||||||
|
assert!(matches!(
|
||||||
|
&result,
|
||||||
|
Err(crate::SyncClientError::HttpStatus { body, .. }) if body.contains("sync wire limit")
|
||||||
|
));
|
||||||
|
join_server(server)
|
||||||
|
}
|
||||||
|
|
||||||
|
fn spawn_snapshot_download_server(
|
||||||
|
data_base64: String,
|
||||||
|
) -> Result<(String, TestServer), Box<dyn Error>> {
|
||||||
|
let body = format!(
|
||||||
|
r#"{{"version":3,"user_id":"user-01","device_id":"device-remote","snapshot":{{"snapshot_id":"snapshot-big","r2_key":"snapshots/user-01","payload_hash":"{hash}","encryption_version":2,"vault_generation":1,"key_id":"{key}","content_hash":"{content}","schema_rev":1,"logical_clock":9,"head_revision":7,"base_head":null,"device_id":"device-remote","size_bytes":256,"created_at":1}},"data_base64":"{data_base64}"}}"#,
|
||||||
|
hash = "ab".repeat(32),
|
||||||
|
key = "ef".repeat(32),
|
||||||
|
content = "12".repeat(32),
|
||||||
|
);
|
||||||
|
spawn_authenticated_server("GET /api/sync/snapshot?snapshot_id=snapshot-big", "200 OK", &body)
|
||||||
|
}
|
||||||
|
|
||||||
fn spawn_logout_server(
|
fn spawn_logout_server(
|
||||||
status_line: &'static str,
|
status_line: &'static str,
|
||||||
body: &str,
|
body: &str,
|
||||||
|
|||||||
Reference in New Issue
Block a user