Skip to content

Commit

Permalink
🛂 zb: Only support one authentication method at a time
Browse files Browse the repository at this point in the history 
Now that we're down to only two authentication mechanisms with one of
them being no-authentication, this really makes sense since we can just
autodetect what authentication method to use for a specific socket type
on a specific platform.

This also simplifies the handshake logic and will allow us to pipeline the
whole client-side handshake in the future, when we can drop the
xdg-dbus-proxy [workarounds].

As they say in the famous Queen song:

Hey! One man, one goal
Ha, one mission
One heart, one soul
Just one solution
One flash of light
Yeah, one God, one vision

One flesh, one bone, one true religion
One voice, one hope, one real decision
Whoa, whoa, whoa, whoa, whoa, whoa
Give me one vision, yeah

Fixes #731.

[workarounds]: #781
  • Loading branch information
@zeenix
zeenix committed Oct 4, 2024
1 parent 467bea5 commit 23bda8e
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 89 deletions.
14 changes: 7 additions & 7 deletions zbus/src/connection/builder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use std::net::TcpStream;
#[cfg(all(unix, not(feature = "tokio")))]
use std::os::unix::net::UnixStream;
use std::{
collections::{HashMap, HashSet, VecDeque},
collections::{HashMap, HashSet},
vec,
};
#[cfg(feature = "tokio")]
Expand Down Expand Up @@ -68,7 +68,7 @@ pub struct Builder<'a> {
internal_executor: bool,
interfaces: Interfaces<'a>,
names: HashSet<WellKnownName<'a>>,
auth_mechanisms: Option<VecDeque<AuthMechanism>>,
auth_mechanism: Option<AuthMechanism>,
#[cfg(feature = "bus-impl")]
unique_name: Option<crate::names::UniqueName<'a>>,
}
Expand Down Expand Up @@ -190,7 +190,7 @@ impl<'a> Builder<'a> {

/// Specify the mechanism to use during authentication.
pub fn auth_mechanism(mut self, auth_mechanism: AuthMechanism) -> Self {
self.auth_mechanisms = Some(VecDeque::from(vec![auth_mechanism]));
self.auth_mechanism = Some(auth_mechanism);

self
}
Expand Down Expand Up @@ -381,7 +381,7 @@ impl<'a> Builder<'a> {
match self.guid {
None => {
// SASL Handshake
Authenticated::client(stream, server_guid, self.auth_mechanisms, is_bus_conn)
Authenticated::client(stream, server_guid, self.auth_mechanism, is_bus_conn)
.await?
}
Some(guid) => {
Expand All @@ -402,15 +402,15 @@ impl<'a> Builder<'a> {
client_uid,
#[cfg(windows)]
client_sid,
self.auth_mechanisms,
self.auth_mechanism,
unique_name,
)
.await?
}
}

#[cfg(not(feature = "p2p"))]
Authenticated::client(stream, server_guid, self.auth_mechanisms, is_bus_conn).await?
Authenticated::client(stream, server_guid, self.auth_mechanism, is_bus_conn).await?
};

// SAFETY: `Authenticated` is always built with these fields set to `Some`.
Expand Down Expand Up @@ -468,7 +468,7 @@ impl<'a> Builder<'a> {
internal_executor: true,
interfaces: HashMap::new(),
names: HashSet::new(),
auth_mechanisms: None,
auth_mechanism: None,
#[cfg(feature = "bus-impl")]
unique_name: None,
}
Expand Down
65 changes: 32 additions & 33 deletions zbus/src/connection/handshake/client.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use async_trait::async_trait;
use std::collections::VecDeque;
use tracing::{debug, instrument, trace, warn};
use tracing::{instrument, trace, warn};

use crate::{conn::socket::ReadHalf, is_flatpak, names::OwnedUniqueName, Message};

Expand All @@ -24,19 +23,14 @@ impl Client {
/// Start a handshake on this client socket
pub fn new(
socket: BoxedSplit,
mechanisms: Option<VecDeque<AuthMechanism>>,
mechanism: Option<AuthMechanism>,
server_guid: Option<OwnedGuid>,
bus: bool,
) -> Client {
let mechanisms = mechanisms.unwrap_or_else(|| {
let mut mechanisms = VecDeque::new();
mechanisms.push_back(AuthMechanism::External);
mechanisms.push_back(AuthMechanism::Anonymous);
mechanisms
});
let mechanism = mechanism.unwrap_or_else(|| socket.read().auth_mechanism());

Client {
common: Common::new(socket, mechanisms),
common: Common::new(socket, mechanism),
server_guid,
bus,
}
Expand Down Expand Up @@ -84,32 +78,37 @@ impl Client {
/// Perform the authentication handshake with the server.
#[instrument(skip(self))]
async fn authenticate(&mut self) -> Result<()> {
loop {
let mechanism = self.common.next_mechanism()?;
trace!("Trying {mechanism} mechanism");
let auth_cmd = match mechanism {
AuthMechanism::Anonymous => Command::Auth(Some(mechanism), Some("zbus".into())),
AuthMechanism::External => {
Command::Auth(Some(mechanism), Some(sasl_auth_id()?.into_bytes()))
}
};
self.common.write_command(auth_cmd).await?;
let mechanism = self.common.mechanism();
trace!("Trying {mechanism} mechanism");
let auth_cmd = match mechanism {
AuthMechanism::Anonymous => Command::Auth(Some(mechanism), Some("zbus".into())),
AuthMechanism::External => {
Command::Auth(Some(mechanism), Some(sasl_auth_id()?.into_bytes()))
}
};
self.common.write_command(auth_cmd).await?;

match self.common.read_command().await? {
Command::Ok(guid) => {
trace!("Received OK from server");
self.set_guid(guid)?;
match self.common.read_command().await? {
Command::Ok(guid) => {
trace!("Received OK from server");
self.set_guid(guid)?;

return Ok(());
}
Command::Rejected(_) => debug!("{mechanism} rejected by the server"),
Command::Error(e) => debug!("Received error from server: {e}"),
cmd => {
return Err(Error::Handshake(format!(
"Unexpected command from server: {cmd}"
)))
}
Ok(())
}
Command::Rejected(accepted) => {
let list = accepted
.iter()
.map(|m| m.to_string())
.collect::<Vec<_>>()
.join(", ");
Err(Error::Handshake(format!(
"{mechanism} rejected by the server. Accepted mechanisms: [{list}]"
)))
}
Command::Error(e) => Err(Error::Handshake(format!("Received error from server: {e}"))),
cmd => Err(Error::Handshake(format!(
"Unexpected command from server: {cmd}"
))),
}
}

Expand Down
25 changes: 8 additions & 17 deletions zbus/src/connection/handshake/common.rs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use std::collections::VecDeque;
use tracing::{instrument, trace};

use super::{AuthMechanism, BoxedSplit, Command};
Expand All @@ -12,21 +11,20 @@ pub(super) struct Common {
#[cfg(unix)]
received_fds: Vec<std::os::fd::OwnedFd>,
cap_unix_fd: bool,
// the current AUTH mechanism is front, ordered by priority
mechanisms: VecDeque<AuthMechanism>,
mechanism: AuthMechanism,
first_command: bool,
}

impl Common {
/// Start a handshake on this client socket
pub fn new(socket: BoxedSplit, mechanisms: VecDeque<AuthMechanism>) -> Self {
pub fn new(socket: BoxedSplit, mechanism: AuthMechanism) -> Self {
Self {
socket,
recv_buffer: Vec::new(),
#[cfg(unix)]
received_fds: Vec::new(),
cap_unix_fd: false,
mechanisms,
mechanism,
first_command: true,
}
}
Expand All @@ -44,9 +42,8 @@ impl Common {
self.cap_unix_fd = cap_unix_fd;
}

#[cfg(feature = "p2p")]
pub fn mechanisms(&self) -> &VecDeque<AuthMechanism> {
&self.mechanisms
pub fn mechanism(&self) -> AuthMechanism {
self.mechanism
}

pub fn into_components(self) -> IntoComponentsReturn {
Expand All @@ -56,7 +53,7 @@ impl Common {
#[cfg(unix)]
self.received_fds,
self.cap_unix_fd,
self.mechanisms,
self.mechanism,
)
}

Expand Down Expand Up @@ -175,12 +172,6 @@ impl Common {

Ok(commands)
}

pub fn next_mechanism(&mut self) -> Result<AuthMechanism> {
self.mechanisms
.pop_front()
.ok_or_else(|| Error::Handshake("Exhausted available AUTH mechanisms".into()))
}
}

#[cfg(unix)]
Expand All @@ -189,7 +180,7 @@ type IntoComponentsReturn = (
Vec<u8>,
Vec<std::os::fd::OwnedFd>,
bool,
VecDeque<AuthMechanism>,
AuthMechanism,
);
#[cfg(not(unix))]
type IntoComponentsReturn = (BoxedSplit, Vec<u8>, bool, VecDeque<AuthMechanism>);
type IntoComponentsReturn = (BoxedSplit, Vec<u8>, bool, AuthMechanism);
14 changes: 7 additions & 7 deletions zbus/src/connection/handshake/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ mod server;
use async_trait::async_trait;
#[cfg(unix)]
use nix::unistd::Uid;
use std::{collections::VecDeque, fmt::Debug};
use std::fmt::Debug;
use zbus_names::OwnedUniqueName;

#[cfg(windows)]
Expand Down Expand Up @@ -51,10 +51,10 @@ impl Authenticated {
pub async fn client(
socket: BoxedSplit,
server_guid: Option<OwnedGuid>,
mechanisms: Option<VecDeque<AuthMechanism>>,
mechanism: Option<AuthMechanism>,
bus: bool,
) -> Result<Self> {
Client::new(socket, mechanisms, server_guid, bus)
Client::new(socket, mechanism, server_guid, bus)
.perform()
.await
}
Expand All @@ -68,7 +68,7 @@ impl Authenticated {
guid: OwnedGuid,
#[cfg(unix)] client_uid: Option<u32>,
#[cfg(windows)] client_sid: Option<String>,
auth_mechanisms: Option<VecDeque<AuthMechanism>>,
auth_mechanism: Option<AuthMechanism>,
unique_name: Option<OwnedUniqueName>,
) -> Result<Self> {
Server::new(
Expand All @@ -78,7 +78,7 @@ impl Authenticated {
client_uid,
#[cfg(windows)]
client_sid,
auth_mechanisms,
auth_mechanism,
unique_name,
)?
.perform()
Expand Down Expand Up @@ -250,7 +250,7 @@ mod tests {
p1.into(),
Guid::generate().into(),
Some(Uid::effective().into()),
Some(vec![AuthMechanism::Anonymous].into()),
Some(AuthMechanism::Anonymous),
None,
)
.unwrap();
Expand All @@ -267,7 +267,7 @@ mod tests {
p1.into(),
Guid::generate().into(),
Some(Uid::effective().into()),
Some(vec![AuthMechanism::Anonymous].into()),
Some(AuthMechanism::Anonymous),
None,
)
.unwrap();
Expand Down
42 changes: 17 additions & 25 deletions zbus/src/connection/handshake/server.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use async_trait::async_trait;
use std::collections::VecDeque;
use tracing::{instrument, trace};

use crate::names::OwnedUniqueName;
Expand Down Expand Up @@ -41,21 +40,13 @@ impl Server {
guid: OwnedGuid,
#[cfg(unix)] client_uid: Option<u32>,
#[cfg(windows)] client_sid: Option<String>,
mechanisms: Option<VecDeque<AuthMechanism>>,
mechanism: Option<AuthMechanism>,
unique_name: Option<OwnedUniqueName>,
) -> Result<Self> {
let mechanisms = match mechanisms {
Some(mechanisms) => mechanisms,
None => {
let mut mechanisms = VecDeque::new();
mechanisms.push_back(AuthMechanism::External);

mechanisms
}
};
let mechanism = mechanism.unwrap_or_else(|| socket.read().auth_mechanism());

Ok(Server {
common: Common::new(socket, mechanisms),
common: Common::new(socket, mechanism),
step: ServerHandshakeStep::WaitingForAuth,
#[cfg(unix)]
client_uid,
Expand Down Expand Up @@ -111,8 +102,7 @@ impl Server {

#[instrument(skip(self))]
async fn rejected_error(&mut self) -> Result<()> {
let mechanisms = self.common.mechanisms().iter().cloned().collect();
let cmd = Command::Rejected(mechanisms);
let cmd = Command::Rejected(vec![self.common.mechanism()]);
trace!("Sending authentication error");
self.common.write_command(cmd).await?;
self.step = ServerHandshakeStep::WaitingForAuth;
Expand Down Expand Up @@ -141,22 +131,24 @@ impl Server {
trace!("Waiting for authentication");
let reply = self.common.read_command().await?;
match reply {
Command::Auth(mech, resp) => {
let mech = mech.filter(|m| self.common.mechanisms().contains(m));
Command::Auth(requested_mech, resp) => {
let mech = self.common.mechanism();
if requested_mech != Some(mech) {
self.rejected_error().await?;

match (mech, &resp) {
(Some(mech), None) => {
return Ok(());
}

match &resp {
None => {
trace!("Sending data request");
self.common.write_command(Command::Data(None)).await?;
self.step = ServerHandshakeStep::WaitingForData(mech);
}
(Some(AuthMechanism::Anonymous), Some(_)) => {
self.auth_ok().await?;
}
(Some(AuthMechanism::External), Some(sasl_id)) => {
self.check_external_auth(sasl_id).await?;
}
_ => self.rejected_error().await?,
Some(sasl_id) => match mech {
AuthMechanism::Anonymous => self.auth_ok().await?,
AuthMechanism::External => self.check_external_auth(sasl_id).await?,
},
}
}
Command::Cancel | Command::Error(_) => {
Expand Down

0 comments on commit 23bda8e

Please sign in to comment.