Merge branch 'upload-descriptor' into 'main'

tor-dirclient: Add support for uploading descriptors.

See merge request tpo/core/arti!1505

crates/tor-dirclient/Cargo.toml

@@ -16,6 +16,8 @@ default = ["xz", "zstd"]
 
 # Enable support for hidden service descriptor downloads.
 hs-client = ["tor-hscrypto"]
+# Enable support for uploading hidden service descriptor downloads.
+hs-service = ["tor-hscrypto"]
 
 xz = ["async-compression/xz"]
 zstd = ["async-compression/zstd"]
@@ -24,6 +26,7 @@ routerdesc = []
 
 full = [
     "hs-client",
+    "hs-service",
     "xz",
     "zstd",
     "routerdesc",

crates/tor-dirclient/semver.md

@@ -0,0 +1,4 @@
+DEPRECATED: `download()`
+ADDED: `send_request()`
+ADDED: `tor-dirclient::request::HsDescUploadRequest`.
+BREAKING: `Requestable::make_request` now returns `http::Request<String>`

crates/tor-dirclient/src/lib.rs

@@ -79,7 +79,7 @@ pub type RequestResult<T> = std::result::Result<T, RequestError>;
 /// constructed using `dirinfo`.
 ///
 /// For more fine-grained control over the circuit and stream used,
-/// construct them yourself, and then call [`download`] instead.
+/// construct them yourself, and then call [`send_request`] instead.
 ///
 /// # TODO
 ///
@@ -122,7 +122,7 @@ where
 
     // TODO: Perhaps we want separate timeouts for each phase of this.
     // For now, we just use higher-level timeouts in `dirmgr`.
-    let r = download(runtime, req, &mut stream, Some(source.clone())).await;
+    let r = send_request(runtime, req, &mut stream, Some(source.clone())).await;
 
     if should_retire_circ(&r) {
         retire_circ(&circ_mgr, &source, "Partial response");
@@ -141,6 +141,22 @@ fn should_retire_circ(result: &Result<DirResponse>) -> bool {
 }
 
 /// Fetch a Tor directory object from a provided stream.
+#[deprecated(since = "0.8.1", note = "Use send_request instead.")]
+pub async fn download<R, S, SP>(
+    runtime: &SP,
+    req: &R,
+    stream: &mut S,
+    source: Option<SourceInfo>,
+) -> Result<DirResponse>
+where
+    R: request::Requestable + ?Sized,
+    S: AsyncRead + AsyncWrite + Send + Unpin,
+    SP: SleepProvider,
+{
+    send_request(runtime, req, stream, source).await
+}
+
+/// Fetch or upload a Tor directory object using the provided stream.
 ///
 /// To do this, we send a simple HTTP/1.0 request for the described
 /// object in `req` over `stream`, and then wait for a response.  In
@@ -158,7 +174,7 @@ fn should_retire_circ(result: &Result<DirResponse>) -> bool {
 /// The only error variant returned is [`Error::RequestFailed`].
 // TODO: should the error return type change to `RequestFailedError`?
 // If so, that would simplify some code in_dirmgr::bridgedesc.
-pub async fn download<R, S, SP>(
+pub async fn send_request<R, S, SP>(
     runtime: &SP,
     req: &R,
     stream: &mut S,
@@ -660,7 +676,7 @@ mod test {
             ) = futures::join!(
                 async {
                     // Run the download function.
-                    let r = download(&rt, &req, &mut s1, None).await;
+                    let r = send_request(&rt, &req, &mut s1, None).await;
                     s1.close().await.map_err(|error| {
                         Error::RequestFailed(RequestFailedError {
                             source: None,
@@ -703,7 +719,7 @@ mod test {
     }
 
     #[test]
-    fn test_download() -> RequestResult<()> {
+    fn test_send_request() -> RequestResult<()> {
         let req: request::MicrodescRequest = vec![[9; 32]].into_iter().collect();
 
         let (response, request) = run_download_test(

crates/tor-dirclient/src/request.rs

@@ -28,7 +28,7 @@ use crate::err::RequestError;
 pub trait Requestable {
     /// Build an [`http::Request`] from this Requestable, if
     /// it is well-formed.
-    fn make_request(&self) -> Result<http::Request<()>>;
+    fn make_request(&self) -> Result<http::Request<String>>;
 
     /// Return true if partial downloads are potentially useful.  This
     /// is true for request types where we're going to be downloading
@@ -190,7 +190,7 @@ impl Default for ConsensusRequest {
 }
 
 impl Requestable for ConsensusRequest {
-    fn make_request(&self) -> Result<http::Request<()>> {
+    fn make_request(&self) -> Result<http::Request<String>> {
         // Build the URL.
         let mut uri = "/tor/status-vote/current/consensus".to_string();
         match self.flavor {
@@ -225,7 +225,7 @@ impl Requestable for ConsensusRequest {
             req = req.header("X-Or-Diff-From-Consensus", &ids);
         }
 
-        Ok(req.body(())?)
+        Ok(req.body(String::new())?)
     }
 
     fn partial_docs_ok(&self) -> bool {
@@ -273,7 +273,7 @@ impl AuthCertRequest {
 }
 
 impl Requestable for AuthCertRequest {
-    fn make_request(&self) -> Result<http::Request<()>> {
+    fn make_request(&self) -> Result<http::Request<String>> {
         if self.ids.is_empty() {
             return Err(RequestError::EmptyRequest);
         }
@@ -296,7 +296,7 @@ impl Requestable for AuthCertRequest {
         let req = http::Request::builder().method("GET").uri(uri);
         let req = add_common_headers(req);
 
-        Ok(req.body(())?)
+        Ok(req.body(String::new())?)
     }
 
     fn partial_docs_ok(&self) -> bool {
@@ -343,7 +343,7 @@ impl MicrodescRequest {
 }
 
 impl Requestable for MicrodescRequest {
-    fn make_request(&self) -> Result<http::Request<()>> {
+    fn make_request(&self) -> Result<http::Request<String>> {
         let d_encode_b64 = |d: &[u8; 32]| Base64Unpadded::encode_string(&d[..]);
         let ids = digest_list_stringify(&self.digests, d_encode_b64, "-")
             .ok_or(RequestError::EmptyRequest)?;
@@ -352,7 +352,7 @@ impl Requestable for MicrodescRequest {
 
         let req = add_common_headers(req);
 
-        Ok(req.body(())?)
+        Ok(req.body(String::new())?)
     }
 
     fn partial_docs_ok(&self) -> bool {
@@ -418,7 +418,7 @@ impl RouterDescRequest {
 
 #[cfg(feature = "routerdesc")]
 impl Requestable for RouterDescRequest {
-    fn make_request(&self) -> Result<http::Request<()>> {
+    fn make_request(&self) -> Result<http::Request<String>> {
         let mut uri = "/tor/server/".to_string();
 
         match self.requested_descriptors {
@@ -438,7 +438,7 @@ impl Requestable for RouterDescRequest {
         let req = http::Request::builder().method("GET").uri(uri);
         let req = add_common_headers(req);
 
-        Ok(req.body(())?)
+        Ok(req.body(String::new())?)
     }
 
     fn partial_docs_ok(&self) -> bool {
@@ -484,12 +484,12 @@ impl RoutersOwnDescRequest {
 
 #[cfg(feature = "routerdesc")]
 impl Requestable for RoutersOwnDescRequest {
-    fn make_request(&self) -> Result<http::Request<()>> {
+    fn make_request(&self) -> Result<http::Request<String>> {
         let uri = "/tor/server/authority.z";
         let req = http::Request::builder().method("GET").uri(uri);
         let req = add_common_headers(req);
 
-        Ok(req.body(())?)
+        Ok(req.body(String::new())?)
     }
 
     fn partial_docs_ok(&self) -> bool {
@@ -530,14 +530,14 @@ impl HsDescDownloadRequest {
 
 #[cfg(feature = "hs-client")]
 impl Requestable for HsDescDownloadRequest {
-    fn make_request(&self) -> Result<http::Request<()>> {
+    fn make_request(&self) -> Result<http::Request<String>> {
         let hsid = Base64Unpadded::encode_string(self.hsid.as_ref());
         // We hardcode version 3 here; if we ever have a v4 onion service
         // descriptor, it will need a different kind of Request.
         let uri = format!("/tor/hs/3/{}", hsid);
         let req = http::Request::builder().method("GET").uri(uri);
         let req = add_common_headers(req);
-        Ok(req.body(())?)
+        Ok(req.body(String::new())?)
     }
 
     fn partial_docs_ok(&self) -> bool {
@@ -548,6 +548,50 @@ impl Requestable for HsDescDownloadRequest {
         self.max_len
     }
 }
+
+/// A request to upload a hidden service descriptor
+///
+/// rend-spec-v3 2.2.6
+#[derive(Debug, Clone)]
+#[cfg(feature = "hs-service")]
+pub struct HsDescUploadRequest(String);
+
+#[cfg(feature = "hs-service")]
+impl HsDescUploadRequest {
+    /// Construct a request for uploading a single onion service descriptor.
+    pub fn new(hsdesc: String) -> Self {
+        HsDescUploadRequest(hsdesc)
+    }
+}
+
+#[cfg(feature = "hs-service")]
+impl Requestable for HsDescUploadRequest {
+    fn make_request(&self) -> Result<http::Request<String>> {
+        /// The upload URI.
+        const URI: &str = "/tor/hs/3/publish";
+
+        let req = http::Request::builder().method("POST").uri(URI);
+        let req = add_common_headers(req);
+        // TODO HSS: we shouldn't have to clone here!
+        Ok(req.body(self.0.clone())?)
+    }
+
+    // TODO HSS: the name of this function doesn't make sense in this case.
+    // Perhaps it should be renamed to `partial_response_ok()`.
+    fn partial_docs_ok(&self) -> bool {
+        false
+    }
+
+    fn max_response_len(&self) -> usize {
+        // We expect the response body to be empty
+        //
+        // TODO HSS: perhaps we shouldn't? In the case of an error response, do we expect the body
+        // to contain e.g. an explanation for the error? If so, we should document this behaviour
+        // in rend-spec.
+        0
+    }
+}
+
 /// List the encodings we accept
 fn encodings() -> String {
     #[allow(unused_mut)]

crates/tor-dirclient/src/util.rs

@@ -3,7 +3,7 @@
 use std::fmt::Write;
 
 /// Encode an HTTP request in a quick and dirty HTTP 1.0 format.
-pub(crate) fn encode_request(req: &http::Request<()>) -> String {
+pub(crate) fn encode_request(req: &http::Request<String>) -> String {
     let mut s = format!("{} {} HTTP/1.0\r\n", req.method(), req.uri());
 
     for (key, val) in req.headers().iter() {
@@ -17,6 +17,7 @@ pub(crate) fn encode_request(req: &http::Request<()>) -> String {
         .unwrap();
     }
     s.push_str("\r\n");
+    s.push_str(req.body());
     s
 }
 
@@ -35,23 +36,33 @@ mod test {
     //! <!-- @@ end test lint list maintained by maint/add_warning @@ -->
     use super::*;
 
+    fn build_request(body: String, headers: &[(&str, &str)]) -> http::Request<String> {
+        let mut builder = http::Request::builder().method("GET").uri("/index.html");
+
+        for (name, value) in headers {
+            builder = builder.header(*name, *value);
+        }
+
+        builder.body(body).unwrap()
+    }
+
     #[test]
     fn format() {
-        let req = http::Request::builder()
+        fn chk_format(body: &str) {
-            .method("GET")
+            let req = build_request(body.to_string(), &[]);
-            .uri("/index.html")
+            assert_eq!(
-            .body(())
+                encode_request(&req),
-            .unwrap();
+                format!("GET /index.html HTTP/1.0\r\n\r\n{body}")
-        assert_eq!(encode_request(&req), "GET /index.html HTTP/1.0\r\n\r\n");
+            );
-        let req = http::Request::builder()
+
-            .method("GET")
+            let req = build_request(body.to_string(), &[("X-Marsupial", "Opossum")]);
-            .uri("/index.html")
+            assert_eq!(
-            .header("X-Marsupial", "Opossum")
+                encode_request(&req),
-            .body(())
+                format!("GET /index.html HTTP/1.0\r\nx-marsupial: Opossum\r\n\r\n{body}")
-            .unwrap();
+            );
-        assert_eq!(
+        }
-            encode_request(&req),
+
-            "GET /index.html HTTP/1.0\r\nx-marsupial: Opossum\r\n\r\n"
+        chk_format("");
-        );
+        chk_format("hello");
     }
 }

crates/tor-dirmgr/src/bridgedesc.rs

@@ -212,11 +212,15 @@ impl<R: Runtime> mockable::MockableAPI<R> for () {
             .await
             .map_err(Error::StreamFailed)?;
         let request = tor_dirclient::request::RoutersOwnDescRequest::new();
-        let response = tor_dirclient::download(runtime, &request, &mut stream, None)
+        let response = tor_dirclient::send_request(runtime, &request, &mut stream, None)
             .await
             .map_err(|dce| match dce {
                 tor_dirclient::Error::RequestFailed(re) => Error::RequestFailed(re),
-                _ => internal!("tor_dirclient::download gave non-RequestFailed {:?}", dce).into(),
+                _ => internal!(
+                    "tor_dirclient::send_request gave non-RequestFailed {:?}",
+                    dce
+                )
+                .into(),
             })?;
         let output = response.into_output_string()?;
         Ok(Some(output))

crates/tor-hsclient/src/connect.rs

@@ -580,7 +580,7 @@ impl<'c, R: Runtime, M: MocksForConnect<R>> Context<'c, R, M> {
             .await
             .map_err(DescriptorErrorDetail::Stream)?;
 
-        let response = tor_dirclient::download(self.runtime, &request, &mut stream, None)
+        let response = tor_dirclient::send_request(self.runtime, &request, &mut stream, None)
             .await
             .map_err(|dir_error| match dir_error {
                 tor_dirclient::Error::RequestFailed(rfe) => DescriptorErrorDetail::from(rfe.error),