Eugeny / russh

Rust SSH client & server library
https://docs.rs/russh
870 stars 91 forks source link

Stack Overflow custom error type #318

Closed FelixKLG closed 1 month ago

FelixKLG commented 1 month ago

So, really strange error I didn't really expect to see. When I used a custom Error type for the Handler, a struct which impls Error and fmt::display, when my authentication fails (I haven't gotten to auth yet so it always fails) my program crashes with Tokio saying a workers stack overflowed.

After switching to anyhow the program works in a way I expect and doesn't crash after auth fail.

MacOS Sonoma - M1 Max Crate version: russh = { version = "0.44.0", features = ["openssl"] }

   Compiling sshtest v0.0.1 (/Users/felixklg/Developer/sshtest)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.78s
     Running `target/debug/sshtest`

thread 'tokio-runtime-worker' has overflowed its stack
fatal runtime error: stack overflow
zsh: abort      cargo r

Pardon the struct names:

use std::{collections::HashMap, error::Error, fmt, sync::Arc};

use async_trait::async_trait;
use russh::{
    keys::key,
    server::{self, Auth, Handler, Msg, Server, Session},
    Channel, ChannelId, MethodSet,
};
use tokio::sync::Mutex;

#[derive(Debug)]
struct CumSockError;

impl Error for CumSockError {}

impl fmt::Display for CumSockError {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        f.write_str("Generic error")
    }
}

impl From<russh::Error> for CumSockError {
    fn from(value: russh::Error) -> Self {
        value.into()
    }
}

#[derive(Clone)]
struct CumSock {
    clients: Arc<Mutex<HashMap<(usize, ChannelId), russh::server::Handle>>>,
    id: usize,
}

#[async_trait]
impl Handler for CumSock {
    type Error = CumSockError;

    async fn channel_open_session(
        &mut self,
        channel: Channel<Msg>,
        session: &mut Session,
    ) -> Result<bool, Self::Error> {
        {
            let mut clients = self.clients.lock().await;
            clients.insert((self.id, channel.id()), session.handle());
        }
        Ok(true)
    }

    async fn auth_publickey(
        &mut self,
        user: &str,
        _public_key: &key::PublicKey,
    ) -> Result<Auth, Self::Error> {
        if user != "felixklg" {
            return Ok(Auth::Reject {
                proceed_with_methods: None,
            });
        }

        Ok(Auth::Accept)
    }
}

impl Server for CumSock {
    type Handler = Self;

    fn new_client(&mut self, _peer_addr: Option<std::net::SocketAddr>) -> Self::Handler {
        let cum_sock = self.clone();

        self.id += 1;

        cum_sock
    }
}

#[tokio::main]
async fn main() {
    let raw_russh_config = server::Config {
        auth_banner: Some("David"),
        methods: MethodSet::PUBLICKEY,
        ..Default::default()
    };
    let russh_config = Arc::new(raw_russh_config);

    let mut russh_server = CumSock {
        id: 0,
        clients: Arc::new(Mutex::new(HashMap::new())),
    };

    russh_server
        .run_on_address(russh_config, "127.0.0.1:2022")
        .await
        .unwrap();
}
Eugeny commented 1 month ago

You have infinite recursion in

    fn from(value: russh::Error) -> Self {
        value.into()
    }

because x.into::<Y>() is just Y::from(x) again.

P.S.

cum_sock