Code

Fix Debian bug #482947: No --nas-ip-address option
authorHolger Weiss <holger@zedat.fu-berlin.de>
Sun, 11 Apr 2010 14:33:44 +0000 (16:33 +0200)
committerHolger Weiss <holger@zedat.fu-berlin.de>
Sun, 11 Apr 2010 14:33:44 +0000 (16:33 +0200)
| check_radius doesn't seem to provide any way to modify the
| NAS-IP-Address attribute that it uses in the packets it sends, but it
| does so for NAS-Identifier.
|
| Instead, it hardcodes the IP address that it gets 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.

Therefore, this commit adds the "-N, --nas-ip-address" option which
allows for specifying the value of the NAS-IP-Address attribute.

| I've also noticed that the original code for NAS-IP-Address hardcoding
| is broken in its error handling - 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. :) I fixed
| that.
|
| 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?"...

[ http://bugs.debian.org/482947 ]

(Contributed by Josip Rodin, forwarded by Jan Wagner.)

NEWS
plugins/check_radius.c

diff --git a/NEWS b/NEWS
index 8ad698d4ef893a2fc4c1ec1e47cd492c17f3f98f..99d48e90664c0e7813cdce1abfa861c51d8d356a 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,7 @@ This file documents the major additions and syntax changes between releases.
        ENHANCEMENTS
        New check_ntp_peer -m and -n options to check the number of usable time sources ("truechimers")
        New check_disk_smb -a option which allows for specifying the IP address of the remote server
+       New check_radius -N option which allows for specifying the value of the NAS-IP-Address attribute
        FIXES
        Fix check_ircd binding to wrong interface (#668778)
        Add proxy-authorization option to check_http (Marcel Kuiper - #1323230, Bryan Irvine - #2863925)
index 57b7090d77d2ec7cd5e71b38c5725d0e069c5a37..37176257be8d276ee4adeb36d75870703e78d184 100644 (file)
@@ -69,6 +69,7 @@ char *server = NULL;
 char *username = NULL;
 char *password = NULL;
 char *nasid = NULL;
+char *nasipaddress = NULL;
 char *expect = NULL;
 char *config_file = NULL;
 unsigned short port = PW_AUTH_UDP_PORT;
@@ -161,19 +162,26 @@ main (int argc, char **argv)
        memset (&data, 0, sizeof(data));
        if (!(my_rc_avpair_add (&data.send_pairs, PW_SERVICE_TYPE, &service, 0) &&
                                my_rc_avpair_add (&data.send_pairs, PW_USER_NAME, username, 0) &&
-                               my_rc_avpair_add (&data.send_pairs, PW_USER_PASSWORD, password, 0) &&
-                               (nasid==NULL || my_rc_avpair_add (&data.send_pairs, PW_NAS_IDENTIFIER, nasid, 0))))
+                               my_rc_avpair_add (&data.send_pairs, PW_USER_PASSWORD, password, 0)
+                               ))
                die (STATE_UNKNOWN, _("Out of Memory?"));
 
-       /*
-        * Fill in NAS-IP-Address
-        */
-
-       if ((client_id = my_rc_own_ipaddress ()) == 0)
-               return (ERROR_RC);
+       if (nasid != NULL) {
+               if (!(my_rc_avpair_add (&data.send_pairs, PW_NAS_IDENTIFIER, nasid, 0)))
+                       die (STATE_UNKNOWN, _("Invalid NAS-Identifier"));
+       }
 
-       if (my_rc_avpair_add (&(data.send_pairs), PW_NAS_IP_ADDRESS, &client_id, 0) ==
-                       NULL) return (ERROR_RC);
+       if (nasipaddress != NULL) {
+               if (rc_good_ipaddr (nasipaddress))
+                       die (STATE_UNKNOWN, _("Invalid NAS-IP-Address"));
+               if ((client_id = rc_get_ipaddr(nasipaddress)) == 0)
+                       die (STATE_UNKNOWN, _("Invalid NAS-IP-Address"));
+       } else {
+               if ((client_id = my_rc_own_ipaddress ()) == 0)
+                       die (STATE_UNKNOWN, _("Can't find local IP for NAS-IP-Address"));
+       }
+       if (my_rc_avpair_add (&(data.send_pairs), PW_NAS_IP_ADDRESS, &client_id, 0) == NULL)
+               die (STATE_UNKNOWN, _("Invalid NAS-IP-Address"));
 
        my_rc_buildreq (&data, PW_ACCESS_REQUEST, server, port, (int)timeout_interval,
                     retries);
@@ -211,6 +219,7 @@ process_arguments (int argc, char **argv)
                {"username", required_argument, 0, 'u'},
                {"password", required_argument, 0, 'p'},
                {"nas-id", required_argument, 0, 'n'},
+               {"nas-ip-address", required_argument, 0, 'N'},
                {"filename", required_argument, 0, 'F'},
                {"expect", required_argument, 0, 'e'},
                {"retries", required_argument, 0, 'r'},
@@ -222,7 +231,7 @@ process_arguments (int argc, char **argv)
        };
 
        while (1) {
-               c = getopt_long (argc, argv, "+hVvH:P:F:u:p:n:t:r:e:", longopts,
+               c = getopt_long (argc, argv, "+hVvH:P:F:u:p:n:N:t:r:e:", longopts,
                                                                         &option);
 
                if (c == -1 || c == EOF || c == 1)
@@ -267,6 +276,9 @@ process_arguments (int argc, char **argv)
                case 'n':                                                                       /* nas id */
                        nasid = optarg;
                        break;
+               case 'N':                                                                       /* nas ip address */
+                       nasipaddress = optarg;
+                       break;
                case 'F':                                                                       /* configuration file */
                        config_file = optarg;
                        break;
@@ -330,6 +342,8 @@ print_help (void)
   printf ("    %s\n", _("Password for autentication (SECURITY RISK)"));
   printf (" %s\n", "-n, --nas-id=STRING");
   printf ("    %s\n", _("NAS identifier"));
+  printf (" %s\n", "-N, --nas-ip-address=STRING");
+  printf ("    %s\n", _("NAS IP Address"));
   printf (" %s\n", "-F, --filename=STRING");
   printf ("    %s\n", _("Configuration file"));
   printf (" %s\n", "-e, --expect=STRING");
@@ -365,8 +379,9 @@ void
 print_usage (void)
 {
   printf (_("Usage:"));
-       printf ("%s -H host -F config_file -u username -p password [-n nas-id] [-P port]\n\
-                  [-t timeout] [-r retries] [-e expect]\n", progname);
+       printf ("%s -H host -F config_file -u username -p password\n\
+                       [-P port] [-t timeout] [-r retries] [-e expect]\n\
+                       [-n nas-id] [-N nas-ip-addr]\n", progname);
 }