Improve resilience against malicious request

5 files changed, 78 insertions, 86 deletions

diff --git a/Cargo.lock b/Cargo.lock
index 3ae4a46..93ac13e 100644
--- 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
index cda1d06..aaed418 100644
--- 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
index bb71613..8839323 100644
--- 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
index 368280d..eb55a09 100644
--- 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
index 7125b3d..59eb6dd 100644
--- 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 {