#291 check_radius wrongly handles NAS-IP-Address

closed-accepted
nobody
Bugfix (123)
5
2010-04-12
2008-05-27
Josip Rodin
No

Hi,

The check_radius plugin is wrongly hardcoding the value of the NAS-IP-Address attribute in the packets it sends.

It gets the address from the rc_own_ipaddress() library call, and that in turn translates into calling gethostbyname() on the result of uname(). This call can easily fail, and its result can easily be unsuitable - for example when the Nagios instance uses its own virtual host, and you don't want the original system hostname leaked to the RADIUS servers you monitor with this.

Furthermore, this behaviour is inconsistent with RFC 2865, which defines the two attributes as analogous and never suggests hardcoding the value of either of them in client software.

I'm attaching a patch which adds a new option (-N) which allows the user to provide the NAS-IP-Address
attribute contents, just like they can for the other attribute (with -n).

While adding this, I also noticed that the error handling there is also broken - it does "return (ERROR_PC)", which is meaningless in the context of check_radius.c. That actually seems to be copy&waste from radiusclient-0.3.2/src/radexample.c. :) so I fixed that as well.

While debugging, I also took the opportunity to decouple the nas-identifier rc_avpair_add() instance from the initial three, because this is just bad practice to lump a fourth optional attribute into the same block with the required attributes, the error handling for which is throwing the same daft message "Out of Memory?"...

Another tidbit - the -n option, to include NAS-Identifier, requires that the radiusclient attribute dictionary includes that attribute - which it doesn't in the old library. radiusclient-ng has this fixed. I'm mentioning this just because of the recent change to the REQUIREMENTS file - just another confirmation that the change is correct, I guess :)

Please include the patch. Thanks.

(I've also filed this at http://bugs.debian.org/482947 FWIW)

Discussion

  • Josip Rodin
    Josip Rodin
    2008-05-27

    fix for plugins/check_radius.c's NAS-IP-Address handling

     
    Attachments
  • Josip Rodin
    Josip Rodin
    2008-06-06

    Logged In: YES
    user_id=119756
    Originator: YES

    > Furthermore, this behaviour is inconsistent with RFC 2865, which defines
    > the two attributes as analogous and never suggests hardcoding the value of
    > either of them in client software.

    I just realized that I forgot to mention the other attribute's name before that paragraph - I'm talking about NAS-Identifier.

     
  • Josip Rodin
    Josip Rodin
    2008-06-07

    Logged In: YES
    user_id=119756
    Originator: YES

    Jan Wagner told me that my original patch doesn't apply to 1.4.12; here's an updated patch for that version.
    The only real change should be the use of my_rc_own_ipaddress(), it seems.

    File Added: check_radius.c.diff

     
  • Josip Rodin
    Josip Rodin
    2008-06-07

    fix for plugins/check_radius.c's NAS-IP-Address handling (1.4.12)

     
    Attachments
  • Holger Weiß
    Holger Weiß
    2010-04-12

    I committed the patch.

    Thank you!

     
  • Holger Weiß
    Holger Weiß
    2010-04-12

    • status: open --> closed-accepted