diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 64c76f3f..112843d8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -101,16 +101,18 @@ jobs: - manifest: crates/dpapi-native-transport/Cargo.toml crate-name: dpapi-native-transport - # Per-OS feature overrides for specific manifests + # Per-OS feature overrides for specific manifests. + # `__test-data` enables the `tests/sspi/client_server` suite (gated on + # `all(network_client, __test-data)`), which otherwise never runs in CI. - os: win manifest: Cargo.toml - additional-args: --features network_client,dns_resolver,scard,tsssp + additional-args: --features network_client,dns_resolver,scard,tsssp,__test-data - os: osx manifest: Cargo.toml - additional-args: --features network_client,scard + additional-args: --features network_client,scard,__test-data - os: linux manifest: Cargo.toml - additional-args: --features network_client,dns_resolver,scard + additional-args: --features network_client,dns_resolver,scard,__test-data - os: win manifest: ffi/Cargo.toml diff --git a/src/credssp/mod.rs b/src/credssp/mod.rs index 4e8ffe9b..b162f9d9 100644 --- a/src/credssp/mod.rs +++ b/src/credssp/mod.rs @@ -454,6 +454,10 @@ pub struct CredSspServer> credentials_handle: Option, ts_request_version: u32, context_config: Option, + /// Set when the security context completed but the final SPNEGO token (accept-completed + + /// `mechListMIC`) still had to be sent to the client: the client cannot send `pubKeyAuth` + /// until it has verified that token, so `pubKeyAuth` arrives on the *next* leg. + awaiting_pub_key_auth: bool, } impl + Send> CredSspServer { @@ -466,6 +470,7 @@ impl + Send> CredSspServe credentials_handle: None, ts_request_version: TS_REQUEST_VERSION, context_config: Some(client_mode), + awaiting_pub_key_auth: false, }) } @@ -483,9 +488,39 @@ impl + Send> CredSspServe credentials_handle: None, ts_request_version, context_config: Some(client_mode), + awaiting_pub_key_auth: false, }) } + /// Decrypts and verifies the client's `pubKeyAuth`, returning the server-side `pubKeyAuth` + /// to send back. + fn exchange_pub_key_auth( + &mut self, + pub_key_auth: Vec, + client_nonce: &Option<[u8; NONCE_SIZE]>, + ) -> crate::Result> { + let peer_version = self + .context + .as_ref() + .unwrap() + .peer_version + .expect("an decrypt public key server function cannot be fired without any incoming TSRequest"); + let context = self.context.as_mut().unwrap(); + context.decrypt_public_key( + self.public_key.as_ref(), + pub_key_auth.as_ref(), + EndpointType::Server, + client_nonce, + peer_version, + )?; + context.encrypt_public_key( + self.public_key.as_ref(), + EndpointType::Server, + client_nonce, + peer_version, + ) + } + #[instrument(fields(state = ?self.state), skip_all)] pub fn process( &mut self, @@ -576,6 +611,32 @@ impl + Send> CredSspServe Ok(ServerState::Finished(auth_identity)) } CredSspState::NegoToken => { + if self.awaiting_pub_key_auth { + // The SPNEGO `mechListMIC` exchange deferred the client's `pubKeyAuth` by one + // leg: the security context is already established and the final SPNEGO token + // has been sent, so this TSRequest carries `pubKeyAuth` alone. Calling the + // acceptor again would be out-of-sequence. + let pub_key_auth = try_cred_ssp_server!( + ts_request.pub_key_auth.take().ok_or_else(|| { + Error::new( + ErrorKind::InvalidToken, + String::from("CredSSP server expected an encrypted public key"), + ) + }), + ts_request + ); + let client_nonce = ts_request.client_nonce; + let pub_key_auth = + try_cred_ssp_server!(self.exchange_pub_key_auth(pub_key_auth, &client_nonce), ts_request); + ts_request.nego_tokens = None; + ts_request.pub_key_auth = Some(pub_key_auth); + + self.awaiting_pub_key_auth = false; + self.state = CredSspState::AuthInfo; + + return Ok(ServerState::ReplyNeeded(ts_request)); + } + let input = ts_request.nego_tokens.take().unwrap_or_default(); let mut input_token = vec![SecurityBuffer::new(input, BufferType::Token)]; let mut output_token = vec![SecurityBuffer::new(Vec::with_capacity(1024), BufferType::Token)]; @@ -628,42 +689,38 @@ impl + Send> CredSspServe self.context.as_mut().unwrap().sspi_context.complete_auth_token(&mut []), ts_request ); - ts_request.nego_tokens = None; - - let pub_key_auth = try_cred_ssp_server!( - ts_request.pub_key_auth.take().ok_or_else(|| { - Error::new( - ErrorKind::InvalidToken, - String::from("CredSSP server expected an encrypted public key"), - ) - }), - ts_request - ); - let peer_version = self.context.as_ref().unwrap().peer_version.expect( - "an decrypt public key server function cannot be fired without any incoming TSRequest", - ); - try_cred_ssp_server!( - self.context.as_mut().unwrap().decrypt_public_key( - self.public_key.as_ref(), - pub_key_auth.as_ref(), - EndpointType::Server, - &ts_request.client_nonce, - peer_version, - ), - ts_request - ); - let pub_key_auth = try_cred_ssp_server!( - self.context.as_mut().unwrap().encrypt_public_key( - self.public_key.as_ref(), - EndpointType::Server, - &ts_request.client_nonce, - peer_version, - ), - ts_request - ); - ts_request.pub_key_auth = Some(pub_key_auth); - self.state = CredSspState::AuthInfo; + let final_token = output_token.remove(0).buffer; + if ts_request.pub_key_auth.is_none() && !final_token.is_empty() { + // SPNEGO `mechListMIC` exchange ([MS-SPNG] 3.3.5.5 / RFC 4178 §5): the + // security context is established, but the client cannot send + // `pubKeyAuth` until it has received and verified our final SPNEGO + // token (accept-completed + `mechListMIC`), so that token must reach + // the wire instead of being dropped. `pubKeyAuth` arrives on the next + // leg. + ts_request.nego_tokens = Some(final_token); + self.awaiting_pub_key_auth = true; + } else { + ts_request.nego_tokens = None; + + let pub_key_auth = try_cred_ssp_server!( + ts_request.pub_key_auth.take().ok_or_else(|| { + Error::new( + ErrorKind::InvalidToken, + String::from("CredSSP server expected an encrypted public key"), + ) + }), + ts_request + ); + let client_nonce = ts_request.client_nonce; + let pub_key_auth = try_cred_ssp_server!( + self.exchange_pub_key_auth(pub_key_auth, &client_nonce), + ts_request + ); + ts_request.pub_key_auth = Some(pub_key_auth); + + self.state = CredSspState::AuthInfo; + } } result => { try_cred_ssp_server!( diff --git a/tests/sspi/client_server/credssp.rs b/tests/sspi/client_server/credssp.rs index 23dc8db6..cb869fe5 100644 --- a/tests/sspi/client_server/credssp.rs +++ b/tests/sspi/client_server/credssp.rs @@ -109,6 +109,54 @@ fn credssp_ntlm() { run_credssp(&mut client, &mut server, &auth_identity, &mut network_client); } +#[test] +fn credssp_negotiate_ntlm() { + // NTLM wrapped in SPNEGO on both sides — the pairing RDP requires (Windows RDP servers + // reject bare NTLM inside CredSSP). Because the client always includes an NTLM MIC, the + // SPNEGO `mechListMIC` exchange is mandatory ([MS-SPNG]): the client defers `pubKeyAuth` + // until it has verified the server's final SPNEGO token, so the handshake takes four + // client legs (NEGOTIATE, AUTHENTICATE+mechListMIC, pubKeyAuth, authInfo) instead of + // NTLM-only's three. Regression test for #687: the server used to drop its final SPNEGO + // token and demand `pubKeyAuth` on the AUTHENTICATE leg, failing the handshake. + let auth_identity = AuthIdentity { + username: Username::parse("test_user").unwrap(), + password: Secret::from("test_password".to_owned()), + }; + let credentials = Credentials::AuthIdentity(auth_identity.clone()); + + let mut client = CredSspClient::new( + PUBLIC_KEY.to_vec(), + credentials.clone(), + CredSspMode::WithCredentials, + ClientMode::Negotiate(NegotiateConfig::new( + Box::new(NtlmConfig { + client_computer_name: Some("DESKTOP-3D83IAN.example.com".to_owned()), + }), + Some("ntlm,!kerberos,!pku2u".to_owned()), + "DESKTOP-3D83IAN.example.com".to_owned(), + )), + TARGET_NAME.to_owned(), + ) + .unwrap(); + + let mut server = CredSspServer::new( + PUBLIC_KEY.to_vec(), + CredentialsProxyImpl::new(&auth_identity), + ServerMode::Negotiate(NegotiateConfig::new( + Box::new(NtlmConfig { + client_computer_name: Some("DESKTOP-3D83IAN.example.com".to_owned()), + }), + Some("ntlm,!kerberos,!pku2u".to_owned()), + "SERVER.example.com".to_owned(), + )), + ) + .unwrap(); + + let mut network_client = NetworkClientMock { kdc: KdcMock::empty() }; + + run_credssp(&mut client, &mut server, &auth_identity, &mut network_client); +} + #[test] fn credssp_kerberos() { // CredSSP with Kerberos inside requires SPNEGO. We cannot use Kerberos inside CredSSP without SPNEGO.