russh server userauth state is not reset when authentication principal changes

@@ -1041,9 +1041,9 @@ async fn read_ssh_id<R: AsyncRead + Unpin>(
     read: &mut SshRead<R>,
 ) -> Result<CommonSession<Arc<Config>>, Error> {
     let sshid = if let Some(t) = config.inactivity_timeout {
-        tokio::time::timeout(t, read.read_ssh_id()).await??
+        tokio::time::timeout(t, read.read_client_ssh_id()).await??
     } else {
-        read.read_ssh_id().await?
+        read.read_client_ssh_id().await?
     };
 
     let session = CommonSession {

@@ -7,6 +7,8 @@ use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, ReadBuf};
 use crate::Error;
 
 const SSH_ID_BUF_SIZE: usize = 256;
+const SSH_ID_MAX_LINE_LEN: usize = 255;
+const SSH_ID_MAX_PRE_BANNER_LINES: usize = 20;
 
 /// The buffer to read the identification string (first line in the
 /// protocol).  Not sensitive data — just protocol version exchange.
@@ -15,6 +17,7 @@ struct ReadSshIdBuffer {
     pub total: usize,
     pub bytes_read: usize,
     pub sshid_len: usize,
+    pub pre_banner_lines: usize,
 }
 
 impl ReadSshIdBuffer {
@@ -29,8 +32,32 @@ impl ReadSshIdBuffer {
             sshid_len: 0,
             bytes_read: 0,
             total: 0,
+            pre_banner_lines: 0,
         }
     }
+
+    fn line(&self) -> Option<(usize, usize)> {
+        if self.total < 2 {
+            return None;
+        }
+        #[allow(clippy::indexing_slicing)] // loop bounds keep i + 1 in range
+        for i in 0..self.total - 1 {
+            if self.buf[i] == b'\r' && self.buf[i + 1] == b'\n' {
+                return Some((i, i + 2));
+            }
+            if self.buf[i + 1] == b'\n' {
+                return Some((i + 1, i + 2));
+            }
+        }
+        None
+    }
+
+    fn discard_line(&mut self) {
+        let remaining = self.total - self.bytes_read;
+        self.buf.copy_within(self.bytes_read..self.total, 0);
+        self.total = remaining;
+        self.bytes_read = 0;
+    }
 }
 
 impl std::fmt::Debug for ReadSshIdBuffer {
@@ -113,45 +140,51 @@ impl<R: AsyncRead + Unpin> SshRead<R> {
         }
     }
 
-    #[allow(clippy::unwrap_used)]
     pub async fn read_ssh_id(&mut self) -> Result<&[u8], Error> {
-        let ssh_id = self.id.as_mut().unwrap();
+        self.read_ssh_id_inner(true).await
+    }
+
+    pub async fn read_client_ssh_id(&mut self) -> Result<&[u8], Error> {
+        self.read_ssh_id_inner(false).await
+    }
+
+    async fn read_ssh_id_inner(&mut self, allow_pre_banner_lines: bool) -> Result<&[u8], Error> {
+        let ssh_id = self.id.as_mut().ok_or(Error::Inconsistent)?;
         loop {
-            let mut i = 0;
-            trace!("read_ssh_id: reading");
+            let i = if let Some((line_len, bytes_read)) = ssh_id.line() {
+                ssh_id.bytes_read = bytes_read;
+                line_len
+            } else {
+                trace!("read_ssh_id: reading");
 
-            #[allow(clippy::indexing_slicing)] // length checked
-            let n = AsyncReadExt::read(&mut self.r, &mut ssh_id.buf[ssh_id.total..]).await?;
-            trace!("read {n:?}");
+                #[allow(clippy::indexing_slicing)] // length checked
+                let n = AsyncReadExt::read(&mut self.r, &mut ssh_id.buf[ssh_id.total..]).await?;
+                trace!("read {n:?}");
 
-            ssh_id.total += n;
-            #[allow(clippy::indexing_slicing)] // length checked
-            {
-                trace!("{:?}", std::str::from_utf8(&ssh_id.buf[..ssh_id.total]));
-            }
-            if n == 0 {
-                return Err(Error::Disconnect);
-            }
-            #[allow(clippy::indexing_slicing)] // length checked
-            loop {
-                if i >= ssh_id.total - 1 {
-                    break;
+                ssh_id.total += n;
+                #[allow(clippy::indexing_slicing)] // length checked
+                {
+                    trace!("{:?}", std::str::from_utf8(&ssh_id.buf[..ssh_id.total]));
+                }
+                if n == 0 {
+                    return Err(Error::Disconnect);
                 }
-                if ssh_id.buf[i] == b'\r' && ssh_id.buf[i + 1] == b'\n' {
-                    ssh_id.bytes_read = i + 2;
-                    break;
-                } else if ssh_id.buf[i + 1] == b'\n' {
-                    // This is really wrong, but OpenSSH 7.4 uses
-                    // it.
-                    ssh_id.bytes_read = i + 2;
-                    i += 1;
-                    break;
+
+                if let Some((line_len, bytes_read)) = ssh_id.line() {
+                    ssh_id.bytes_read = bytes_read;
+                    line_len
+                } else if ssh_id.total >= SSH_ID_MAX_LINE_LEN {
+                    return Err(Error::Version);
                 } else {
-                    i += 1;
+                    trace!("bytes_read: {:?}", ssh_id.bytes_read);
+                    continue;
                 }
-            }
+            };
 
             if ssh_id.bytes_read > 0 {
+                if ssh_id.bytes_read > SSH_ID_MAX_LINE_LEN {
+                    return Err(Error::Version);
+                }
                 // If we have a full line, handle it.
                 if i >= 8 {
                     // Check if we have a valid SSH protocol identifier
@@ -163,11 +196,17 @@ impl<R: AsyncRead + Unpin> SshRead<R> {
                         }
                     }
                 }
+                if !allow_pre_banner_lines {
+                    return Err(Error::Version);
+                }
+                ssh_id.pre_banner_lines += 1;
+                if ssh_id.pre_banner_lines > SSH_ID_MAX_PRE_BANNER_LINES {
+                    return Err(Error::Version);
+                }
                 // Else, it is a "preliminary" (see
                 // https://tools.ietf.org/html/rfc4253#section-4.2),
                 // and we can discard it and read the next one.
-                ssh_id.total = 0;
-                ssh_id.bytes_read = 0;
+                ssh_id.discard_line();
             }
             trace!("bytes_read: {:?}", ssh_id.bytes_read);
         }
@@ -204,7 +243,46 @@ mod tests {
         let mut read = SshRead::new(data.as_bytes());
 
         let received = read.read_ssh_id().await;
-        assert!(matches!(received.err(), Some(Error::Disconnect)));
+        assert!(matches!(received.err(), Some(Error::Version)));
+    }
+
+    #[tokio::test]
+    async fn test_ssh_id_accepts_maximum_line_length() {
+        let data = format!("SSH-2.0-{}\r\n", "A".repeat(245));
+        let mut read = SshRead::new(data.as_bytes());
+
+        let received = read.read_ssh_id().await.unwrap();
+        assert_eq!(received.len(), SSH_ID_MAX_LINE_LEN - 2);
+    }
+
+    #[tokio::test]
+    async fn test_ssh_id_rejects_oversized_line_with_terminator() {
+        let data = format!("SSH-2.0-{}\r\n", "A".repeat(246));
+        let mut read = SshRead::new(data.as_bytes());
+
+        let received = read.read_ssh_id().await;
+        assert!(matches!(received.err(), Some(Error::Version)));
+    }
+
+    #[tokio::test]
+    async fn test_ssh_id_rejects_too_many_pre_banner_lines() {
+        let data = format!(
+            "{}SSH-2.0-OpenSSH_10.2\r\n",
+            "debug\r\n".repeat(SSH_ID_MAX_PRE_BANNER_LINES + 1)
+        );
+        let mut read = SshRead::new(data.as_bytes());
+
+        let received = read.read_ssh_id().await;
+        assert!(matches!(received.err(), Some(Error::Version)));
+    }
+
+    #[tokio::test]
+    async fn test_server_ssh_id_rejects_pre_banner_line() {
+        let data = "debug\r\nSSH-2.0-OpenSSH_10.2\r\n";
+        let mut read = SshRead::new(data.as_bytes());
+
+        let received = read.read_client_ssh_id().await;
+        assert!(matches!(received.err(), Some(Error::Version)));
     }
 
     #[tokio::test]

@@ -170,8 +170,13 @@ impl Session {
                                 let n_prompts = map_err!(u32::decode(&mut r))?;
 
                                 // read prompts
-                                let mut prompts =
-                                    Vec::with_capacity(n_prompts.try_into().unwrap_or(0));
+                                // Each prompt needs at least a 4-byte length plus a 1-byte echo flag.
+                                let max_prompts = r.remaining_len() / 5;
+                                let n_prompts = n_prompts as usize;
+                                if n_prompts > max_prompts {
+                                    return Err(crate::Error::Inconsistent.into());
+                                }
+                                let mut prompts = Vec::with_capacity(n_prompts);
                                 for _i in 0..n_prompts {
                                     let prompt = map_err!(String::decode(&mut r))?;
 

@@ -1664,6 +1664,118 @@ async fn reply<H: Handler>(
     session.client_read_encrypted(handler, pkt).await
 }
 
+#[cfg(test)]
+mod tests {
+    use std::collections::HashMap;
+    use std::num::Wrapping;
+    use std::sync::Arc;
+
+    use ssh_encoding::Encode;
+    use tokio::sync::mpsc::channel;
+    use tokio::sync::mpsc::unbounded_channel;
+
+    use super::*;
+    use crate::auth::{AuthRequest, Method};
+    use crate::compression::{Compression, Decompress};
+    use crate::kex::{KEXES, NONE};
+    use crate::session::{CommonSession, Encrypted, EncryptedState, Exchange};
+    use crate::{CryptoVec, cipher, mac};
+
+    struct TestHandler;
+
+    impl Handler for TestHandler {
+        type Error = crate::Error;
+
+        async fn check_server_key(
+            &mut self,
+            _: &ssh_key::PublicKey,
+        ) -> Result<bool, Self::Error> {
+            Ok(true)
+        }
+    }
+
+    fn keyboard_interactive_session() -> (
+        Session,
+        tokio::sync::mpsc::Sender<Msg>,
+        tokio::sync::mpsc::UnboundedReceiver<Reply>,
+    ) {
+        let config = Arc::new(Config::default());
+        let (sender, receiver) = channel(config.channel_buffer_size);
+        let (reply_sender, reply_receiver) = unbounded_channel();
+        let auth_request = AuthRequest::new(&Method::KeyboardInteractive {
+            submethods: String::new(),
+        });
+        let session = Session::new(
+            config.window_size,
+            CommonSession {
+                auth_user: "user".to_owned(),
+                auth_attempts: 0,
+                auth_method: Some(Method::KeyboardInteractive {
+                    submethods: String::new(),
+                }),
+                remote_to_local: Box::new(cipher::clear::Key),
+                encrypted: Some(Encrypted {
+                    state: EncryptedState::WaitingAuthRequest(auth_request),
+                    exchange: Some(Exchange::default()),
+                    kex: KEXES.get(&NONE).unwrap().make(),
+                    key: 0,
+                    client_mac: mac::NONE,
+                    server_mac: mac::NONE,
+                    session_id: CryptoVec::new(),
+                    channels: HashMap::new(),
+                    last_channel_id: Wrapping(0),
+                    write: Vec::new(),
+                    write_cursor: 0,
+                    last_rekey: russh_util::time::Instant::now(),
+                    server_compression: Compression::None,
+                    client_compression: Compression::None,
+                    decompress: Decompress::None,
+                    rekey_wanted: false,
+                    received_extensions: Vec::new(),
+                    extension_info_awaiters: HashMap::new(),
+                }),
+                config,
+                wants_reply: false,
+                disconnected: false,
+                buffer: Vec::new(),
+                strict_kex: false,
+                alive_timeouts: 0,
+                received_data: false,
+                remote_sshid: b"SSH-2.0-test".to_vec(),
+                packet_writer: PacketWriter::clear(),
+            },
+            receiver,
+            reply_sender,
+        );
+        (session, sender, reply_receiver)
+    }
+
+    fn oversized_prompt_count_packet() -> Vec<u8> {
+        let mut packet = Vec::new();
+        msg::USERAUTH_INFO_REQUEST_OR_USERAUTH_PK_OK
+            .encode(&mut packet)
+            .unwrap();
+        "name".encode(&mut packet).unwrap();
+        "instructions".encode(&mut packet).unwrap();
+        "".encode(&mut packet).unwrap();
+        u32::MAX.encode(&mut packet).unwrap();
+        packet
+    }
+
+    #[tokio::test]
+    async fn oversized_keyboard_interactive_prompt_count_is_rejected() {
+        let (mut session, _sender, mut replies) = keyboard_interactive_session();
+        let mut handler = TestHandler;
+        let err = session
+            .process_packet(&mut handler, &oversized_prompt_count_packet())
+            .await
+            .expect_err("malformed prompt count must fail");
+
+        assert!(matches!(err, crate::Error::Inconsistent));
+        assert!(replies.try_recv().is_err(), "malformed packet must not emit a reply");
+    }
+}
+
 fn initial_encrypted_state(session: &Session) -> EncryptedState {
     if session.common.config.anonymous {
         EncryptedState::Authenticated

@@ -226,14 +226,23 @@ pub enum Method {
 #[doc(hidden)]
 #[derive(Debug)]
 pub struct AuthRequest {
+    initial_methods: MethodSet,
     pub methods: MethodSet,
     #[cfg_attr(target_arch = "wasm32", allow(dead_code))]
     pub partial_success: bool,
     pub current: Option<CurrentRequest>,
+    pub(crate) principal: Option<AuthPrincipal>,
     #[cfg_attr(target_arch = "wasm32", allow(dead_code))]
     pub rejection_count: usize,
 }
 
+#[doc(hidden)]
+#[derive(Debug)]
+pub(crate) struct AuthPrincipal {
+    user: String,
+    service: String,
+}
+
 #[doc(hidden)]
 #[derive(Debug)]
 pub enum CurrentRequest {
@@ -252,22 +261,53 @@ pub enum CurrentRequest {
 }
 
 impl AuthRequest {
+    pub(crate) fn server(methods: MethodSet) -> Self {
+        Self {
+            initial_methods: methods.clone(),
+            methods,
+            partial_success: false,
+            current: None,
+            principal: None,
+            rejection_count: 0,
+        }
+    }
+
     pub(crate) fn new(method: &Method) -> Self {
         match method {
             Method::KeyboardInteractive { submethods } => Self {
+                initial_methods: MethodSet::all(),
                 methods: MethodSet::all(),
                 partial_success: false,
                 current: Some(CurrentRequest::KeyboardInteractive {
                     submethods: submethods.to_string(),
                 }),
+                principal: None,
                 rejection_count: 0,
             },
             _ => Self {
+                initial_methods: MethodSet::all(),
                 methods: MethodSet::all(),
                 partial_success: false,
                 current: None,
+                principal: None,
                 rejection_count: 0,
             },
         }
     }
+
+    pub(crate) fn bind_or_reset_principal(&mut self, user: &str, service: &str) -> bool {
+        match &self.principal {
+            Some(bound) if bound.user == user && bound.service == service => false,
+            _ => {
+                self.principal = Some(AuthPrincipal {
+                    user: user.to_owned(),
+                    service: service.to_owned(),
+                });
+                self.methods = self.initial_methods.clone();
+                self.partial_success = false;
+                self.current = None;
+                true
+            }
+        }
+    }
 }

@@ -333,12 +333,7 @@ fn server_accept_service(
         })
     }
 
-    Ok(AuthRequest {
-        methods,
-        partial_success: false, // not used immediately anway.
-        current: None,
-        rejection_count: 0,
-    })
+    Ok(AuthRequest::server(methods))
 }
 
 impl Encrypted {
@@ -359,6 +354,18 @@ impl Encrypted {
         debug!("name: {user:?} {service_name:?} {method:?}",);
 
         if service_name == "ssh-connection" {
+            {
+                let auth_request = if let EncryptedState::WaitingAuthRequest(ref mut a) = self.state
+                {
+                    a
+                } else {
+                    unreachable!()
+                };
+                if auth_request.bind_or_reset_principal(&user, &service_name) {
+                    auth_user.clear();
+                }
+            }
+
             if method == "password" {
                 let auth_request = if let EncryptedState::WaitingAuthRequest(ref mut a) = self.state
                 {

@@ -0,0 +1,91 @@
+use std::sync::Arc;
+use std::time::Duration;
+
+use russh::client;
+use russh::keys::PrivateKey;
+use russh::{MethodKind, MethodSet, server};
+
+struct AcceptTestServerKey;
+
+impl client::Handler for AcceptTestServerKey {
+    type Error = russh::Error;
+
+    async fn check_server_key(
+        &mut self,
+        _server_public_key: &russh::keys::ssh_key::PublicKey,
+    ) -> Result<bool, Self::Error> {
+        Ok(true)
+    }
+}
+
+struct RemainingMethodsUserSwitchServer;
+
+impl server::Handler for RemainingMethodsUserSwitchServer {
+    type Error = russh::Error;
+
+    async fn auth_none(&mut self, user: &str) -> Result<server::Auth, Self::Error> {
+        if user == "alice" {
+            Ok(server::Auth::Reject {
+                proceed_with_methods: Some(MethodSet::from(&[MethodKind::Password][..])),
+                partial_success: true,
+            })
+        } else {
+            Ok(server::Auth::reject())
+        }
+    }
+}
+
+#[tokio::test]
+async fn auth_does_not_carry_remaining_methods_across_username_change() {
+    let mut server_config = server::Config::default();
+    server_config.inactivity_timeout = None;
+    server_config.auth_rejection_time = Duration::from_millis(1);
+    server_config.auth_rejection_time_initial = Some(Duration::from_millis(1));
+    server_config.keys.push(
+        PrivateKey::random(&mut rand::rng(), russh::keys::ssh_key::Algorithm::Ed25519).unwrap(),
+    );
+    let server_config = Arc::new(server_config);
+
+    let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
+    let addr = listener.local_addr().unwrap();
+    let server = tokio::spawn(async move {
+        let (socket, _) = listener.accept().await.unwrap();
+        let running = server::run_stream(server_config, socket, RemainingMethodsUserSwitchServer)
+            .await
+            .unwrap();
+        running.await
+    });
+
+    let mut session = client::connect(
+        Arc::new(client::Config::default()),
+        addr,
+        AcceptTestServerKey,
+    )
+    .await
+    .unwrap();
+
+    let alice = session.authenticate_none("alice").await.unwrap();
+    assert!(
+        matches!(
+            alice,
+            client::AuthResult::Failure {
+                ref remaining_methods,
+                ..
+            } if *remaining_methods == MethodSet::from(&[MethodKind::Password][..])
+        ),
+        "unexpected Alice auth result: {alice:?}"
+    );
+
+    let bob = session.authenticate_none("bob").await.unwrap();
+    if let client::AuthResult::Failure {
+        remaining_methods, ..
+    } = bob
+    {
+        assert!(
+            remaining_methods.contains(&MethodKind::PublicKey),
+            "server reused Alice's narrowed remaining methods for Bob: {remaining_methods:?}"
+        );
+    }
+
+    server.abort();
+}

@@ -6,7 +6,7 @@ edition = "2024"
 license = "Apache-2.0"
 name = "russh-cryptovec"
 repository = "https://github.com/warp-tech/russh"
-version = "0.60.3"
+version = "0.61.0"
 rust-version = "1.85"
 
 [dependencies]

@@ -6,7 +6,7 @@ edition = "2024"
 license = "Apache-2.0"
 name = "pageant"
 repository = "https://github.com/warp-tech/russh"
-version = "0.2.0"
+version = "0.2.1"
 rust-version = "1.85"
 
 [dependencies]

@@ -9,7 +9,7 @@ license = "Apache-2.0"
 name = "russh"
 readme = "../README.md"
 repository = "https://github.com/warp-tech/russh"
-version = "0.60.3"
+version = "0.61.0"
 rust-version = "1.85"
 
 [features]
@@ -81,7 +81,7 @@ rand_core = { version = "0.10.0" }
 rand.workspace = true
 ring = { version = "0.17.14", optional = true }
 rsa = { version = "=0.10.0-rc.18", optional = true }
-russh-cryptovec = { version = "0.60.3", path = "../cryptovec", features = [
+russh-cryptovec = { version = "0.61.0", path = "../cryptovec", features = [
   "ssh-encoding",
 ] }
 russh-util = { version = "0.52.0", path = "../russh-util" }