Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Signal keyconfig problem #153

Merged
merged 5 commits into from
Dec 28, 2023
+56 −19
Merged

Signal keyconfig problem #153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
51fc9ee
Match HandlerError::to_response variant order
DanGould Dec 26, 2023
737c9b8
Mutate Response on HandlerError match
DanGould Dec 26, 2023
1df3165
Signal OHTTP Key Configuration problem
DanGould Dec 26, 2023
e92d6a4
Test bad OHTTP key response
DanGould Dec 26, 2023
9e43b90
Display Bad HTTP response in payjoin cli
DanGould Dec 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 20 additions & 8 deletions payjoin-cli/src/app.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,9 @@ impl App {
);
let (req, ctx) = enroller.extract_req()?;
log::debug!("Enrolling receiver");
let http = http_agent()?;
let ohttp_response = spawn_blocking(move || {
let http = http_agent()?;
http.post(req.url.as_ref())
.send_bytes(&req.body)
.with_context(|| "HTTP request failed")
http.post(req.url.as_ref()).send_bytes(&req.body).map_err(map_ureq_err)
})
.await??;

Expand Down Expand Up @@ -172,7 +170,7 @@ impl App {
.extract_v2_req()
.map_err(|e| anyhow!("v2 req extraction failed {}", e))?;
let http = http_agent()?;
let res = http.post(req.url.as_str()).send_bytes(&req.body)?;
let res = http.post(req.url.as_str()).send_bytes(&req.body).map_err(map_ureq_err)?;
let mut buf = Vec::new();
let _ = res.into_reader().read_to_end(&mut buf)?;
let res = payjoin_proposal.deserialize_res(buf, ohttp_ctx);
Expand Down Expand Up @@ -204,7 +202,7 @@ impl App {
http.post(req.url.as_ref())
.set("Content-Type", "text/plain")
.send_bytes(&req.body)
.with_context(|| "HTTP request failed")
.map_err(map_ureq_err)
})
.await??;

Expand All @@ -229,8 +227,10 @@ impl App {
enrolled.extract_req().map_err(|_| anyhow!("Failed to extract request"))?;
log::debug!("GET fallback_psbt");
let http = http_agent()?;
let ohttp_response =
spawn_blocking(move || http.post(req.url.as_str()).send_bytes(&req.body)).await??;
let ohttp_response = spawn_blocking(move || {
http.post(req.url.as_str()).send_bytes(&req.body).map_err(map_ureq_err)
})
.await??;

let proposal = enrolled
.process_res(ohttp_response.into_reader(), context)
Expand Down Expand Up @@ -905,3 +905,15 @@ fn http_agent() -> Result<ureq::Agent> {

#[cfg(not(feature = "danger-local-https"))]
fn http_agent() -> Result<ureq::Agent> { Ok(ureq::Agent::new()) }

fn map_ureq_err(e: ureq::Error) -> anyhow::Error {
let e_string = e.to_string();
match e.into_response() {
Some(res) => anyhow!(
"HTTP request failed: {} {}",
res.status(),
res.into_string().unwrap_or_default()
),
None => anyhow!("No HTTP response: {}", e_string),
}
}
33 changes: 22 additions & 11 deletions payjoin-relay/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use std::sync::Arc;

use anyhow::Result;
use bitcoin::{self, base64};
use hyper::header::HeaderValue;
use hyper::server::conn::AddrIncoming;
use hyper::server::Builder;
use hyper::service::{make_service_fn, service_fn};
Expand Down Expand Up @@ -151,8 +152,9 @@ async fn handle_ohttp(
let ohttp_body =
hyper::body::to_bytes(body).await.map_err(|e| HandlerError::BadRequest(e.into()))?;
let mut ohttp_locked = ohttp.lock().await;
let (bhttp_req, res_ctx) =
ohttp_locked.decapsulate(&ohttp_body).map_err(|e| HandlerError::BadRequest(e.into()))?;
let (bhttp_req, res_ctx) = ohttp_locked
.decapsulate(&ohttp_body)
.map_err(|e| HandlerError::OhttpKeyRejection(e.into()))?;
drop(ohttp_locked);
let mut cursor = std::io::Cursor::new(bhttp_req);
let req =
Expand Down Expand Up @@ -206,25 +208,34 @@ async fn handle_v2(pool: DbPool, req: Request<Body>) -> Result<Response<Body>, H
enum HandlerError {
PayloadTooLarge,
InternalServerError(anyhow::Error),
OhttpKeyRejection(anyhow::Error),
BadRequest(anyhow::Error),
}

impl HandlerError {
fn to_response(&self) -> Response<Body> {
let status = match self {
HandlerError::PayloadTooLarge => StatusCode::PAYLOAD_TOO_LARGE,
HandlerError::BadRequest(e) => {
error!("Bad request: {}", e);
StatusCode::BAD_REQUEST
}
let mut res = Response::default();
match self {
HandlerError::PayloadTooLarge => *res.status_mut() = StatusCode::PAYLOAD_TOO_LARGE,
HandlerError::InternalServerError(e) => {
error!("Internal server error: {}", e);
StatusCode::INTERNAL_SERVER_ERROR
*res.status_mut() = StatusCode::INTERNAL_SERVER_ERROR
}
HandlerError::OhttpKeyRejection(e) => {
const OHTTP_KEY_REJECTION_RES_JSON: &str = r#"{"type":"https://iana.org/assignments/http-problem-types#ohttp-key", "title": "key identifier unknown"}"#;
Copy link
Contributor

@jbesraa jbesraa Dec 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think the title is a bit opaque, maybe its worth mentioning the specific problematic param ie ohttp_config

edit:
I see this is following the spec, but still we might want to add a message or something

Copy link
Contributor Author

@DanGould DanGould Dec 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, it would be instructive to show something like "Key identifier unknown. Refresh your payjoin directory configuration at $OHTTP_GATEWAY/ohttp-config" or similar.

I think this is a separate task for payjoin-cli, and eventually an implementation library doing I/O. Fortunately the client should be able to automate this problem away with automatic ip protected fetch as described here


error!("Bad request: Key configuration rejected: {}", e);
*res.status_mut() = StatusCode::BAD_REQUEST;
res.headers_mut()
.insert("Content-Type", HeaderValue::from_static("application/problem+json"));
*res.body_mut() = Body::from(OHTTP_KEY_REJECTION_RES_JSON);
}
HandlerError::BadRequest(e) => {
error!("Bad request: {}", e);
*res.status_mut() = StatusCode::BAD_REQUEST
}
};

let mut res = Response::new(Body::empty());
*res.status_mut() = status;
res
}
}
Expand Down
14 changes: 14 additions & 0 deletions payjoin/tests/integration.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ mod integration {
use super::*;

const PJ_RELAY_URL: &str = "https://localhost:8088";
const BAD_OHTTP_CONFIG: &str = "AQAg3WpRjS0aqAxQUoLvpas2VYjT2oIg6-3XSiB-QiYI1BAABAABAAM";
const OH_RELAY_URL: &str = "https://localhost:8088";
const LOCAL_CERT_FILE: &str = "localhost.der";

Expand All @@ -223,6 +224,19 @@ mod integration {

// **********************
// Inside the Receiver:
// Try enroll with bad relay ohttp-config
let mut bad_enroller =
Enroller::from_relay_config(&PJ_RELAY_URL, &BAD_OHTTP_CONFIG, &OH_RELAY_URL);
let (req, _ctx) = bad_enroller.extract_req()?;
let res =
spawn_blocking(move || http_agent().post(req.url.as_str()).send_bytes(&req.body))
.await?;
assert!(res.is_err());
assert!(
res.unwrap_err().into_response().unwrap().content_type()
== "application/problem+json"
);

// Enroll with relay
let mut enroller =
Enroller::from_relay_config(&PJ_RELAY_URL, &ohttp_config, &OH_RELAY_URL);
Expand Down
Loading