Skip to content

Commit 13179b4

Browse files
MikhailMSMikhailMS
committed
WIP #28 
1 parent e517ef0 commit 13179b4

File tree

4 files changed

+127
-28
lines changed

4 files changed

+127
-28
lines changed

‎CHANGELOG.md

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,20 @@
1+
=============
2+
# v0.4.3 (XX Mar 2024)
3+
4+
This release fixes issue reported in [#28](/../../issues/28) (thanks to CoderChristopher for reporting and suggesting the solution)
5+
6+
## What's new
7+
8+
## What's removed or deprecated
9+
10+
## What's changed
11+
* Changed `initialise_packet_from_bytes` function of `radius_packet` to:
12+
* Handle packets of the length less than 20 or more than 4096 octets - returns `RadiusError` (to comply with [RFC2865](https://datatracker.ietf.org/doc/html/rfc2865))
13+
* Derive packet length from `RadiusPacket` (Length field) instead of relying on `bytes.len()`
14+
* If derived packet length is greater than `bytes.len()` - returns `RadiusError` (to comply with [RFC2865](https://datatracker.ietf.org/doc/html/rfc2865))
15+
* Fixed incorrect tests in `protocol/host.rs` (flagged by the changed above)
16+
17+
118
=============
219
# v0.4.2 (05 Aug 2023)
320

@@ -16,14 +33,15 @@ This release fixes some outstanding items and, hopefully, makes it's easier to u
1633
* Remove validation in `verify_original_value` for `ByteString` & `Concat` because it is not really possible to validate those values once received
1734

1835
## What's changed
19-
* Closes #17
36+
* Closes [#17](/../../issues/17)
2037
* Fix for `timestamp_to_bytes` function - it was incorrectly expecting `u64` while RADIUS expects timestamps to be `u32`
2138
* `verify_original_value` function now handles verify for `Integer64` & `InterfaceId` data types
2239
* `original_string_value` function now handles retrieval of string value for `IPv4Prefix` & `InterfaceId` data types
2340
* Functions to encode to/decode from `IPv4` bytes now also handle values with prefix/subnet
2441
* Functions to encode to/decode from `IPv6` bytes now also handle values with prefix/subnet
2542
* Not related to RADIUS implementation - Github Action CI/CD add support for newer Rust versions and drop support for older versions (because unfortunately Action fails on those)
2643

44+
2745
=============
2846
# v0.4.1 (10 Aug 2022)
2947

@@ -45,7 +63,7 @@ This is small release/patch fixing a few bits here & there
4563
* `rand`, `0.7.3` --> `0.8.5`
4664
* `thiserror`, `1.0.23` --> `1.0.32`
4765
* `log` library is moved into `dev-dependencies` and bumped to `0.4.17`
48-
* Added code from PR #24 - ensure dictionary parser not failing when file has tabs as well as whitespaces
66+
* Added code from PR [#24](/../../pull/24) - ensure dictionary parser not failing when file has tabs as well as whitespaces
4967

5068

5169
=============
@@ -61,7 +79,7 @@ Got a couple of PRs & issues raised with some of them introducing breaking chang
6179

6280
## What's changed
6381
* Breaking change - Changed **encrypt_data()** function signature, so **data** parameter is now of type **&[u8]** instead of **&str**. Was reported in [#4](/../../issues/4) by Istvan91
64-
* Breaking change - RADIUS packet creation now doesn't require **Vec<RadiusAttribute>**. To set attributes for packet, call **set_attributes()** function. For examples have a look at **examples/*_client.rs** (Fixes #11)
82+
* Breaking change - RADIUS packet creation now doesn't require **Vec<RadiusAttribute>**. To set attributes for packet, call **set_attributes()** function. For examples have a look at **examples/*_client.rs** (Fixes [#11](/../../issues/11))
6583
* Rewrote **encrypt_data()** a bit to remove unneeded allocations (thanks to Istvan91 [!2](/../../pull/2))
6684
* Rewrote **decrypt_data()** a bit to remove unneeded allocations (thanks to Istvan91 [!2](/../../pull/2))
6785

‎src/protocol/host.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ mod tests {
194194
let dictionary = Dictionary::from_file("./dict_examples/integration_dict").unwrap();
195195
let host = Host::initialise_host(1812, 1813, 3799, dictionary);
196196

197-
let packet_bytes = [4, 43, 0, 83, 215, 189, 213, 172, 57, 94, 141, 70, 134, 121, 101, 57, 187, 220, 227, 73, 4, 6, 192, 168, 1, 10, 5, 6, 0, 0, 0, 0, 32, 10, 116, 114, 105, 108, 108, 105, 97, 110, 30, 19, 48, 48, 45, 48, 52, 45, 53, 70, 45, 48, 48, 45, 48, 70, 45, 68, 49, 31, 19, 48, 48, 45, 48, 49, 45, 50, 52, 45, 56, 48, 45, 66, 51, 45, 57, 67, 8, 6, 10, 0, 0, 100];
197+
let packet_bytes = [4, 43, 0, 86, 215, 189, 213, 172, 57, 94, 141, 70, 134, 121, 101, 57, 187, 220, 227, 73, 4, 6, 192, 168, 1, 10, 5, 6, 0, 0, 0, 0, 32, 10, 116, 114, 105, 108, 108, 105, 97, 110, 30, 19, 48, 48, 45, 48, 52, 45, 53, 70, 45, 48, 48, 45, 48, 70, 45, 68, 49, 31, 19, 48, 48, 45, 48, 49, 45, 50, 52, 45, 56, 48, 45, 66, 51, 45, 57, 67, 8, 6, 10, 0, 0, 100];
198198

199199
match host.verify_packet_attributes(&packet_bytes) {
200200
Err(_err) => {
@@ -209,7 +209,7 @@ mod tests {
209209
let dictionary = Dictionary::from_file("./dict_examples/integration_dict").unwrap();
210210
let host = Host::initialise_host(1812, 1813, 3799, dictionary);
211211

212-
let packet_bytes = [4, 43, 0, 82, 215, 189, 213, 172, 57, 94, 141, 70, 134, 121, 101, 57, 187, 220, 227, 73, 4, 5, 192, 168, 10, 5, 6, 0, 0, 0, 0, 32, 10, 116, 114, 105, 108, 108, 105, 97, 110, 30, 19, 48, 48, 45, 48, 52, 45, 53, 70, 45, 48, 48, 45, 48, 70, 45, 68, 49, 31, 19, 48, 48, 45, 48, 49, 45, 50, 52, 45, 56, 48, 45, 66, 51, 45, 57, 67, 8, 6, 10, 0, 0, 100];
212+
let packet_bytes = [4, 43, 0, 85, 215, 189, 213, 172, 57, 94, 141, 70, 134, 121, 101, 57, 187, 220, 227, 73, 4, 5, 192, 168, 10, 5, 6, 0, 0, 0, 0, 32, 10, 116, 114, 105, 108, 108, 105, 97, 110, 30, 19, 48, 48, 45, 48, 52, 45, 53, 70, 45, 48, 48, 45, 48, 70, 45, 68, 49, 31, 19, 48, 48, 45, 48, 49, 45, 50, 52, 45, 56, 48, 45, 66, 51, 45, 57, 67, 8, 6, 10, 0, 0, 100];
213213

214214
match host.verify_packet_attributes(&packet_bytes) {
215215
Err(err) => {
@@ -241,7 +241,7 @@ mod tests {
241241
let host = Host::initialise_host(1812, 1813, 3799, dictionary);
242242
let secret = "secret";
243243

244-
let packet_bytes = [4, 43, 0, 83, 215, 189, 213, 172, 57, 94, 141, 70, 134, 121, 101, 57, 187, 220, 227, 73, 4, 6, 192, 168, 1, 10, 5, 6, 0, 0, 0, 0, 32, 10, 116, 114, 105, 108, 108, 105, 97, 110, 30, 19, 48, 48, 45, 48, 52, 45, 53, 70, 45, 48, 48, 45, 48, 70, 45, 68, 49, 31, 19, 48, 48, 45, 48, 49, 45, 50, 52, 45, 56, 48, 45, 66, 51, 45, 57, 67, 8, 6, 10, 0, 0, 100];
244+
let packet_bytes = [4, 43, 0, 86, 215, 189, 213, 172, 57, 94, 141, 70, 134, 121, 101, 57, 187, 220, 227, 73, 4, 6, 192, 168, 1, 10, 5, 6, 0, 0, 0, 0, 32, 10, 116, 114, 105, 108, 108, 105, 97, 110, 30, 19, 48, 48, 45, 48, 52, 45, 53, 70, 45, 48, 48, 45, 48, 70, 45, 68, 49, 31, 19, 48, 48, 45, 48, 49, 45, 50, 52, 45, 56, 48, 45, 66, 51, 45, 57, 67, 8, 6, 10, 0, 0, 100];
245245

246246
match host.verify_message_authenticator(&secret, &packet_bytes) {
247247
Err(err) => {

‎src/protocol/radius_packet.rs

Lines changed: 102 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use crate::tools::{
1010
bytes_to_ipv4_string,
1111
bytes_to_ipv6_string,
1212
bytes_to_timestamp,
13+
u16_from_be_bytes
1314
};
1415

1516
use hmac::{ Hmac, Mac };
@@ -197,7 +198,7 @@ impl RadiusAttribute {
197198
},
198199
Some(SupportedAttributeTypes::Concat) => {
199200
// Behaves similar to ByteString but allowed to be longer than 253 octets
200-
Ok(())
201+
Ok(())
201202
},
202203
Some(SupportedAttributeTypes::Integer) => {
203204
match self.value().try_into() {
@@ -342,12 +343,12 @@ impl RadiusAttribute {
342343
/*
343344
*
344345
* 0 1 2
345-
0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0
346-
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-
347-
| Type | Length | Value ...
348-
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-
349-
* Taken from https://tools.ietf.org/html/rfc2865#page-23
350-
*/
346+
0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0
347+
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-
348+
| Type | Length | Value ...
349+
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-
350+
* Taken from https://tools.ietf.org/html/rfc2865#page-23
351+
*/
351352
[ &[self.id], &[(2 + self.value.len()) as u8], self.value.as_slice() ].concat()
352353
}
353354
}
@@ -375,16 +376,30 @@ impl RadiusPacket {
375376

376377
/// Initialises RADIUS packet from raw bytes
377378
pub fn initialise_packet_from_bytes(dictionary: &Dictionary, bytes: &[u8]) -> Result<RadiusPacket, RadiusError> {
379+
if bytes.len() < 20 || bytes.len() > 4096 {
380+
return Err( RadiusError::MalformedPacketError {error: String::from("packet length should be of size between 20 and 4096 octets")} )
381+
}
382+
378383
let code = TypeCode::from_u8(bytes[0])?;
379384
let id = bytes[1];
385+
let packet_len = u16_from_be_bytes(&bytes[2..4]) as usize;
380386
let authenticator = bytes[4..20].to_vec();
381387
let mut attributes = Vec::new();
382388

389+
if packet_len > bytes.len() {
390+
return Err( RadiusError::MalformedPacketError {error:format!("defined packet length: [{}] is greater than actual packet length received: [{}]", packet_len, bytes.len())} )
391+
}
392+
383393
let mut last_index = 20;
384394

385-
while last_index != bytes.len() {
395+
while last_index != packet_len {
386396
let attr_id = bytes[last_index];
387397
let attr_length = bytes[last_index + 1] as usize;
398+
399+
if attr_length == 0 {
400+
return Err( RadiusError::MalformedPacketError {error:format!("attribute with ID: {} has invalid length 0", attr_id)} )
401+
}
402+
388403
let attr_value = &bytes[(last_index + 2)..=(last_index + attr_length - 1)];
389404

390405
match RadiusAttribute::create_by_id(dictionary, attr_id, attr_value.to_vec()) {
@@ -499,17 +514,17 @@ impl RadiusPacket {
499514
/* Prepare packet for a transmission to server/client
500515
*
501516
* 0 1 2 3
502-
0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
503-
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
504-
| Code | Identifier | Length |
505-
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
506-
| |
507-
| Authenticator |
508-
| |
509-
| |
510-
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
511-
| Attributes ...
512-
+-+-+-+-+-+-+-+-+-+-+-+-+-
517+
0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
518+
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
519+
| Code | Identifier | Length |
520+
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
521+
| |
522+
| Authenticator |
523+
| |
524+
| |
525+
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
526+
| Attributes ...
527+
+-+-+-+-+-+-+-+-+-+-+-+-+-
513528
* Taken from https://tools.ietf.org/html/rfc2865#page-14
514529
*
515530
*/
@@ -627,12 +642,78 @@ mod tests {
627642
expected_packet.override_id(43);
628643
expected_packet.override_authenticator(authenticator);
629644

630-
let bytes = [4, 43, 0, 83, 215, 189, 213, 172, 57, 94, 141, 70, 134, 121, 101, 57, 187, 220, 227, 73, 4, 6, 192, 168, 1, 10, 5, 6, 0, 0, 0, 0, 32, 10, 116, 114, 105, 108, 108, 105, 97, 110, 30, 19, 48, 48, 45, 48, 52, 45, 53, 70, 45, 48, 48, 45, 48, 70, 45, 68, 49, 31, 19, 48, 48, 45, 48, 49, 45, 50, 52, 45, 56, 48, 45, 66, 51, 45, 57, 67, 8, 6, 10, 0, 0, 100];
645+
let bytes = [4, 43, 0, 86, 215, 189, 213, 172, 57, 94, 141, 70, 134, 121, 101, 57, 187, 220, 227, 73, 4, 6, 192, 168, 1, 10, 5, 6, 0, 0, 0, 0, 32, 10, 116, 114, 105, 108, 108, 105, 97, 110, 30, 19, 48, 48, 45, 48, 52, 45, 53, 70, 45, 48, 48, 45, 48, 70, 45, 68, 49, 31, 19, 48, 48, 45, 48, 49, 45, 50, 52, 45, 56, 48, 45, 66, 51, 45, 57, 67, 8, 6, 10, 0, 0, 100];
631646
let packet_from_bytes = RadiusPacket::initialise_packet_from_bytes(&dict, &bytes).unwrap();
632647

633648
assert_eq!(expected_packet, packet_from_bytes);
634649
}
635650

651+
#[test]
652+
fn test_initialise_packet_from_bytes_invalid_length() {
653+
let dictionary_path = "./dict_examples/integration_dict";
654+
let dict = Dictionary::from_file(dictionary_path).unwrap();
655+
656+
let bytes = [4, 43, 0, 26, 215, 189, 213, 172, 57, 94, 141, 70, 134, 121, 101, 57, 187, 220, 227, 73];
657+
658+
659+
match RadiusPacket::initialise_packet_from_bytes(&dict, &bytes) {
660+
Err(err) => assert_eq!(String::from("Radius packet is malformed: defined packet length: [26] is greater than actual packet length received: [20]"), err.to_string()),
661+
_ => assert!(false)
662+
}
663+
}
664+
665+
#[test]
666+
fn test_initialise_packet_from_bytes_invalid_length_less_than_20() {
667+
let dictionary_path = "./dict_examples/integration_dict";
668+
let dict = Dictionary::from_file(dictionary_path).unwrap();
669+
670+
let bytes = [4, 43, 0, 19, 215, 189, 213, 172, 57, 94, 141, 70, 134, 121, 101, 57, 187, 220, 227];
671+
672+
match RadiusPacket::initialise_packet_from_bytes(&dict, &bytes) {
673+
Err(err) => assert_eq!(String::from("Radius packet is malformed: packet length should be of size between 20 and 4096 octets"), err.to_string()),
674+
_ => assert!(false)
675+
}
676+
}
677+
678+
#[test]
679+
fn test_initialise_packet_from_bytes_invalid_length_greater_than_4096() {
680+
let dictionary_path = "./dict_examples/integration_dict";
681+
let dict = Dictionary::from_file(dictionary_path).unwrap();
682+
683+
let bytes = [0; 4097];
684+
685+
match RadiusPacket::initialise_packet_from_bytes(&dict, &bytes) {
686+
Err(err) => assert_eq!(String::from("Radius packet is malformed: packet length should be of size between 20 and 4096 octets"), err.to_string()),
687+
_ => assert!(false)
688+
}
689+
}
690+
691+
#[test]
692+
fn test_initialise_packet_from_bytes_invalid_attr_length() {
693+
let dictionary_path = "./dict_examples/integration_dict";
694+
let dict = Dictionary::from_file(dictionary_path).unwrap();
695+
696+
let bytes = [4, 43, 0, 26, 215, 189, 213, 172, 57, 94, 141, 70, 134, 121, 101, 57, 187, 220, 227, 73, 4, 0, 192, 168, 1, 10];
697+
698+
match RadiusPacket::initialise_packet_from_bytes(&dict, &bytes) {
699+
Err(err) => assert_eq!(String::from("Radius packet is malformed: attribute with ID: 4 has invalid length 0"), err.to_string()),
700+
_ => assert!(false)
701+
}
702+
}
703+
704+
#[test]
705+
fn test_initialise_packet_from_bytes_missing_attr() {
706+
let dictionary_path = "./dict_examples/integration_dict";
707+
let dict = Dictionary::from_file(dictionary_path).unwrap();
708+
709+
let bytes = [4, 43, 0, 26, 215, 189, 213, 172, 57, 94, 141, 70, 134, 121, 101, 57, 187, 220, 227, 73, 234, 6, 192, 168, 1, 10];
710+
711+
match RadiusPacket::initialise_packet_from_bytes(&dict, &bytes) {
712+
Err(err) => assert_eq!(String::from("Radius packet is malformed: attribute with ID: 234 is not found in dictionary"), err.to_string()),
713+
_ => assert!(false)
714+
}
715+
}
716+
636717
#[test]
637718
fn test_radius_packet_override_id() {
638719
let attributes: Vec<RadiusAttribute> = Vec::with_capacity(1);
@@ -747,5 +828,5 @@ mod tests {
747828

748829
assert_eq!(expected_message_authenticator, packet.message_authenticator().unwrap());
749830
}
750-
831+
751832
}

‎src/tools/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,7 @@ fn u16_to_be_bytes(u16_data: u16) -> [u8;2] {
397397
u16_data.to_be_bytes()
398398
}
399399

400-
fn u16_from_be_bytes(bytes: &[u8]) -> u16 {
400+
pub(crate) fn u16_from_be_bytes(bytes: &[u8]) -> u16 {
401401
u16::from_be_bytes(bytes.try_into().expect("slice with incorrect length"))
402402
}
403403
// -----------------------------------------

0 commit comments

Comments
 (0)