Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🛂 zb: Only support one authentication method at a time #1041

Merged
merged 3 commits into from
Oct 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions zbus/src/blocking/connection/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ use zvariant::ObjectPath;
#[cfg(feature = "p2p")]
use crate::Guid;
use crate::{
address::ToAddresses, blocking::Connection, connection::socket::BoxedSplit,
names::WellKnownName, object_server::Interface, utils::block_on, AuthMechanism, Error, Result,
address::ToAddresses, blocking::Connection, conn::AuthMechanism,
connection::socket::BoxedSplit, names::WellKnownName, object_server::Interface,
utils::block_on, Error, Result,
};

/// A builder for [`zbus::blocking::Connection`].
Expand Down
14 changes: 7 additions & 7 deletions zbus/src/connection/builder.rs
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
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
2 changes: 1 addition & 1 deletion zbus/src/connection/handshake/command.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::{fmt, str::FromStr};

use crate::{AuthMechanism, Error, Guid, OwnedGuid, Result};
use crate::{conn::AuthMechanism, Error, Guid, OwnedGuid, Result};

// The plain-text SASL profile authentication protocol described here:
// <https://dbus.freedesktop.org/doc/dbus-specification.html#auth-protocol>
Expand Down
25 changes: 8 additions & 17 deletions zbus/src/connection/handshake/common.rs
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
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
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
Loading