commit f9fe9248934080e57b5359dab4b53d0b1852f9b5
parent b4876e58b2d95e3745978717ed6efa763b054d08
Author: Antoine A <>
Date: Thu, 9 Dec 2021 16:52:55 +0100
Improve resilience against malicious request
Diffstat:
5 files changed, 78 insertions(+), 87 deletions(-)
diff --git a/Cargo.lock b/Cargo.lock
@@ -47,19 +47,6 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "38de00daab4eac7d753e97697066238d67ce9d7e2d823ab4f72fe14af29f3f33"
[[package]]
-name = "async-compression"
-version = "0.3.8"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "5443ccbb270374a2b1055fc72da40e1f237809cd6bb0e97e66d264cd138473a6"
-dependencies = [
- "flate2",
- "futures-core",
- "memchr",
- "pin-project-lite",
- "tokio",
-]
-
-[[package]]
name = "async-trait"
version = "0.1.51"
source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -265,15 +252,6 @@ dependencies = [
]
[[package]]
-name = "crc32fast"
-version = "1.3.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "738c290dfaea84fc1ca15ad9c168d083b05a714e1efddd8edaab678dc28d2836"
-dependencies = [
- "cfg-if",
-]
-
-[[package]]
name = "criterion"
version = "0.3.5"
source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -461,18 +439,6 @@ dependencies = [
]
[[package]]
-name = "flate2"
-version = "1.0.22"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "1e6988e897c1c9c485f43b47a529cef42fde0547f9d8d41a7062518f1d8fc53f"
-dependencies = [
- "cfg-if",
- "crc32fast",
- "libc",
- "miniz_oxide",
-]
-
-[[package]]
name = "flexi_logger"
version = "0.20.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -835,12 +801,11 @@ dependencies = [
[[package]]
name = "miniz_oxide"
-version = "0.4.4"
+version = "0.5.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "a92518e98c078586bc6c934028adcca4c92a53d6a958196de835170a01d84e4b"
+checksum = "d2b29bd4bc3f33391105ebee3589c19197c4271e3e5a9ec9bfe8127eeff8f082"
dependencies = [
"adler",
- "autocfg",
]
[[package]]
@@ -1772,10 +1737,10 @@ checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f"
name = "wire-gateway"
version = "0.1.0"
dependencies = [
- "async-compression",
"base32",
"bitcoin",
"hyper",
+ "miniz_oxide",
"rand",
"serde",
"serde_json",
diff --git a/script/test_gateway.sh b/script/test_gateway.sh
@@ -2,11 +2,15 @@
set -eu
+# Create temp file
+TEMP_FILE=$(mktemp)
+
# Cleanup to run whenever we exit
function cleanup() {
for n in `jobs -p`; do
kill $n 2> /dev/null || true
done
+ rm -f $TEMP_FILE
wait
}
@@ -30,7 +34,7 @@ for n in `seq 1 50`; do
done
echo ""
-echo "---- Gateway API -----"
+echo "---- Gateway API -----"
echo -n "Making wire transfer to exchange:"
for n in `seq 1 9`; do
@@ -81,7 +85,7 @@ for bad_payto in http://bitcoin/$ADDRESS payto://btc/$ADDRESS payto://bitcoin/$A
done
echo ""
-echo -n "Bad bitcoin address..."
+echo -n "Bad bitcoin address:"
taler-exchange-wire-gateway-client -b $BANK_ENDPOINT -C payto://bitcoin/42$ADDRESS -a BTC:0.00042 2>&1 | grep -q "(400/26)" && echo " OK" || echo " Failed"
echo -n "Bad transaction amount:"
@@ -104,7 +108,7 @@ function check_delta() {
}
for endpoint in incoming outgoing; do
- echo -n "History $endpoint"
+ echo -n "History $endpoint:"
check_delta ${endpoint}?delta=-9 9 "seq 1 9" && echo -n " OK" || echo -n " Failed"
check_delta ${endpoint}?delta=9 9 "seq 1 9" && echo -n " OK" || echo -n " Failed"
check_delta ${endpoint}?delta=-4 4 "seq 6 9" && echo -n " OK" || echo -n " Failed"
@@ -114,5 +118,21 @@ for endpoint in incoming outgoing; do
echo ""
done
+echo "----- Security -----"
+
+# Generate big random file
+printf 'HelloWorld%s' {1..1000} >> $TEMP_FILE
+
+echo -n "Handle huge body:"
+test `curl -w %{http_code} -X POST -s -o /dev/null -d @$TEMP_FILE ${BANK_ENDPOINT}transfer` -eq 400 && echo " OK" || echo " Failed"
+
+echo -n "Handle body length liar:"
+test `curl -w %{http_code} -X POST -H"Content-Length:1024" -s -o /dev/null -d @$TEMP_FILE ${BANK_ENDPOINT}transfer` -eq 400 && echo " OK" || echo " Failed"
+
+# Generate compression bomb
+printf 'HelloWorld%s' {1..1000} | pigz -z9 >> $TEMP_FILE
+
+echo -n "Handle compression bomb:"
+test `curl -w %{http_code} -X POST -H"Content-Encoding:deflate" -s -o /dev/null --data-binary @$TEMP_FILE ${BANK_ENDPOINT}transfer` -eq 400 && echo " OK" || echo " Failed"
echo "All tests passed"
diff --git a/wire-gateway/Cargo.toml b/wire-gateway/Cargo.toml
@@ -11,13 +11,7 @@ test = []
# Http library
hyper = { version = "0.14.15", features = ["http1", "server", "runtime"] }
# Async runtime
-tokio = { version = "1.14.0", features = [
- "net",
- "macros",
- "rt-multi-thread",
- "io-std",
- "io-util",
-] }
+tokio = { version = "1.14.0", features = ["net", "macros", "rt-multi-thread"] }
# Serialization framework
serde = { version = "1.0.130", features = ["derive"] }
# Serialization helper
@@ -30,8 +24,8 @@ serde_urlencoded = "0.7.0"
base32 = "0.4.0"
# Error macros
thiserror = "1.0.30"
-# Async friendly compression
-async-compression = { version = "0.3.8", features = ["tokio", "zlib"] }
+# Deflate compression
+miniz_oxide = "0.5.1"
# Rng
rand = { version = "0.8.4", features = ["getrandom"] }
# Url format
@@ -39,8 +33,8 @@ url = { version = "2.2.2", features = ["serde"] }
# Async postgres client
tokio-postgres = { version = "0.7.5" }
# Logging
-taler-log = {path = "../taler-log"}
+taler-log = { path = "../taler-log" }
# TODO Put this behind a feature
# Bitcoin data structure
-bitcoin = "0.27.1"
-\ No newline at end of file
+bitcoin = "0.27.1"
diff --git a/wire-gateway/src/json.rs b/wire-gateway/src/json.rs
@@ -1,52 +1,68 @@
-use async_compression::tokio::{bufread::ZlibDecoder, write::ZlibEncoder};
-use hyper::{header, http::request::Parts, Body, Response, StatusCode};
-use tokio::io::{AsyncReadExt, AsyncWriteExt};
+use hyper::{body::HttpBody, header, http::request::Parts, Body, Response, StatusCode};
+use miniz_oxide::inflate::TINFLStatus;
+
+const MAX_ALLOWED_RESPONSE_SIZE: u64 = 4 * 1024; // 4MB
#[derive(Debug, thiserror::Error)]
-pub enum ParseError {
+pub enum ParseBodyError {
#[error(transparent)]
Body(#[from] hyper::Error),
#[error(transparent)]
Json(#[from] serde_json::Error),
- #[error(transparent)]
- Deflate(#[from] tokio::io::Error),
+ #[error("deflate decompression error")]
+ Deflate,
+ #[error("body is suspiciously big")]
+ SuspiciousBody,
+ #[error("decompression is suspiciously big")]
+ SuspiciousCompression,
}
-pub async fn parse_json<J: serde::de::DeserializeOwned>(
+/// Parse json body, perform security check and decompression
+pub async fn parse_body<J: serde::de::DeserializeOwned>(
parts: &Parts,
body: Body,
-) -> Result<J, ParseError> {
+) -> Result<J, ParseBodyError> {
+ // Check announced body size
+ if body.size_hint().upper().unwrap_or(u64::MAX) > MAX_ALLOWED_RESPONSE_SIZE {
+ return Err(ParseBodyError::SuspiciousBody);
+ }
+ // Read body
let bytes = hyper::body::to_bytes(body).await?;
- let mut buf = Vec::new();
- let decompressed = if parts
+
+ // Decompress if necessary
+ if parts
.headers
.get(header::CONTENT_ENCODING)
.map(|it| it == "deflate")
.unwrap_or(false)
{
- let mut decoder = ZlibDecoder::new(bytes.as_ref());
- decoder.read_to_end(&mut buf).await?;
- &buf
+ let decompressed = miniz_oxide::inflate::decompress_to_vec_zlib_with_limit(
+ &bytes,
+ MAX_ALLOWED_RESPONSE_SIZE as usize,
+ )
+ .map_err(|s| match s {
+ TINFLStatus::HasMoreOutput => ParseBodyError::SuspiciousCompression,
+ _ => ParseBodyError::Deflate,
+ })?;
+ // Parse json
+ Ok(serde_json::from_slice(&decompressed)?)
} else {
- bytes.as_ref()
- };
-
- Ok(serde_json::from_slice(&decompressed)?)
+ // Parse json
+ Ok(serde_json::from_slice(&bytes)?)
+ }
}
#[derive(Debug, thiserror::Error)]
-pub enum JsonRespError {
+pub enum EncodeBodyError {
#[error(transparent)]
Json(#[from] serde_json::Error),
- #[error(transparent)]
- Deflate(#[from] tokio::io::Error),
}
-pub async fn json_response<J: serde::Serialize>(
+pub async fn encode_body<J: serde::Serialize>(
parts: &Parts,
status: StatusCode,
json: &J,
-) -> Result<Response<Body>, JsonRespError> {
+) -> Result<Response<Body>, EncodeBodyError> {
let json = serde_json::to_vec(json)?;
if parts
.headers
@@ -55,10 +71,7 @@ pub async fn json_response<J: serde::Serialize>(
.map(|str| str.contains("deflate"))
.unwrap_or(false)
{
- let mut encoder = ZlibEncoder::new(Vec::new());
- encoder.write_all(&json).await?;
- encoder.shutdown().await?;
- let compressed = encoder.into_inner();
+ let compressed = miniz_oxide::deflate::compress_to_vec_zlib(&json, 6);
Ok(Response::builder()
.status(status)
.header(header::CONTENT_TYPE, "application/json")
diff --git a/wire-gateway/src/main.rs b/wire-gateway/src/main.rs
@@ -4,7 +4,7 @@ use hyper::{
service::{make_service_fn, service_fn},
Body, Error, Method, Response, Server, StatusCode,
};
-use json::parse_json;
+use json::parse_body;
use std::{process::exit, str::FromStr, time::Instant};
use taler_log::log::{error, info, log, Level};
use tokio_postgres::{Client, NoTls};
@@ -18,7 +18,7 @@ use wire_gateway::{
error_codes::ErrorCode,
};
-use crate::json::json_response;
+use crate::json::encode_body;
mod error;
mod json;
@@ -149,7 +149,7 @@ async fn router(
let response = match parts.uri.path() {
"/transfer" => {
assert_method(&parts, Method::POST)?;
- let request: TransferRequest = parse_json(&parts, body).await.catch_code(
+ let request: TransferRequest = parse_body(&parts, body).await.catch_code(
StatusCode::BAD_REQUEST,
ErrorCode::GENERIC_PARAMETER_MALFORMED,
)?;
@@ -169,7 +169,7 @@ async fn router(
let row = state.client.query_one("INSERT INTO tx_out (_date, amount, wtid, debit_acc, credit_acc, exchange_url, status) VALUES (now(), $1, $2, $3, $4, $5, $6) RETURNING id", &[
&request.amount.to_string(), &request.wtid.as_ref(), &SELF_PAYTO, &request.credit_account.to_string(), &request.exchange_base_url.to_string(), &0i16
]).await.unwrap();
- json_response(
+ encode_body(
parts,
StatusCode::OK,
&TransferResponse {
@@ -213,7 +213,7 @@ async fn router(
credit_account: Url::parse(row.get(5)).unwrap(),
})
.collect();
- json_response(
+ encode_body(
parts,
StatusCode::OK,
&IncomingHistory {
@@ -254,7 +254,7 @@ async fn router(
exchange_base_url: Url::parse(row.get(6)).unwrap(),
})
.collect();
- json_response(
+ encode_body(
parts,
StatusCode::OK,
&OutgoingHistory {
@@ -269,12 +269,12 @@ async fn router(
// We do not check input as this is a test admin endpoint
assert_method(&parts, Method::POST).unwrap();
let request: wire_gateway::api_wire::AddIncomingRequest =
- parse_json(&parts, body).await.unwrap();
+ parse_body(&parts, body).await.unwrap();
let timestamp = Timestamp::now();
let row = state.client.query_one("INSERT INTO tx_in (_date, amount, reserve_pub, debit_acc, credit_acc) VALUES (now(), $1, $2, $3, $4) RETURNING id", &[
&request.amount.to_string(), &request.reserve_pub.as_ref(), &request.debit_account.to_string(), &"payto://bitcoin/bcrt1qgkgxkjj27g3f7s87mcvjjsghay7gh34cx39prj"
]).await.unwrap();
- json_response(
+ encode_body(
parts,
StatusCode::OK,
&TransferResponse {
