Make _danger-local-https feature additive

See: <https://doc.rust-lang.org/cargo/reference/features.html#feature-unification>

"Rust features should be *additive*. That is, enabling a feature should
not disable functionality, and it should usually be safe to enable any
combination of features"

Before this change `io::fetch_ohttp_keys` changed its signature based
on the feature flag. I noticed downstream that this creates a feature
flag mess in bindings and it's one opportunity to address tech debt.

`io` now enables the `v2` feature since it only contains methods for
fetching OHTTP keys which is only useful in V2. In the future perhaps
it will also include generic HTTP client behavior taking `Request` as
a parameter in which case it should not enable `v2`, because such
functionality would be useful in a V1 setting as well.

payjoin-cli/src/app/v2.rs

@@ -360,14 +360,14 @@ async fn unwrap_ohttp_keys_or_else_fetch(config: &AppConfig) -> Result<payjoin::
         let ohttp_relay = config.ohttp_relay.clone();
         let payjoin_directory = config.pj_directory.clone();
         #[cfg(feature = "_danger-local-https")]
-        let cert_der = crate::app::read_local_cert()?;
-        Ok(payjoin::io::fetch_ohttp_keys(
-            ohttp_relay,
-            payjoin_directory,
-            #[cfg(feature = "_danger-local-https")]
-            cert_der,
-        )
-        .await?)
+        let ohttp_keys = {
+            let cert_der = crate::app::read_local_cert()?;
+            payjoin::io::fetch_ohttp_keys_with_cert(ohttp_relay, payjoin_directory, cert_der)
+                .await?
+        };
+        #[cfg(not(feature = "_danger-local-https"))]
+        let ohttp_keys = payjoin::io::fetch_ohttp_keys(ohttp_relay, payjoin_directory).await?;
+        Ok(ohttp_keys)
     }
 }
 

payjoin-cli/tests/e2e.rs

@@ -240,9 +240,12 @@ mod e2e {
 
             // fetch for setup here since ohttp_relay doesn't know the certificate for the directory
             // so payjoin-cli is set up with the mock_ohttp_relay which is the directory
-            let ohttp_keys =
-                payjoin::io::fetch_ohttp_keys(ohttp_relay.clone(), directory.clone(), cert.clone())
-                    .await?;
+            let ohttp_keys = payjoin::io::fetch_ohttp_keys_with_cert(
+                ohttp_relay.clone(),
+                directory.clone(),
+                cert.clone(),
+            )
+            .await?;
             let ohttp_keys_path = temp_dir.join("ohttp_keys");
             tokio::fs::write(&ohttp_keys_path, ohttp_keys.encode()?).await?;
 

payjoin/Cargo.toml

@@ -20,8 +20,9 @@ send = []
 receive = ["bitcoin/rand"]
 base64 = ["bitcoin/base64"]
 v2 = ["bitcoin/rand", "bitcoin/serde", "hpke", "dep:http", "bhttp", "ohttp", "serde", "url/serde" ]
-io = ["reqwest/rustls-tls"]
-_danger-local-https = ["io", "reqwest/rustls-tls", "rustls"]
+#[doc = "Functions to fetch OHTTP keys via CONNECT proxy using reqwest. Enables `v2` since only `v2` uses OHTTP."]
+io = ["v2", "reqwest/rustls-tls"]
+_danger-local-https = ["reqwest/rustls-tls", "rustls"]
 
 [dependencies]
 bitcoin = { version = "0.32.5", features = ["base64"] }

payjoin/src/io.rs

@@ -1,4 +1,5 @@
-#[cfg(feature = "v2")]
+use reqwest::{Client, Proxy};
+
 use crate::{OhttpKeys, Url};
 
 /// Fetch the ohttp keys from the specified payjoin directory via proxy.
@@ -9,22 +10,36 @@ use crate::{OhttpKeys, Url};
 ///
 /// * `payjoin_directory`: The payjoin directory from which to fetch the ohttp keys.  This
 ///   directory stores and forwards payjoin client payloads.
-///
-/// * `cert_der` (optional): The DER-encoded certificate to use for local HTTPS connections.  This
-///   parameter is only available when the "_danger-local-https" feature is enabled.
-#[cfg(feature = "v2")]
 pub async fn fetch_ohttp_keys(
     ohttp_relay: Url,
     payjoin_directory: Url,
-    #[cfg(feature = "_danger-local-https")] cert_der: Vec<u8>,
 ) -> Result<OhttpKeys, Error> {
-    use reqwest::{Client, Proxy};
-
     let ohttp_keys_url = payjoin_directory.join("/ohttp-keys")?;
     let proxy = Proxy::all(ohttp_relay.as_str())?;
-    #[cfg(not(feature = "_danger-local-https"))]
     let client = Client::builder().proxy(proxy).build()?;
-    #[cfg(feature = "_danger-local-https")]
+    let res = client.get(ohttp_keys_url).send().await?;
+    let body = res.bytes().await?.to_vec();
+    OhttpKeys::decode(&body).map_err(|e| Error(InternalError::InvalidOhttpKeys(e.to_string())))
+}
+
+/// Fetch the ohttp keys from the specified payjoin directory via proxy.
+///
+/// * `ohttp_relay`: The http CONNNECT method proxy to request the ohttp keys from a payjoin
+///   directory.  Proxying requests for ohttp keys ensures a client IP address is never revealed to
+///   the payjoin directory.
+///
+/// * `payjoin_directory`: The payjoin directory from which to fetch the ohttp keys.  This
+///   directory stores and forwards payjoin client payloads.
+///
+/// * `cert_der`: The DER-encoded certificate to use for local HTTPS connections.
+#[cfg(feature = "_danger-local-https")]
+pub async fn fetch_ohttp_keys_with_cert(
+    ohttp_relay: Url,
+    payjoin_directory: Url,
+    cert_der: Vec<u8>,
+) -> Result<OhttpKeys, Error> {
+    let ohttp_keys_url = payjoin_directory.join("/ohttp-keys")?;
+    let proxy = Proxy::all(ohttp_relay.as_str())?;
     let client = Client::builder()
         .danger_accept_invalid_certs(true)
         .use_rustls_tls()
@@ -46,7 +61,6 @@ enum InternalError {
     Io(std::io::Error),
     #[cfg(feature = "_danger-local-https")]
     Rustls(rustls::Error),
-    #[cfg(feature = "v2")]
     InvalidOhttpKeys(String),
 }
 
@@ -72,7 +86,6 @@ impl std::fmt::Display for Error {
             Reqwest(e) => e.fmt(f),
             ParseUrl(e) => e.fmt(f),
             Io(e) => e.fmt(f),
-            #[cfg(feature = "v2")]
             InvalidOhttpKeys(e) => {
                 write!(f, "Invalid ohttp keys returned from payjoin directory: {}", e)
             }
@@ -90,7 +103,6 @@ impl std::error::Error for Error {
             Reqwest(e) => Some(e),
             ParseUrl(e) => Some(e),
             Io(e) => Some(e),
-            #[cfg(feature = "v2")]
             InvalidOhttpKeys(_) => None,
             #[cfg(feature = "_danger-local-https")]
             Rustls(e) => Some(e),

payjoin/tests/integration.rs

@@ -1,4 +1,4 @@
-#[cfg(all(feature = "send", feature = "receive"))]
+#[cfg(all(feature = "send", feature = "receive", feature = "_danger-local-https"))]
 mod integration {
     use std::collections::HashMap;
     use std::env;
@@ -171,8 +171,7 @@ mod integration {
         }
     }
 
-    #[cfg(feature = "_danger-local-https")]
-    #[cfg(feature = "v2")]
+    #[cfg(all(feature = "io", feature = "v2"))]
     mod v2 {
         use std::sync::Arc;
         use std::time::Duration;
@@ -252,9 +251,12 @@ mod integration {
                 let agent = Arc::new(http_agent(cert_der.clone())?);
                 wait_for_service_ready(ohttp_relay.clone(), agent.clone()).await.unwrap();
                 wait_for_service_ready(directory.clone(), agent.clone()).await.unwrap();
-                let ohttp_keys =
-                    payjoin::io::fetch_ohttp_keys(ohttp_relay, directory.clone(), cert_der.clone())
-                        .await?;
+                let ohttp_keys = payjoin::io::fetch_ohttp_keys_with_cert(
+                    ohttp_relay,
+                    directory.clone(),
+                    cert_der,
+                )
+                .await?;
 
                 // **********************
                 // Inside the Receiver:
@@ -321,9 +323,12 @@ mod integration {
                 let agent = Arc::new(http_agent(cert_der.clone())?);
                 wait_for_service_ready(ohttp_relay.clone(), agent.clone()).await.unwrap();
                 wait_for_service_ready(directory.clone(), agent.clone()).await.unwrap();
-                let ohttp_keys =
-                    payjoin::io::fetch_ohttp_keys(ohttp_relay, directory.clone(), cert_der.clone())
-                        .await?;
+                let ohttp_keys = payjoin::io::fetch_ohttp_keys_with_cert(
+                    ohttp_relay,
+                    directory.clone(),
+                    cert_der.clone(),
+                )
+                .await?;
                 // **********************
                 // Inside the Receiver:
                 let address = receiver.get_new_address(None, None)?.assume_checked();
@@ -450,9 +455,12 @@ mod integration {
                 let agent = Arc::new(http_agent(cert_der.clone())?);
                 wait_for_service_ready(ohttp_relay.clone(), agent.clone()).await.unwrap();
                 wait_for_service_ready(directory.clone(), agent.clone()).await.unwrap();
-                let ohttp_keys =
-                    payjoin::io::fetch_ohttp_keys(ohttp_relay, directory.clone(), cert_der.clone())
-                        .await?;
+                let ohttp_keys = payjoin::io::fetch_ohttp_keys_with_cert(
+                    ohttp_relay,
+                    directory.clone(),
+                    cert_der,
+                )
+                .await?;
                 // **********************
                 // Inside the Receiver:
                 // make utxos with different script types
@@ -662,9 +670,12 @@ mod integration {
                 let agent: Arc<Client> = Arc::new(http_agent(cert_der.clone())?);
                 wait_for_service_ready(ohttp_relay.clone(), agent.clone()).await?;
                 wait_for_service_ready(directory.clone(), agent.clone()).await?;
-                let ohttp_keys =
-                    payjoin::io::fetch_ohttp_keys(ohttp_relay, directory.clone(), cert_der.clone())
-                        .await?;
+                let ohttp_keys = payjoin::io::fetch_ohttp_keys_with_cert(
+                    ohttp_relay,
+                    directory.clone(),
+                    cert_der.clone(),
+                )
+                .await?;
                 let address = receiver.get_new_address(None, None)?.assume_checked();
 
                 let mut session = initialize_session(address, directory, ohttp_keys.clone(), None);