Skip to content

Commit

Permalink
💥 zb: Drop support for DBUS_COOKIE_SHA1 auth mechanism
Browse files Browse the repository at this point in the history 
* It drags the `sha1` crate as a dependency, which can be [problematic for some users].
* It makes the handshake more complex, not allowing to pipeline all the commands.
* It's not widely used. If `EXTERNAL` is not an option, you might as well just use `ANONYMOUS`.

Fixes #727.

[problematic for some users]: #543
  • Loading branch information
@zeenix
zeenix committed Oct 3, 2024
1 parent 2c4f8e3 commit 817d636
Show file tree
Hide file tree
Showing 9 changed files with 32 additions and 474 deletions.
29 changes: 1 addition & 28 deletions zbus/src/blocking/connection/builder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<C>(self, context: C) -> Result<Self>
where
C: Into<Str<'a>>,
{
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.
Expand Down
39 changes: 1 addition & 38 deletions zbus/src/connection/builder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -71,8 +71,6 @@ pub struct Builder<'a> {
auth_mechanisms: Option<VecDeque<AuthMechanism>>,
#[cfg(feature = "bus-impl")]
unique_name: Option<crate::names::UniqueName<'a>>,
cookie_context: Option<super::handshake::CookieContext<'a>>,
cookie_id: Option<usize>,
}

assert_impl_all!(Builder<'_>: Send, Sync, Unpin);
Expand Down Expand Up @@ -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<C>(mut self, context: C) -> Result<Self>
where
C: Into<Str<'a>>,
{
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.
Expand Down Expand Up @@ -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?
Expand Down Expand Up @@ -506,8 +471,6 @@ impl<'a> Builder<'a> {
auth_mechanisms: None,
#[cfg(feature = "bus-impl")]
unique_name: None,
cookie_id: None,
cookie_context: None,
}
}

Expand Down
17 changes: 10 additions & 7 deletions zbus/src/connection/handshake/auth_mechanism.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,23 @@ 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 <https://dbus.freedesktop.org/doc/dbus-specification.html#auth-mechanisms>
///
/// [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
/// transferred out-of-band, in particular Unix platforms that can perform credentials-passing
/// 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,
Expand All @@ -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}")
Expand All @@ -38,9 +42,8 @@ impl FromStr for AuthMechanism {
fn from_str(s: &str) -> Result<Self> {
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}"))),
}
}
}
78 changes: 7 additions & 71 deletions zbus/src/connection/handshake/client.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
});
Expand All @@ -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<u8>) -> Result<Command> {
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 => {
Expand Down Expand Up @@ -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<Option<Command>> {
async fn authenticate(&mut self) -> Result<()> {
loop {
let mechanism = self.common.next_mechanism()?;
trace!("Trying {mechanism} mechanism");
Expand All @@ -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?;

Expand All @@ -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}"),
Expand All @@ -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<Command>,
) -> Result<usize> {
async fn send_secondary_commands(&mut self) -> Result<usize> {
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 {
Expand Down Expand Up @@ -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}"
Expand All @@ -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)
Expand Down
6 changes: 6 additions & 0 deletions zbus/src/connection/handshake/command.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,3 +105,9 @@ impl FromStr for Command {
Ok(cmd)
}
}

impl From<hex::FromHexError> for Error {
fn from(e: hex::FromHexError) -> Self {
Error::Handshake(format!("Invalid hexcode: {e}"))
}
}
Loading

0 comments on commit 817d636

Please sign in to comment.