add "deny" to limited networks

diff --git a/raddb/sites-available/dynamic-clients b/raddb/sites-available/dynamic-clients
index 8903e93a16016a3dd3cd3219ac86347b1af983fb..5964e68a8a0e2fb9f71d79244feb2c9131f54682 100644 (file)
--- a/raddb/sites-available/dynamic-clients
+++ b/raddb/sites-available/dynamic-clients
@@ -155,7 +155,7 @@ server dynamic_clients {
                        #  If dynamic clients are not allowed, then
                        #  this section is ignored.
                        #
-                       allow {
+                       networks {
                                #
                                #  Allow packets from these networks
                                #  to define dynamic clients.
@@ -170,8 +170,25 @@ server dynamic_clients {
                                #  There is no limit to the number of
                                #  networks which can be listed here.
                                #
-                               network = 127/8
-                               network = 192.0.2/24
+                               allow = 127/8
+                               allow = 192.0.2/24
+
+                               #
+                               #  The default is to deny all networks
+                               #  which are not in the "allow" list.
+                               #
+                               #  However, it may be useful to allow
+                               #  a large range, and then deny a
+                               #  subset of IPs in that range.  For
+                               #  that situation, use a "deny"
+                               #
+                               #  In this example, 127.0.0.1 can be
+                               #  used to define a dynamic client.
+                               #  But the (hypothetical) 127.1.0.1
+                               #  address cannot be usef to define
+                               #  a dynamic client.
+                               #
+                               deny = 127.1/16
                        }
                }
        }

diff --git a/src/modules/proto_radius/io.c b/src/modules/proto_radius/io.c
index b5c1990991da2a35fc742be34c68c9fb35458231..4c5919356df7d91199642244626b25f495065dfc 100644 (file)
--- a/src/modules/proto_radius/io.c
+++ b/src/modules/proto_radius/io.c
@@ -868,9 +868,17 @@ redo:
                                return 0;
                        }
 
+                       /*
+                        *      Look up the allowed networks.
+                        */
                        network = fr_trie_lookup(inst->networks, &address.src_ipaddr.addr, address.src_ipaddr.prefix);
                        if (!network) goto ignore;
 
+                       /*
+                        *      It exists, but it's a "deny" rule, ignore it.
+                        */
+                       if (network->af == AF_UNSPEC) goto ignore;
+
                        /*
                         *      Allocate our local radclient as a
                         *      placeholder for the dynamic client.

diff --git a/src/modules/proto_radius/proto_radius.h b/src/modules/proto_radius/proto_radius.h
index 92e1d224649c58960ee2934aa49cd41ffd99ebb2..2b53279cda5a8904de78c90a1fef1bd05d75797e 100644 (file)
--- a/src/modules/proto_radius/proto_radius.h
+++ b/src/modules/proto_radius/proto_radius.h
@@ -1,4 +1,3 @@
-
 #pragma once
 /*
  *  This program is free software; you can redistribute it and/or modify

diff --git a/src/modules/proto_radius/proto_radius_udp.c b/src/modules/proto_radius/proto_radius_udp.c
index 90d869247feafb5458a2b51b95d3bad09a7e10f1..c70b13f2d5c57fcd66d4b75cb55948c0023e9f96 100644 (file)
--- a/src/modules/proto_radius/proto_radius_udp.c
+++ b/src/modules/proto_radius/proto_radius_udp.c
@@ -62,15 +62,17 @@ typedef struct proto_radius_udp_t {
        bool                            dynamic_clients;        //!< whether we have dynamic clients
 
        fr_trie_t                       *trie;                  //!< for parsed networks
-       fr_ipaddr_t                     *network;               //!< network for dynamic clients
+       fr_ipaddr_t                     *allow;                 //!< allowed networks for dynamic clients
+       fr_ipaddr_t                     *deny;                  //!< denied networks for dynamic clients
 
        proto_radius_connection_t       *connection;            //!< for connected sockets.
 
 } proto_radius_udp_t;
 
 
-static const CONF_PARSER allow_config[] = {
-       { FR_CONF_OFFSET("network", FR_TYPE_COMBO_IP_PREFIX | FR_TYPE_MULTI, proto_radius_udp_t, network) },
+static const CONF_PARSER networks_config[] = {
+       { FR_CONF_OFFSET("allow", FR_TYPE_COMBO_IP_PREFIX | FR_TYPE_MULTI, proto_radius_udp_t, allow) },
+       { FR_CONF_OFFSET("deny", FR_TYPE_COMBO_IP_PREFIX | FR_TYPE_MULTI, proto_radius_udp_t, deny) },
 
        CONF_PARSER_TERMINATOR
 };
@@ -88,7 +90,7 @@ static const CONF_PARSER udp_listen_config[] = {
        { FR_CONF_IS_SET_OFFSET("recv_buff", FR_TYPE_UINT32, proto_radius_udp_t, recv_buff) },
 
        { FR_CONF_OFFSET("dynamic_clients", FR_TYPE_BOOL, proto_radius_udp_t, dynamic_clients) } ,
-       { FR_CONF_POINTER("allow", FR_TYPE_SUBSECTION, NULL), .subcs = (void const *) allow_config },
+       { FR_CONF_POINTER("networks", FR_TYPE_SUBSECTION, NULL), .subcs = (void const *) networks_config },
 
        { FR_CONF_OFFSET("max_packet_size", FR_TYPE_UINT32, proto_radius_udp_t, max_packet_size), .dflt = "4096" } ,
                { FR_CONF_OFFSET("max_attributes", FR_TYPE_UINT32, proto_radius_udp_t, max_attributes), .dflt = STRINGIFY(RADIUS_MAX_ATTRIBUTES) } ,
@@ -473,7 +475,7 @@ static int mod_bootstrap(void *instance, CONF_SECTION *cs)
         *      e.g. allow clients from a /16, but not from a /24
         *      within that /16.
         */
-       num = talloc_array_length(inst->network);
+       num = talloc_array_length(inst->allow);
        if (!num) {
                if (inst->dynamic_clients) {
                        cf_log_err(cs, "The 'allow' subsection MUST contain at least one 'network' entry when 'dynamic_clients = true'.");
@@ -489,9 +491,9 @@ static int mod_bootstrap(void *instance, CONF_SECTION *cs)
                        /*
                         *      Can't add v4 networks to a v6 socket, or vice versa.
                         */
-                       if (inst->network[i].af != inst->ipaddr.af) {
-                               fr_value_box_snprint(buffer, sizeof(buffer), fr_box_ipaddr(inst->network[i]), 0);
-                               cf_log_err(cs, "Address family in entry %zd - 'network = %s' does not match 'ipaddr'", i + 1, buffer);
+                       if (inst->allow[i].af != inst->ipaddr.af) {
+                               fr_value_box_snprint(buffer, sizeof(buffer), fr_box_ipaddr(inst->allow[i]), 0);
+                               cf_log_err(cs, "Address family in entry %zd - 'allow = %s' does not match 'ipaddr'", i + 1, buffer);
                                return -1;
                        }
 
@@ -499,10 +501,10 @@ static int mod_bootstrap(void *instance, CONF_SECTION *cs)
                         *      Duplicates are bad.
                         */
                        network = fr_trie_match(inst->trie,
-                                               &inst->network[i].addr, inst->network[i].prefix);
+                                               &inst->allow[i].addr, inst->allow[i].prefix);
                        if (network) {
-                               fr_value_box_snprint(buffer, sizeof(buffer), fr_box_ipaddr(inst->network[i]), 0);
-                               cf_log_err(cs, "Cannot add duplicate entry 'network = %s'", buffer);
+                               fr_value_box_snprint(buffer, sizeof(buffer), fr_box_ipaddr(inst->allow[i]), 0);
+                               cf_log_err(cs, "Cannot add duplicate entry 'allow = %s'", buffer);
                                return -1;
                        }
 
@@ -518,10 +520,10 @@ static int mod_bootstrap(void *instance, CONF_SECTION *cs)
                         *      have terminal fr_trie_user_t nodes"
                         */
                        network = fr_trie_lookup(inst->trie,
-                                                &inst->network[i].addr, inst->network[i].prefix);
-                       if (network && (network->prefix <= inst->network[i].prefix)) {
-                               fr_value_box_snprint(buffer, sizeof(buffer), fr_box_ipaddr(inst->network[i]), 0);
-                               cf_log_err(cs, "Cannot add overlapping entry 'network = %s'", buffer);
+                                                &inst->allow[i].addr, inst->allow[i].prefix);
+                       if (network && (network->prefix <= inst->allow[i].prefix)) {
+                               fr_value_box_snprint(buffer, sizeof(buffer), fr_box_ipaddr(inst->allow[i]), 0);
+                               cf_log_err(cs, "Cannot add overlapping entry 'allow = %s'", buffer);
                                cf_log_err(cs, "Entry is completely enclosed inside of a previously defined network.");
                                return -1;
                        }
@@ -532,13 +534,90 @@ static int mod_bootstrap(void *instance, CONF_SECTION *cs)
                         *      the network.
                         */
                        if (fr_trie_insert(inst->trie,
-                                          &inst->network[i].addr, inst->network[i].prefix,
-                                          &inst->network[i]) < 0) {
-                               fr_value_box_snprint(buffer, sizeof(buffer), fr_box_ipaddr(inst->network[i]), 0);
-                               cf_log_err(cs, "Failed adding 'network = %s' to tracking table.", buffer);
+                                          &inst->allow[i].addr, inst->allow[i].prefix,
+                                          &inst->allow[i]) < 0) {
+                               fr_value_box_snprint(buffer, sizeof(buffer), fr_box_ipaddr(inst->allow[i]), 0);
+                               cf_log_err(cs, "Failed adding 'allow = %s' to tracking table.", buffer);
                                return -1;
                        }
                }
+
+               /*
+                *      And now check denied networks.
+                */
+               num = talloc_array_length(inst->deny);
+               if (!num) return 0;
+
+               /*
+                *      Since the default is to deny, you can only add
+                *      a "deny" inside of a previous "allow".
+                */
+               for (i = 0; i < num; i++) {
+                       fr_ipaddr_t *network;
+                       char buffer[256];
+
+                       /*
+                        *      Can't add v4 networks to a v6 socket, or vice versa.
+                        */
+                       if (inst->deny[i].af != inst->ipaddr.af) {
+                               fr_value_box_snprint(buffer, sizeof(buffer), fr_box_ipaddr(inst->deny[i]), 0);
+                               cf_log_err(cs, "Address family in entry %zd - 'deny = %s' does not match 'ipaddr'", i + 1, buffer);
+                               return -1;
+                       }
+
+                       /*
+                        *      Duplicates are bad.
+                        */
+                       network = fr_trie_match(inst->trie,
+                                               &inst->deny[i].addr, inst->deny[i].prefix);
+                       if (network) {
+                               fr_value_box_snprint(buffer, sizeof(buffer), fr_box_ipaddr(inst->deny[i]), 0);
+                               cf_log_err(cs, "Cannot add duplicate entry 'deny = %s'", buffer);
+                               return -1;
+                       }
+
+                       /*
+                        *      A "deny" can only be within a previous "allow".
+                        */
+                       network = fr_trie_lookup(inst->trie,
+                                               &inst->deny[i].addr, inst->deny[i].prefix);
+                       if (!network) {
+                               fr_value_box_snprint(buffer, sizeof(buffer), fr_box_ipaddr(inst->deny[i]), 0);
+                               cf_log_err(cs, "The network in entry %zd - 'deny = %s' is not contained within a previous 'allow'",
+                                          i + 1, buffer);
+                               return -1;
+                       }
+
+                       /*
+                        *      We hack the AF in "deny" rules.  If
+                        *      the lookup gets AF_UNSPEC, then we're
+                        *      adding a "deny" inside of a "deny".
+                        */
+                       if (network->af != inst->ipaddr.af) {
+                               fr_value_box_snprint(buffer, sizeof(buffer), fr_box_ipaddr(inst->deny[i]), 0);
+                               cf_log_err(cs, "The network in entry %zd - 'deny = %s' is overlaps with another 'deny' rule",
+                                          i + 1, buffer);
+                               return -1;
+                       }
+
+                       /*
+                        *      Insert the network into the trie.
+                        *      Lookups will return the fr_ipaddr_t of
+                        *      the network.
+                        */
+                       if (fr_trie_insert(inst->trie,
+                                          &inst->deny[i].addr, inst->deny[i].prefix,
+                                          &inst->deny[i]) < 0) {
+                               fr_value_box_snprint(buffer, sizeof(buffer), fr_box_ipaddr(inst->deny[i]), 0);
+                               cf_log_err(cs, "Failed adding 'deny = %s' to tracking table.", buffer);
+                               return -1;
+                       }
+
+                       /*
+                        *      Hack it to make it a deny rule.
+                        */
+                       inst->deny[i].af = AF_UNSPEC;
+               }
        }
 
        return 0;