tough: always use unix-style paths in clean_name

Using `std::path::Path` to manipulate `Url` paths only has the correct
behavior on unix systems where the properties of the paths are nearly
identical.

This change moves to using unix-style path manipulation regardless of
platform.

It also adds a utility for safely creating filesystem paths based on
`Url` paths for both unix and windows (which is complicated by drive
lettering in absolute paths).

tough/Cargo.toml

@@ -15,7 +15,6 @@ globset = { version = "0.4" }
 hex = "0.4"
 log = "0.4"
 olpc-cjson = { version = "0.1", path = "../olpc-cjson" }
-path-absolutize = "3"
 pem = "1"
 percent-encoding = "2"
 reqwest = { version = "0.11", optional = true, default-features = false, features = ["blocking"] }
@@ -25,6 +24,7 @@ serde_json = "1"
 serde_plain = "1"
 snafu = "0.7"
 tempfile = "3"
+typed-path = "0.4"
 untrusted = "0.7"
 url = "2"
 walkdir = "2"

tough/src/error.rs

@@ -306,6 +306,12 @@ pub enum Error {
     #[snafu(display("Path {} is not valid UTF-8", path.display()))]
     PathUtf8 { path: PathBuf, backtrace: Backtrace },
 
+    #[snafu(display("Path {} is not valid UTF-8", path.display()))]
+    UnixPathUtf8 {
+        path: typed_path::UnixPathBuf,
+        backtrace: Backtrace,
+    },
+
     #[snafu(display("Failed to remove existing target path '{}': {}", path.display(), source))]
     RemoveTarget {
         path: PathBuf,

tough/src/lib.rs

@@ -43,6 +43,7 @@ pub mod schema;
 pub mod sign;
 mod target_name;
 mod transport;
+mod urlpath;
 
 use crate::datastore::Datastore;
 use crate::error::Result;
@@ -57,6 +58,7 @@ pub use crate::target_name::TargetName;
 pub use crate::transport::{
     DefaultTransport, FilesystemTransport, Transport, TransportError, TransportErrorKind,
 };
+pub use crate::urlpath::SafeUrlPath;
 use chrono::{DateTime, Utc};
 use log::warn;
 use percent_encoding::{utf8_percent_encode, AsciiSet, NON_ALPHANUMERIC};

tough/src/target_name.rs

@@ -1,11 +1,10 @@
 use crate::error::{self, Result};
-use path_absolutize::Absolutize;
 use serde::de::Error;
 use serde::{Deserialize, Deserializer, Serialize, Serializer};
-use snafu::{ensure, OptionExt, ResultExt};
+use snafu::{ensure, OptionExt};
 use std::convert::TryFrom;
-use std::path::PathBuf;
 use std::str::FromStr;
+use typed_path::{Component, UnixPath, UnixPathBuf};
 
 /// Represents the name of a target in the repository. Path-like constructs are resolved (e.g.
 /// `foo/../bar` becomes `bar`). Certain unsafe names are rejected when constructing a `TargetName`.
@@ -113,13 +112,11 @@ fn clean_name(name: &str) -> Result<String> {
     ensure!(!name.is_empty(), error::UnsafeTargetNameEmptySnafu { name });
 
     // If our name starts with absolute, then we need to remember this so we can restore it later.
-    let name_path = PathBuf::from(name);
+    let name_path = UnixPathBuf::from(name);
     let absolute = name_path.is_absolute();
 
     let clean = {
-        let proposed = name_path
-            .absolutize_from(&PathBuf::from("/"))
-            .context(error::TargetNameResolveSnafu { name })?;
+        let proposed = UnixPath::new("/").join(name_path).normalize();
 
         // `absolutize_from` will give us a path that starts with `/`, so we remove it if the
         // original name did not start with `/`
@@ -135,12 +132,12 @@ fn clean_name(name: &str) -> Result<String> {
                 .next()
                 // If this error occurs then there is a bug or behavior change in absolutize_from.
                 .context(error::TargetNameComponentsEmptySnafu { name })?
-                .as_os_str();
+                .as_bytes();
 
             // If the first component isn't the main separator ( unix `/`, windows '\' )
             // then there is a bug or behavior change in absolutize_from.
             ensure!(
-                first_component == std::path::MAIN_SEPARATOR_STR,
+                first_component == typed_path::unix::SEPARATOR_STR.as_bytes(),
                 error::TargetNameRootMissingSnafu { name }
             );
 
@@ -149,9 +146,8 @@ fn clean_name(name: &str) -> Result<String> {
     };
 
     let final_name = clean
-        .as_os_str()
         .to_str()
-        .context(error::PathUtf8Snafu { path: &clean })?
+        .context(error::UnixPathUtf8Snafu { path: &clean })?
         .to_string();
 
     // Check again to make sure we didn't end up with an empty string.

tough/src/transport.rs

@@ -1,10 +1,10 @@
+use crate::SafeUrlPath;
 #[cfg(feature = "http")]
 use crate::{HttpTransport, HttpTransportBuilder};
 use dyn_clone::DynClone;
 use std::error::Error;
 use std::fmt::{Debug, Display, Formatter};
 use std::io::{ErrorKind, Read};
-use std::path::PathBuf;
 use url::Url;
 
 /// A trait to abstract over the method/protocol by which files are obtained.
@@ -149,10 +149,7 @@ impl Transport for FilesystemTransport {
             ));
         }
 
-        // Convert the file URL into a file path. We need to use url.path() and not
-        // url.to_file_path() because to_file_path will decode the percent encoding which could
-        // restore path traversal characters.
-        let file_path = PathBuf::from(url.path());
+        let file_path = url.safe_url_filepath();
 
         // And open the file
         let f = std::fs::File::open(file_path).map_err(|e| {

tough/src/urlpath.rs

@@ -0,0 +1,73 @@
+//! This module contains utilities for mapping URL paths to local Paths.
+use std::path::PathBuf;
+use url::Url;
+
+/// Converts a file URL into a file path.
+/// Needed because `url.to_file_path()` will decode any percent encoding, which could restore path
+/// traversal characters, and `url.path()` roots paths to '/' on Windows.
+pub trait SafeUrlPath {
+    /// Returns the path component of a URL as a filesystem path.
+    fn safe_url_filepath(&self) -> PathBuf;
+}
+
+#[cfg(windows)]
+impl SafeUrlPath for Url {
+    fn safe_url_filepath(&self) -> PathBuf {
+        let url_path = self.path();
+
+        // Windows filepaths when written as `file://` URLs have path components prefixed with a /.
+        PathBuf::from(if let Some(stripped) = url_path.strip_prefix('/') {
+            stripped
+        } else {
+            url_path
+        })
+    }
+}
+
+#[cfg(unix)]
+impl SafeUrlPath for Url {
+    fn safe_url_filepath(&self) -> PathBuf {
+        PathBuf::from(self.path())
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    use crate::encode_filename;
+    use std::path::PathBuf;
+
+    fn manifest_dir() -> PathBuf {
+        PathBuf::from(env!("CARGO_MANIFEST_DIR"))
+    }
+
+    #[test]
+    fn test_safe_simple() {
+        let cargo_toml = manifest_dir().join("Cargo.toml");
+        let cargo_toml_url = Url::from_file_path(&cargo_toml)
+            .expect("Could not create URL from Cargo.toml filepath");
+
+        let safe_url_path = cargo_toml_url.safe_url_filepath();
+
+        assert_eq!(cargo_toml, safe_url_path);
+        assert!(safe_url_path.is_absolute());
+    }
+
+    #[test]
+    fn test_safe_traversals() {
+        let url_base = Url::from_directory_path(manifest_dir())
+            .expect("Could not create URL from CARGO_MANIFEST_DIR");
+
+        let escaped_test_path = encode_filename("a/../b/././c/..");
+        let traversal_url = url_base.join(&escaped_test_path).expect(&format!(
+            "Could not create URL from unusual traveral path '{}' + '{}'",
+            url_base.to_string(),
+            escaped_test_path
+        ));
+
+        assert_eq!(
+            manifest_dir().join("a%2F..%2Fb%2F.%2F.%2Fc%2F.."),
+            traversal_url.safe_url_filepath(),
+        );
+    }
+}

tuftool/src/error.rs

@@ -127,6 +127,9 @@ pub(crate) enum Error {
         backtrace: Backtrace,
     },
 
+    #[snafu(display("Can't build URL from path '{}'", path.display()))]
+    FileUrl { path: PathBuf, backtrace: Backtrace },
+
     #[snafu(display("Failed to write to {}: {}", path.display(), source))]
     FileWrite {
         path: PathBuf,

tuftool/src/source.rs

@@ -34,9 +34,10 @@
 //! "aws-ssm:///a/key" (notice the 3 slashes after the colon)
 
 use crate::error::{self, Result};
-use snafu::ResultExt;
-use std::path::PathBuf;
+use snafu::{OptionExt, ResultExt};
+use std::path::Path;
 use tough::key_source::{KeySource, LocalKeySource};
+use tough::SafeUrlPath;
 use tough_kms::{KmsKeySource, KmsSigningAlgorithm};
 use tough_ssm::SsmKeySource;
 use url::Url;
@@ -50,16 +51,19 @@ use url::Url;
 /// the `KeySource` trait in the `tough` library. A user can then add
 /// to this parser to support them in `tuftool`.
 pub(crate) fn parse_key_source(input: &str) -> Result<Box<dyn KeySource>> {
-    let pwd_url =
-        Url::from_directory_path(std::env::current_dir().context(error::CurrentDirSnafu)?)
-            .expect("expected current directory to be absolute");
-    let url = Url::options()
-        .base_url(Some(&pwd_url))
-        .parse(input)
-        .context(error::UrlParseSnafu { url: input })?;
+    let input_as_path = Path::new(input);
+    let url = if input_as_path.exists() {
+        Url::from_file_path(input)
+            .ok() // dump unhelpful `()` error
+            .context(error::FileUrlSnafu {
+                path: input_as_path.to_owned(),
+            })?
+    } else {
+        Url::parse(input).context(error::UrlParseSnafu { url: input })?
+    };
     match url.scheme() {
         "file" => Ok(Box::new(LocalKeySource {
-            path: PathBuf::from(url.path()),
+            path: url.safe_url_filepath(),
         })),
         #[cfg(any(feature = "aws-sdk-rust-native-tls", feature = "aws-sdk-rust-rustls"))]
         "aws-ssm" => Ok(Box::new(SsmKeySource {