fixup

Author: Eugeny

russh/src/auth.rs

@@ -126,6 +126,16 @@ impl MethodSet {
         ])
     }
 
+    pub(crate) fn server_supported() -> Self {
+        Self(vec![
+            MethodKind::None,
+            MethodKind::Password,
+            MethodKind::PublicKey,
+            MethodKind::HostBased,
+            MethodKind::KeyboardInteractive,
+        ])
+    }
+
     pub fn remove(&mut self, method: MethodKind) {
         self.0.retain(|x| *x != method);
     }
@@ -168,27 +178,71 @@ pub trait Signer: Sized {
     ) -> impl Future<Output = Result<Vec<u8>, Self::Error>> + Send;
 }
 
+/// One step of a GSSAPI security context exchange, as produced by a
+/// [`GssapiAuthenticator`].
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum GssapiStep {
+    /// The context is not established yet; send `token` to the server and
+    /// wait for its next token.
     Continue {
         token: Vec<u8>,
     },
+    /// The context is established. `token` is the final output token, if any.
+    /// `mic` is the MIC computed over the `mic_data` passed to
+    /// [`GssapiAuthenticator::gssapi_step`]. Implementations MUST produce a
+    /// MIC whenever the established context supports integrity protection
+    /// (RFC 4462, Section 3.5); `None` falls back to
+    /// `SSH_MSG_USERAUTH_GSSAPI_EXCHANGE_COMPLETE`.
     Complete {
         token: Option<Vec<u8>>,
         mic: Option<Vec<u8>>,
     },
 }
 
+/// A GSS-API error reported by the server during `gssapi-with-mic`
+/// authentication. Informational: the server follows up with an
+/// authentication failure.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub enum GssapiError {
+    /// `SSH_MSG_USERAUTH_GSSAPI_ERROR` (RFC 4462, Section 3.8).
+    Status {
+        major_status: u32,
+        minor_status: u32,
+        message: String,
+    },
+    /// `SSH_MSG_USERAUTH_GSSAPI_ERRTOK` (RFC 4462, Section 3.10). May be
+    /// passed to `GSS_Init_sec_context()` to obtain mechanism-specific
+    /// error details.
+    ErrorToken(Vec<u8>),
+}
+
 #[cfg_attr(feature = "async-trait", async_trait::async_trait)]
 pub trait GssapiAuthenticator: Sized {
     type Error: From<crate::SendError>;
 
+    /// Advance the GSSAPI security context.
+    ///
+    /// `selected_mechanism` is `Some` on the first step and carries the
+    /// DER-encoded OID of the mechanism the server selected; implementations
+    /// must verify it is one of the mechanisms they offered (RFC 4462,
+    /// Section 3.3). It is `None` on subsequent steps.
+    ///
+    /// `input_token` is the token received from the server, if any.
+    /// `mic_data` is the data to compute the final MIC over once the context
+    /// is established.
     fn gssapi_step(
         &mut self,
-        selected_mechanism: Vec<u8>,
+        selected_mechanism: Option<Vec<u8>>,
         input_token: Option<Vec<u8>>,
         mic_data: Vec<u8>,
     ) -> impl Future<Output = Result<GssapiStep, Self::Error>> + Send;
+
+    /// Called when the server reports a GSS-API error; the server follows up
+    /// with an authentication failure. The default implementation ignores
+    /// the error.
+    fn gssapi_error(&mut self, _error: GssapiError) -> impl Future<Output = ()> + Send {
+        async {}
+    }
 }
 
 #[derive(Debug, Error)]
@@ -304,33 +358,22 @@ impl AuthRequest {
     }
 
     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 {
+        let current = match method {
+            Method::KeyboardInteractive { submethods } => {
+                Some(CurrentRequest::KeyboardInteractive {
                     submethods: submethods.to_string(),
-                }),
-                principal: None,
-                rejection_count: 0,
-            },
-            Method::GssapiWithMic { .. } => Self {
-                initial_methods: MethodSet::all(),
-                methods: MethodSet::all(),
-                partial_success: false,
-                current: Some(CurrentRequest::GssapiWithMic),
-                principal: None,
-                rejection_count: 0,
-            },
-            _ => Self {
-                initial_methods: MethodSet::all(),
-                methods: MethodSet::all(),
-                partial_success: false,
-                current: None,
-                principal: None,
-                rejection_count: 0,
-            },
+                })
+            }
+            Method::GssapiWithMic { .. } => Some(CurrentRequest::GssapiWithMic),
+            _ => None,
+        };
+        Self {
+            initial_methods: MethodSet::all(),
+            methods: MethodSet::all(),
+            partial_success: false,
+            current,
+            principal: None,
+            rejection_count: 0,
         }
     }
 

russh/src/client/encrypted.rs

@@ -342,9 +342,18 @@ impl Session {
                                 let message = map_err!(String::decode(&mut r))?;
                                 let _language_tag = map_err!(String::decode(&mut r))?;
                                 map_err!(ensure_end(&r))?;
-                                debug!(
+                                warn!(
                                     "userauth_gssapi_error major={major_status} minor={minor_status}: {message}"
                                 );
+                                self.sender
+                                    .send(Reply::AuthGssapiError {
+                                        error: auth::GssapiError::Status {
+                                            major_status,
+                                            minor_status,
+                                            message,
+                                        },
+                                    })
+                                    .map_err(|_| crate::Error::SendError)?;
                                 return Ok(());
                             }
                             return Err(crate::Error::Inconsistent.into());
@@ -353,9 +362,14 @@ impl Session {
                             if let Some(auth::CurrentRequest::GssapiWithMic) =
                                 auth_request.current
                             {
-                                let _token = map_err!(Bytes::decode(&mut r))?;
+                                let token = map_err!(Bytes::decode(&mut r))?.to_vec();
                                 map_err!(ensure_end(&r))?;
-                                debug!("userauth_gssapi_errtok");
+                                warn!("userauth_gssapi_errtok ({} bytes)", token.len());
+                                self.sender
+                                    .send(Reply::AuthGssapiError {
+                                        error: auth::GssapiError::ErrorToken(token),
+                                    })
+                                    .map_err(|_| crate::Error::SendError)?;
                                 return Ok(());
                             }
                             return Err(crate::Error::Inconsistent.into());
@@ -1010,7 +1024,7 @@ mod tests {
     use crate::compression::{Compression, Decompress};
     use crate::kex::{KEXES, NONE};
     use crate::session::Exchange;
-    use crate::{CryptoVec, MethodKind};
+    use crate::{CryptoVec, MethodKind, MethodSet};
 
     fn test_encrypted() -> Encrypted {
         Encrypted {
@@ -1150,6 +1164,19 @@ mod tests {
         );
         ensure_end(&complete).unwrap();
     }
+
+    #[test]
+    fn client_send_gssapi_mic_skips_empty_final_token() {
+        let mut encrypted = test_encrypted();
+        encrypted.client_send_gssapi_mic(Some(b""), b"mic").unwrap();
+
+        let payloads = payloads(&encrypted.write);
+        assert_eq!(payloads.len(), 1);
+        let mut mic = payloads[0];
+        assert_eq!(u8::decode(&mut mic).unwrap(), msg::USERAUTH_GSSAPI_MIC);
+        assert_eq!(Vec::<u8>::decode(&mut mic).unwrap(), b"mic".to_vec());
+        ensure_end(&mic).unwrap();
+    }
 }
 
 impl Encrypted {
@@ -1242,8 +1269,7 @@ impl Encrypted {
                     user.as_bytes().encode(&mut self.write)?;
                     "ssh-connection".encode(&mut self.write)?;
                     "gssapi-with-mic".encode(&mut self.write)?;
-                    (mechanism_oids.len().try_into().unwrap_or(0) as u32)
-                        .encode(&mut self.write)?;
+                    (mechanism_oids.len() as u32).encode(&mut self.write)?;
                     for oid in mechanism_oids {
                         oid.as_slice().encode(&mut self.write)?;
                     }
@@ -1280,9 +1306,7 @@ impl Encrypted {
         token: Option<&[u8]>,
         mic: &[u8],
     ) -> Result<(), crate::Error> {
-        if let Some(token) = token
-            && !token.is_empty()
-        {
+        if let Some(token) = token.filter(|t| !t.is_empty()) {
             self.client_send_gssapi_token(token)?;
         }
         push_packet!(self.write, {
@@ -1296,9 +1320,7 @@ impl Encrypted {
         &mut self,
         token: Option<&[u8]>,
     ) -> Result<(), crate::Error> {
-        if let Some(token) = token
-            && !token.is_empty()
-        {
+        if let Some(token) = token.filter(|t| !t.is_empty()) {
             self.client_send_gssapi_token(token)?;
         }
         push_packet!(self.write, {

russh/src/client/mod.rs

@@ -138,6 +138,9 @@ enum Reply {
         token: Vec<u8>,
         mic_data: Vec<u8>,
     },
+    AuthGssapiError {
+        error: auth::GssapiError,
+    },
 }
 
 #[derive(Debug)]
@@ -579,16 +582,19 @@ impl<H: Handler> Handle<H> {
                     mic_data,
                 }) => {
                     let step = authenticator
-                        .gssapi_step(selected_mechanism, None, mic_data)
+                        .gssapi_step(Some(selected_mechanism), None, mic_data)
                         .await?;
                     self.send_gssapi_step(step).await?;
                 }
                 Some(Reply::AuthGssapiToken { token, mic_data }) => {
                     let step = authenticator
-                        .gssapi_step(Vec::new(), Some(token), mic_data)
+                        .gssapi_step(None, Some(token), mic_data)
                         .await?;
                     self.send_gssapi_step(step).await?;
                 }
+                Some(Reply::AuthGssapiError { error }) => {
+                    authenticator.gssapi_error(error).await;
+                }
                 None => {
                     return Ok(AuthResult::Failure {
                         remaining_methods: MethodSet::empty(),

russh/src/lib_inner.rs

@@ -32,7 +32,6 @@ mod ssh_read;
 mod sshbuffer;
 
 pub use negotiation::{Names, Preferred};
-pub use auth::{GssapiAuthenticator, GssapiStep};
 
 mod pty;
 
@@ -292,7 +291,9 @@ impl Default for Limits {
     }
 }
 
-pub use auth::{AgentAuthError, MethodKind, MethodSet, Signer};
+pub use auth::{
+    AgentAuthError, GssapiAuthenticator, GssapiError, GssapiStep, MethodKind, MethodSet, Signer,
+};
 
 /// A reason for disconnection.
 #[allow(missing_docs)] // This should be relatively self-explanatory.

russh/src/server/mod.rs

@@ -108,7 +108,7 @@ impl Default for Config {
                 "_",
                 env!("CARGO_PKG_VERSION")
             ))),
-            methods: auth::MethodSet::all(),
+            methods: auth::MethodSet::server_supported(),
             auth_rejection_time: std::time::Duration::from_secs(1),
             auth_rejection_time_initial: None,
             keys: Vec::new(),