diff --git a/sable_ircd/src/command/handlers/services/sasl.rs b/sable_ircd/src/command/handlers/services/sasl.rs index 9fa74fc3..4f2895e1 100644 --- a/sable_ircd/src/command/handlers/services/sasl.rs +++ b/sable_ircd/src/command/handlers/services/sasl.rs @@ -59,7 +59,7 @@ async fn handle_authenticate( match services .require()? - .send_remote_request(authenticate_request.into()) + .send_remote_request(authenticate_request.clone().into()) .await { Ok(RemoteServerResponse::Services(RemoteServicesServerResponse::Authenticate(status))) => { @@ -95,8 +95,9 @@ async fn handle_authenticate( } } } - _ => { - response.numeric(make_numeric!(SaslAborted)); + msg => { + tracing::error!("Unexpected services response to {authenticate_request:?}: {msg:?}"); + response.numeric(make_numeric!(UnknownError, "Unknown SASL error")); } } Ok(()) diff --git a/sable_ircd/src/messages/numeric.rs b/sable_ircd/src/messages/numeric.rs index edfbc932..5267751b 100644 --- a/sable_ircd/src/messages/numeric.rs +++ b/sable_ircd/src/messages/numeric.rs @@ -74,6 +74,7 @@ define_messages! { 374(EndOfInfo) => { () => ":End of /INFO list" }, + 400(UnknownError) => { (reason: &str) => ":{reason}" }, 401(NoSuchTarget) => { (unknown: &str) => "{unknown} :No such nick/channel" }, 402(NoSuchServer) => { (server_name: &ServerName) => "{server_name} :No such server" }, 403(NoSuchChannel) => { (chname: &ChannelName) => "{chname} :No such channel" }, diff --git a/sable_services/src/server/command/sasl_commands.rs b/sable_services/src/server/command/sasl_commands.rs index ec637eaf..7e139b03 100644 --- a/sable_services/src/server/command/sasl_commands.rs +++ b/sable_services/src/server/command/sasl_commands.rs @@ -23,18 +23,27 @@ impl ServicesServer { } pub fn authenticate(&self, session_id: SaslSessionId, data: Vec) -> CommandResult { - let Some(session) = self.sasl_sessions.get(&session_id) else { + let session_entry = self.sasl_sessions.entry(session_id); + let dashmap::mapref::entry::Entry::Occupied(session_entry) = session_entry else { return Ok(Authenticate(Fail).into()); }; + let session = session_entry.get(); let Some(mechanism) = self.sasl_mechanisms.get(&session.mechanism) else { - self.sasl_sessions.remove(&session_id); + session_entry.remove(); return Ok(Authenticate(Fail).into()); }; - let response = mechanism.step(self, &session, data)?; - - Ok(Authenticate(response).into()) + match mechanism.step(self, session, data) { + Ok(response) => Ok(Authenticate(response).into()), + Err(e) => { + tracing::debug!(?session_id, "SASL {} step failed: {e}", mechanism.name()); + // Equivalent to self.fail_authenticate(session_id) but we can't call it here + // because we already have the lock. + session_entry.remove(); + Ok(Authenticate(Fail).into()) + } + } } pub fn abort_authenticate(&self, session_id: SaslSessionId) -> CommandResult { diff --git a/sable_services/src/server/sasl/mod.rs b/sable_services/src/server/sasl/mod.rs index eb382a05..3279eb03 100644 --- a/sable_services/src/server/sasl/mod.rs +++ b/sable_services/src/server/sasl/mod.rs @@ -7,6 +7,8 @@ use AuthenticateStatus::*; pub type SaslResult = Result; pub trait SaslMechanism: Send + Sync + 'static { + fn name(&self) -> String; + fn step(&self, server: &ServicesServer, session: &SaslSession, data: Vec) -> SaslResult; } diff --git a/sable_services/src/server/sasl/plain.rs b/sable_services/src/server/sasl/plain.rs index 1ab58887..d3b70e38 100644 --- a/sable_services/src/server/sasl/plain.rs +++ b/sable_services/src/server/sasl/plain.rs @@ -6,6 +6,10 @@ use sable_network::prelude::*; pub struct SaslPlain; impl SaslMechanism for SaslPlain { + fn name(&self) -> String { + "PLAIN".to_string() + } + fn step( &self, server: &ServicesServer,