Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong attribute lengths #35

Open
fakuivan opened this issue Dec 15, 2022 · 3 comments
Open

Wrong attribute lengths #35

fakuivan opened this issue Dec 15, 2022 · 3 comments

Comments

@fakuivan
Copy link

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.

pub fn add_tunnel_type(packet: &mut Packet, tag: Option<&Tag>, value: TunnelType) {
packet.add(AVP::from_tagged_u32(TUNNEL_TYPE_TYPE, tag, value as u32));
}

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
Copy link

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

@fakuivan
Copy link
Author

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: fakuivan@c226e30

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

@elonen
Copy link

elonen commented Nov 1, 2024

This is still an issue, and makes at least JunOS fail parsing the messages. Here's the workaround I ended up using:

// Causes 'MALFORMED DATA' on JunOS:

rfc2868::add_tunnel_type(&mut resp_packet, tag, rfc3580::TUNNEL_TYPE_VLAN);
rfc2868::add_tunnel_medium_type(&mut resp_packet, tag, rfc2868::TUNNEL_MEDIUM_TYPE_IEEE_802);


// Works:

fn u32_as_u24_str(v: u32) -> String {
    String::from_utf8(v.to_be_bytes()[1..].to_vec()).unwrap()
}

resp_packet.add(AVP::from_tagged_string(rfc2868::TUNNEL_TYPE_TYPE, tag, &u32_as_u24_str(rfc3580::TUNNEL_TYPE_VLAN)));
resp_packet.add(AVP::from_tagged_string(rfc2868::TUNNEL_MEDIUM_TYPE_TYPE, tag, &u32_as_u24_str(rfc2868::TUNNEL_MEDIUM_TYPE_IEEE_802)));

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants