Skip to content

Commit fa4efe5

Browse files
CBenoitclaude
andauthored
refactor: change nonce parameter type from byte slice to u32 (#762)
Co-authored-by: Claude <noreply@anthropic.com>
1 parent f7335d5 commit fa4efe5

7 files changed

Lines changed: 35 additions & 46 deletions

File tree

‎src/kerberos/client/change_password.rs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,14 +39,13 @@ pub async fn change_password<'a>(
3939
let realm = &get_client_principal_realm(username, domain);
4040

4141
let mut rand = StdRng::try_from_rng(&mut SysRng)?;
42-
let nonce = &generate_nonce(&mut rand).to_be_bytes();
42+
let nonce = generate_nonce(&mut rand);
4343

4444
let options = GenerateAsReqOptions {
4545
realm,
4646
username,
4747
cname_type,
4848
snames: &[KADMIN, CHANGE_PASSWORD_SERVICE_NAME],
49-
// 4 = size of u32
5049
nonce,
5150
hostname: &client.config.client_computer_name,
5251
context_requirements: ClientRequestFlags::empty(),

‎src/kerberos/client/generators.rs‎

Lines changed: 25 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -253,20 +253,18 @@ pub(crate) fn generate_nonce(rng: &mut impl Rng) -> u32 {
253253
rng.next_u32() & 0x7fff_ffff
254254
}
255255

256-
/// Encodes a 4-byte big-endian nonce as a minimal DER INTEGER in the `Int32` range.
256+
/// Encodes a nonce as a minimal DER INTEGER in the `Int32` range.
257257
///
258258
/// The TGS-REQ authenticator checksum covers the DER encoded KDC-REQ-BODY and the Windows KDC
259-
/// verifies it against its own re-encoding of the body. Random bytes used as-is are not minimal
260-
/// DER when they start with `00` followed by a byte below `0x80`, or `FF` followed by a byte of
261-
/// `0x80` or above. The re-encoded body then differs and the KDC fails the request with
262-
/// `KRB_AP_ERR_MODIFIED`.
259+
/// verifies it against its own re-encoding of the body. A nonce that is not minimal DER makes the
260+
/// re-encoded body differ and the KDC fails the request with `KRB_AP_ERR_MODIFIED`.
263261
///
264-
/// The bytes are encoded as a signed value, the way Windows re-encodes the nonce. A nonce with the
262+
/// The value is encoded as a signed `Int32`, the way Windows re-encodes the nonce. A nonce with the
265263
/// high bit set stays a 4-byte negative INTEGER, an unsigned encoding would add a fifth `00` octet
266264
/// and the Windows KDC rejects the request outright. Nonces from [generate_nonce] are positive and
267265
/// encode the same either way.
268-
pub(crate) fn nonce_to_asn1(nonce: &[u8]) -> IntegerAsn1 {
269-
IntegerAsn1::from_bytes_be_signed(nonce.to_vec())
266+
pub(crate) fn nonce_to_asn1(nonce: u32) -> IntegerAsn1 {
267+
IntegerAsn1::from_bytes_be_signed(nonce.to_be_bytes().to_vec())
270268
}
271269

272270
/// Parameters for generating [AsReq].
@@ -276,7 +274,8 @@ pub struct GenerateAsReqOptions<'a> {
276274
pub username: &'a str,
277275
pub cname_type: u8,
278276
pub snames: &'a [&'a str],
279-
pub nonce: &'a [u8],
277+
/// KDC-REQ nonce, encoded with [nonce_to_asn1].
278+
pub nonce: u32,
280279
pub hostname: &'a str,
281280
pub context_requirements: ClientRequestFlags,
282281
}
@@ -340,7 +339,7 @@ pub fn generate_as_req_kdc_body(options: &GenerateAsReqOptions<'_>) -> Result<Kd
340339
rtime: Optional::from(Some(ExplicitContextTag6::from(GeneralizedTimeAsn1::from(
341340
GeneralizedTime::from(expiration_date),
342341
)))),
343-
nonce: ExplicitContextTag7::from(nonce_to_asn1(nonce)),
342+
nonce: ExplicitContextTag7::from(nonce_to_asn1(*nonce)),
344343
etype: ExplicitContextTag8::from(Asn1SequenceOf::from(vec![
345344
IntegerAsn1::from(vec![CipherSuite::Aes256CtsHmacSha196.into()]),
346345
IntegerAsn1::from(vec![CipherSuite::Aes128CtsHmacSha196.into()]),
@@ -406,7 +405,7 @@ pub fn generate_tgs_req(options: GenerateTgsReqOptions<'_>) -> Result<TgsReq> {
406405
}
407406

408407
let mut rng = StdRng::try_from_rng(&mut SysRng)?;
409-
let nonce = generate_nonce(&mut rng).to_be_bytes();
408+
let nonce = generate_nonce(&mut rng);
410409

411410
let req_body = KdcReqBody {
412411
kdc_options: ExplicitContextTag0::from(KerberosFlags::from(BitString::with_bytes(
@@ -424,7 +423,7 @@ pub fn generate_tgs_req(options: GenerateTgsReqOptions<'_>) -> Result<TgsReq> {
424423
from: Optional::from(None),
425424
till: ExplicitContextTag5::from(GeneralizedTimeAsn1::from(GeneralizedTime::from(expiration_date))),
426425
rtime: Optional::from(None),
427-
nonce: ExplicitContextTag7::from(nonce_to_asn1(&nonce)),
426+
nonce: ExplicitContextTag7::from(nonce_to_asn1(nonce)),
428427
etype: ExplicitContextTag8::from(Asn1SequenceOf::from(vec![
429428
IntegerAsn1::from(vec![CipherSuite::Aes256CtsHmacSha196.into()]),
430429
IntegerAsn1::from(vec![CipherSuite::Aes128CtsHmacSha196.into()]),
@@ -902,20 +901,20 @@ mod tests {
902901
fn nonce_is_minimal_der() {
903902
for (nonce, expected) in [
904903
// Previously sent verbatim, the Windows KDC failed these with KRB_AP_ERR_MODIFIED.
905-
([0x00, 0x09, 0xa7, 0x92], vec![0x02, 0x03, 0x09, 0xa7, 0x92]),
906-
([0x00, 0x00, 0x00, 0x01], vec![0x02, 0x01, 0x01]),
907-
([0x00, 0x00, 0x00, 0x00], vec![0x02, 0x01, 0x00]),
908-
([0x7f, 0xa0, 0xb3, 0xdf], vec![0x02, 0x04, 0x7f, 0xa0, 0xb3, 0xdf]),
909-
([0x12, 0x34, 0x56, 0x78], vec![0x02, 0x04, 0x12, 0x34, 0x56, 0x78]),
904+
(0x0009_a792, vec![0x02, 0x03, 0x09, 0xa7, 0x92]),
905+
(0x0000_0001, vec![0x02, 0x01, 0x01]),
906+
(0x0000_0000, vec![0x02, 0x01, 0x00]),
907+
(0x7fa0_b3df, vec![0x02, 0x04, 0x7f, 0xa0, 0xb3, 0xdf]),
908+
(0x1234_5678, vec![0x02, 0x04, 0x12, 0x34, 0x56, 0x78]),
910909
// A high bit must not add a fifth `00` octet, the Windows KDC rejects a nonce outside the
911910
// `Int32` range.
912-
([0x9a, 0xbc, 0xde, 0xf0], vec![0x02, 0x04, 0x9a, 0xbc, 0xde, 0xf0]),
913-
([0x80, 0x00, 0x00, 0x00], vec![0x02, 0x04, 0x80, 0x00, 0x00, 0x00]),
914-
([0xff, 0xff, 0xff, 0xff], vec![0x02, 0x01, 0xff]),
911+
(0x9abc_def0, vec![0x02, 0x04, 0x9a, 0xbc, 0xde, 0xf0]),
912+
(0x8000_0000, vec![0x02, 0x04, 0x80, 0x00, 0x00, 0x00]),
913+
(0xffff_ffff, vec![0x02, 0x01, 0xff]),
915914
// Non-minimal negative, previously failed with KRB_AP_ERR_MODIFIED.
916-
([0xff, 0xa0, 0xb3, 0xdf], vec![0x02, 0x03, 0xa0, 0xb3, 0xdf]),
915+
(0xffa0_b3df, vec![0x02, 0x03, 0xa0, 0xb3, 0xdf]),
917916
] {
918-
assert_eq!(picky_asn1_der::to_vec(&nonce_to_asn1(&nonce)).unwrap(), expected);
917+
assert_eq!(picky_asn1_der::to_vec(&nonce_to_asn1(nonce)).unwrap(), expected);
919918
}
920919
}
921920

@@ -934,22 +933,16 @@ mod tests {
934933
#[test]
935934
fn as_req_kdc_body_nonce_is_minimal_der() {
936935
for (nonce, expected) in [
937-
(
938-
[0x00, 0x09, 0xa7, 0x92],
939-
&[0xa7, 0x05, 0x02, 0x03, 0x09, 0xa7, 0x92][..],
940-
),
936+
(0x0009_a792, &[0xa7, 0x05, 0x02, 0x03, 0x09, 0xa7, 0x92][..]),
941937
// An unrestricted random nonce (e.g. `next_u32()`) must stay within 4 octets.
942-
(
943-
[0x9a, 0xbc, 0xde, 0xf0],
944-
&[0xa7, 0x06, 0x02, 0x04, 0x9a, 0xbc, 0xde, 0xf0][..],
945-
),
938+
(0x9abc_def0, &[0xa7, 0x06, 0x02, 0x04, 0x9a, 0xbc, 0xde, 0xf0][..]),
946939
] {
947940
let body = generate_as_req_kdc_body(&GenerateAsReqOptions {
948941
realm: "CRABKA.TEST",
949942
username: "alice",
950943
cname_type: NT_PRINCIPAL,
951944
snames: &["krbtgt", "CRABKA.TEST"],
952-
nonce: &nonce,
945+
nonce,
953946
hostname: "host",
954947
context_requirements: ClientRequestFlags::empty(),
955948
})
@@ -1021,7 +1014,7 @@ mod tests {
10211014
username,
10221015
cname_type: NT_PRINCIPAL,
10231016
snames: &["krbtgt", "CRABKA.TEST"],
1024-
nonce: &[0, 0, 0, 1],
1017+
nonce: 1,
10251018
hostname: "host",
10261019
context_requirements: ClientRequestFlags::empty(),
10271020
})

‎src/kerberos/client/mod.rs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -186,8 +186,7 @@ pub async fn initialize_security_context<'a>(
186186
username: &username,
187187
cname_type,
188188
snames: &[TGT_SERVICE_NAME, &realm],
189-
// 4 = size of u32
190-
nonce: &generate_nonce(&mut rand).to_be_bytes(),
189+
nonce: generate_nonce(&mut rand),
191190
hostname: &client.config.client_computer_name,
192191
context_requirements: builder.context_requirements,
193192
};
@@ -237,7 +236,7 @@ pub async fn initialize_security_context<'a>(
237236
smart_card.sign(digest)
238237
}),
239238
with_pre_auth: false,
240-
authenticator_nonce: generate_nonce(&mut rand).to_be_bytes(),
239+
authenticator_nonce: generate_nonce(&mut rand),
241240
}))
242241
}
243242
};

‎src/kerberos/server/as_exchange.rs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,7 @@ pub(crate) async fn request_tgt(
7777
username: &username,
7878
cname_type,
7979
snames: &[TGT_SERVICE_NAME, &realm],
80-
// 4 = size of u32
81-
nonce: &nonce.to_be_bytes(),
80+
nonce,
8281
hostname: &server.config.client_computer_name,
8382
context_requirements: ClientRequestFlags::empty(),
8483
};

‎src/pk_init.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ pub(crate) struct GenerateAsPaDataOptions<'a> {
6868
pub dh_parameters: DhParameters,
6969
pub sign_data: SignDataFn,
7070
pub with_pre_auth: bool,
71-
pub authenticator_nonce: [u8; 4],
71+
pub authenticator_nonce: u32,
7272
}
7373

7474
#[instrument(level = "trace", skip_all, ret)]
@@ -127,7 +127,7 @@ pub(crate) fn generate_pa_datas_for_as_req_at(
127127
pk_authenticator: ExplicitContextTag0::from(PkAuthenticator {
128128
cusec: ExplicitContextTag0::from(IntegerAsn1::from(microseconds.to_be_bytes().to_vec())),
129129
ctime: ExplicitContextTag1::from(KerberosTime::from(GeneralizedTime::from(current_date))),
130-
nonce: ExplicitContextTag2::from(nonce_to_asn1(authenticator_nonce)),
130+
nonce: ExplicitContextTag2::from(nonce_to_asn1(*authenticator_nonce)),
131131
pa_checksum: Optional::from(Some(ExplicitContextTag3::from(OctetStringAsn1::from(
132132
kdc_req_body_sha1_hash,
133133
)))),

‎src/pku2u/extractors.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ mod tests {
148148
username: "AzureAD\\issuer\\subject",
149149
cname_type: 0x80,
150150
snames: &["host"],
151-
nonce: &[1, 2, 3, 4],
151+
nonce: 0x0102_0304,
152152
hostname: "hostname",
153153
context_requirements: ClientRequestFlags::empty(),
154154
})
@@ -177,7 +177,7 @@ mod tests {
177177
username: "AzureAD\\issuer\\subject",
178178
cname_type: 0x80,
179179
snames: &["host"],
180-
nonce: &[1, 2, 3, 4],
180+
nonce: 0x0102_0304,
181181
hostname: "hostname",
182182
context_requirements: ClientRequestFlags::empty(),
183183
})

‎src/pku2u/mod.rs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1147,14 +1147,13 @@ impl Pku2u {
11471147
let next_seq_number = self.next_seq_number();
11481148
let mut rng = StdRng::try_from_rng(&mut SysRng)?;
11491149
let request_nonce = generate_nonce(&mut rng);
1150-
let request_nonce_bytes = request_nonce.to_be_bytes();
11511150
self.request_nonce = Some(request_nonce);
11521151
let kdc_req_body = generate_as_req_kdc_body(&GenerateAsReqOptions {
11531152
realm: WELLKNOWN_REALM,
11541153
username: &generate_as_req_username_from_certificate(&self.config.p2p_certificate)?,
11551154
cname_type: 0x80,
11561155
snames: &snames,
1157-
nonce: &request_nonce_bytes,
1156+
nonce: request_nonce,
11581157
hostname: &self.config.client_hostname,
11591158
context_requirements: builder.context_requirements,
11601159
})?;
@@ -1172,7 +1171,7 @@ impl Pku2u {
11721171
})
11731172
}),
11741173
with_pre_auth: true,
1175-
authenticator_nonce: request_nonce_bytes,
1174+
authenticator_nonce: request_nonce,
11761175
})?;
11771176
let as_req = generate_as_req(pa_datas, kdc_req_body);
11781177

0 commit comments

Comments
 (0)