From: Alan T. DeKok Date: Mon, 23 Apr 2018 15:21:47 +0000 (-0400) Subject: add "deny" to limited networks X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=fd127330d797f7f560502c5a546f2e20dfebe477;p=thirdparty%2Ffreeradius-server.git add "deny" to limited networks --- diff --git a/raddb/sites-available/dynamic-clients b/raddb/sites-available/dynamic-clients index 8903e93a160..5964e68a8a0 100644 --- 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 b5c1990991d..4c5919356df 100644 --- 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 92e1d224649..2b53279cda5 100644 --- 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 90d869247fe..c70b13f2d5c 100644 --- 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;