search

moznion / radius-rs

An async/await native implementation of the RADIUS server and client for Rust.
MIT License
88 stars 14 forks source link

Wrong attribute lengths #35

Open fakuivan opened 1 year ago

fakuivan commented 1 year ago

APVs like Tunnel-Type are added to a packet using the function from_tagged_u32, which does not take into account the appropriate length of the attribute. Something similar happens with Tunnel-Medium-Type.

https://github.com/moznion/radius-rs/blob/149e2706a257e7c2d66d115d179d97cdc94d7e32/radius/src/core/rfc2868.rs#L76-L78

As mentioned in RFC2868, the length for Tunnel-Type is always 6, and this mismatch can cause problems with clients that strictly follow this. radtest will show attributes that do not follow these standards as Attr-N instead of decoding it into the appropriate name, so this can be useful when trying to detect these issues.

part1cleth1ef commented 5 months ago

Hey, sorry for the bump but is there any information on this??

fakuivan commented 5 months ago

Well, I did end up fixing this, but I didn't continue using radius and opted for something else instead, so I didn't follow up on it. I did a few changes but basically it seems like some attribute byte sizes were interpreted to be the same size as the integral type used, meaning if the value taken was u32, the attribute length would end up being 4 bytes. Since rust has no 3 byte integrals (no u24/i24) and some APVs are sized to 3 bytes, I ended up implementing a macro to trim the integral and panic on overflows.

Here are the changes I made: https://github.com/fakuivan/radius-rs/commit/c226e3075af178fdd869cdb8ea065e6ffb8b2be4

Maybe you can make a PR out of this? I'm not aware of these issues being solved, maybe they are?