diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 0a05e8a52..23e530cef 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -85,9 +85,6 @@ jobs: dbus-run-session --config-file /tmp/dbus-session.conf -- \ cargo --locked test --profile "$PROFILE" --verbose --features uuid,url,time,chrono,option-as-array,vsock,bus-impl \ -- --skip fdpass_systemd - # check cookie-sha1 auth against dbus-daemon - sed -i s/EXTERNAL/DBUS_COOKIE_SHA1/g /tmp/dbus-session.conf - dbus-run-session --config-file /tmp/dbus-session.conf -- cargo --locked test --profile "$PROFILE" --verbose -- basic_connection # Test tokio support. dbus-run-session --config-file /tmp/dbus-session.conf -- \ cargo --locked test --profile "$PROFILE" --verbose --tests -p zbus --no-default-features \ diff --git a/Cargo.lock b/Cargo.lock index 1d009dcba..5cf81dc29 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -261,15 +261,6 @@ version = "2.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b048fb63fd8b5923fc5aa7b340d8e156aec7ec02f0c78fa8a6ddc2613f6f71de" -[[package]] -name = "block-buffer" -version = "0.10.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3078c7629b62d3f0439517fa394996acacc5cbc91c5a20d8c658e77abd503a71" -dependencies = [ - "generic-array", -] - [[package]] name = "blocking" version = "1.6.1" @@ -434,15 +425,6 @@ dependencies = [ "crossbeam-utils", ] -[[package]] -name = "cpufeatures" -version = "0.2.12" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "53fe5e26ff1b7aef8bca9c6080520cfb8d9333c7568e1829cef191a9723e5504" -dependencies = [ - "libc", -] - [[package]] name = "criterion" version = "0.5.1" @@ -510,16 +492,6 @@ version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7a81dae078cea95a014a339291cec439d2f232ebe854a9d672b796c6afafa9b7" -[[package]] -name = "crypto-common" -version = "0.1.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1bfb12502f3fc46cca1bb51ac28df9d618d813cdc3d2f25b9fe775a34af26bb3" -dependencies = [ - "generic-array", - "typenum", -] - [[package]] name = "deranged" version = "0.3.11" @@ -536,16 +508,6 @@ version = "0.1.13" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "56254986775e3233ffa9c4d7d3faaf6d36a2c09d30b20687e9f88bc8bafc16c8" -[[package]] -name = "digest" -version = "0.10.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9ed9a281f7bc9b7576e61468ba615a66a5c8cfdff42420a70aa82701a3b1e292" -dependencies = [ - "block-buffer", - "crypto-common", -] - [[package]] name = "doc-comment" version = "0.3.3" @@ -739,16 +701,6 @@ dependencies = [ "slab", ] -[[package]] -name = "generic-array" -version = "0.14.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "85649ca51fd72272d7821adaf274ad91c288277713d9c18820d8499a7ff69e9a" -dependencies = [ - "typenum", - "version_check", -] - [[package]] name = "getrandom" version = "0.2.15" @@ -1513,17 +1465,6 @@ dependencies = [ "serde", ] -[[package]] -name = "sha1" -version = "0.10.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e3bf829a2d51ab4a5ddf1352d8470c140cadc8301b2ae1789db023f01cedd6ba" -dependencies = [ - "cfg-if", - "cpufeatures", - "digest", -] - [[package]] name = "sharded-slab" version = "0.1.7" @@ -1898,12 +1839,6 @@ dependencies = [ "toml", ] -[[package]] -name = "typenum" -version = "1.17.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "42ff0bf0c66b8238c6f3b578df37d0b7848e55df8577b3f74f92a69acceeb825" - [[package]] name = "uds_windows" version = "1.1.0" @@ -1969,12 +1904,6 @@ version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "852e951cb7832cb45cb1169900d19760cfa39b82bc0ea9c0e5a14ae88411c98b" -[[package]] -name = "version_check" -version = "0.9.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "49874b5167b65d7193b8aba1567f5c7d93d001cafc34600cee003eda787e483f" - [[package]] name = "vsock" version = "0.3.0" @@ -2249,7 +2178,6 @@ dependencies = [ "rand", "serde", "serde_repr", - "sha1", "static_assertions", "tempfile", "test-log", diff --git a/zbus/Cargo.toml b/zbus/Cargo.toml index 01fee9ca7..b4b19ac4b 100644 --- a/zbus/Cargo.toml +++ b/zbus/Cargo.toml @@ -58,7 +58,6 @@ async-broadcast = "0.7.0" hex = "0.4.3" ordered-stream = "0.2" rand = "0.8.5" -sha1 = { version = "0.10.6", features = ["std"] } event-listener = "5.3.0" static_assertions = "1.1.0" async-trait = "0.1.80" diff --git a/zbus/src/abstractions/file.rs b/zbus/src/abstractions/file.rs deleted file mode 100644 index 83145b6de..000000000 --- a/zbus/src/abstractions/file.rs +++ /dev/null @@ -1,80 +0,0 @@ -//! Runtime-agnostic File I/O abstractions. -//! -//! Proving only specific API that we need internally. - -#[cfg(unix)] -use std::fs::Metadata; -use std::{ - io::Result, - path::Path, - pin::Pin, - task::{Context, Poll}, -}; - -use futures_core::Stream; - -#[cfg(not(feature = "tokio"))] -#[derive(Debug)] -pub struct FileLines(futures_util::io::Lines>); -#[cfg(feature = "tokio")] -#[derive(Debug)] -pub struct FileLines(tokio::io::Lines>); - -impl FileLines { - pub async fn open(path: impl AsRef) -> Result { - #[cfg(not(feature = "tokio"))] - { - async_fs::File::open(path) - .await - .map(futures_util::io::BufReader::new) - .map(futures_util::AsyncBufReadExt::lines) - .map(Self) - } - - #[cfg(feature = "tokio")] - { - tokio::fs::File::open(path) - .await - .map(tokio::io::BufReader::new) - .map(tokio::io::AsyncBufReadExt::lines) - .map(Self) - } - } -} - -impl Stream for FileLines { - type Item = Result; - - fn poll_next(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { - #[cfg(not(feature = "tokio"))] - { - Stream::poll_next(Pin::new(&mut self.get_mut().0), cx) - } - - #[cfg(feature = "tokio")] - { - let fut = self.get_mut().0.next_line(); - futures_util::pin_mut!(fut); - std::future::Future::poll(Pin::new(&mut fut), cx).map(Result::transpose) - } - } - - #[cfg(not(feature = "tokio"))] - fn size_hint(&self) -> (usize, Option) { - self.0.size_hint() - } -} - -// Not unix-specific itself but only used on unix. -#[cfg(unix)] -pub async fn metadata(path: impl AsRef) -> Result { - #[cfg(not(feature = "tokio"))] - { - async_fs::metadata(path).await - } - - #[cfg(feature = "tokio")] - { - tokio::fs::metadata(path).await - } -} diff --git a/zbus/src/abstractions/mod.rs b/zbus/src/abstractions/mod.rs index 473f7465f..06c20eec2 100644 --- a/zbus/src/abstractions/mod.rs +++ b/zbus/src/abstractions/mod.rs @@ -7,7 +7,6 @@ pub use executor::*; mod async_drop; pub(crate) mod async_lock; pub use async_drop::*; -pub(crate) mod file; // Not macOS-specific itself but only used on macOS. #[cfg(target_os = "macos")] diff --git a/zbus/src/blocking/connection/builder.rs b/zbus/src/blocking/connection/builder.rs index 7d2fd4908..496645772 100644 --- a/zbus/src/blocking/connection/builder.rs +++ b/zbus/src/blocking/connection/builder.rs @@ -10,7 +10,7 @@ use tokio::net::UnixStream; #[cfg(all(windows, not(feature = "tokio")))] use uds_windows::UnixStream; -use zvariant::{ObjectPath, Str}; +use zvariant::ObjectPath; #[cfg(feature = "p2p")] use crate::Guid; @@ -94,33 +94,6 @@ impl<'a> Builder<'a> { Self(self.0.auth_mechanism(auth_mechanism)) } - /// The cookie context to use during authentication. - /// - /// This is only used when the `cookie` authentication mechanism is enabled and only valid for - /// server connections. - /// - /// If not specified, the default cookie context of `org_freedesktop_general` will be used. - /// - /// # Errors - /// - /// If the given string is not a valid cookie context. - pub fn cookie_context(self, context: C) -> Result - where - C: Into>, - { - self.0.cookie_context(context).map(Self) - } - - /// The ID of the cookie to use during authentication. - /// - /// This is only used when the `cookie` authentication mechanism is enabled and only valid for - /// server connections. - /// - /// If not specified, the first cookie found in the cookie context file will be used. - pub fn cookie_id(self, id: usize) -> Self { - Self(self.0.cookie_id(id)) - } - /// The to-be-created connection will be a peer-to-peer connection. /// /// This method is only available when the `p2p` feature is enabled. diff --git a/zbus/src/connection/builder.rs b/zbus/src/connection/builder.rs index 35fddcda1..dd81bf100 100644 --- a/zbus/src/connection/builder.rs +++ b/zbus/src/connection/builder.rs @@ -21,7 +21,7 @@ use uds_windows::UnixStream; #[cfg(all(feature = "vsock", not(feature = "tokio")))] use vsock::VsockStream; -use zvariant::{ObjectPath, Str}; +use zvariant::ObjectPath; use crate::{ address::{Address, ToAddresses}, @@ -71,8 +71,6 @@ pub struct Builder<'a> { auth_mechanisms: Option>, #[cfg(feature = "bus-impl")] unique_name: Option>, - cookie_context: Option>, - cookie_id: Option, } assert_impl_all!(Builder<'_>: Send, Sync, Unpin); @@ -197,37 +195,6 @@ impl<'a> Builder<'a> { self } - /// The cookie context to use during authentication. - /// - /// This is only used when the `cookie` authentication mechanism is enabled and only valid for - /// server connections. - /// - /// If not specified, the default cookie context of `org_freedesktop_general` will be used. - /// - /// # Errors - /// - /// If the given string is not a valid cookie context. - pub fn cookie_context(mut self, context: C) -> Result - where - C: Into>, - { - self.cookie_context = Some(context.into().try_into()?); - - Ok(self) - } - - /// The ID of the cookie to use during authentication. - /// - /// This is only used when the `cookie` authentication mechanism is enabled and only valid for - /// server connections. - /// - /// If not specified, the first cookie found in the cookie context file will be used. - pub fn cookie_id(mut self, id: usize) -> Self { - self.cookie_id = Some(id); - - self - } - /// The to-be-created connection will be a peer-to-peer connection. /// /// This method is only available when the `p2p` feature is enabled. @@ -436,8 +403,6 @@ impl<'a> Builder<'a> { #[cfg(windows)] client_sid, self.auth_mechanisms, - self.cookie_id, - self.cookie_context.unwrap_or_default(), unique_name, ) .await? @@ -506,8 +471,6 @@ impl<'a> Builder<'a> { auth_mechanisms: None, #[cfg(feature = "bus-impl")] unique_name: None, - cookie_id: None, - cookie_context: None, } } diff --git a/zbus/src/connection/handshake/auth_mechanism.rs b/zbus/src/connection/handshake/auth_mechanism.rs index 8aae2b372..2d34cd3a3 100644 --- a/zbus/src/connection/handshake/auth_mechanism.rs +++ b/zbus/src/connection/handshake/auth_mechanism.rs @@ -4,7 +4,16 @@ use crate::{Error, Result}; /// Authentication mechanisms /// +/// Note that the `DBUS_COOKIE_SHA1` mechanism is not supported by zbus since version 5.0. The +/// reasons are: +/// +/// * It drags the `sha1` crate as a dependency, which can be [problematic for some users]. +/// * It makes the handshake more complex, now allowing use to pipeline all the commands. +/// * It's not widely used. If `EXTERNAL` is not an option, you might as well just use `ANONYMOUS`. +/// /// See +/// +/// [problematic for some users]: https://github.com/dbus2/zbus/issues/543 #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum AuthMechanism { /// This is the recommended authentication mechanism on platforms where credentials can be @@ -12,10 +21,6 @@ pub enum AuthMechanism { /// over the `unix:` transport. External, - /// This mechanism is designed to establish that a client has the ability to read a private - /// file owned by the user being authenticated. - Cookie, - /// Does not perform any authentication at all, and should not be accepted by message buses. /// However, it might sometimes be useful for non-message-bus uses of D-Bus. Anonymous, @@ -25,7 +30,6 @@ impl fmt::Display for AuthMechanism { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let mech = match self { AuthMechanism::External => "EXTERNAL", - AuthMechanism::Cookie => "DBUS_COOKIE_SHA1", AuthMechanism::Anonymous => "ANONYMOUS", }; write!(f, "{mech}") @@ -38,9 +42,8 @@ impl FromStr for AuthMechanism { fn from_str(s: &str) -> Result { match s { "EXTERNAL" => Ok(AuthMechanism::External), - "DBUS_COOKIE_SHA1" => Ok(AuthMechanism::Cookie), "ANONYMOUS" => Ok(AuthMechanism::Anonymous), - _ => Err(Error::Handshake(format!("Unknown mechanism: {s}"))), + _ => Err(Error::Handshake(format!("Unsupported mechanism: {s}"))), } } } diff --git a/zbus/src/connection/handshake/client.rs b/zbus/src/connection/handshake/client.rs index 9e290ff46..c85010df2 100644 --- a/zbus/src/connection/handshake/client.rs +++ b/zbus/src/connection/handshake/client.rs @@ -2,13 +2,11 @@ use async_trait::async_trait; use std::collections::VecDeque; use tracing::{debug, instrument, trace, warn}; -use sha1::{Digest, Sha1}; - use crate::{conn::socket::ReadHalf, is_flatpak, names::OwnedUniqueName, Message}; use super::{ - random_ascii, sasl_auth_id, AuthMechanism, Authenticated, BoxedSplit, Command, Common, Cookie, - Error, Handshake, OwnedGuid, Result, Str, + sasl_auth_id, AuthMechanism, Authenticated, BoxedSplit, Command, Common, Error, Handshake, + OwnedGuid, Result, }; /// A representation of an in-progress handshake, client-side @@ -33,7 +31,6 @@ impl Client { let mechanisms = mechanisms.unwrap_or_else(|| { let mut mechanisms = VecDeque::new(); mechanisms.push_back(AuthMechanism::External); - mechanisms.push_back(AuthMechanism::Cookie); mechanisms.push_back(AuthMechanism::Anonymous); mechanisms }); @@ -45,37 +42,6 @@ impl Client { } } - /// Respond to a cookie authentication challenge from the server. - /// - /// Returns the next command to send to the server. - async fn handle_cookie_challenge(&mut self, data: Vec) -> Result { - let context = std::str::from_utf8(&data) - .map_err(|_| Error::Handshake("Cookie context was not valid UTF-8".into()))?; - let mut split = context.split_ascii_whitespace(); - let context = split - .next() - .ok_or_else(|| Error::Handshake("Missing cookie context name".into()))?; - let context = Str::from(context).try_into()?; - let id = split - .next() - .ok_or_else(|| Error::Handshake("Missing cookie ID".into()))?; - let id = id - .parse() - .map_err(|e| Error::Handshake(format!("Invalid cookie ID `{id}`: {e}")))?; - let server_challenge = split - .next() - .ok_or_else(|| Error::Handshake("Missing cookie challenge".into()))?; - - let cookie = Cookie::lookup(&context, id).await?; - let cookie = cookie.cookie(); - let client_challenge = random_ascii(16); - let sec = format!("{server_challenge}:{client_challenge}:{cookie}"); - let sha1 = hex::encode(Sha1::digest(sec)); - let data = format!("{client_challenge} {sha1}").into_bytes(); - - Ok(Command::Data(Some(data))) - } - fn set_guid(&mut self, guid: OwnedGuid) -> Result<()> { match &self.server_guid { Some(server_guid) if *server_guid != guid => { @@ -116,11 +82,8 @@ impl Client { } /// Perform the authentication handshake with the server. - /// - /// In case of cookie auth, it returns the challenge response to send to the server, so it can - /// be batched with rest of the commands. #[instrument(skip(self))] - async fn authenticate(&mut self) -> Result> { + async fn authenticate(&mut self) -> Result<()> { loop { let mechanism = self.common.next_mechanism()?; trace!("Trying {mechanism} mechanism"); @@ -129,10 +92,6 @@ impl Client { AuthMechanism::External => { Command::Auth(Some(mechanism), Some(sasl_auth_id()?.into_bytes())) } - AuthMechanism::Cookie => Command::Auth( - Some(AuthMechanism::Cookie), - Some(sasl_auth_id()?.into_bytes()), - ), }; self.common.write_command(auth_cmd).await?; @@ -141,16 +100,7 @@ impl Client { trace!("Received OK from server"); self.set_guid(guid)?; - return Ok(None); - } - Command::Data(data) if mechanism == AuthMechanism::Cookie => { - let data = data.ok_or_else(|| { - Error::Handshake("Received DATA with no data from server".into()) - })?; - trace!("Received cookie challenge from server"); - let response = self.handle_cookie_challenge(data).await?; - - return Ok(Some(response)); + return Ok(()); } Command::Rejected(_) => debug!("{mechanism} rejected by the server"), Command::Error(e) => debug!("Received error from server: {e}"), @@ -164,18 +114,9 @@ impl Client { } /// Sends out all commands after authentication. - /// - /// This includes the challenge response for cookie auth, if any and returns the number of - /// responses expected from the server. #[instrument(skip(self))] - async fn send_secondary_commands( - &mut self, - challenge_response: Option, - ) -> Result { + async fn send_secondary_commands(&mut self) -> Result { let mut commands = Vec::with_capacity(4); - if let Some(response) = challenge_response { - commands.push(response); - } let can_pass_fd = self.common.socket_mut().read_mut().can_pass_unix_fd(); if can_pass_fd { @@ -222,11 +163,6 @@ impl Client { } Command::AgreeUnixFD => self.common.set_cap_unix_fd(true), Command::Error(e) => warn!("UNIX file descriptor passing rejected: {e}"), - // This also covers "REJECTED", which would mean that the server has rejected the - // authentication challenge response (likely cookie) since it already agreed to the - // mechanism. Theoretically we should be just trying the next auth mechanism but - // this most likely means something is very wrong and we're already too deep into - // the handshake to recover. cmd => { return Err(Error::Handshake(format!( "Unexpected command from server: {cmd}" @@ -248,8 +184,8 @@ impl Handshake for Client { #[cfg(any(target_os = "freebsd", target_os = "dragonfly"))] self.send_zero_byte().await?; - let challenge_response = self.authenticate().await?; - let expected_n_responses = self.send_secondary_commands(challenge_response).await?; + self.authenticate().await?; + let expected_n_responses = self.send_secondary_commands().await?; if expected_n_responses > 0 { self.receive_secondary_responses(expected_n_responses) diff --git a/zbus/src/connection/handshake/command.rs b/zbus/src/connection/handshake/command.rs index 8fea97890..be2252461 100644 --- a/zbus/src/connection/handshake/command.rs +++ b/zbus/src/connection/handshake/command.rs @@ -105,3 +105,9 @@ impl FromStr for Command { Ok(cmd) } } + +impl From for Error { + fn from(e: hex::FromHexError) -> Self { + Error::Handshake(format!("Invalid hexcode: {e}")) + } +} diff --git a/zbus/src/connection/handshake/cookies.rs b/zbus/src/connection/handshake/cookies.rs deleted file mode 100644 index 54b830fab..000000000 --- a/zbus/src/connection/handshake/cookies.rs +++ /dev/null @@ -1,147 +0,0 @@ -use std::{fmt, path::PathBuf}; - -use futures_util::StreamExt; -use tracing::trace; -use xdg_home::home_dir; -use zvariant::Str; - -use crate::{file::FileLines, Error, Result}; - -#[derive(Debug)] -pub(super) struct Cookie { - id: usize, - cookie: String, -} - -impl Cookie { - #[cfg(feature = "p2p")] - pub fn id(&self) -> usize { - self.id - } - - pub fn cookie(&self) -> &str { - &self.cookie - } - - fn keyring_path() -> Result { - let mut path = home_dir() - .ok_or_else(|| Error::Handshake("Failed to determine home directory".into()))?; - path.push(".dbus-keyrings"); - Ok(path) - } - - async fn read_keyring(context: &CookieContext<'_>) -> Result> { - let mut path = Cookie::keyring_path()?; - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - - let perms = crate::file::metadata(&path).await?.permissions().mode(); - if perms & 0o066 != 0 { - return Err(Error::Handshake( - "DBus keyring has invalid permissions".into(), - )); - } - } - #[cfg(not(unix))] - { - // FIXME: add code to check directory permissions - } - path.push(&*context.0); - trace!("Reading keyring {:?}", path); - let mut lines = FileLines::open(&path).await?.enumerate(); - let mut cookies = vec![]; - while let Some((n, line)) = lines.next().await { - let line = line?; - let mut split = line.split_whitespace(); - let id = split - .next() - .ok_or_else(|| { - Error::Handshake(format!( - "DBus cookie `{}` missing ID at line {n}", - path.display(), - )) - })? - .parse() - .map_err(|e| { - Error::Handshake(format!( - "Failed to parse cookie ID in file `{}` at line {n}: {e}", - path.display(), - )) - })?; - let _ = split.next().ok_or_else(|| { - Error::Handshake(format!( - "DBus cookie `{}` missing creation time at line {n}", - path.display(), - )) - })?; - let cookie = split - .next() - .ok_or_else(|| { - Error::Handshake(format!( - "DBus cookie `{}` missing cookie data at line {}", - path.to_str().unwrap(), - n - )) - })? - .to_string(); - cookies.push(Cookie { id, cookie }) - } - trace!("Loaded keyring {:?}", cookies); - Ok(cookies) - } - - pub async fn lookup(context: &CookieContext<'_>, id: usize) -> Result { - let keyring = Self::read_keyring(context).await?; - keyring - .into_iter() - .find(|c| c.id == id) - .ok_or_else(|| Error::Handshake(format!("DBus cookie ID {id} not found"))) - } - - #[cfg(feature = "p2p")] - pub async fn first(context: &CookieContext<'_>) -> Result { - let keyring = Self::read_keyring(context).await?; - keyring - .into_iter() - .next() - .ok_or_else(|| Error::Handshake("No cookies available".into())) - } -} - -#[derive(Debug)] -pub struct CookieContext<'c>(Str<'c>); - -impl<'c> TryFrom> for CookieContext<'c> { - type Error = Error; - - fn try_from(value: Str<'c>) -> Result { - if value.is_empty() { - return Err(Error::Handshake("Empty cookie context".into())); - } else if !value.is_ascii() || value.contains(['/', '\\', ' ', '\n', '\r', '\t', '.']) { - return Err(Error::Handshake( - "Invalid characters in cookie context".into(), - )); - } - - Ok(Self(value)) - } -} - -impl fmt::Display for CookieContext<'_> { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", self.0) - } -} - -impl Default for CookieContext<'_> { - fn default() -> Self { - Self(Str::from_static("org_freedesktop_general")) - } -} - -impl From for Error { - fn from(e: hex::FromHexError) -> Self { - Error::Handshake(format!("Invalid hexcode: {e}")) - } -} diff --git a/zbus/src/connection/handshake/mod.rs b/zbus/src/connection/handshake/mod.rs index 21e7db585..56a665cbd 100644 --- a/zbus/src/connection/handshake/mod.rs +++ b/zbus/src/connection/handshake/mod.rs @@ -2,7 +2,6 @@ mod auth_mechanism; mod client; mod command; mod common; -mod cookies; #[cfg(feature = "p2p")] mod server; @@ -11,7 +10,6 @@ use async_trait::async_trait; use nix::unistd::Uid; use std::{collections::VecDeque, fmt::Debug}; use zbus_names::OwnedUniqueName; -use zvariant::Str; #[cfg(windows)] use crate::win32; @@ -23,8 +21,6 @@ pub use auth_mechanism::AuthMechanism; use client::Client; use command::Command; use common::Common; -use cookies::Cookie; -pub(crate) use cookies::CookieContext; #[cfg(feature = "p2p")] use server::Server; @@ -73,8 +69,6 @@ impl Authenticated { #[cfg(unix)] client_uid: Option, #[cfg(windows)] client_sid: Option, auth_mechanisms: Option>, - cookie_id: Option, - cookie_context: CookieContext<'_>, unique_name: Option, ) -> Result { Server::new( @@ -85,8 +79,6 @@ impl Authenticated { #[cfg(windows)] client_sid, auth_mechanisms, - cookie_id, - cookie_context, unique_name, )? .perform() @@ -103,18 +95,6 @@ pub trait Handshake { async fn perform(mut self) -> Result; } -fn random_ascii(len: usize) -> String { - use rand::{distributions::Alphanumeric, thread_rng, Rng}; - use std::iter; - - let mut rng = thread_rng(); - iter::repeat(()) - .map(|()| rng.sample(Alphanumeric)) - .map(char::from) - .take(len) - .collect() -} - fn sasl_auth_id() -> Result { let id = { #[cfg(unix)] @@ -178,16 +158,8 @@ mod tests { let guid = OwnedGuid::from(Guid::generate()); let client = Client::new(p0.into(), None, Some(guid.clone()), false); - let server = Server::new( - p1.into(), - guid, - Some(Uid::effective().into()), - None, - None, - CookieContext::default(), - None, - ) - .unwrap(); + let server = + Server::new(p1.into(), guid, Some(Uid::effective().into()), None, None).unwrap(); // proceed to the handshakes let (client, server) = crate::utils::block_on(join( @@ -209,8 +181,6 @@ mod tests { Some(Uid::effective().into()), None, None, - CookieContext::default(), - None, ) .unwrap(); @@ -239,8 +209,6 @@ mod tests { Some(Uid::effective().into()), None, None, - CookieContext::default(), - None, ) .unwrap(); @@ -267,8 +235,6 @@ mod tests { Some(Uid::effective().into()), None, None, - CookieContext::default(), - None, ) .unwrap(); @@ -286,8 +252,6 @@ mod tests { Some(Uid::effective().into()), Some(vec![AuthMechanism::Anonymous].into()), None, - CookieContext::default(), - None, ) .unwrap(); @@ -305,8 +269,6 @@ mod tests { Some(Uid::effective().into()), Some(vec![AuthMechanism::Anonymous].into()), None, - CookieContext::default(), - None, ) .unwrap(); diff --git a/zbus/src/connection/handshake/server.rs b/zbus/src/connection/handshake/server.rs index 063ec8582..518a556f7 100644 --- a/zbus/src/connection/handshake/server.rs +++ b/zbus/src/connection/handshake/server.rs @@ -1,13 +1,11 @@ use async_trait::async_trait; -use sha1::{Digest, Sha1}; use std::collections::VecDeque; use tracing::{instrument, trace}; use crate::names::OwnedUniqueName; use super::{ - random_ascii, sasl_auth_id, AuthMechanism, Authenticated, BoxedSplit, Command, Common, Cookie, - CookieContext, Error, Handshake, OwnedGuid, Result, + AuthMechanism, Authenticated, BoxedSplit, Command, Common, Error, Handshake, OwnedGuid, Result, }; /* @@ -26,7 +24,7 @@ enum ServerHandshakeStep { /// /// This would typically be used to implement a D-Bus broker, or in the context of a P2P connection. #[derive(Debug)] -pub struct Server<'s> { +pub struct Server { common: Common, step: ServerHandshakeStep, guid: OwnedGuid, @@ -34,22 +32,18 @@ pub struct Server<'s> { client_uid: Option, #[cfg(windows)] client_sid: Option, - cookie_id: Option, - cookie_context: CookieContext<'s>, unique_name: Option, } -impl<'s> Server<'s> { +impl Server { pub fn new( socket: BoxedSplit, guid: OwnedGuid, #[cfg(unix)] client_uid: Option, #[cfg(windows)] client_sid: Option, mechanisms: Option>, - cookie_id: Option, - cookie_context: CookieContext<'s>, unique_name: Option, - ) -> Result> { + ) -> Result { let mechanisms = match mechanisms { Some(mechanisms) => mechanisms, None => { @@ -67,8 +61,6 @@ impl<'s> Server<'s> { client_uid, #[cfg(windows)] client_sid, - cookie_id, - cookie_context, guid, unique_name, }) @@ -109,55 +101,6 @@ impl<'s> Server<'s> { } } - #[instrument(skip(self))] - async fn check_cookie_auth(&mut self, sasl_id: &[u8]) -> Result<()> { - let cookie = match self.cookie_id { - Some(cookie_id) => Cookie::lookup(&self.cookie_context, cookie_id).await?, - None => Cookie::first(&self.cookie_context).await?, - }; - let id = std::str::from_utf8(sasl_id) - .map_err(|e| Error::Handshake(format!("Invalid ID: {e}")))?; - if sasl_auth_id()? != id { - // While the spec will make you believe that DBUS_COOKIE_SHA1 can be used to - // authenticate any user, it is not even possible (or correct) for the server to manage - // contents in random users' home directories. - // - // The dbus reference implementation also has the same limitation/behavior. - self.rejected_error().await?; - return Ok(()); - } - let server_challenge = random_ascii(16); - let data = format!("{} {} {server_challenge}", self.cookie_context, cookie.id()); - let cmd = Command::Data(Some(data.into_bytes())); - trace!("Sending DBUS_COOKIE_SHA1 authentication challenge"); - self.common.write_command(cmd).await?; - - let auth_data = match self.common.read_command().await? { - Command::Data(data) => data, - _ => None, - }; - let auth_data = auth_data.ok_or_else(|| { - Error::Handshake("Expected DBUS_COOKIE_SHA1 authentication challenge response".into()) - })?; - let client_auth = std::str::from_utf8(&auth_data) - .map_err(|e| Error::Handshake(format!("Invalid COOKIE authentication data: {e}")))?; - let mut split = client_auth.split_ascii_whitespace(); - let client_challenge = split - .next() - .ok_or_else(|| Error::Handshake("Missing cookie challenge".into()))?; - let client_sha1 = split - .next() - .ok_or_else(|| Error::Handshake("Missing client cookie data".into()))?; - let sec = format!("{server_challenge}:{client_challenge}:{}", cookie.cookie()); - let sha1 = hex::encode(Sha1::digest(sec)); - - if sha1 == client_sha1 { - self.auth_ok().await - } else { - self.rejected_error().await - } - } - #[instrument(skip(self))] async fn unsupported_command_error(&mut self) -> Result<()> { let cmd = Command::Error("Unsupported or misplaced command".to_string()); @@ -213,9 +156,6 @@ impl<'s> Server<'s> { (Some(AuthMechanism::External), Some(sasl_id)) => { self.check_external_auth(sasl_id).await?; } - (Some(AuthMechanism::Cookie), Some(sasl_id)) => { - self.check_cookie_auth(sasl_id).await?; - } _ => self.rejected_error().await?, } } @@ -242,7 +182,6 @@ impl<'s> Server<'s> { self.check_external_auth(&data).await?; } (AuthMechanism::Anonymous, Command::Data(_)) => self.auth_ok().await?, - (_, Command::Data(_)) => self.rejected_error().await?, (_, _) => self.unsupported_command_error().await?, } Ok(()) @@ -286,7 +225,7 @@ impl<'s> Server<'s> { } #[async_trait] -impl Handshake for Server<'_> { +impl Handshake for Server { #[instrument(skip(self))] async fn perform(mut self) -> Result { while !self.next_step().await? {} diff --git a/zbus/src/connection/mod.rs b/zbus/src/connection/mod.rs index dfb0a0e1c..a76e9bc88 100644 --- a/zbus/src/connection/mod.rs +++ b/zbus/src/connection/mod.rs @@ -1816,83 +1816,6 @@ mod p2p_tests { ) } - #[cfg(any(unix, not(feature = "tokio")))] - #[test] - #[timeout(15000)] - fn unix_p2p_cookie_auth() { - use crate::utils::block_on; - use std::{ - fs::{create_dir_all, remove_file, write}, - time::{SystemTime as Time, UNIX_EPOCH}, - }; - #[cfg(unix)] - use std::{ - fs::{set_permissions, Permissions}, - os::unix::fs::PermissionsExt, - }; - use xdg_home::home_dir; - - let cookie_context = "zbus-test-cookie-context"; - let cookie_id = 123456789; - let cookie = hex::encode(b"our cookie"); - - // Ensure cookie directory exists. - let cookie_dir = home_dir().unwrap().join(".dbus-keyrings"); - create_dir_all(&cookie_dir).unwrap(); - #[cfg(unix)] - set_permissions(&cookie_dir, Permissions::from_mode(0o700)).unwrap(); - - // Create a cookie file. - let cookie_file = cookie_dir.join(cookie_context); - let ts = Time::now().duration_since(UNIX_EPOCH).unwrap().as_secs(); - let cookie_entry = format!("{cookie_id} {ts} {cookie}"); - write(&cookie_file, cookie_entry).unwrap(); - - // Explicit cookie ID. - let res1 = block_on(test_unix_p2p_cookie_auth(cookie_context, Some(cookie_id))); - // Implicit cookie ID (first one should be picked). - let res2 = block_on(test_unix_p2p_cookie_auth(cookie_context, None)); - - // Remove the cookie file. - remove_file(&cookie_file).unwrap(); - - res1.unwrap(); - res2.unwrap(); - } - - #[cfg(any(unix, not(feature = "tokio")))] - async fn test_unix_p2p_cookie_auth( - cookie_context: &'static str, - cookie_id: Option, - ) -> Result<()> { - #[cfg(all(unix, not(feature = "tokio")))] - use std::os::unix::net::UnixStream; - #[cfg(all(unix, feature = "tokio"))] - use tokio::net::UnixStream; - #[cfg(all(windows, not(feature = "tokio")))] - use uds_windows::UnixStream; - - let guid = Guid::generate(); - - let (p0, p1) = UnixStream::pair().unwrap(); - let mut server_builder = Builder::unix_stream(p0) - .server(guid) - .unwrap() - .p2p() - .auth_mechanism(AuthMechanism::Cookie) - .cookie_context(cookie_context) - .unwrap(); - if let Some(cookie_id) = cookie_id { - server_builder = server_builder.cookie_id(cookie_id); - } - - futures_util::try_join!( - Builder::unix_stream(p1).p2p().build(), - server_builder.build(), - ) - .map(|_| ()) - } - #[test] #[timeout(15000)] fn channel_pair() {