Skip to content

Commit 27ce28d

Browse files
TheBestTvarynkaCBenoit
authored andcommitted
fix(sspi): server-side Kerberos fixes (#457)
1 parent 943a297 commit 27ce28d

File tree

18 files changed

+173
-44
lines changed

18 files changed

+173
-44
lines changed

Cargo.toml

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,13 @@ sha1 = { version = "0.10", default-features = false }
4444
sha2 = "0.10"
4545
num-derive = "0.4"
4646
num-traits = { version = "0.2", default-features = false }
47-
picky = { version = "7.0.0-rc.12", default-features = false }
47+
48+
picky = { version = "7.0.0-rc.15", default-features = false }
4849
picky-asn1 = "0.10"
4950
picky-asn1-der = "0.5"
5051
picky-asn1-x509 = "0.14"
51-
picky-krb = "0.10"
52+
picky-krb = "0.11"
53+
5254
tokio = "1.45"
5355
ffi-types = { path = "crates/ffi-types" }
5456
winscard = { version = "0.2", path = "crates/winscard" }

crates/dpapi/src/client.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ async fn get_key<T: Transport>(
213213
let mut rpc = RpcClient::<T>::connect(
214214
&connection_options,
215215
AuthProvider::new(
216-
SspiContext::Negotiate(Negotiate::new(negotiate_config.clone()).map_err(AuthError::from)?),
216+
SspiContext::Negotiate(Negotiate::new_client(negotiate_config.clone()).map_err(AuthError::from)?),
217217
Credentials::AuthIdentity(AuthIdentity {
218218
username: username.clone(),
219219
password: password.clone(),
@@ -246,7 +246,7 @@ async fn get_key<T: Transport>(
246246
let mut rpc = RpcClient::<T>::connect(
247247
&connection_options,
248248
AuthProvider::new(
249-
SspiContext::Negotiate(Negotiate::new(negotiate_config).map_err(AuthError::from)?),
249+
SspiContext::Negotiate(Negotiate::new_client(negotiate_config).map_err(AuthError::from)?),
250250
Credentials::AuthIdentity(AuthIdentity { username, password }),
251251
server,
252252
network_client,

ffi/src/dpapi/network_client.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@ use sspi::{Error, ErrorKind, NetworkRequest, Result};
99
pub struct SyncNetworkClient;
1010

1111
impl AsyncNetworkClient for SyncNetworkClient {
12-
fn send<'a>(&'a mut self, request: &'a NetworkRequest) -> Pin<Box<dyn Future<Output = Result<Vec<u8>>> + 'a>> {
12+
fn send<'a>(
13+
&'a mut self,
14+
request: &'a NetworkRequest,
15+
) -> Pin<Box<dyn Future<Output = Result<Vec<u8>>> + Send + 'a>> {
1316
let request = request.clone();
1417
Box::pin(async move {
1518
tokio::task::spawn_blocking(move || ReqwestNetworkClient.send(&request))

ffi/src/sspi/sec_handle.rs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -230,23 +230,23 @@ pub struct CredentialsHandle {
230230
fn create_negotiate_context(attributes: &CredentialsAttributes) -> Result<Negotiate> {
231231
let client_computer_name = attributes.hostname()?;
232232

233-
if let Some(kdc_url) = attributes.kdc_url() {
233+
let negotiate_config = if let Some(kdc_url) = attributes.kdc_url() {
234234
let kerberos_config = KerberosConfig::new(&kdc_url, client_computer_name.clone());
235-
let negotiate_config = NegotiateConfig::new(
235+
236+
NegotiateConfig::new(
236237
Box::new(kerberos_config),
237238
attributes.package_list.clone(),
238239
client_computer_name,
239-
);
240-
241-
Negotiate::new(negotiate_config)
240+
)
242241
} else {
243-
let negotiate_config = NegotiateConfig {
242+
NegotiateConfig {
244243
protocol_config: Box::new(NtlmConfig::new(client_computer_name.clone())),
245244
package_list: attributes.package_list.clone(),
246245
client_computer_name,
247-
};
248-
Negotiate::new(negotiate_config)
249-
}
246+
}
247+
};
248+
249+
Negotiate::new_client(negotiate_config)
250250
}
251251

252252
/// Transforms [&mut PCtxtHandle] to [*mut SspiHandle].
@@ -1244,7 +1244,7 @@ pub unsafe extern "system" fn ChangeAccountPasswordA(
12441244
package_list: None,
12451245
client_computer_name: try_execute!(hostname()),
12461246
};
1247-
SspiContext::Negotiate(try_execute!(Negotiate::new(negotiate_config)))
1247+
SspiContext::Negotiate(try_execute!(Negotiate::new_client(negotiate_config)))
12481248
},
12491249
kerberos::PKG_NAME => {
12501250
let krb_config = KerberosConfig{

src/auth_identity.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,14 @@ impl AuthIdentityBuffers {
170170
pub fn is_empty(&self) -> bool {
171171
self.user.is_empty()
172172
}
173+
174+
pub fn from_utf8(user: &str, domain: &str, password: &str) -> Self {
175+
Self {
176+
user: utils::string_to_utf16(user),
177+
domain: utils::string_to_utf16(domain),
178+
password: utils::string_to_utf16(password).into(),
179+
}
180+
}
173181
}
174182

175183
impl fmt::Debug for AuthIdentityBuffers {

src/credssp/mod.rs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ macro_rules! try_cred_ssp_server {
6161
$ts_request.error_code = Some(construct_error(&error));
6262

6363
return Err(ServerError {
64-
ts_request: $ts_request,
64+
ts_request: Some(Box::new($ts_request)),
6565
error,
6666
});
6767
}
@@ -100,7 +100,7 @@ pub enum ServerState {
100100
/// Contains `TsRequest` with non-empty `error_code`, and the error which caused the server to fail.
101101
#[derive(Debug, Clone)]
102102
pub struct ServerError {
103-
pub ts_request: TsRequest,
103+
pub ts_request: Option<Box<TsRequest>>,
104104
pub error: crate::Error,
105105
}
106106

@@ -248,7 +248,7 @@ impl CredSspClient {
248248
.expect("CredSsp client mode should never be empty")
249249
{
250250
ClientMode::Negotiate(negotiate_config) => Some(CredSspContext::new(SspiContext::Negotiate(
251-
Negotiate::new(negotiate_config)?,
251+
Negotiate::new_client(negotiate_config)?,
252252
))),
253253
ClientMode::Kerberos(kerberos_config) => Some(CredSspContext::new(SspiContext::Kerberos(
254254
Kerberos::new_client_from_config(kerberos_config)?,
@@ -437,7 +437,6 @@ impl<C: CredentialsProxy<AuthenticationData = AuthIdentity> + Send> CredSspServe
437437
})
438438
}
439439

440-
#[allow(clippy::result_large_err)]
441440
#[instrument(fields(state = ?self.state), skip_all)]
442441
pub fn process(
443442
&mut self,
@@ -460,7 +459,7 @@ impl<C: CredentialsProxy<AuthenticationData = AuthIdentity> + Send> CredSspServe
460459
.expect("CredSsp client mode should never be empty")
461460
{
462461
ServerMode::Negotiate(neg_config) => Some(CredSspContext::new(SspiContext::Negotiate(
463-
try_cred_ssp_server!(Negotiate::new(neg_config), ts_request),
462+
try_cred_ssp_server!(Negotiate::new_server(neg_config), ts_request),
464463
))),
465464
ServerMode::Kerberos(kerberos_mode) => {
466465
let (kerberos_config, server_properties) = *kerberos_mode;
@@ -630,7 +629,7 @@ impl<C: CredentialsProxy<AuthenticationData = AuthIdentity> + Send> CredSspServe
630629
Ok(ServerState::ReplyNeeded(ts_request))
631630
}
632631
CredSspState::Final => Err(ServerError {
633-
ts_request,
632+
ts_request: Some(Box::new(ts_request)),
634633
error: Error::new(
635634
ErrorKind::UnsupportedFunction,
636635
"CredSSP server's 'process' method must not be fired after the 'Finished' state",

src/generator.rs

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use std::task::{Context, Poll, Wake, Waker};
55

66
use url::Url;
77

8+
use crate::credssp::ServerError;
89
use crate::network_client::{AsyncNetworkClient, NetworkClient, NetworkProtocol};
910
use crate::{AcceptSecurityContextResult, Error, InitializeSecurityContextResult};
1011

@@ -139,11 +140,11 @@ fn execute_one_step<OutTy>(task: &mut PinnedFuture<OutTy>) -> Option<OutTy> {
139140
}
140141

141142
/// Utility types and methods
142-
impl<'a, YieldTy, ResumeTy, OutTy> Generator<'a, YieldTy, ResumeTy, Result<OutTy, crate::Error>>
143+
impl<'a, YieldTy, ResumeTy, OutTy> Generator<'a, YieldTy, ResumeTy, Result<OutTy, Error>>
143144
where
144145
OutTy: Send + 'a,
145146
{
146-
pub fn resolve_to_result(&mut self) -> Result<OutTy, crate::Error> {
147+
pub fn resolve_to_result(&mut self) -> Result<OutTy, Error> {
147148
let state = self.start();
148149
match state {
149150
GeneratorState::Suspended(_) => Err(Error::new(
@@ -162,6 +163,23 @@ where
162163
self.resolve_to_result().expect(msg)
163164
}
164165
}
166+
167+
impl<'a, YieldTy, ResumeTy, OutTy> Generator<'a, YieldTy, ResumeTy, Result<OutTy, ServerError>>
168+
where
169+
OutTy: Send + 'a,
170+
{
171+
pub fn resolve_to_result(&mut self) -> Result<OutTy, ServerError> {
172+
let state = self.start();
173+
match state {
174+
GeneratorState::Suspended(_) => Err(ServerError {
175+
ts_request: None,
176+
error: Error::new(crate::ErrorKind::UnsupportedFunction, "cannot finish generator"),
177+
}),
178+
GeneratorState::Completed(res) => res,
179+
}
180+
}
181+
}
182+
165183
#[derive(Debug, Clone)]
166184
pub struct NetworkRequest {
167185
pub protocol: NetworkProtocol,

src/kerberos/client/extractors.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,7 @@ pub fn extract_status_code_from_krb_priv_response(
171171
}
172172

173173
/// Extracts [ApRep] from the [NegTokenTarg1] .
174+
#[instrument(ret, level = "trace")]
174175
pub fn extract_ap_rep_from_neg_token_targ(token: &NegTokenTarg1) -> Result<ApRep> {
175176
let resp_token = &token
176177
.0

src/kerberos/config.rs

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,11 @@ use std::str::FromStr;
44
use url::Url;
55

66
use crate::kdc::detect_kdc_url;
7+
use crate::kerberos::ServerProperties;
78
use crate::negotiate::{NegotiatedProtocol, ProtocolConfig};
89
use crate::{Kerberos, Result};
910

11+
/// Kerberos client configuration.
1012
#[derive(Clone, Debug)]
1113
pub struct KerberosConfig {
1214
/// KDC URL
@@ -29,14 +31,14 @@ pub struct KerberosConfig {
2931
}
3032

3133
impl ProtocolConfig for KerberosConfig {
32-
fn new_client(&self) -> Result<NegotiatedProtocol> {
34+
fn new_instance(&self) -> Result<NegotiatedProtocol> {
3335
Ok(NegotiatedProtocol::Kerberos(Kerberos::new_client_from_config(
34-
Clone::clone(self),
36+
self.clone(),
3537
)?))
3638
}
3739

3840
fn box_clone(&self) -> Box<dyn ProtocolConfig> {
39-
Box::new(Clone::clone(self))
41+
Box::new(self.clone())
4042
}
4143
}
4244

@@ -75,3 +77,25 @@ impl KerberosConfig {
7577
}
7678
}
7779
}
80+
81+
/// Kerberos server configuration.
82+
#[derive(Clone, Debug)]
83+
pub struct KerberosServerConfig {
84+
/// General Kerberos configuration.
85+
pub kerberos_config: KerberosConfig,
86+
/// Kerberos server specific parameters.
87+
pub server_properties: ServerProperties,
88+
}
89+
90+
impl ProtocolConfig for KerberosServerConfig {
91+
fn new_instance(&self) -> Result<NegotiatedProtocol> {
92+
Ok(NegotiatedProtocol::Kerberos(Kerberos::new_server_from_config(
93+
self.kerberos_config.clone(),
94+
self.server_properties.clone(),
95+
)?))
96+
}
97+
98+
fn box_clone(&self) -> Box<dyn ProtocolConfig> {
99+
Box::new(self.clone())
100+
}
101+
}

src/kerberos/server/extractors.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use picky_krb::messages::{ApReq, TgtReq};
1414
use crate::{Error, ErrorKind, Result};
1515

1616
/// Extract TGT request and mech types from the first token returned by the Kerberos client.
17+
#[instrument(ret, level = "trace")]
1718
pub fn decode_initial_neg_init(data: &[u8]) -> Result<(Option<TgtReq>, MechTypeList)> {
1819
let token: ApplicationTag0<GssApiNegInit> = picky_asn1_der::from_bytes(data)?;
1920
let NegTokenInit {

0 commit comments

Comments
 (0)