commit b4876e58b2d95e3745978717ed6efa763b054d08
parent 92fcf0505bc718f4a3a2225c64f4161d0dd5745c
Author: Antoine A <>
Date: Thu, 9 Dec 2021 14:35:04 +0100
Improve error handling and validate bitcoin addresses
Diffstat:
8 files changed, 227 insertions(+), 318 deletions(-)
diff --git a/Cargo.lock b/Cargo.lock
@@ -1774,6 +1774,7 @@ version = "0.1.0"
dependencies = [
"async-compression",
"base32",
+ "bitcoin",
"hyper",
"rand",
"serde",
diff --git a/research.md b/research.md
@@ -1,150 +1,5 @@
# Depolymerization
-## The problem
-
-We want to store metadata in the blockchain to identify taler wire transaction.
-
-## Bitcoin
-
-### Methods
-
-#### Fake legacy address
-
-The good:
-
-- Work with common wallets
-- Multiple payload of 20B
-
-The bad:
-
-- Two fees: higher weight fee and a a dust to burn (546 satoshis for a legacy
- address)
-- Pollute the blockchain by bloating UTXO (Unspent Transaction Outputs)
-- Bitcoin URI does not support multiple recipients
-
-Assuming people using cryptocurrencies are techies, asking them to enter
-multiple recipients manually doe no sound that bad.
-
-#### Fake SegWit address
-
-Same as fake legacy address but:
-
-- dust burn reduced from 546sa/20B to 294sa/20B (0.3247€ to 0.1748€)
-- for huge address 330sa/32B (0.1784€) but against standard
-
-The good:
-
-- Work with common wallets
-- Multiple payload of 20B
-
-The bad:
-
-- Two fees: higher weight fee and a a dust to burn (546 satoshis for a legacy
- address)
-- Pollute the blockchain by bloating UTXO (Unspent Transaction Outputs)
-- Bitcoin URI does not support multiple recipients
-
-Assuming people using cryptocurrencies are techies, asking them to enter
-multiple recipients manually doe no sound that bad.
-
-#### OP_RETURN
-
-The good:
-
-- Recommended way to store metadata
-- No blockchain abuse
-- Only cost is a higher weight fee
-
-The bad:
-
-- Require custom raw transaction
-- Require custom client (need more research)
-- Max 40B or 80B payload (one OP_RETURN by transaction)
-
-Solution:
-
-- Create a custom online wallet middleware (https://coinb.in/#txinputs)
-- Good user interface
-- Lot of complexity
-
-#### Unique address
-
-The good:
-
-- The normal way to do transaction
-
-The bad:
-
-- We need to store state in a database
-- We need to add a new API routine
-- Complicated to do correctly
-
-`sendmany` shuffle outputs for privacy reasons in RPC since v0.17, and the order
-cannot be relied upon for other wallets.
-
-Solutions:
-
-- Encode order in payload
-
-### Blockchain trust
-
-We can virtually never be sure that a transaction have been made in durable
-manner. By waiting for 6 blocks (~1h) we ca, have a significant assurance.
-
-- Make the number of confirmation blocks configurable
-
-TODO:
-
-- Find a way to detect extended forks using bitcoin alert (-alertnotify or
- getnetworkinfo RPC)
-- Or detect it ourselves by reading row block using RPC
-- How to handle removed transaction ? Do the wire history have to be consistent
- ?
-
-### Refunds
-
-There is no reliable way to know how send a transaction
-
-#### Cheapest and least reliable way
-
-Send back to unspent transaction (there may be none and the user may not control
-all of them)
-
-#### Better but still limited
-
-Get the transaction address from every input and send back to them Require to
-keep a transaction index -txindex
-
-To refund we could send bitcoins back to the sender address (minus some fee of
-course). However some people use wallet solutions using a shared address for
-multiple users, in that case the refund will go to the solution but not to the
-designated user.
-
-- get raw transaction details
-- remove refund fee from sent amount
-- send an equal part to each remaining addresses making them pay transactions
- fees
-- ignore insufficient_funds
-
-RPC error code :
-https://github.com/bitcoin/bitcoin/blob/master/src/rpc/protocol.h
-
-Do we care ?
-
-### Sources
-
-Pompianu, Livio & Bartoletti, Massimo & Bellomy, Bryn. (2019). A Journey into
-Bitcoin Metadata. Journal of Grid Computing. 10.1007/s10723-019-09473-3.
-
-## Out metadata with OP_RETURN
-
-- `createrawtransaction` with one output and a data
-- `fundrawtransaction` for automatic correctness
-- `signrawtransactionwithwallet`
-- `sendrawtransaction`
-
-OP_RETURN test
-
## Ethereum
- A transaction is from one address to another
@@ -154,16 +9,12 @@ Can we use smart contract to read metadata ?
Rust client library OpenEthereum - JSON RCP API
-## TODO
+## wire-gateway security
+
+- Maximum body size
+- Maximum body decompresses size
-- Magic bit detection
-- Optimize magic bit
-- lower level WitnessProgram alternative ?
-- OP_RETURN logic
+## TODO
-- Postgres database
-- Protocol rest api
-- test fork
-- log stderr
- detect small fork -> warning
- fork -> ERR panic
\ No newline at end of file
diff --git a/script/test_gateway.sh b/script/test_gateway.sh
@@ -14,6 +14,7 @@ function cleanup() {
trap cleanup EXIT
BANK_ENDPOINT=http://127.0.0.1:8080/
+ADDRESS=mpTJZxWPerz1Gife6mQSdHT8mMuJK6FP85
echo "---- Setup -----"
@@ -29,10 +30,6 @@ for n in `seq 1 50`; do
done
echo ""
-# Get client address
-ADDRESS=address
-BANK_ENDPOINT=http://127.0.0.1:8080/
-
echo "---- Gateway API -----"
echo -n "Making wire transfer to exchange:"
@@ -84,8 +81,8 @@ for bad_payto in http://bitcoin/$ADDRESS payto://btc/$ADDRESS payto://bitcoin/$A
done
echo ""
-#echo -n "Bad bitcoin address..."
-#taler-exchange-wire-gateway-client -b $BANK_ENDPOINT -C payto://bitcoin/ADDRESS -a BTC:0.00042 2>&1 | grep -q "(400/26)" && echo " OK" || echo " Failed"
+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:"
taler-exchange-wire-gateway-client -b $BANK_ENDPOINT -C payto://bitcoin/$ADDRESS -a ATC:0.00042 2>&1 | grep -q "(400/26)" && echo " OK" || echo " Failed"
diff --git a/taler-log/src/lib.rs b/taler-log/src/lib.rs
@@ -11,11 +11,12 @@ fn custom_format(
now: &mut DeferredNow,
record: &Record,
) -> Result<(), std::io::Error> {
+ let path = record.module_path().unwrap_or("");
write!(
w,
"{} {} {} {}",
now.format(TIME_FORMAT),
- record.module_path().unwrap_or("<unnamed>"),
+ path.split_once(':').map(|s| s.0).unwrap_or(path),
record.level(),
&record.args()
)
diff --git a/wire-gateway/Cargo.toml b/wire-gateway/Cargo.toml
@@ -39,4 +39,8 @@ url = { version = "2.2.2", features = ["serde"] }
# Async postgres client
tokio-postgres = { version = "0.7.5" }
# Logging
-taler-log = {path = "../taler-log"}
-\ No newline at end of file
+taler-log = {path = "../taler-log"}
+
+# TODO Put this behind a feature
+# Bitcoin data structure
+bitcoin = "0.27.1"
+\ No newline at end of file
diff --git a/wire-gateway/src/error.rs b/wire-gateway/src/error.rs
@@ -0,0 +1,77 @@
+use hyper::{Body, Response, StatusCode};
+use taler_log::log::error;
+use wire_gateway::{api_common::ErrorDetail, error_codes::ErrorCode};
+
+/// Generic http error
+#[derive(Debug)]
+pub struct ServerError {
+ status: StatusCode,
+ body: Body,
+}
+
+impl ServerError {
+ fn new(status: StatusCode, body: Body) -> Self {
+ Self { status, body }
+ }
+
+ pub fn response(self) -> Response<Body> {
+ Response::builder()
+ .status(self.status)
+ .body(self.body)
+ .unwrap()
+ }
+
+ pub fn unexpected<E: std::error::Error>(e: E) -> Self {
+ error!("unexpected: {}", e);
+ Self::new(StatusCode::INTERNAL_SERVER_ERROR, Body::empty())
+ }
+
+ fn detail(status: StatusCode, code: ErrorCode) -> Self {
+ let detail = &ErrorDetail {
+ code: code as i64,
+ hint: None,
+ detail: None,
+ parameter: None,
+ path: None,
+ offset: None,
+ index: None,
+ object: None,
+ currency: None,
+ type_expected: None,
+ type_actual: None,
+ };
+ match serde_json::to_vec(&detail) {
+ Ok(json) => Self::new(status, Body::from(json)),
+ Err(e) => Self::unexpected(e),
+ }
+ }
+
+ pub fn code(status: StatusCode, code: ErrorCode) -> Self {
+ error!(
+ "standard {}: {}",
+ code as i64,
+ status.canonical_reason().unwrap_or("")
+ );
+ Self::detail(status, code)
+ }
+
+ pub fn catch_code<E: std::error::Error>(e: E, status: StatusCode, code: ErrorCode) -> Self {
+ error!("standard {}: {}", code as i64, e);
+ Self::detail(status, code)
+ }
+}
+
+pub trait CatchResult<T: Sized> {
+ fn unexpected(self) -> Result<T, ServerError>;
+ fn catch_code(self, status: StatusCode, code: ErrorCode) -> Result<T, ServerError>;
+}
+
+impl<T, E: std::error::Error> CatchResult<T> for Result<T, E> {
+ fn unexpected(self) -> Result<T, ServerError> {
+ self.map_err(|e| ServerError::unexpected(e))
+ }
+
+ fn catch_code(self, status: StatusCode, code: ErrorCode) -> Result<T, ServerError> {
+ self.map_err(|e| ServerError::catch_code(e, status, code))
+ }
+}
diff --git a/wire-gateway/src/json.rs b/wire-gateway/src/json.rs
@@ -0,0 +1,75 @@
+use async_compression::tokio::{bufread::ZlibDecoder, write::ZlibEncoder};
+use hyper::{header, http::request::Parts, Body, Response, StatusCode};
+use tokio::io::{AsyncReadExt, AsyncWriteExt};
+
+#[derive(Debug, thiserror::Error)]
+pub enum ParseError {
+ #[error(transparent)]
+ Body(#[from] hyper::Error),
+ #[error(transparent)]
+ Json(#[from] serde_json::Error),
+ #[error(transparent)]
+ Deflate(#[from] tokio::io::Error),
+}
+
+pub async fn parse_json<J: serde::de::DeserializeOwned>(
+ parts: &Parts,
+ body: Body,
+) -> Result<J, ParseError> {
+ let bytes = hyper::body::to_bytes(body).await?;
+ let mut buf = Vec::new();
+ let decompressed = 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
+ } else {
+ bytes.as_ref()
+ };
+
+ Ok(serde_json::from_slice(&decompressed)?)
+}
+
+#[derive(Debug, thiserror::Error)]
+pub enum JsonRespError {
+ #[error(transparent)]
+ Json(#[from] serde_json::Error),
+ #[error(transparent)]
+ Deflate(#[from] tokio::io::Error),
+}
+
+pub async fn json_response<J: serde::Serialize>(
+ parts: &Parts,
+ status: StatusCode,
+ json: &J,
+) -> Result<Response<Body>, JsonRespError> {
+ let json = serde_json::to_vec(json)?;
+ if parts
+ .headers
+ .get(header::ACCEPT_ENCODING)
+ .and_then(|it| it.to_str().ok())
+ .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();
+ Ok(Response::builder()
+ .status(status)
+ .header(header::CONTENT_TYPE, "application/json")
+ .header(header::CONTENT_ENCODING, "deflate")
+ .body(Body::from(compressed))
+ .unwrap())
+ } else {
+ Ok(Response::builder()
+ .status(status)
+ .header(header::CONTENT_TYPE, "application/json")
+ .body(Body::from(json))
+ .unwrap())
+ }
+}
diff --git a/wire-gateway/src/main.rs b/wire-gateway/src/main.rs
@@ -1,37 +1,27 @@
-use std::{process::exit, str::FromStr, time::Instant};
-
-use api_common::{Amount, SafeUint64, ShortHashCode, Timestamp};
-use api_wire::{OutgoingBankTransaction, OutgoingHistory};
-use async_compression::tokio::{bufread::ZlibDecoder, write::ZlibEncoder};
-use error_codes::ErrorCode;
+use error::{CatchResult, ServerError};
use hyper::{
- header,
http::request::Parts,
service::{make_service_fn, service_fn},
Body, Error, Method, Response, Server, StatusCode,
};
+use json::parse_json;
+use std::{process::exit, str::FromStr, time::Instant};
use taler_log::log::{error, info, log, Level};
-use tokio::io::{AsyncReadExt, AsyncWriteExt};
use tokio_postgres::{Client, NoTls};
use url::Url;
-
-use crate::{
- api_common::ErrorDetail,
+use wire_gateway::{
+ api_common::{Amount, SafeUint64, ShortHashCode, Timestamp},
api_wire::{
- HistoryParams, IncomingBankTransaction, IncomingHistory, TransferRequest, TransferResponse,
+ HistoryParams, IncomingBankTransaction, IncomingHistory, OutgoingBankTransaction,
+ OutgoingHistory, TransferRequest, TransferResponse,
},
+ error_codes::ErrorCode,
};
-mod error_codes;
+use crate::json::json_response;
-fn check_pay_to(url: &Url) -> bool {
- return url.domain() == Some("bitcoin")
- && url.scheme() == "payto"
- && url.username() == ""
- && url.password().is_none()
- && url.query().is_none()
- && url.fragment().is_none();
-}
+mod error;
+mod json;
const SELF_PAYTO: &str = "payto://bitcoin/bcrt1qgkgxkjj27g3f7s87mcvjjsghay7gh34cx39prj";
#[cfg(target_family = "windows")]
@@ -43,6 +33,9 @@ const DB_URL: &str = "postgres://localhost?user=postgres&password=password";
async fn main() {
taler_log::init();
+ #[cfg(feature = "test")]
+ taler_log::log::warn!("Running with test admin endpoint unsuitable for production");
+
let (client, connection) = tokio_postgres::connect(DB_URL, NoTls).await.unwrap();
tokio::spawn(async move {
@@ -60,31 +53,9 @@ async fn main() {
let (parts, body) = req.into_parts();
let response = match router(&parts, body, state).await {
Ok(resp) => resp,
- Err((status, err)) => json_response(
- &parts,
- status,
- &ErrorDetail {
- code: err as i64,
- hint: None,
- detail: None,
- parameter: None,
- path: None,
- offset: None,
- index: None,
- object: None,
- currency: None,
- type_expected: None,
- type_actual: None,
- },
- )
- .await
- .unwrap_or(
- Response::builder()
- .status(StatusCode::INTERNAL_SERVER_ERROR)
- .body(Body::empty())
- .unwrap(),
- ),
+ Err(err) => err.response(),
};
+ // TODO log error message inlined into response log OR use a request id to link error message and response
let status = response.status().as_u16();
let level = if status >= 500 {
Level::Error
@@ -118,104 +89,38 @@ struct ServerState {
client: Client,
}
-pub mod api_common;
-pub mod api_wire;
-
-#[derive(Debug, thiserror::Error)]
-enum ParseError {
- #[error(transparent)]
- Body(#[from] hyper::Error),
- #[error(transparent)]
- Json(#[from] serde_json::Error),
- #[error(transparent)]
- Deflate(#[from] tokio::io::Error),
-}
-
-async fn parse_json<J: serde::de::DeserializeOwned>(
- parts: &Parts,
- body: Body,
-) -> Result<J, ParseError> {
- let bytes = hyper::body::to_bytes(body).await?;
- let mut buf = Vec::new();
- let decompressed = 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
- } else {
- bytes.as_ref()
- };
-
- Ok(serde_json::from_slice(&decompressed)?)
-}
-
-#[derive(Debug, thiserror::Error)]
-enum JsonRespError {
- #[error(transparent)]
- Json(#[from] serde_json::Error),
- #[error(transparent)]
- Deflate(#[from] tokio::io::Error),
-}
-
-async fn json_response<J: serde::Serialize>(
- parts: &Parts,
- status: StatusCode,
- json: &J,
-) -> Result<Response<Body>, JsonRespError> {
- let json = serde_json::to_vec(json)?;
- if parts
- .headers
- .get(header::ACCEPT_ENCODING)
- .and_then(|it| it.to_str().ok())
- .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();
- Ok(Response::builder()
- .status(status)
- .header(header::CONTENT_TYPE, "application/json")
- .header(header::CONTENT_ENCODING, "deflate")
- .body(Body::from(compressed))
- .unwrap())
- } else {
- Ok(Response::builder()
- .status(status)
- .header(header::CONTENT_TYPE, "application/json")
- .body(Body::from(json))
- .unwrap())
- }
+/// Check if an url is a valid bitcoin payto url
+fn check_pay_to(url: &Url) -> bool {
+ return url.domain() == Some("bitcoin")
+ && url.scheme() == "payto"
+ && url.username() == ""
+ && url.password().is_none()
+ && url.query().is_none()
+ && url.fragment().is_none()
+ && bitcoin::Address::from_str(url.path().trim_start_matches('/')).is_ok();
}
-type ServerErr = (StatusCode, ErrorCode);
-
-fn assert_method(parts: &Parts, method: Method) -> Result<(), ServerErr> {
+/// Assert request method match expected
+fn assert_method(parts: &Parts, method: Method) -> Result<(), ServerError> {
if parts.method == method {
Ok(())
} else {
- Err((
+ Err(ServerError::code(
StatusCode::METHOD_NOT_ALLOWED,
ErrorCode::GENERIC_METHOD_INVALID,
))
}
}
-fn history_params(parts: &Parts) -> Result<HistoryParams, ServerErr> {
+/// Parse history params from request
+fn history_params(parts: &Parts) -> Result<HistoryParams, ServerError> {
let params: HistoryParams = serde_urlencoded::from_str(parts.uri.query().unwrap_or(""))
- .map_err(|_| {
- (
- StatusCode::BAD_REQUEST,
- ErrorCode::GENERIC_PARAMETER_MALFORMED,
- )
- })?;
+ .catch_code(
+ StatusCode::BAD_REQUEST,
+ ErrorCode::GENERIC_PARAMETER_MALFORMED,
+ )?;
if params.delta == 0 {
- return Err((
+ return Err(ServerError::code(
StatusCode::BAD_REQUEST,
ErrorCode::GENERIC_PARAMETER_MALFORMED,
));
@@ -223,6 +128,7 @@ fn history_params(parts: &Parts) -> Result<HistoryParams, ServerErr> {
Ok(params)
}
+/// Generate sql query filter from history params
fn sql_history_filter(params: &HistoryParams) -> String {
let asc = params.delta > 0;
let limit = params.delta.abs();
@@ -239,24 +145,22 @@ async fn router(
parts: &Parts,
body: Body,
state: &'static ServerState,
-) -> Result<Response<Body>, (StatusCode, ErrorCode)> {
+) -> Result<Response<Body>, ServerError> {
let response = match parts.uri.path() {
"/transfer" => {
assert_method(&parts, Method::POST)?;
- let request: TransferRequest = parse_json(&parts, body).await.map_err(|_| {
- (
- StatusCode::BAD_REQUEST,
- ErrorCode::GENERIC_PARAMETER_MALFORMED,
- )
- })?;
+ let request: TransferRequest = parse_json(&parts, body).await.catch_code(
+ StatusCode::BAD_REQUEST,
+ ErrorCode::GENERIC_PARAMETER_MALFORMED,
+ )?;
if !check_pay_to(&request.credit_account) {
- return Err((
+ return Err(ServerError::code(
StatusCode::BAD_REQUEST,
ErrorCode::GENERIC_PAYTO_URI_MALFORMED,
));
}
if request.amount.currency != "BTC" {
- return Err((
+ return Err(ServerError::code(
StatusCode::BAD_REQUEST,
ErrorCode::GENERIC_PARAMETER_MALFORMED,
));
@@ -277,6 +181,7 @@ async fn router(
},
)
.await
+ .unexpected()?
}
"/history/incoming" => {
assert_method(&parts, Method::GET)?;
@@ -316,6 +221,7 @@ async fn router(
},
)
.await
+ .unexpected()?
}
"/history/outgoing" => {
assert_method(&parts, Method::GET)?;
@@ -356,18 +262,14 @@ async fn router(
},
)
.await
+ .unexpected()?
}
#[cfg(feature = "test")]
"/admin/add-incoming" => {
- assert_method(&parts, Method::POST)?;
- let request: api_wire::AddIncomingRequest =
- parse_json(&parts, body).await.map_err(|_| {
- (
- StatusCode::BAD_REQUEST,
- ErrorCode::GENERIC_PARAMETER_MALFORMED,
- )
- })?;
- // We do not check input as this is an admin endpoint
+ // 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();
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"
@@ -384,13 +286,14 @@ async fn router(
},
)
.await
+ .unexpected()?
+ }
+ _ => {
+ return Err(ServerError::code(
+ StatusCode::NOT_FOUND,
+ ErrorCode::GENERIC_ENDPOINT_UNKNOWN,
+ ))
}
- _ => return Err((StatusCode::NOT_FOUND, ErrorCode::GENERIC_ENDPOINT_UNKNOWN)),
};
- return Ok(response.unwrap_or(
- Response::builder()
- .status(StatusCode::INTERNAL_SERVER_ERROR)
- .body(Body::empty())
- .unwrap(),
- ));
+ return Ok(response);
}
