On Tue, Jul 09, 2024 at 11:44:58PM +0200, Bernhard Schmidt wrote: > Control: tags -1 help security > > Am 09.07.24 um 18:15 schrieb Herwin Weststrate: > > Package: freeradius > > Version: 3.2.1+dfsg-4+deb12u1 > > > > FreeRADIUS 3.2.5 has just been released, which includes some security > > fixes for BlastRADIUS: a vulnerability with a name and a website[0] and > > a logo (hadn't seen one of those in a while). > > > > The FreeRADIUS security page[1] (scroll to "2024.07.09", there is no > > anchor to link directly to the relevant article) describes some new > > configuration options to resolve everything. Since this will be the > > first thing people read, it would be nice to have those backported to > > the Debian packages. > > > > At first glance, it looks like this requires just two commits[2] [3] to > > be cherry-picked, but there may be some hidden dependencies in previous > > commits. > > > [2] > > https://github.com/FreeRADIUS/freeradius-server/commit/0947439f2569d2b8c2b4949be24250263934e260 > > [3] > > https://github.com/FreeRADIUS/freeradius-server/commit/6616be90346beb6050446bd00c8ed5bca1b8ef29 > > I haven't looked closer yet, but the patches do not apply at all > > Given that the freeradius codebase is really complicated I'm not entirely > sure whether we can do this (_I_ can't), or ask the security team for a > newer upstream version in stable.
I looked a bit deeper into it: there was a lot more needed than just these two commits. Pretty much every commit of July 8 was relevant. I've created a new git repo where I imported the extracted Debian package, added the upstream repository as a new remote, and git cherry-picked every commit of that day except for the changelogs and the CentOS CI updates. The conflicts were all related to missing code that has been added in recent upstream versions and pretty easy to fix. The result is this 2500 line behemoth of `git log -p`. I tested it with the user `bob` enabled (the default test user of FreeRADIUS) and setting `require_message_authenticator = auto` and using a different machine to send requests to it. The `auto` settings looks to be working: if I start with an `Access-Request` with no `Message-Authenticator` attribute, the logging of FreeRADIUS shows this client is vulnerable and further checking will be disabled. After a restart of the server and sending an `Access-Request` with the `Message-Authenticator` attribute the client will have checking enabled, and sending a next request without the attribute will result in the package being dropped. Replies of the server now always include a `Message-Authenticator` attribute, which they did not have before (with the default config). A simple authentication with 802.1X (PEAP) looked like it's still working as well. I have not yet tested the proxy settings, it takes a while to set that up and I would first like to know if there is a chance that this patch set will be accepted, if it gets rejected right away for whatever reason I'd rather save myself the trouble. All the commits have been cherry-picked in order from the upstream changes, so a code review can compare these commits side by side. -- Herwin Weststrate
commit 9a4ffa1bc47c3ac99653b29327f873fedf52228f Author: Alan T. DeKok <al...@freeradius.org> Date: Fri Feb 16 08:09:54 2024 -0500 add and document global require_message_authenticator diff --git a/raddb/radiusd.conf.in b/raddb/radiusd.conf.in index 49003a41b9..8eafdac377 100644 --- a/raddb/radiusd.conf.in +++ b/raddb/radiusd.conf.in @@ -572,6 +572,17 @@ security { # status_server = yes + # + # Global configuration for requiring Message-Authenticator + # in all Access-* packets. + # + # This flag sets the global default for all clients and home + # servers. It can be over-ridden in individual client or + # home server by adding a flag to that section which says + # "require_message_authenticator = false". + # + require_message_authenticator = yes + @openssl_version_check_config@ } diff --git a/src/include/radiusd.h b/src/include/radiusd.h index c3e8cdd142..289f9623a3 100644 --- a/src/include/radiusd.h +++ b/src/include/radiusd.h @@ -174,6 +174,7 @@ typedef struct main_config { bool exiting; //!< are we exiting? + bool require_ma; //!< global configuration for all clients and home servers #ifdef ENABLE_OPENSSL_VERSION_CHECK char const *allow_vulnerable_openssl; //!< The CVE number of the last security issue acknowledged. diff --git a/src/main/mainconfig.c b/src/main/mainconfig.c index 960a312006..e7a8209541 100644 --- a/src/main/mainconfig.c +++ b/src/main/mainconfig.c @@ -160,6 +160,7 @@ static const CONF_PARSER security_config[] = { { "max_attributes", FR_CONF_POINTER(PW_TYPE_INTEGER, &fr_max_attributes), STRINGIFY(0) }, { "reject_delay", FR_CONF_POINTER(PW_TYPE_TIMEVAL, &main_config.reject_delay), STRINGIFY(0) }, { "status_server", FR_CONF_POINTER(PW_TYPE_BOOLEAN, &main_config.status_server), "no"}, + { "require_message_authenticator", FR_CONF_POINTER(PW_TYPE_BOOLEAN, &main_config.require_ma), "yes"}, #ifdef ENABLE_OPENSSL_VERSION_CHECK { "allow_vulnerable_openssl", FR_CONF_POINTER(PW_TYPE_STRING, &main_config.allow_vulnerable_openssl), "no"}, #endif commit 4f41abc224c6f107635063fab5c39e925d3a52b8 Author: Alan T. DeKok <al...@freeradius.org> Date: Fri Feb 16 08:16:12 2024 -0500 rename for consistency diff --git a/src/include/clients.h b/src/include/clients.h index fc6034a2f5..ed1fca0cd2 100644 --- a/src/include/clients.h +++ b/src/include/clients.h @@ -43,7 +43,7 @@ typedef struct radclient { char const *secret; //!< Secret PSK. - bool message_authenticator; //!< Require RADIUS message authenticator in requests. + bool require_ma; //!< Require RADIUS message authenticator in requests. char const *nas_type; //!< Type of client (arbitrary). diff --git a/src/main/client.c b/src/main/client.c index b0b3e24028..0db702cc92 100644 --- a/src/main/client.c +++ b/src/main/client.c @@ -327,7 +327,7 @@ check_list: (old->coa_home_server == client->coa_home_server) && (old->coa_home_pool == client->coa_home_pool) && #endif - (old->message_authenticator == client->message_authenticator)) { + (old->require_ma == client->require_ma)) { WARN("Ignoring duplicate client %s", client->longname); client_free(client); return true; @@ -511,7 +511,7 @@ static const CONF_PARSER client_config[] = { { "src_ipaddr", FR_CONF_POINTER(PW_TYPE_STRING, &cl_srcipaddr), NULL }, - { "require_message_authenticator", FR_CONF_OFFSET(PW_TYPE_BOOLEAN, RADCLIENT, message_authenticator), "no" }, + { "require_message_authenticator", FR_CONF_OFFSET(PW_TYPE_BOOLEAN, RADCLIENT, require_ma), "no" }, { "secret", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_SECRET, RADCLIENT, secret), NULL }, { "shortname", FR_CONF_OFFSET(PW_TYPE_STRING, RADCLIENT, shortname), NULL }, @@ -719,7 +719,7 @@ static const CONF_PARSER dynamic_config[] = { { "FreeRADIUS-Client-Src-IP-Address", FR_CONF_OFFSET(PW_TYPE_IPV4_ADDR, RADCLIENT, src_ipaddr), NULL }, { "FreeRADIUS-Client-Src-IPv6-Address", FR_CONF_OFFSET(PW_TYPE_IPV6_ADDR, RADCLIENT, src_ipaddr), NULL }, - { "FreeRADIUS-Client-Require-MA", FR_CONF_OFFSET(PW_TYPE_BOOLEAN, RADCLIENT, message_authenticator), NULL }, + { "FreeRADIUS-Client-Require-MA", FR_CONF_OFFSET(PW_TYPE_BOOLEAN, RADCLIENT, require_ma), NULL }, { "FreeRADIUS-Client-Secret", FR_CONF_OFFSET(PW_TYPE_STRING, RADCLIENT, secret), "" }, { "FreeRADIUS-Client-Shortname", FR_CONF_OFFSET(PW_TYPE_STRING, RADCLIENT, shortname), "" }, @@ -1214,7 +1214,7 @@ RADCLIENT *client_afrom_query(TALLOC_CTX *ctx, char const *identifier, char cons if (shortname) c->shortname = talloc_typed_strdup(c, shortname); if (type) c->nas_type = talloc_typed_strdup(c, type); if (server) c->server = talloc_typed_strdup(c, server); - c->message_authenticator = require_ma; + c->require_ma = require_ma; return c; } diff --git a/src/main/listen.c b/src/main/listen.c index b160d4f361..b6383cb484 100644 --- a/src/main/listen.c +++ b/src/main/listen.c @@ -1766,7 +1766,7 @@ static int auth_socket_recv(rad_listen_t *listener) * Now that we've sanity checked everything, receive the * packet. */ - packet = rad_recv(ctx, listener->fd, client->message_authenticator); + packet = rad_recv(ctx, listener->fd, client->require_ma); if (!packet) { FR_STATS_INC(auth, total_malformed_requests); if (DEBUG_ENABLED) ERROR("Receive - %s", fr_strerror()); @@ -2162,7 +2162,7 @@ static int coa_socket_recv(rad_listen_t *listener) * Now that we've sanity checked everything, receive the * packet. */ - packet = rad_recv(ctx, listener->fd, client->message_authenticator); + packet = rad_recv(ctx, listener->fd, client->require_ma); if (!packet) { FR_STATS_INC(coa, total_malformed_requests); if (DEBUG_ENABLED) ERROR("Receive - %s", fr_strerror()); commit 4ffee189fc27c6bca7b9c025687f1a0c35b1072d Author: Alan T. DeKok <al...@freeradius.org> Date: Fri Feb 16 08:23:09 2024 -0500 add and use "ignore default" flag which means that if the configuration item is missing, we do not set the value from the default. This change allows the value to be set before the configuration file is parsed, and then only changed if the named configuration item exists, and is manually set by the admin diff --git a/src/include/conffile.h b/src/include/conffile.h index b99688196b..237469c880 100644 --- a/src/include/conffile.h +++ b/src/include/conffile.h @@ -140,6 +140,7 @@ typedef struct timeval _timeval_t; #define PW_TYPE_MULTI (1 << 18) //!< CONF_PAIR can have multiple copies. #define PW_TYPE_NOT_EMPTY (1 << 19) //!< CONF_PAIR is required to have a non zero length value. #define PW_TYPE_FILE_EXISTS ((1 << 20) | PW_TYPE_STRING) //!< File matching value must exist +#define PW_TYPE_IGNORE_DEFAULT (1 << 21) //!< don't set from .dflt if the CONF_PAIR is missing /* @} **/ #define FR_INTEGER_COND_CHECK(_name, _var, _cond, _new)\ diff --git a/src/main/conffile.c b/src/main/conffile.c index 7fe658711f..f0ba148a18 100644 --- a/src/main/conffile.c +++ b/src/main/conffile.c @@ -1423,6 +1423,7 @@ int cf_item_parse(CONF_SECTION *cs, char const *name, unsigned int type, void *d { int rcode; bool deprecated, required, attribute, secret, file_input, cant_be_empty, tmpl, multi, file_exists; + bool ignore_dflt; char **q; char const *value; CONF_PAIR *cp = NULL; @@ -1446,6 +1447,7 @@ int cf_item_parse(CONF_SECTION *cs, char const *name, unsigned int type, void *d cant_be_empty = (type & PW_TYPE_NOT_EMPTY); tmpl = (type & PW_TYPE_TMPL); multi = (type & PW_TYPE_MULTI); + ignore_dflt = (type & PW_TYPE_IGNORE_DEFAULT); if (attribute) required = true; if (required) cant_be_empty = true; /* May want to review this in the future... */ @@ -1469,7 +1471,7 @@ int cf_item_parse(CONF_SECTION *cs, char const *name, unsigned int type, void *d * section, use the default value. */ if (!cp) { - if (deprecated) return 0; /* Don't set the default value */ + if (deprecated || ignore_dflt) return 0; /* Don't set the default value */ rcode = 1; value = dflt; commit c6600345d9aed526a14c2555de989d58dcee02e7 Author: Alan T. DeKok <al...@freeradius.org> Date: Fri Feb 16 08:29:54 2024 -0500 make require_message_authenticator the default for clients and document the behavior change diff --git a/raddb/clients.conf b/raddb/clients.conf index 60f9f4bf8a..e4cb532833 100644 --- a/raddb/clients.conf +++ b/raddb/clients.conf @@ -100,15 +100,17 @@ client localhost { secret = testing123 # - # Old-style clients do not send a Message-Authenticator - # in an Access-Request. RFC 5080 suggests that all clients - # SHOULD include it in an Access-Request. The configuration - # item below allows the server to require it. If a client - # is required to include a Message-Authenticator and it does - # not, then the packet will be silently discarded. + # The global configuration "security.require_message_authenticator" + # flag sets the default for all clients. That default can be + # over-ridden here, by setting it to "no". + # + # This flag exists solely for legacy clients which do not send + # Message-Authenticator in all Access-Request packets. We do not + # recommend setting it to "no". # # allowed values: yes, no - require_message_authenticator = no + # +# require_message_authenticator = no # # The short name is used as an alias for the fully qualified diff --git a/src/main/client.c b/src/main/client.c index 0db702cc92..f4cebe09b8 100644 --- a/src/main/client.c +++ b/src/main/client.c @@ -511,7 +511,7 @@ static const CONF_PARSER client_config[] = { { "src_ipaddr", FR_CONF_POINTER(PW_TYPE_STRING, &cl_srcipaddr), NULL }, - { "require_message_authenticator", FR_CONF_OFFSET(PW_TYPE_BOOLEAN, RADCLIENT, require_ma), "no" }, + { "require_message_authenticator", FR_CONF_OFFSET(PW_TYPE_BOOLEAN | PW_TYPE_IGNORE_DEFAULT, RADCLIENT, require_ma), NULL }, { "secret", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_SECRET, RADCLIENT, secret), NULL }, { "shortname", FR_CONF_OFFSET(PW_TYPE_STRING, RADCLIENT, shortname), NULL }, @@ -901,6 +901,13 @@ RADCLIENT *client_afrom_cs(TALLOC_CTX *ctx, CONF_SECTION *cs, bool in_server, bo c = talloc_zero(ctx, RADCLIENT); c->cs = cs; + /* + * Set the "require message authenticator" flag from the + * global default. If the configuration item exists, AND + * is set, it will over-ride this flag. + */ + c->require_ma = main_config.require_ma; + memset(&cl_ipaddr, 0, sizeof(cl_ipaddr)); cl_netmask = 255; commit 0542f92d68fd714467e1b78e9b3dff145082f9ec Author: Alan T. DeKok <al...@freeradius.org> Date: Fri Feb 16 08:33:54 2024 -0500 add tls flag to packets and set it for TLS transport send / receive. This lets the packet encoder and verification routines behave differently for TLS and non-TLS transport diff --git a/src/include/libradius.h b/src/include/libradius.h index e5f2251949..6a48642eb6 100644 --- a/src/include/libradius.h +++ b/src/include/libradius.h @@ -407,6 +407,7 @@ typedef struct radius_packet { size_t partial; int proto; #endif + bool tls; //!< uses secure transport } RADIUS_PACKET; typedef enum { diff --git a/src/main/tls_listen.c b/src/main/tls_listen.c index 5995fa390a..1d3f4c5d3c 100644 --- a/src/main/tls_listen.c +++ b/src/main/tls_listen.c @@ -598,6 +598,8 @@ read_application_data: packet->vps = NULL; PTHREAD_MUTEX_UNLOCK(&sock->mutex); + packet->tls = true; + if (!rad_packet_ok(packet, 0, NULL)) { if (DEBUG_ENABLED) ERROR("Receive - %s", fr_strerror()); DEBUG("(TLS) Closing TLS socket from client"); @@ -1152,6 +1154,8 @@ int proxy_tls_recv(rad_listen_t *listener) memcpy(packet->data, data, packet->data_len); memcpy(packet->vector, packet->data + 4, 16); + packet->tls = true; + /* * FIXME: Client MIB updates? */ @@ -1238,6 +1242,7 @@ int proxy_tls_send(rad_listen_t *listener, REQUEST *request) * if there's no packet, encode it here. */ if (!request->proxy->data) { + request->reply->tls = true; request->proxy_listener->proxy_encode(request->proxy_listener, request); } @@ -1290,6 +1295,8 @@ int proxy_tls_send_reply(rad_listen_t *listener, REQUEST *request) if ((listener->status != RAD_LISTEN_STATUS_INIT && (listener->status != RAD_LISTEN_STATUS_KNOWN))) return 0; + request->reply->tls = true; + /* * Pack the VPs */ commit 54f947a402b5a9de056c678b6d2a4fef43384ade Author: Alan T. DeKok <al...@freeradius.org> Date: Fri Feb 16 08:46:11 2024 -0500 always add Message-Authenticator for replies to Access-Request diff --git a/src/lib/radius.c b/src/lib/radius.c index 524e68088e..f9398165fd 100644 --- a/src/lib/radius.c +++ b/src/lib/radius.c @@ -1806,6 +1806,7 @@ int rad_encode(RADIUS_PACKET *packet, RADIUS_PACKET const *original, uint16_t total_length; int len; VALUE_PAIR const *reply; + bool seen_ma = false; /* * A 4K packet, aligned on 64-bits. @@ -1869,6 +1870,25 @@ int rad_encode(RADIUS_PACKET *packet, RADIUS_PACKET const *original, * memcpy. */ + /* + * Always add Message-Authenticator for replies to + * Access-Request packets. + * + * It must be the FIRST attribute in the packet. + */ + if (!packet->tls && original && (original->code == PW_CODE_ACCESS_REQUEST)) { + seen_ma = true; + + packet->offset = RADIUS_HDR_LEN; + + ptr[0] = PW_MESSAGE_AUTHENTICATOR; + ptr[1] = 18; + memset(ptr + 2, 0, 16); + + ptr += 18; + total_length += 18; + } + /* * Loop over the reply attributes for the packet. */ @@ -1926,6 +1946,14 @@ int rad_encode(RADIUS_PACKET *packet, RADIUS_PACKET const *original, * length and initial value. */ if (!reply->da->vendor && (reply->da->attr == PW_MESSAGE_AUTHENTICATOR)) { + /* + * We have already encoded the Message-Authenticator, don't do it again. + */ + if (seen_ma) { + reply = reply->next; + continue; + } + if (room < 18) break; /* commit 42bd8538a6d2e102d17e19ff1c553abee847e6a1 Author: Alan T. DeKok <al...@freeradius.org> Date: Fri Feb 16 09:12:35 2024 -0500 add and set require_message_authenticator for home servers diff --git a/raddb/proxy.conf b/raddb/proxy.conf index fe6d5789e4..e0be4c0497 100644 --- a/raddb/proxy.conf +++ b/raddb/proxy.conf @@ -250,6 +250,20 @@ home_server localhost { # secret = testing123 + # + # The global configuration "security.require_message_authenticator" + # flag sets the default for all home servers. That default can be + # over-ridden here, by setting it to "no". + # + # This flag exists solely for legacy home servers which do + # not send Message-Authenticator in all Access-Accept, + # Access-Reject, or Access-Challenge packets. We do not + # recommend setting it to "no". + # + # allowed values: yes, no + # +# require_message_authenticator = no + ############################################################ # # The rest of the configuration items listed here are optional, diff --git a/src/include/realms.h b/src/include/realms.h index b845840cd3..4f82ca4be5 100644 --- a/src/include/realms.h +++ b/src/include/realms.h @@ -66,6 +66,8 @@ typedef struct home_server { bool dual; //!< One of a pair of homeservers on consecutive ports. bool dynamic; //!< is this a dynamically added home server? + bool require_ma; //!< for all replies to Access-Request and Status-Server + #ifdef WITH_COA_TUNNEL bool recv_coa; //!< receive CoA packets, too #endif diff --git a/src/main/process.c b/src/main/process.c index 410c5713d0..79117cd998 100644 --- a/src/main/process.c +++ b/src/main/process.c @@ -2750,11 +2750,27 @@ int request_proxy_reply(RADIUS_PACKET *packet) * ignore it. This does the MD5 calculations in the * server core, but I guess we can fix that later. */ - if (!request->proxy_reply && - (rad_verify(packet, request->proxy, - request->home_server->secret) != 0)) { - DEBUG("Ignoring spoofed proxy reply. Signature is invalid"); - return 0; + if (!request->proxy_reply) { + decode_fail_t reason; + + /* + * If the home server configuration requires a Message-Authenticator, then set the flag, + * but only if the proxied packet is Access-Request or Status-Sercer. + * + * The realms.c file already clears require_ma for TLS connections. + */ + bool require_ma = request->home_server->require_ma && (request->proxy->code == PW_CODE_ACCESS_REQUEST); + + if (!rad_packet_ok(packet, require_ma, &reason)) { + DEBUG("Ignoring invalid packet - %s", fr_strerror()); + return 0; + } + + if (rad_verify(packet, request->proxy, + request->home_server->secret) != 0) { + DEBUG("Ignoring spoofed proxy reply. Signature is invalid"); + return 0; + } } /* diff --git a/src/main/realms.c b/src/main/realms.c index 497c8991d1..bf9a8d5ca2 100644 --- a/src/main/realms.c +++ b/src/main/realms.c @@ -473,6 +473,7 @@ static CONF_PARSER home_server_recv_coa[] = { #endif static CONF_PARSER home_server_config[] = { + { "require_message_authenticator", FR_CONF_OFFSET(PW_TYPE_BOOLEAN | PW_TYPE_IGNORE_DEFAULT, home_server_t, require_ma), NULL }, { "ipaddr", FR_CONF_OFFSET(PW_TYPE_COMBO_IP_ADDR, home_server_t, ipaddr), NULL }, { "ipv4addr", FR_CONF_OFFSET(PW_TYPE_IPV4_ADDR, home_server_t, ipaddr), NULL }, { "ipv6addr", FR_CONF_OFFSET(PW_TYPE_IPV6_ADDR, home_server_t, ipaddr), NULL }, @@ -763,6 +764,7 @@ home_server_t *home_server_afrom_cs(TALLOC_CTX *ctx, realm_config_t *rc, CONF_SE home->cs = cs; home->state = HOME_STATE_UNKNOWN; home->proto = IPPROTO_UDP; + home->require_ma = main_config.require_ma; /* * Parse the configuration into the home server @@ -1099,6 +1101,11 @@ home_server_t *home_server_afrom_cs(TALLOC_CTX *ctx, realm_config_t *rc, CONF_SE if (tls) { int rcode; + /* + * We don't require this for TLS connections. + */ + home->require_ma = false; + home->tls = tls_client_conf_parse(tls); if (!home->tls) { goto error; commit eb2822588028505ae8f64290c86c1d7d785f9198 Author: Alan T. DeKok <al...@freeradius.org> Date: Fri Feb 16 10:36:54 2024 -0500 add Message-Authenticator to all Access-Request packets diff --git a/src/lib/radius.c b/src/lib/radius.c index f9398165fd..20ea045c2b 100644 --- a/src/lib/radius.c +++ b/src/lib/radius.c @@ -1876,7 +1876,9 @@ int rad_encode(RADIUS_PACKET *packet, RADIUS_PACKET const *original, * * It must be the FIRST attribute in the packet. */ - if (!packet->tls && original && (original->code == PW_CODE_ACCESS_REQUEST)) { + if (!packet->tls && + ((original && (original->code == PW_CODE_ACCESS_REQUEST)) || + (packet->code == PW_CODE_ACCESS_REQUEST))) { seen_ma = true; packet->offset = RADIUS_HDR_LEN; commit fe8c71fc394c1d39ed5bb051d53ff6a5f2404056 Author: Alan T. DeKok <al...@freeradius.org> Date: Thu Feb 22 05:36:33 2024 -0500 add and document global limit_proxy_state diff --git a/raddb/radiusd.conf.in b/raddb/radiusd.conf.in index 8eafdac377..43d86bd047 100644 --- a/raddb/radiusd.conf.in +++ b/raddb/radiusd.conf.in @@ -581,8 +581,42 @@ security { # home server by adding a flag to that section which says # "require_message_authenticator = false". # + # If the server produces error message which says "Packet + # does not contain required Message-Authenticator attribute", + # then this configuration item has to be updated. + # + # WARNING: This item should always be left as "yes", + # otherwise it is possible for MITM attackers to create fake + # Access-Accept packets to the NAS! + # require_message_authenticator = yes + # + # Global configuration for requiring Message-Authenticator + # Access-Request packets from a NAS, but only if those + # packets also contain Proxy-State. + # + # If "require_message_authenticator" is set to "yes", this + # configuration item is ignored. + # + # If "require_message_authenticator" is set to "no", this + # configuration item is checked. + # + # This configuration item should ALWAYS be set to "yes". + # + # The only reason to set it to "no" is when the client is a + # proxy, AND the proxy does not send Message-Authenticator in + # Access-Request packets. Even then, the best approach to + # fix the issue is to (1) update the client to send + # Message-Authenticator, and if that can't be done, then (2) + # set this flag to "no", but ONLY on a per-client basis. + # + # WARNING: This item should always be left as "yes", + # otherwise it is possible for MITM attackers to create fake + # Access-Accept packets to the NAS! + # + limit_proxy_state = yes + @openssl_version_check_config@ } diff --git a/src/include/radiusd.h b/src/include/radiusd.h index 289f9623a3..1bcde9b31f 100644 --- a/src/include/radiusd.h +++ b/src/include/radiusd.h @@ -176,6 +176,8 @@ typedef struct main_config { bool require_ma; //!< global configuration for all clients and home servers + bool limit_proxy_state; //!< global configuration for all clients + #ifdef ENABLE_OPENSSL_VERSION_CHECK char const *allow_vulnerable_openssl; //!< The CVE number of the last security issue acknowledged. #endif diff --git a/src/main/mainconfig.c b/src/main/mainconfig.c index e7a8209541..b93e64f1f4 100644 --- a/src/main/mainconfig.c +++ b/src/main/mainconfig.c @@ -161,6 +161,7 @@ static const CONF_PARSER security_config[] = { { "reject_delay", FR_CONF_POINTER(PW_TYPE_TIMEVAL, &main_config.reject_delay), STRINGIFY(0) }, { "status_server", FR_CONF_POINTER(PW_TYPE_BOOLEAN, &main_config.status_server), "no"}, { "require_message_authenticator", FR_CONF_POINTER(PW_TYPE_BOOLEAN, &main_config.require_ma), "yes"}, + { "limit_proxy_state", FR_CONF_POINTER(PW_TYPE_BOOLEAN, &main_config.limit_proxy_state), "yes"}, #ifdef ENABLE_OPENSSL_VERSION_CHECK { "allow_vulnerable_openssl", FR_CONF_POINTER(PW_TYPE_STRING, &main_config.allow_vulnerable_openssl), "no"}, #endif commit ec8fced91c348f2854740488832bb3d8b3c6f28a Author: Alan T. DeKok <al...@freeradius.org> Date: Thu Feb 22 05:44:07 2024 -0500 make limit_proxy_state the default for clients diff --git a/raddb/clients.conf b/raddb/clients.conf index e4cb532833..6a354dc8b7 100644 --- a/raddb/clients.conf +++ b/raddb/clients.conf @@ -112,6 +112,19 @@ client localhost { # # require_message_authenticator = no + # + # The global configuration "security.limit_proxy_state" + # flag sets the default for all clients. That default can be + # over-ridden here, by setting it to "no". + # + # This flag exists solely for legacy clients which do not send + # Message-Authenticator in all Access-Request packets. We do not + # recommend setting it to "no". + # + # allowed values: yes, no + # +# limit_proxy_state = yes + # # The short name is used as an alias for the fully qualified # domain name, or the IP address. diff --git a/src/include/clients.h b/src/include/clients.h index ed1fca0cd2..e7601f3aa5 100644 --- a/src/include/clients.h +++ b/src/include/clients.h @@ -45,6 +45,8 @@ typedef struct radclient { bool require_ma; //!< Require RADIUS message authenticator in requests. + bool limit_proxy_state; //!< Limit Proxy-State in requests + char const *nas_type; //!< Type of client (arbitrary). char const *login; //!< Username to use for simultaneous use checks. diff --git a/src/main/client.c b/src/main/client.c index f4cebe09b8..b439f9ef3c 100644 --- a/src/main/client.c +++ b/src/main/client.c @@ -327,7 +327,8 @@ check_list: (old->coa_home_server == client->coa_home_server) && (old->coa_home_pool == client->coa_home_pool) && #endif - (old->require_ma == client->require_ma)) { + (old->require_ma == client->require_ma) && + (old->limit_proxy_state == client->limit_proxy_state)) { WARN("Ignoring duplicate client %s", client->longname); client_free(client); return true; @@ -512,6 +513,7 @@ static const CONF_PARSER client_config[] = { { "src_ipaddr", FR_CONF_POINTER(PW_TYPE_STRING, &cl_srcipaddr), NULL }, { "require_message_authenticator", FR_CONF_OFFSET(PW_TYPE_BOOLEAN | PW_TYPE_IGNORE_DEFAULT, RADCLIENT, require_ma), NULL }, + { "limit_proxy_state", FR_CONF_OFFSET(PW_TYPE_BOOLEAN | PW_TYPE_IGNORE_DEFAULT, RADCLIENT, limit_proxy_state), NULL }, { "secret", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_SECRET, RADCLIENT, secret), NULL }, { "shortname", FR_CONF_OFFSET(PW_TYPE_STRING, RADCLIENT, shortname), NULL }, @@ -902,11 +904,13 @@ RADCLIENT *client_afrom_cs(TALLOC_CTX *ctx, CONF_SECTION *cs, bool in_server, bo c->cs = cs; /* - * Set the "require message authenticator" flag from the - * global default. If the configuration item exists, AND - * is set, it will over-ride this flag. + * Set the "require message authenticator" and "limit + * proxy state" flags from the global default. If the + * configuration item exists, AND is set, it will + * over-ride the flag. */ c->require_ma = main_config.require_ma; + c->limit_proxy_state = main_config.limit_proxy_state; memset(&cl_ipaddr, 0, sizeof(cl_ipaddr)); cl_netmask = 255; commit 7ac4580ef8ba2fc76d4d8cf0a62eb11e503f1297 Author: Alan T. DeKok <al...@freeradius.org> Date: Thu Feb 22 06:01:22 2024 -0500 use and enforce limit_proxy_state for Access-Request packets diff --git a/src/include/libradius.h b/src/include/libradius.h index 6a48642eb6..fa79aa1740 100644 --- a/src/include/libradius.h +++ b/src/include/libradius.h @@ -515,6 +515,12 @@ DICT_VENDOR *dict_vendorbyvalue(int vendor); /* radius.c */ int rad_send(RADIUS_PACKET *, RADIUS_PACKET const *, char const *secret); bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason); + +/* + * 1 == require_ma + * 2 == msg_peek + * 3 == limit_proxy_state + */ RADIUS_PACKET *rad_recv(TALLOC_CTX *ctx, int fd, int flags); ssize_t rad_recv_header(int sockfd, fr_ipaddr_t *src_ipaddr, uint16_t *src_port, int *code); void rad_recv_discard(int sockfd); diff --git a/src/lib/radius.c b/src/lib/radius.c index 20ea045c2b..3c69d57fc7 100644 --- a/src/lib/radius.c +++ b/src/lib/radius.c @@ -2450,6 +2450,8 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason) radius_packet_t *hdr; char host_ipaddr[128]; bool require_ma = false; + bool limit_proxy_state = false; + bool seen_proxy_state = false; bool seen_ma = false; uint32_t num_attributes; decode_fail_t failure = DECODE_FAIL_NONE; @@ -2500,13 +2502,14 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason) /* * Message-Authenticator is required in Status-Server * packets, otherwise they can be trivially forged. - */ - if (hdr->code == PW_CODE_STATUS_SERVER) require_ma = true; - - /* + * * It's also required if the caller asks for it. + * + * We only limit Proxy-State if we're not requiring + * Message-Authenticator. */ - if (flags) require_ma = true; + require_ma = ((flags & 0x01) != 0) || (hdr->code == PW_CODE_STATUS_SERVER); + limit_proxy_state = ((flags & 0x04) != 0) & !require_ma; /* * Repeat the length checks. This time, instead of @@ -2669,6 +2672,10 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason) non_eap = true; break; + case PW_PROXY_STATE: + seen_proxy_state = true; + break; + case PW_MESSAGE_AUTHENTICATOR: if (attr[1] != 2 + AUTH_VECTOR_LEN) { FR_DEBUG_STRERROR_PRINTF("Malformed RADIUS packet from host %s: Message-Authenticator has invalid length %d", @@ -2744,6 +2751,18 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason) goto finish; } + /* + * The client is a NAS which shouldn't send Proxy-State, but it did! + */ + if (limit_proxy_state && seen_proxy_state && !seen_ma) { + FR_DEBUG_STRERROR_PRINTF("Insecure packet from host %s: Packet does not contain required Message-Authenticator attribute, but still has one or more Proxy-State attributes", + inet_ntop(packet->src_ipaddr.af, + &packet->src_ipaddr.ipaddr, + host_ipaddr, sizeof(host_ipaddr))); + failure = DECODE_FAIL_MA_MISSING; + goto finish; + } + if (eap && non_eap) { FR_DEBUG_STRERROR_PRINTF("Bad packet from host %s: Packet contains EAP-Message and non-EAP authentication attribute", inet_ntop(packet->src_ipaddr.af, diff --git a/src/main/listen.c b/src/main/listen.c index b6383cb484..d5ce4cb310 100644 --- a/src/main/listen.c +++ b/src/main/listen.c @@ -1766,7 +1766,7 @@ static int auth_socket_recv(rad_listen_t *listener) * Now that we've sanity checked everything, receive the * packet. */ - packet = rad_recv(ctx, listener->fd, client->require_ma); + packet = rad_recv(ctx, listener->fd, client->require_ma | (((int) client->limit_proxy_state) << 2)); if (!packet) { FR_STATS_INC(auth, total_malformed_requests); if (DEBUG_ENABLED) ERROR("Receive - %s", fr_strerror()); commit c344480243c27d7591d7a7ee128fc153565cdd86 Author: Alan T. DeKok <al...@freeradius.org> Date: Wed Apr 10 17:33:38 2024 -0400 word smithing diff --git a/raddb/radiusd.conf.in b/raddb/radiusd.conf.in index 43d86bd047..8ca67f1eec 100644 --- a/raddb/radiusd.conf.in +++ b/raddb/radiusd.conf.in @@ -574,12 +574,14 @@ security { # # Global configuration for requiring Message-Authenticator - # in all Access-* packets. + # in all Access-* packets sent over UDP or TCP. This flag + # is ignored for TLS. # # This flag sets the global default for all clients and home - # servers. It can be over-ridden in individual client or - # home server by adding a flag to that section which says - # "require_message_authenticator = false". + # servers. It can be over-ridden in an individual client or + # home server definition by adding a flag to that section: + # + # require_message_authenticator = no # # If the server produces error message which says "Packet # does not contain required Message-Authenticator attribute", @@ -594,7 +596,15 @@ security { # # Global configuration for requiring Message-Authenticator # Access-Request packets from a NAS, but only if those - # packets also contain Proxy-State. + # packets also contain Proxy-State. This flag only applies + # to packets sent over UDP or TCP. This flag is ignored for + # TLS. + # + # This flag sets the global default for all clients. It can + # be over-ridden in an individual client definition by adding + # a flag to that section: + # + # limit_proxy_state = no # # If "require_message_authenticator" is set to "yes", this # configuration item is ignored. @@ -607,13 +617,15 @@ security { # The only reason to set it to "no" is when the client is a # proxy, AND the proxy does not send Message-Authenticator in # Access-Request packets. Even then, the best approach to - # fix the issue is to (1) update the client to send + # fix the issue is to (1) update the proxy to send # Message-Authenticator, and if that can't be done, then (2) # set this flag to "no", but ONLY on a per-client basis. # - # WARNING: This item should always be left as "yes", - # otherwise it is possible for MITM attackers to create fake - # Access-Accept packets to the NAS! + # WARNING: Setting both this flag and the + # "require_message_authenticator" flag to "no" will allow + # MITM attackers to create fake Access-Accept packets to the + # NAS! At least one of them MUST be set to "yes" for the + # system to have any protection against the attack. # limit_proxy_state = yes commit 016994ff2f5d53029744c1aeb0e6a5bf791e3b39 Author: Alan T. DeKok <al...@freeradius.org> Date: Wed Apr 10 17:34:31 2024 -0400 add Blast RADIUS checks to radclient diff --git a/man/man1/radclient.1 b/man/man1/radclient.1 index 229dcae0c7..b83bee931a 100644 --- a/man/man1/radclient.1 +++ b/man/man1/radclient.1 @@ -1,10 +1,11 @@ -.TH RADCLIENT 1 "22 March 2019" "" "FreeRADIUS Daemon" +.TH RADCLIENT 1 "21 May 2024" "" "FreeRADIUS Daemon" .SH NAME radclient - send packets to a RADIUS server, show reply .SH SYNOPSIS .B radclient .RB [ \-4 ] .RB [ \-6 ] +.RB [ \-b ] .RB [ \-c .IR count ] .RB [ \-d @@ -52,6 +53,13 @@ automatically encrypted before the packet is sent to the server. Use IPv4 (default) .IP \-6 Use IPv6 +.IP \-b +Enforce the Blast RADIUS checks. All replies to an Access-Request packet +must contain a Message-Authenticator as the first attribute. + +For compatibility with old servers, this flag is not set by default. +However, radclient still checks for the Blast RADIUS signature, and +discards packets which match the attack. .IP \-c\ \fIcount\fP Send each packet \fIcount\fP times. .IP \-d\ \fIraddb_directory\fP diff --git a/src/main/radclient.c b/src/main/radclient.c index 09d27c8711..fe3a6130c7 100644 --- a/src/main/radclient.c +++ b/src/main/radclient.c @@ -60,6 +60,7 @@ static fr_ipaddr_t server_ipaddr; static int resend_count = 1; static bool done = true; static bool print_filename = false; +static bool blast_radius = false; static fr_ipaddr_t client_ipaddr; static uint16_t client_port = 0; @@ -95,6 +96,7 @@ static void NEVER_RETURNS usage(void) fprintf(stderr, " <command> One of auth, acct, status, coa, disconnect or auto.\n"); fprintf(stderr, " -4 Use IPv4 address of server\n"); fprintf(stderr, " -6 Use IPv6 address of server.\n"); + fprintf(stderr, " -b Mandate checks for Blast RADIUS issue (this is not set by default).\n"); fprintf(stderr, " -c <count> Send each packet 'count' times.\n"); fprintf(stderr, " -d <raddb> Set user dictionary directory (defaults to " RADDBDIR ").\n"); fprintf(stderr, " -D <dictdir> Set main dictionary directory (defaults to " DICTDIR ").\n"); @@ -1056,6 +1058,130 @@ static int send_one_packet(rc_request_t *request) return 0; } +/* + * Do Blast RADIUS checks. + * + * The request is an Access-Request, and does NOT contain Proxy-State. + * + * The reply is a raw packet, and is NOT yet decoded. + */ +static int blast_radius_check(rc_request_t *request, RADIUS_PACKET *reply) +{ + uint8_t *attr, *end; + VALUE_PAIR *vp; + bool have_message_authenticator = false; + + /* + * We've received a raw packet. Nothing has (as of yet) checked + * anything in it other than the length, and that it's a + * well-formed RADIUS packet. + */ + switch (reply->data[0]) { + case PW_CODE_ACCESS_ACCEPT: + case PW_CODE_ACCESS_REJECT: + case PW_CODE_ACCESS_CHALLENGE: + if (reply->data[1] != request->packet->id) { + ERROR("Invalid reply ID %d to Access-Request ID %d", reply->data[1], request->packet->id); + return -1; + } + break; + + default: + ERROR("Invalid reply code %d to Access-Request", reply->data[0]); + return -1; + } + + /* + * If the reply has a Message-Authenticator, then it MIGHT be fine. + */ + attr = reply->data + 20; + end = reply->data + reply->data_len; + + /* + * It should be the first attribute, so we warn if it isn't there. + * + * But it's not a fatal error. + */ + if (blast_radius && (attr[0] != PW_MESSAGE_AUTHENTICATOR)) { + RDEBUG("WARNING The %s reply packet does not have Message-Authenticator as the first attribute. The packet may be vulnerable to Blast RADIUS attacks.", + fr_packet_codes[reply->data[0]]); + } + + /* + * Set up for Proxy-State checks. + * + * If we see a Proxy-State in the reply which we didn't send, then it's a Blast RADIUS attack. + */ + vp = fr_pair_find_by_num(request->packet->vps, PW_PROXY_STATE, 0, TAG_ANY); + + while (attr < end) { + /* + * Blast RADIUS work-arounds require that + * Message-Authenticator is the first attribute in the + * reply. Note that we don't check for it being the + * first attribute, but simply that it exists. + * + * That check is a balance between securing the reply + * packet from attacks, and not violating the RFCs which + * say that there is no order to attributes in the + * packet. + * + * However, no matter the status of the '-b' flag we + * still can check for the signature of the attack, and + * discard packets which are suspicious. This behavior + * protects radclient from the attack, without mandating + * new behavior on the server side. + * + * Note that we don't set the '-b' flag by default. + * radclient is intended for testing / debugging, and is + * not intended to be used as part of a secure login / + * user checking system. + */ + if (attr[0] == PW_MESSAGE_AUTHENTICATOR) { + have_message_authenticator = true; + goto next; + } + + /* + * If there are Proxy-State attributes in the reply, they must + * match EXACTLY the Proxy-State attributes in the request. + * + * Note that we don't care if there are more Proxy-States + * in the request than in the reply. The Blast RADIUS + * issue requires _adding_ Proxy-State attributes, and + * cannot work when the server _deletes_ Proxy-State + * attributes. + */ + if (attr[0] == PW_PROXY_STATE) { + if (!vp || (vp->length != (size_t) (attr[1] - 2)) || (memcmp(vp->vp_octets, attr + 2, vp->length) != 0)) { + ERROR("Invalid reply to Access-Request ID %d - Discarding packet due to Blast RADIUS attack being detected.", request->packet->id); + ERROR("We received a Proxy-State in the reply which we did not send, or which is different from what we sent."); + return -1; + } + + vp = fr_pair_find_by_num(vp->next, PW_PROXY_STATE, 0, TAG_ANY); + } + + next: + attr += attr[1]; + } + + /* + * If "-b" is set, then we require Message-Authenticator in the reply. + */ + if (blast_radius && !have_message_authenticator) { + ERROR("The %s reply packet does not contain Message-Authenticator - discarding packet due to Blast RADIUS checks.", + fr_packet_codes[reply->data[0]]); + return -1; + } + + /* + * The packet doesn't look like it's a Blast RADIUS attack. The + * caller will now verify the packet signature. + */ + return 0; +} + /* * Receive one packet, maybe. */ @@ -1107,6 +1233,20 @@ static int recv_one_packet(int wait_time) } request = fr_packet2myptr(rc_request_t, packet, packet_p); + /* + * We want radclient to be able to send any packet, including + * imperfect ones. However, we do NOT want to be vulnerable to + * the "Blast RADIUS" issue. Instead of adding command-line + * flags to enable/disable similar flags to what the server + * sends, we just do a few more smart checks to double-check + * things. + */ + if ((request->packet->code == PW_CODE_ACCESS_REQUEST) && + blast_radius_check(request, reply) < 0) { + rad_free(&reply); + return -1; + } + /* * Fails the signature validation: not a real reply. * FIXME: Silently drop it and listen for another packet. @@ -1239,7 +1379,7 @@ int main(int argc, char **argv) exit(1); } - while ((c = getopt(argc, argv, "46c:d:D:f:Fhn:p:qr:sS:t:vx" + while ((c = getopt(argc, argv, "46bc:d:D:f:Fhn:p:qr:sS:t:vx" #ifdef WITH_TCP "P:" #endif @@ -1252,6 +1392,10 @@ int main(int argc, char **argv) force_af = AF_INET6; break; + case 'b': + blast_radius = true; + break; + case 'c': if (!isdigit((int) *optarg)) usage(); commit 2cd9b70a3feb56b3f05f88284930c82d481ef8d0 Author: Alan T. DeKok <al...@freeradius.org> Date: Sat May 11 15:41:03 2024 -0400 Add M-A processing for Status-Server and replies from home server diff --git a/src/include/libradius.h b/src/include/libradius.h index fa79aa1740..acf79802cf 100644 --- a/src/include/libradius.h +++ b/src/include/libradius.h @@ -519,7 +519,8 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason); /* * 1 == require_ma * 2 == msg_peek - * 3 == limit_proxy_state + * 4 == limit_proxy_state + * 8 == require_ma for Access-* replies and Protocol-Error */ RADIUS_PACKET *rad_recv(TALLOC_CTX *ctx, int fd, int flags); ssize_t rad_recv_header(int sockfd, fr_ipaddr_t *src_ipaddr, uint16_t *src_port, int *code); @@ -711,7 +712,7 @@ extern bool fr_dns_lookups; /* do IP -> hostname lookups? */ extern bool fr_hostname_lookups; /* do hostname -> IP lookups? */ extern int fr_debug_lvl; /* 0 = no debugging information */ extern uint32_t fr_max_attributes; /* per incoming packet */ -#define FR_MAX_PACKET_CODE (52) +#define FR_MAX_PACKET_CODE (53) extern char const *fr_packet_codes[FR_MAX_PACKET_CODE]; #define is_radius_code(_x) ((_x > 0) && (_x < FR_MAX_PACKET_CODE)) extern FILE *fr_log_fp; diff --git a/src/include/radius.h b/src/include/radius.h index 473528d65d..147d674eed 100644 --- a/src/include/radius.h +++ b/src/include/radius.h @@ -61,6 +61,7 @@ typedef enum { PW_CODE_COA_REQUEST = 43, //!< RFC3575/RFC5176 - CoA-Request PW_CODE_COA_ACK = 44, //!< RFC3575/RFC5176 - CoA-Ack (positive) PW_CODE_COA_NAK = 45, //!< RFC3575/RFC5176 - CoA-Nak (not willing to perform) + PW_CODE_PROTOCOL_ERROR = 52, //!< RFC7930 - Protocol layer issue PW_CODE_MAX = 255, //!< Maximum possible code } PW_CODE; diff --git a/src/lib/radius.c b/src/lib/radius.c index 3c69d57fc7..5bb08cc911 100644 --- a/src/lib/radius.c +++ b/src/lib/radius.c @@ -143,8 +143,9 @@ char const *fr_packet_codes[FR_MAX_PACKET_CODE] = { "47", "48", "49", - "IP-Address-Allocate", - "IP-Address-Release", //!< 50 + "IP-Address-Allocate", //!< 50 + "IP-Address-Release", + "Protocol-Error", }; @@ -1794,6 +1795,14 @@ int rad_vp2attr(RADIUS_PACKET const *packet, RADIUS_PACKET const *original, return rad_vp2vsa(packet, original, secret, pvp, start, room); } +static const bool code2ma[FR_MAX_PACKET_CODE] = { + [ PW_CODE_ACCESS_REQUEST ] = true, + [ PW_CODE_ACCESS_ACCEPT ] = true, + [ PW_CODE_ACCESS_REJECT ] = true, + [ PW_CODE_ACCESS_CHALLENGE ] = true, + [ PW_CODE_STATUS_SERVER ] = true, + [ PW_CODE_PROTOCOL_ERROR ] = true, +}; /** Encode a packet * @@ -1872,13 +1881,13 @@ int rad_encode(RADIUS_PACKET *packet, RADIUS_PACKET const *original, /* * Always add Message-Authenticator for replies to - * Access-Request packets. + * Access-Request packets, and for all Access-Accept, + * Access-Reject, Access-Challenge. * * It must be the FIRST attribute in the packet. */ - if (!packet->tls && - ((original && (original->code == PW_CODE_ACCESS_REQUEST)) || - (packet->code == PW_CODE_ACCESS_REQUEST))) { + if (!packet->tls && + ((code2ma[packet->code]) || (original && code2ma[original->code]))) { seen_ma = true; packet->offset = RADIUS_HDR_LEN; @@ -2500,16 +2509,23 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason) } /* - * Message-Authenticator is required in Status-Server - * packets, otherwise they can be trivially forged. + * If the caller requires Message-Authenticator, then set + * the flag. * - * It's also required if the caller asks for it. + * We also require Message-Authenticator if the packet + * code is Status-Server. * + * If we're receiving packets from a proxy socket, then + * require Message-Authenticator for Access-* replies, + * and for Protocol-Error. + */ + require_ma = ((flags & 0x01) != 0) || (hdr->code == PW_CODE_STATUS_SERVER) || (((flags & 0x08) != 0) && code2ma[hdr->code]); + + /* * We only limit Proxy-State if we're not requiring * Message-Authenticator. */ - require_ma = ((flags & 0x01) != 0) || (hdr->code == PW_CODE_STATUS_SERVER); - limit_proxy_state = ((flags & 0x04) != 0) & !require_ma; + limit_proxy_state = ((flags & 0x04) != 0) && !require_ma; /* * Repeat the length checks. This time, instead of diff --git a/src/main/listen.c b/src/main/listen.c index d5ce4cb310..7b49b7b384 100644 --- a/src/main/listen.c +++ b/src/main/listen.c @@ -2193,7 +2193,7 @@ static int proxy_socket_recv(rad_listen_t *listener) #endif char buffer[128]; - packet = rad_recv(NULL, listener->fd, 0); + packet = rad_recv(NULL, listener->fd, 8); /* SOME packets don't require a Message-Authenticator attribute */ if (!packet) { if (DEBUG_ENABLED) ERROR("Receive - %s", fr_strerror()); return 0; commit 2f6a2918df572cda3d8c09a73303f12c8a2f5465 Author: Alan T. DeKok <al...@freeradius.org> Date: Sat Jun 29 10:27:47 2024 -0400 Enforce BlastRADIUS checks for TCP sockets, too. Though TBH, no one should use TCP for anything. diff --git a/src/main/listen.c b/src/main/listen.c index 7b49b7b384..75df668c0d 100644 --- a/src/main/listen.c +++ b/src/main/listen.c @@ -584,6 +584,16 @@ static int dual_tcp_recv(rad_listen_t *listener) switch (packet->code) { case PW_CODE_ACCESS_REQUEST: if (listener->type != RAD_LISTEN_AUTH) goto bad_packet; + + /* + * Enforce BlastRADIUS checks on TCP, too. + */ + if (!rad_packet_ok(packet, client->require_ma | (((int) client->limit_proxy_state) << 2), NULL)) { + FR_STATS_INC(auth, total_malformed_requests); + rad_free(&sock->packet); + return 0; + } + FR_STATS_INC(auth, total_requests); fun = rad_authenticate; break; commit 9c46dfc9d3524a19180556fe15554496cd3181e7 Author: Alan T. DeKok <al...@freeradius.org> Date: Sat Jun 29 12:05:04 2024 -0400 implement and document "limit_proxy_state = auto" Also add a standard function which complains loudly about security issues. diff --git a/raddb/radiusd.conf.in b/raddb/radiusd.conf.in index 8ca67f1eec..d34a70959b 100644 --- a/raddb/radiusd.conf.in +++ b/raddb/radiusd.conf.in @@ -594,17 +594,14 @@ security { require_message_authenticator = yes # - # Global configuration for requiring Message-Authenticator - # Access-Request packets from a NAS, but only if those - # packets also contain Proxy-State. This flag only applies - # to packets sent over UDP or TCP. This flag is ignored for - # TLS. + # Global configuration for limiting the combination of + # Proxy-State and Message-Authenticator. This flag only + # applies to packets sent over UDP or TCP. This flag is + # ignored for TLS. # # This flag sets the global default for all clients. It can # be over-ridden in an individual client definition by adding - # a flag to that section: - # - # limit_proxy_state = no + # the same flag to that section with an appropriate value: # # If "require_message_authenticator" is set to "yes", this # configuration item is ignored. @@ -612,22 +609,62 @@ security { # If "require_message_authenticator" is set to "no", this # configuration item is checked. # - # This configuration item should ALWAYS be set to "yes". - # - # The only reason to set it to "no" is when the client is a - # proxy, AND the proxy does not send Message-Authenticator in - # Access-Request packets. Even then, the best approach to - # fix the issue is to (1) update the proxy to send - # Message-Authenticator, and if that can't be done, then (2) - # set this flag to "no", but ONLY on a per-client basis. - # - # WARNING: Setting both this flag and the - # "require_message_authenticator" flag to "no" will allow - # MITM attackers to create fake Access-Accept packets to the - # NAS! At least one of them MUST be set to "yes" for the - # system to have any protection against the attack. - # - limit_proxy_state = yes + # The possible values and meanings for "limit_proxy_state" are; + # + # * "no" - allow any packets from the client, even packets + # which contain the BlastRADIUS attack. Please be aware + # that in this configuration the server will complain for + # EVERY packet which it receives. + # + # The only reason to set this flag to "no" is when the + # client is a proxy, AND the proxy does not send + # Message-Authenticator in Access-Request packets. Even + # then, the best approach to fix the issue is to (1) update + # the proxy to send Message-Authenticator, and if that + # can't be done, then (2) set this flag to "no", but ONLY + # for that one client. The global flag SHOULD still be set + # to a safe value: "yes". + # + # WARNING: Setting both this flag and the + # "require_message_authenticator" flag to "no" will allow + # MITM attackers to create fake Access-Accept packets to the + # NAS! At least one of them MUST be set to "yes" for the + # system to have any protection against the attack. + # + # * "yes" - Allow packets without Message-Authenticator, + # but only when they do not contain Proxy-State. + # packets which contain Proxy-State MUST also contain + # Message-Authenticator, otherwise they are discarded. + # + # This setting is safe for all NASes, GGSNs, BRAS, etc. + # No known RADIUS client sends Proxy-State for normal + # Access-Request packets. + # + # * "auto" - Automatically determine the value of the flag, + # based on the first packet received from that client. + # + # If the packet contains Proxy-State but no + # Message-Authenticator, then the value of the flag is + # automatically switched to "no". + # + # For all other situations, the value of the flag is + # automatically switched to "yes". + # + # WARNING: This switch is done for the first packet + # received from that client. The change does NOT persist + # across server restarts. You MUST change the to "yes" + # manually, in order to make a permanent change to the + # configuration. + # + # WARNING: If there are multiple NASes with the same source + # IP and client definitions, BUT the NASes have different + # behavior, then this flag WILL LIKELY BREAK YOUR NETWORK. + # + # The only solution to that rare configuration is to set + # this flag to "no", in which case the network will work, + # but will be vulnerable to the attack. + # + limit_proxy_state = auto @openssl_version_check_config@ } diff --git a/src/include/clients.h b/src/include/clients.h index e7601f3aa5..a9b66429cc 100644 --- a/src/include/clients.h +++ b/src/include/clients.h @@ -26,6 +26,13 @@ * @copyright 2015 The FreeRADIUS server project */ +typedef enum { + FR_BOOL_FALSE = 0, + FR_BOOL_TRUE, + FR_BOOL_AUTO, +} fr_bool_auto_t; + + typedef struct radclient_list RADCLIENT_LIST; @@ -45,7 +52,7 @@ typedef struct radclient { bool require_ma; //!< Require RADIUS message authenticator in requests. - bool limit_proxy_state; //!< Limit Proxy-State in requests + fr_bool_auto_t limit_proxy_state; //!< Limit Proxy-State in requests char const *nas_type; //!< Type of client (arbitrary). diff --git a/src/include/libradius.h b/src/include/libradius.h index acf79802cf..7479d0f02e 100644 --- a/src/include/libradius.h +++ b/src/include/libradius.h @@ -408,6 +408,10 @@ typedef struct radius_packet { int proto; #endif bool tls; //!< uses secure transport + + bool message_authenticator; + bool proxy_state; + bool eap_message; } RADIUS_PACKET; typedef enum { diff --git a/src/include/radiusd.h b/src/include/radiusd.h index 1bcde9b31f..cf08627dfb 100644 --- a/src/include/radiusd.h +++ b/src/include/radiusd.h @@ -176,7 +176,7 @@ typedef struct main_config { bool require_ma; //!< global configuration for all clients and home servers - bool limit_proxy_state; //!< global configuration for all clients + fr_bool_auto_t limit_proxy_state; //!< global configuration for all clients #ifdef ENABLE_OPENSSL_VERSION_CHECK char const *allow_vulnerable_openssl; //!< The CVE number of the last security issue acknowledged. @@ -567,6 +567,8 @@ int main_config_free(void); void main_config_hup(void); void hup_logfile(void); +int fr_bool_auto_parse(CONF_PAIR *cp, fr_bool_auto_t *out, char const *str); + /* listen.c */ void listen_free(rad_listen_t **head); int listen_init(CONF_SECTION *cs, rad_listen_t **head, bool spawn_flag); diff --git a/src/lib/radius.c b/src/lib/radius.c index 5bb08cc911..1e92d8b66e 100644 --- a/src/lib/radius.c +++ b/src/lib/radius.c @@ -2680,6 +2680,7 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason) case PW_EAP_MESSAGE: require_ma = true; eap = true; + packet->eap_message = true; break; case PW_USER_PASSWORD: @@ -2690,6 +2691,7 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason) case PW_PROXY_STATE: seen_proxy_state = true; + packet->proxy_state = true; break; case PW_MESSAGE_AUTHENTICATOR: @@ -2703,6 +2705,7 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason) goto finish; } seen_ma = true; + packet->message_authenticator = true; break; } diff --git a/src/main/client.c b/src/main/client.c index b439f9ef3c..310d64db02 100644 --- a/src/main/client.c +++ b/src/main/client.c @@ -490,6 +490,7 @@ static fr_ipaddr_t cl_ipaddr; static uint32_t cl_netmask; static char const *cl_srcipaddr = NULL; static char const *hs_proto = NULL; +static char const *limit_proxy_state = NULL; #ifdef WITH_TCP static CONF_PARSER limit_config[] = { @@ -513,7 +514,7 @@ static const CONF_PARSER client_config[] = { { "src_ipaddr", FR_CONF_POINTER(PW_TYPE_STRING, &cl_srcipaddr), NULL }, { "require_message_authenticator", FR_CONF_OFFSET(PW_TYPE_BOOLEAN | PW_TYPE_IGNORE_DEFAULT, RADCLIENT, require_ma), NULL }, - { "limit_proxy_state", FR_CONF_OFFSET(PW_TYPE_BOOLEAN | PW_TYPE_IGNORE_DEFAULT, RADCLIENT, limit_proxy_state), NULL }, + { "limit_proxy_state", FR_CONF_POINTER(PW_TYPE_STRING| PW_TYPE_IGNORE_DEFAULT, &limit_proxy_state), NULL }, { "secret", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_SECRET, RADCLIENT, secret), NULL }, { "shortname", FR_CONF_OFFSET(PW_TYPE_STRING, RADCLIENT, shortname), NULL }, @@ -914,6 +915,7 @@ RADCLIENT *client_afrom_cs(TALLOC_CTX *ctx, CONF_SECTION *cs, bool in_server, bo memset(&cl_ipaddr, 0, sizeof(cl_ipaddr)); cl_netmask = 255; + limit_proxy_state = NULL; if (cf_section_parse(cs, c, client_config) < 0) { cf_log_err_cs(cs, "Error parsing client section"); @@ -923,6 +925,7 @@ RADCLIENT *client_afrom_cs(TALLOC_CTX *ctx, CONF_SECTION *cs, bool in_server, bo hs_proto = NULL; cl_srcipaddr = NULL; #endif + limit_proxy_state = NULL; return NULL; } @@ -1181,6 +1184,10 @@ done_coa: } #endif + if (fr_bool_auto_parse(cf_pair_find(cs, "limit_proxy_state"), &c->limit_proxy_state, limit_proxy_state) < 0) { + goto error; + } + return c; } diff --git a/src/main/listen.c b/src/main/listen.c index 75df668c0d..fb35a3c3b9 100644 --- a/src/main/listen.c +++ b/src/main/listen.c @@ -508,6 +508,79 @@ int rad_status_server(REQUEST *request) return 0; } +static void blastradius_checks(RADIUS_PACKET *packet, RADCLIENT *client) +{ + if (client->require_ma) return; + + /* + * If all of the checks are turned off, then complain for every packet we receive. + */ + if (client->limit_proxy_state == FR_BOOL_FALSE) { + /* + * We have a Message-Authenticator, and it's valid. We don't need to compain. + */ + if (packet->message_authenticator) return; + + DEBUG("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); + DEBUG("BlastRADIUS check: Received packet without Message-Authenticator."); + DEBUG("YOU MUST SET \"require_message_authenticator = true\", or"); + DEBUG("YOU MUST SET \"limit_proxy_state = true\" for client %s", client->shortname); + DEBUG("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); + DEBUG("The packet does not contain Message-Authenticator, which is a security issue"); + DEBUG("UPGRADE THE CLIENT AS YOUR NETWORK IS VULNERABLE TO THE BLASTRADIUS ATTACK."); + DEBUG("Once the client is upgraded, set \"require_message_authenticator = true\" for this client."); + DEBUG("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); + return; + } + + /* + * Don't complain here. rad_packet_ok() will instead + * complain about every packet with Proxy-State but which + * is missing Message-Authenticator. + */ + if (client->limit_proxy_state == FR_BOOL_TRUE) { + return; + } + + if (packet->proxy_state && !packet->message_authenticator) { + ERROR("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); + ERROR("BlastRADIUS check: Received packet with Proxy-State, but without Message-Authenticator."); + ERROR("This is either a BlastRADIUS attack, OR"); + ERROR("the client is a proxy RADIUS server which has not been upgraded."); + ERROR("Setting \"limit_proxy_state = false\" for client %s", client->shortname); + ERROR("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); + ERROR("UPGRADE THE CLIENT AS YOUR NETWORK IS VULNERABLE TO THE BLASTRADIUS ATTACK."); + ERROR("Once the client is upgraded, set \"require_message_authenticator = true\" for this client."); + ERROR("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); + + client->limit_proxy_state = FR_BOOL_FALSE; + + } else { + client->limit_proxy_state = FR_BOOL_TRUE; + + ERROR("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); + if (!packet->proxy_state) { + ERROR("BlastRADIUS check: Received packet without Proxy-State."); + } else { + ERROR("BlastRADIUS check: Received packet with Proxy-State and Message-Authenticator."); + } + + ERROR("Setting \"limit_proxy_state = true\" for client %s", client->shortname); + ERROR("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); + + if (!packet->message_authenticator) { + ERROR("The packet does not contain Message-Authenticator, which is a security issue."); + ERROR("UPGRADE THE CLIENT AS YOUR NETWORK MAY BE VULNERABLE TO THE BLASTRADIUS ATTACK."); + ERROR("Once the client is upgraded, set \"require_message_authenticator = true\" for this client."); + } else { + ERROR("The packet contains Message-Authenticator."); + if (!packet->eap_message) ERROR("The client has likely been upgraded to protect from the attack."); + ERROR("Please set \"require_message_authenticator = true\" for this client"); + } + ERROR("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); + } +} + #ifdef WITH_TCP static int dual_tcp_recv(rad_listen_t *listener) { @@ -588,12 +661,17 @@ static int dual_tcp_recv(rad_listen_t *listener) /* * Enforce BlastRADIUS checks on TCP, too. */ - if (!rad_packet_ok(packet, client->require_ma | (((int) client->limit_proxy_state) << 2), NULL)) { + if (!rad_packet_ok(packet, client->require_ma | ((client->limit_proxy_state == FR_BOOL_TRUE) << 2), NULL)) { FR_STATS_INC(auth, total_malformed_requests); rad_free(&sock->packet); return 0; } + /* + * Perform BlastRADIUS checks and warnings. + */ + if (packet->code == PW_CODE_ACCESS_REQUEST) blastradius_checks(packet, client); + FR_STATS_INC(auth, total_requests); fun = rad_authenticate; break; @@ -1700,7 +1778,6 @@ static int stats_socket_recv(rad_listen_t *listener) } #endif - /* * Check if an incoming request is "ok" * @@ -1776,7 +1853,7 @@ static int auth_socket_recv(rad_listen_t *listener) * Now that we've sanity checked everything, receive the * packet. */ - packet = rad_recv(ctx, listener->fd, client->require_ma | (((int) client->limit_proxy_state) << 2)); + packet = rad_recv(ctx, listener->fd, client->require_ma | ((client->limit_proxy_state == FR_BOOL_TRUE) << 2)); if (!packet) { FR_STATS_INC(auth, total_malformed_requests); if (DEBUG_ENABLED) ERROR("Receive - %s", fr_strerror()); @@ -1784,6 +1861,11 @@ static int auth_socket_recv(rad_listen_t *listener) return 0; } + /* + * Perform BlastRADIUS checks and warnings. + */ + if (packet->code == PW_CODE_ACCESS_REQUEST) blastradius_checks(packet, client); + #ifdef __APPLE__ #ifdef WITH_UDPFROMTO /* diff --git a/src/main/mainconfig.c b/src/main/mainconfig.c index b93e64f1f4..6c6dad9cfe 100644 --- a/src/main/mainconfig.c +++ b/src/main/mainconfig.c @@ -73,6 +73,7 @@ static char const *gid_name = NULL; static char const *chroot_dir = NULL; static bool allow_core_dumps = false; static char const *radlog_dest = NULL; +static char const *limit_proxy_state = NULL; /* * These are not used anywhere else.. @@ -87,6 +88,52 @@ static bool do_colourise = false; static char const *radius_dir = NULL; //!< Path to raddb directory +static const FR_NAME_NUMBER fr_bool_auto_names[] = { + { "false", FR_BOOL_FALSE }, + { "no", FR_BOOL_FALSE }, + { "0", FR_BOOL_FALSE }, + + { "true", FR_BOOL_TRUE }, + { "yes", FR_BOOL_TRUE }, + { "1", FR_BOOL_TRUE }, + + { "auto", FR_BOOL_AUTO }, + + { NULL, 0 } +}; + +/* + * Get decent values for false / true / auto + */ +int fr_bool_auto_parse(CONF_PAIR *cp, fr_bool_auto_t *out, char const *str) +{ + int value; + + /* + * Don't change anything. + */ + if (!str) return 0; + + value = fr_str2int(fr_bool_auto_names, str, -1); + if (value >= 0) { + *out = value; + return 0; + } + + /* + * This should never happen, as the defaults are in the + * source code. If there's no CONF_PAIR, and there's a + * parse error, then the source code is wrong. + */ + if (!cp) { + fprintf(stderr, "%s: Error - Invalid value in configuration", main_config.name); + return -1; + } + + cf_log_err(cf_pair_to_item(cp), "Invalid value for \"%s\"", cf_pair_attr(cp)); + return -1; +} + /********************************************************************** * * We need to figure out where the logs go, before doing anything @@ -161,7 +208,7 @@ static const CONF_PARSER security_config[] = { { "reject_delay", FR_CONF_POINTER(PW_TYPE_TIMEVAL, &main_config.reject_delay), STRINGIFY(0) }, { "status_server", FR_CONF_POINTER(PW_TYPE_BOOLEAN, &main_config.status_server), "no"}, { "require_message_authenticator", FR_CONF_POINTER(PW_TYPE_BOOLEAN, &main_config.require_ma), "yes"}, - { "limit_proxy_state", FR_CONF_POINTER(PW_TYPE_BOOLEAN, &main_config.limit_proxy_state), "yes"}, + { "limit_proxy_state", FR_CONF_POINTER(PW_TYPE_STRING, &limit_proxy_state), "auto"}, #ifdef ENABLE_OPENSSL_VERSION_CHECK { "allow_vulnerable_openssl", FR_CONF_POINTER(PW_TYPE_STRING, &main_config.allow_vulnerable_openssl), "no"}, #endif @@ -850,6 +897,7 @@ int main_config_init(void) if (!main_config.dictionary_dir) { main_config.dictionary_dir = DICTDIR; } + main_config.limit_proxy_state = FR_BOOL_AUTO; /* * About sizeof(REQUEST) + sizeof(RADIUS_PACKET) * 2 + sizeof(VALUE_PAIR) * 400 @@ -1145,6 +1193,18 @@ do {\ main_config.init_delay.tv_sec = 0; main_config.init_delay.tv_usec = 2* (1000000 / 3); + { + CONF_PAIR *cp = NULL; + + subcs = cf_section_sub_find(cs, "security"); + if (subcs) cp = cf_pair_find(subcs, "limit_proxy_state"); + + if (fr_bool_auto_parse(cp, &main_config.limit_proxy_state, limit_proxy_state) < 0) { + cf_file_free(cs); + return -1; + } + } + /* * Free the old configuration items, and replace them * with the new ones. commit 1ba2c0fd9c9f577c14bd99909f9d564ae44b0fc6 Author: Alan T. DeKok <al...@freeradius.org> Date: Sat Jun 29 13:54:31 2024 -0400 add more helpful error messages diff --git a/src/lib/radius.c b/src/lib/radius.c index 1e92d8b66e..82e70d8c92 100644 --- a/src/lib/radius.c +++ b/src/lib/radius.c @@ -2762,7 +2762,7 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason) * Message-Authenticator attributes. */ if (require_ma && !seen_ma) { - FR_DEBUG_STRERROR_PRINTF("Insecure packet from host %s: Packet does not contain required Message-Authenticator attribute", + FR_DEBUG_STRERROR_PRINTF("Insecure packet from host %s: Packet does not contain required Message-Authenticator attribute. You may need to set \"require_message_authenticator = no\" in the configuration.", inet_ntop(packet->src_ipaddr.af, &packet->src_ipaddr.ipaddr, host_ipaddr, sizeof(host_ipaddr))); commit a9c5d43141733f46f025b48fda61c4204967a96b Author: Alan T. DeKok <al...@freeradius.org> Date: Sat Jun 29 21:53:48 2024 -0400 implement and document "require_message_authenticator = auto" diff --git a/raddb/clients.conf b/raddb/clients.conf index 6a354dc8b7..a584120b68 100644 --- a/raddb/clients.conf +++ b/raddb/clients.conf @@ -102,13 +102,25 @@ client localhost { # # The global configuration "security.require_message_authenticator" # flag sets the default for all clients. That default can be - # over-ridden here, by setting it to "no". + # over-ridden here, by setting it to a value. If no value is set, + # then the default from the "radiusd.conf" file is used. + # + # See that file for full documentation on the flag, along + # with allowed values and meanings. # # This flag exists solely for legacy clients which do not send # Message-Authenticator in all Access-Request packets. We do not # recommend setting it to "no". # - # allowed values: yes, no + # The number one way to protect yourself from the BlastRADIUS + # attack is to update all RADIUS servers, and then set this + # flag to "yes". If all RADIUS servers are updated, and if + # all of them have this flag set to "yes" for all clients, + # then your network is safe. You can then upgrade the + # clients when it is convenient, instead of rushing the + # upgrades. + # + # allowed values: yes, no, auto # # require_message_authenticator = no @@ -117,11 +129,14 @@ client localhost { # flag sets the default for all clients. That default can be # over-ridden here, by setting it to "no". # + # See that file for full documentation on the flag, along + # with allowed values,and meanings. + # # This flag exists solely for legacy clients which do not send # Message-Authenticator in all Access-Request packets. We do not # recommend setting it to "no". # - # allowed values: yes, no + # allowed values: yes, no, auto # # limit_proxy_state = yes diff --git a/raddb/proxy.conf b/raddb/proxy.conf index e0be4c0497..7295538d5e 100644 --- a/raddb/proxy.conf +++ b/raddb/proxy.conf @@ -253,14 +253,18 @@ home_server localhost { # # The global configuration "security.require_message_authenticator" # flag sets the default for all home servers. That default can be - # over-ridden here, by setting it to "no". + # over-ridden here, by setting it to a value. If no value is set, + # then the default from the "radiusd.conf" file is used. + # + # See that file for full documentation on the flag, along + # with allowed values and meanings. # # This flag exists solely for legacy home servers which do # not send Message-Authenticator in all Access-Accept, # Access-Reject, or Access-Challenge packets. We do not # recommend setting it to "no". # - # allowed values: yes, no + # allowed values: yes, no, auto # # require_message_authenticator = no diff --git a/raddb/radiusd.conf.in b/raddb/radiusd.conf.in index d34a70959b..cf2d247ff7 100644 --- a/raddb/radiusd.conf.in +++ b/raddb/radiusd.conf.in @@ -573,25 +573,102 @@ security { status_server = yes # - # Global configuration for requiring Message-Authenticator - # in all Access-* packets sent over UDP or TCP. This flag - # is ignored for TLS. + # Global configuration for requiring Message-Authenticator in + # all Access-* packets sent over UDP or TCP. This flag is + # ignored for TLS. + # + # The number one way to protect yourself from the BlastRADIUS + # attack is to update all RADIUS servers, and then set this + # flag to "yes". If all RADIUS servers are updated, and if + # all of them have this flag set to "yes" for all clients, + # then your network is safe. You can then upgrade the + # clients when it is convenient, instead of rushing the + # upgrades. # # This flag sets the global default for all clients and home # servers. It can be over-ridden in an individual client or - # home server definition by adding a flag to that section: + # home_server definition by adding the same flag to that + # section with an appropriate value. + # + # All upgraded RADIUS implementations should send + # Message-Authenticator in all Access-Request, Access-Accept, + # Access-Reject, and Access-Challenge packets. Once all + # systems are upgraded, setting this flag to "yes" is the + # best protection from the attack. + # + # The possible values and meanings for + # "require_message_authenticator" are; + # + # * "no" - allow Access-* packet which do not contain + # Message-Authenticator + # + # For a client, if this flag is set to "no", then the + # "limit_proxy_state" flag, below, is also checked. + # + # For a home_server, if this flag is set to "no", then the + # Access-Accept, Access-Reject, and Access-Challenge + # packets do not need to contain Message-Authenticator. + # + # The only reason to set this flag to "no" is when the + # RADIUS client or home server has not been updated. It is + # always safer to set this flag "no" in the individual + # client or home_server definition. The global flag SHOULD + # still be set to a safe value: "yes". + # + # WARNING: Setting this flag and the "limit_proxy_state" + # flag to "no" will allow MITM attackers to create fake + # Access-Accept packets to the NAS! At least one of them + # MUST be set to "yes" for the system to have any + # protection against the attack. + # + # * "yes" - Require that all Access-* packets (client and + # home_server) contain Message-Authenticator. If a packet + # does not contain Message-Authenticator, then it is + # discarded. + # + # * "auto" - Automatically determine the value of the flag, + # based on the first packet received from that client or + # home_server. + # + # If the packet does not contain Message-Authenticator, + # then the value of the flag is automatically switched to + # "no". + # + # If the packet contains Message-Authenticator but not + # EAP-Message, then the value of the flag is automatically + # switched to "yes". The server has to check for + # EAP-Message, because the previous RFCs require that the + # packet contains Message-Authenticator when it also + # contains EAP-Message. So having a Message-Authenticator + # in those packets doesn't give the server enough + # information to determined if the client or home_server + # has been updated. + # + # If the packet contains Message-Authenticator and + # EAP-Message, then the flag is left at the "auto" value. + # + # WARNING: This switch is done for the first packet + # received from that client or home server. The change + # does NOT persist across server restarts. You MUST change + # the to "yes" manually, in order to make a permanent + # change to the configuration. # - # require_message_authenticator = no + # WARNING: If there are multiple NASes with the same source + # IP and client definitions, BUT the NASes have different + # behavior, then this flag WILL LIKELY BREAK YOUR NETWORK. # - # If the server produces error message which says "Packet - # does not contain required Message-Authenticator attribute", - # then this configuration item has to be updated. + # That is, when there are multiple different RADIUS clients + # behind one NATed IP address, then these security settings + # have to be set to allow the MOST INSECURE packets to be + # processed. This is a terrible idea, and will leave your + # network vulnerable to the attack. Please upgrade all + # clients immediately. # - # WARNING: This item should always be left as "yes", - # otherwise it is possible for MITM attackers to create fake - # Access-Accept packets to the NAS! + # The only solution to that rare configuration is to set + # this flag to "no", in which case the network will work, + # but will be vulnerable to the attack. # - require_message_authenticator = yes + require_message_authenticator = auto # # Global configuration for limiting the combination of @@ -601,7 +678,7 @@ security { # # This flag sets the global default for all clients. It can # be over-ridden in an individual client definition by adding - # the same flag to that section with an appropriate value: + # the same flag to that section with an appropriate value. # # If "require_message_authenticator" is set to "yes", this # configuration item is ignored. @@ -660,6 +737,13 @@ security { # IP and client definitions, BUT the NASes have different # behavior, then this flag WILL LIKELY BREAK YOUR NETWORK. # + # That is, when there are multiple different RADIUS clients + # behind one NATed IP address, then these security settings + # have to be set to allow the MOST INSECURE packets to be + # processed. This is a terrible idea, and will leave your + # network vulnerable to the attack. Please upgrade all + # clients immediately. + # # The only solution to that rare configuration is to set # this flag to "no", in which case the network will work, # but will be vulnerable to the attack. diff --git a/src/include/clients.h b/src/include/clients.h index a9b66429cc..7d3288a489 100644 --- a/src/include/clients.h +++ b/src/include/clients.h @@ -26,13 +26,6 @@ * @copyright 2015 The FreeRADIUS server project */ -typedef enum { - FR_BOOL_FALSE = 0, - FR_BOOL_TRUE, - FR_BOOL_AUTO, -} fr_bool_auto_t; - - typedef struct radclient_list RADCLIENT_LIST; @@ -50,7 +43,9 @@ typedef struct radclient { char const *secret; //!< Secret PSK. - bool require_ma; //!< Require RADIUS message authenticator in requests. + fr_bool_auto_t require_ma; //!< Require RADIUS message authenticator in requests. + + bool dynamic_require_ma; //!< for dynamic clients fr_bool_auto_t limit_proxy_state; //!< Limit Proxy-State in requests diff --git a/src/include/libradius.h b/src/include/libradius.h index 7479d0f02e..209051e129 100644 --- a/src/include/libradius.h +++ b/src/include/libradius.h @@ -954,6 +954,12 @@ int fr_socket_wait_for_connect(int sockfd, struct timeval *timeout); } #endif +typedef enum { + FR_BOOL_FALSE = 0, + FR_BOOL_TRUE, + FR_BOOL_AUTO, +} fr_bool_auto_t; + #include <freeradius-devel/packet.h> #ifdef WITH_TCP diff --git a/src/include/radiusd.h b/src/include/radiusd.h index cf08627dfb..b2a95247ef 100644 --- a/src/include/radiusd.h +++ b/src/include/radiusd.h @@ -174,7 +174,7 @@ typedef struct main_config { bool exiting; //!< are we exiting? - bool require_ma; //!< global configuration for all clients and home servers + fr_bool_auto_t require_ma; //!< global configuration for all clients and home servers fr_bool_auto_t limit_proxy_state; //!< global configuration for all clients diff --git a/src/include/realms.h b/src/include/realms.h index 4f82ca4be5..5f30ae13b5 100644 --- a/src/include/realms.h +++ b/src/include/realms.h @@ -66,7 +66,7 @@ typedef struct home_server { bool dual; //!< One of a pair of homeservers on consecutive ports. bool dynamic; //!< is this a dynamically added home server? - bool require_ma; //!< for all replies to Access-Request and Status-Server + fr_bool_auto_t require_ma; //!< for all replies to Access-Request and Status-Server #ifdef WITH_COA_TUNNEL bool recv_coa; //!< receive CoA packets, too diff --git a/src/main/client.c b/src/main/client.c index 310d64db02..c04259d497 100644 --- a/src/main/client.c +++ b/src/main/client.c @@ -490,6 +490,7 @@ static fr_ipaddr_t cl_ipaddr; static uint32_t cl_netmask; static char const *cl_srcipaddr = NULL; static char const *hs_proto = NULL; +static char const *require_message_authenticator = NULL; static char const *limit_proxy_state = NULL; #ifdef WITH_TCP @@ -513,7 +514,7 @@ static const CONF_PARSER client_config[] = { { "src_ipaddr", FR_CONF_POINTER(PW_TYPE_STRING, &cl_srcipaddr), NULL }, - { "require_message_authenticator", FR_CONF_OFFSET(PW_TYPE_BOOLEAN | PW_TYPE_IGNORE_DEFAULT, RADCLIENT, require_ma), NULL }, + { "require_message_authenticator", FR_CONF_POINTER(PW_TYPE_STRING| PW_TYPE_IGNORE_DEFAULT, &require_message_authenticator), NULL }, { "limit_proxy_state", FR_CONF_POINTER(PW_TYPE_STRING| PW_TYPE_IGNORE_DEFAULT, &limit_proxy_state), NULL }, { "secret", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_SECRET, RADCLIENT, secret), NULL }, @@ -722,7 +723,7 @@ static const CONF_PARSER dynamic_config[] = { { "FreeRADIUS-Client-Src-IP-Address", FR_CONF_OFFSET(PW_TYPE_IPV4_ADDR, RADCLIENT, src_ipaddr), NULL }, { "FreeRADIUS-Client-Src-IPv6-Address", FR_CONF_OFFSET(PW_TYPE_IPV6_ADDR, RADCLIENT, src_ipaddr), NULL }, - { "FreeRADIUS-Client-Require-MA", FR_CONF_OFFSET(PW_TYPE_BOOLEAN, RADCLIENT, require_ma), NULL }, + { "FreeRADIUS-Client-Require-MA", FR_CONF_OFFSET(PW_TYPE_BOOLEAN, RADCLIENT, dynamic_require_ma), NULL }, { "FreeRADIUS-Client-Secret", FR_CONF_OFFSET(PW_TYPE_STRING, RADCLIENT, secret), "" }, { "FreeRADIUS-Client-Shortname", FR_CONF_OFFSET(PW_TYPE_STRING, RADCLIENT, shortname), "" }, @@ -915,6 +916,7 @@ RADCLIENT *client_afrom_cs(TALLOC_CTX *ctx, CONF_SECTION *cs, bool in_server, bo memset(&cl_ipaddr, 0, sizeof(cl_ipaddr)); cl_netmask = 255; + require_message_authenticator = NULL; limit_proxy_state = NULL; if (cf_section_parse(cs, c, client_config) < 0) { @@ -925,6 +927,7 @@ RADCLIENT *client_afrom_cs(TALLOC_CTX *ctx, CONF_SECTION *cs, bool in_server, bo hs_proto = NULL; cl_srcipaddr = NULL; #endif + require_message_authenticator = NULL; limit_proxy_state = NULL; return NULL; @@ -1184,10 +1187,16 @@ done_coa: } #endif - if (fr_bool_auto_parse(cf_pair_find(cs, "limit_proxy_state"), &c->limit_proxy_state, limit_proxy_state) < 0) { + if (fr_bool_auto_parse(cf_pair_find(cs, "require_message_authenticator"), &c->require_ma, require_message_authenticator) < 0) { goto error; } + if (c->require_ma != FR_BOOL_TRUE) { + if (fr_bool_auto_parse(cf_pair_find(cs, "limit_proxy_state"), &c->limit_proxy_state, limit_proxy_state) < 0) { + goto error; + } + } + return c; } @@ -1512,6 +1521,11 @@ validate: goto error; } + /* + * It can't be set to "auto". Too bad. + */ + c->require_ma = (fr_bool_auto_t) c->dynamic_require_ma; + if (!client_add_dynamic(clients, request->client, c)) { return NULL; } diff --git a/src/main/listen.c b/src/main/listen.c index fb35a3c3b9..44ca2853d8 100644 --- a/src/main/listen.c +++ b/src/main/listen.c @@ -510,7 +510,49 @@ int rad_status_server(REQUEST *request) static void blastradius_checks(RADIUS_PACKET *packet, RADCLIENT *client) { - if (client->require_ma) return; + if (client->require_ma == FR_BOOL_TRUE) return; + + if (client->require_ma == FR_BOOL_AUTO) { + if (!packet->message_authenticator) { + ERROR("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); + ERROR("BlastRADIUS check: Received packet without Message-Authenticator."); + ERROR("Setting \"require_message_authenticator = false\" for client %s", client->shortname); + ERROR("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); + ERROR("UPGRADE THE CLIENT AS YOUR NETWORK IS VULNERABLE TO THE BLASTRADIUS ATTACK."); + ERROR("Once the client is upgraded, set \"require_message_authenticator = true\" for this client."); + ERROR("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); + client->require_ma = FR_BOOL_FALSE; + + /* + * And fall through to the + * limit_proxy_state checks, which might + * complain again. Oh well, maybe that + * will make people read the messages. + */ + + } else if (packet->eap_message) { + /* + * Don't set it to "true" for packets + * with EAP-Message. It's already + * required there, and we might get a + * non-EAP packet with (or without) + * Message-Authenticator + */ + return; + } else { + ERROR("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); + ERROR("BlastRADIUS check: Received packet with Message-Authenticator."); + ERROR("Setting \"require_message_authenticator = true\" for client %s", client->shortname); + ERROR("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); + ERROR("It looks like the client has been uppdated to protect from the BlastRADIUS attack."); + ERROR("Please set \"require_message_authenticator = true\" for this client."); + ERROR("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); + + client->require_ma = FR_BOOL_TRUE; + return; + } + + } /* * If all of the checks are turned off, then complain for every packet we receive. @@ -521,6 +563,8 @@ static void blastradius_checks(RADIUS_PACKET *packet, RADCLIENT *client) */ if (packet->message_authenticator) return; + if (!fr_debug_lvl) return; /* easier than checking for each line below */ + DEBUG("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); DEBUG("BlastRADIUS check: Received packet without Message-Authenticator."); DEBUG("YOU MUST SET \"require_message_authenticator = true\", or"); @@ -661,7 +705,7 @@ static int dual_tcp_recv(rad_listen_t *listener) /* * Enforce BlastRADIUS checks on TCP, too. */ - if (!rad_packet_ok(packet, client->require_ma | ((client->limit_proxy_state == FR_BOOL_TRUE) << 2), NULL)) { + if (!rad_packet_ok(packet, (client->require_ma == FR_BOOL_TRUE) | ((client->limit_proxy_state == FR_BOOL_TRUE) << 2), NULL)) { FR_STATS_INC(auth, total_malformed_requests); rad_free(&sock->packet); return 0; @@ -1853,7 +1897,7 @@ static int auth_socket_recv(rad_listen_t *listener) * Now that we've sanity checked everything, receive the * packet. */ - packet = rad_recv(ctx, listener->fd, client->require_ma | ((client->limit_proxy_state == FR_BOOL_TRUE) << 2)); + packet = rad_recv(ctx, listener->fd, (client->require_ma == FR_BOOL_TRUE) | ((client->limit_proxy_state == FR_BOOL_TRUE) << 2)); if (!packet) { FR_STATS_INC(auth, total_malformed_requests); if (DEBUG_ENABLED) ERROR("Receive - %s", fr_strerror()); @@ -2285,7 +2329,7 @@ static int proxy_socket_recv(rad_listen_t *listener) #endif char buffer[128]; - packet = rad_recv(NULL, listener->fd, 8); /* SOME packets don't require a Message-Authenticator attribute */ + packet = rad_recv(NULL, listener->fd, 0x08); /* SOME packets don't require a Message-Authenticator attribute */ if (!packet) { if (DEBUG_ENABLED) ERROR("Receive - %s", fr_strerror()); return 0; diff --git a/src/main/mainconfig.c b/src/main/mainconfig.c index 6c6dad9cfe..4e10de4225 100644 --- a/src/main/mainconfig.c +++ b/src/main/mainconfig.c @@ -73,6 +73,7 @@ static char const *gid_name = NULL; static char const *chroot_dir = NULL; static bool allow_core_dumps = false; static char const *radlog_dest = NULL; +static char const *require_message_authenticator = NULL; static char const *limit_proxy_state = NULL; /* @@ -207,7 +208,7 @@ static const CONF_PARSER security_config[] = { { "max_attributes", FR_CONF_POINTER(PW_TYPE_INTEGER, &fr_max_attributes), STRINGIFY(0) }, { "reject_delay", FR_CONF_POINTER(PW_TYPE_TIMEVAL, &main_config.reject_delay), STRINGIFY(0) }, { "status_server", FR_CONF_POINTER(PW_TYPE_BOOLEAN, &main_config.status_server), "no"}, - { "require_message_authenticator", FR_CONF_POINTER(PW_TYPE_BOOLEAN, &main_config.require_ma), "yes"}, + { "require_message_authenticator", FR_CONF_POINTER(PW_TYPE_STRING, &require_message_authenticator), "auto"}, { "limit_proxy_state", FR_CONF_POINTER(PW_TYPE_STRING, &limit_proxy_state), "auto"}, #ifdef ENABLE_OPENSSL_VERSION_CHECK { "allow_vulnerable_openssl", FR_CONF_POINTER(PW_TYPE_STRING, &main_config.allow_vulnerable_openssl), "no"}, @@ -897,6 +898,7 @@ int main_config_init(void) if (!main_config.dictionary_dir) { main_config.dictionary_dir = DICTDIR; } + main_config.require_ma = FR_BOOL_AUTO; main_config.limit_proxy_state = FR_BOOL_AUTO; /* @@ -1197,8 +1199,13 @@ do {\ CONF_PAIR *cp = NULL; subcs = cf_section_sub_find(cs, "security"); - if (subcs) cp = cf_pair_find(subcs, "limit_proxy_state"); + if (subcs) cp = cf_pair_find(subcs, "require_message_authenticator"); + if (fr_bool_auto_parse(cp, &main_config.require_ma, require_message_authenticator) < 0) { + cf_file_free(cs); + return -1; + } + if (subcs) cp = cf_pair_find(subcs, "limit_proxy_state"); if (fr_bool_auto_parse(cp, &main_config.limit_proxy_state, limit_proxy_state) < 0) { cf_file_free(cs); return -1; diff --git a/src/main/process.c b/src/main/process.c index 79117cd998..7d0ccae7a9 100644 --- a/src/main/process.c +++ b/src/main/process.c @@ -2759,7 +2759,7 @@ int request_proxy_reply(RADIUS_PACKET *packet) * * The realms.c file already clears require_ma for TLS connections. */ - bool require_ma = request->home_server->require_ma && (request->proxy->code == PW_CODE_ACCESS_REQUEST); + bool require_ma = (request->home_server->require_ma == FR_BOOL_TRUE) && (request->proxy->code == PW_CODE_ACCESS_REQUEST); if (!rad_packet_ok(packet, require_ma, &reason)) { DEBUG("Ignoring invalid packet - %s", fr_strerror()); @@ -2771,6 +2771,53 @@ int request_proxy_reply(RADIUS_PACKET *packet) DEBUG("Ignoring spoofed proxy reply. Signature is invalid"); return 0; } + + /* + * BlastRADIUS checks. We're running in the main + * listener thread, so there's no conflict + * checking or setting these fields. + */ + if ((request->proxy->code == PW_CODE_ACCESS_REQUEST) && +#ifdef WITH_TLS + !request->home_server->tls && +#endif + !packet->eap_message) { + if (request->home_server->require_ma == FR_BOOL_AUTO) { + if (!packet->message_authenticator) { + RERROR("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); + RERROR("BlastRADIUS check: Received response to Access-Request without Message-Authenticator."); + RERROR("Setting \"require_message_authenticator = false\" for home_server %s", request->home_server->name); + RERROR("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); + RERROR("UPGRADE THE HOME SERVER AS YOUR NETWORK IS VULNERABLE TO THE BLASTRADIUS ATTACK."); + RERROR("Once the home_server is upgraded, set \"require_message_authenticator = true\" for this home_server."); + RERROR("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); + + request->home_server->require_ma = FR_BOOL_FALSE; + } else { + RERROR("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); + RERROR("BlastRADIUS check: Received response to Access-Request with Message-Authenticator."); + RERROR("Setting \"require_message_authenticator = true\" for home_server %s", request->home_server->name); + RERROR("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); + RERROR("It looks like the home server has been uppdated to protect from the BlastRADIUS attack."); + RERROR("Please set \"require_message_authenticator = true\" for this home_server."); + RERROR("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); + + request->home_server->require_ma = FR_BOOL_TRUE; + } + + } else if (fr_debug_lvl && (request->home_server->require_ma == FR_BOOL_FALSE) && !packet->message_authenticator) { + /* + * If it's "no" AND we don't have a Message-Authenticator, then complain on every packet. + */ + RDEBUG("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); + RDEBUG("BlastRADIUS check: Received packet without Message-Authenticator from home_server %s", request->home_server->name); + RDEBUG("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); + RDEBUG("The packet does not contain Message-Authenticator, which is a security issue"); + RDEBUG("UPGRADE THE HOME SERVER AS YOUR NETWORK IS VULNERABLE TO THE BLASTRADIUS ATTACK."); + RDEBUG("Once the home server is upgraded, set \"require_message_authenticator = true\" for this home_server."); + RDEBUG("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); + } + } } /* diff --git a/src/main/realms.c b/src/main/realms.c index bf9a8d5ca2..0a3f6796d5 100644 --- a/src/main/realms.c +++ b/src/main/realms.c @@ -472,8 +472,10 @@ static CONF_PARSER home_server_recv_coa[] = { #endif +static const char *require_message_authenticator = NULL; + static CONF_PARSER home_server_config[] = { - { "require_message_authenticator", FR_CONF_OFFSET(PW_TYPE_BOOLEAN | PW_TYPE_IGNORE_DEFAULT, home_server_t, require_ma), NULL }, + { "require_message_authenticator", FR_CONF_POINTER(PW_TYPE_STRING| PW_TYPE_IGNORE_DEFAULT, &require_message_authenticator), NULL }, { "ipaddr", FR_CONF_OFFSET(PW_TYPE_COMBO_IP_ADDR, home_server_t, ipaddr), NULL }, { "ipv4addr", FR_CONF_OFFSET(PW_TYPE_IPV4_ADDR, home_server_t, ipaddr), NULL }, { "ipv6addr", FR_CONF_OFFSET(PW_TYPE_IPV6_ADDR, home_server_t, ipaddr), NULL }, @@ -766,12 +768,18 @@ home_server_t *home_server_afrom_cs(TALLOC_CTX *ctx, realm_config_t *rc, CONF_SE home->proto = IPPROTO_UDP; home->require_ma = main_config.require_ma; + require_message_authenticator = false; + /* * Parse the configuration into the home server * struct. */ if (cf_section_parse(cs, home, home_server_config) < 0) goto error; + if (fr_bool_auto_parse(cf_pair_find(cs, "require_message_authenticator"), &home->require_ma, require_message_authenticator) < 0) { + goto error; + } + /* * It has an IP address, it must be a remote server. */ commit 1d9ea1fdd5b6a77439cabc172aa2ffdeb23265fe Author: Alan T. DeKok <al...@freeradius.org> Date: Thu Jul 4 15:52:10 2024 -0400 typos and clarifications diff --git a/src/main/listen.c b/src/main/listen.c index 44ca2853d8..5d2ec0241e 100644 --- a/src/main/listen.c +++ b/src/main/listen.c @@ -519,7 +519,7 @@ static void blastradius_checks(RADIUS_PACKET *packet, RADCLIENT *client) ERROR("Setting \"require_message_authenticator = false\" for client %s", client->shortname); ERROR("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); ERROR("UPGRADE THE CLIENT AS YOUR NETWORK IS VULNERABLE TO THE BLASTRADIUS ATTACK."); - ERROR("Once the client is upgraded, set \"require_message_authenticator = true\" for this client."); + ERROR("Once the client is upgraded, set \"require_message_authenticator = true\" for client %s", client->shortname); ERROR("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); client->require_ma = FR_BOOL_FALSE; @@ -544,8 +544,8 @@ static void blastradius_checks(RADIUS_PACKET *packet, RADCLIENT *client) ERROR("BlastRADIUS check: Received packet with Message-Authenticator."); ERROR("Setting \"require_message_authenticator = true\" for client %s", client->shortname); ERROR("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); - ERROR("It looks like the client has been uppdated to protect from the BlastRADIUS attack."); - ERROR("Please set \"require_message_authenticator = true\" for this client."); + ERROR("It looks like the client has been updated to protect from the BlastRADIUS attack."); + ERROR("Please set \"require_message_authenticator = true\" for client %s", client->shortname); ERROR("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); client->require_ma = FR_BOOL_TRUE; @@ -572,7 +572,7 @@ static void blastradius_checks(RADIUS_PACKET *packet, RADCLIENT *client) DEBUG("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); DEBUG("The packet does not contain Message-Authenticator, which is a security issue"); DEBUG("UPGRADE THE CLIENT AS YOUR NETWORK IS VULNERABLE TO THE BLASTRADIUS ATTACK."); - DEBUG("Once the client is upgraded, set \"require_message_authenticator = true\" for this client."); + DEBUG("Once the client is upgraded, set \"require_message_authenticator = true\" for client %s", client->shortname); DEBUG("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); return; } @@ -594,7 +594,7 @@ static void blastradius_checks(RADIUS_PACKET *packet, RADCLIENT *client) ERROR("Setting \"limit_proxy_state = false\" for client %s", client->shortname); ERROR("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); ERROR("UPGRADE THE CLIENT AS YOUR NETWORK IS VULNERABLE TO THE BLASTRADIUS ATTACK."); - ERROR("Once the client is upgraded, set \"require_message_authenticator = true\" for this client."); + ERROR("Once the client is upgraded, set \"require_message_authenticator = true\" for client %s", client->shortname); ERROR("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); client->limit_proxy_state = FR_BOOL_FALSE; @@ -615,11 +615,11 @@ static void blastradius_checks(RADIUS_PACKET *packet, RADCLIENT *client) if (!packet->message_authenticator) { ERROR("The packet does not contain Message-Authenticator, which is a security issue."); ERROR("UPGRADE THE CLIENT AS YOUR NETWORK MAY BE VULNERABLE TO THE BLASTRADIUS ATTACK."); - ERROR("Once the client is upgraded, set \"require_message_authenticator = true\" for this client."); + ERROR("Once the client is upgraded, set \"require_message_authenticator = true\" for client %s", client->shortname); } else { ERROR("The packet contains Message-Authenticator."); if (!packet->eap_message) ERROR("The client has likely been upgraded to protect from the attack."); - ERROR("Please set \"require_message_authenticator = true\" for this client"); + ERROR("Please set \"require_message_authenticator = true\" for client %s", client->shortname); } ERROR("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); } diff --git a/src/main/process.c b/src/main/process.c index 7d0ccae7a9..6c5103cfc2 100644 --- a/src/main/process.c +++ b/src/main/process.c @@ -2789,7 +2789,7 @@ int request_proxy_reply(RADIUS_PACKET *packet) RERROR("Setting \"require_message_authenticator = false\" for home_server %s", request->home_server->name); RERROR("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); RERROR("UPGRADE THE HOME SERVER AS YOUR NETWORK IS VULNERABLE TO THE BLASTRADIUS ATTACK."); - RERROR("Once the home_server is upgraded, set \"require_message_authenticator = true\" for this home_server."); + RERROR("Once the home_server is upgraded, set \"require_message_authenticator = true\" for home_server %s.", request->home_server->name); RERROR("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); request->home_server->require_ma = FR_BOOL_FALSE; @@ -2798,8 +2798,8 @@ int request_proxy_reply(RADIUS_PACKET *packet) RERROR("BlastRADIUS check: Received response to Access-Request with Message-Authenticator."); RERROR("Setting \"require_message_authenticator = true\" for home_server %s", request->home_server->name); RERROR("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); - RERROR("It looks like the home server has been uppdated to protect from the BlastRADIUS attack."); - RERROR("Please set \"require_message_authenticator = true\" for this home_server."); + RERROR("It looks like the home server has been updated to protect from the BlastRADIUS attack."); + RERROR("Please set \"require_message_authenticator = true\" for home_server %s", request->home_server->name); RERROR("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); request->home_server->require_ma = FR_BOOL_TRUE; @@ -2814,7 +2814,7 @@ int request_proxy_reply(RADIUS_PACKET *packet) RDEBUG("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); RDEBUG("The packet does not contain Message-Authenticator, which is a security issue"); RDEBUG("UPGRADE THE HOME SERVER AS YOUR NETWORK IS VULNERABLE TO THE BLASTRADIUS ATTACK."); - RDEBUG("Once the home server is upgraded, set \"require_message_authenticator = true\" for this home_server."); + RDEBUG("Once the home server is upgraded, set \"require_message_authenticator = true\" for home_server %s", request->home_server->name); RDEBUG("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); } } commit c0192db04519d9aa083a07bc929f01c93a2817d9 Author: Alan T. DeKok <al...@freeradius.org> Date: Sat Jul 6 11:49:54 2024 -0400 don't enforce require_ma on packet reception diff --git a/src/main/listen.c b/src/main/listen.c index 5d2ec0241e..2f1ef6804f 100644 --- a/src/main/listen.c +++ b/src/main/listen.c @@ -2329,7 +2329,7 @@ static int proxy_socket_recv(rad_listen_t *listener) #endif char buffer[128]; - packet = rad_recv(NULL, listener->fd, 0x08); /* SOME packets don't require a Message-Authenticator attribute */ + packet = rad_recv(NULL, listener->fd, 0); if (!packet) { if (DEBUG_ENABLED) ERROR("Receive - %s", fr_strerror()); return 0; commit 82965776c51e9a4d7517a84ce596c0b443255cdc Author: Alan T. DeKok <al...@freeradius.org> Date: Sun Jul 7 09:50:43 2024 -0400 handle dynamic require Message-Authenticator diff --git a/src/main/client.c b/src/main/client.c index c04259d497..51f9ba5837 100644 --- a/src/main/client.c +++ b/src/main/client.c @@ -1427,10 +1427,10 @@ RADCLIENT *client_afrom_request(RADCLIENT_LIST *clients, REQUEST *request) *pi = vp->vp_integer; /* - * Same nastiness as above. + * Same nastiness as above, but hard-coded for require Message-Authenticator. */ for (parse = client_config; parse->name; parse++) { - if (parse->offset == dynamic_config[i].offset) break; + if (parse->type == PW_TYPE_BOOLEAN) break; } if (!parse) break; commit 83b2b349301458351268358854169870bd994db7 Author: Terry Burton <t...@terryburton.co.uk> Date: Mon Jul 8 15:55:44 2024 +0100 Config docs: Clients aggregators may be RADIUS proxies and set proxy-state diff --git a/raddb/radiusd.conf.in b/raddb/radiusd.conf.in index cf2d247ff7..0cf3de89a9 100644 --- a/raddb/radiusd.conf.in +++ b/raddb/radiusd.conf.in @@ -713,9 +713,16 @@ security { # packets which contain Proxy-State MUST also contain # Message-Authenticator, otherwise they are discarded. # - # This setting is safe for all NASes, GGSNs, BRAS, etc. - # No known RADIUS client sends Proxy-State for normal - # Access-Request packets. + # This setting is safe for most NASes, GGSNs, BRAS, etc. + # Most regular RADIUS clients do not send Proxy-State + # attributes for Access-Request packets that they originate. + # However some aggregators (e.g. Wireless LAN Controllers) + # may act as a RADIUS proxy for requests from their cohort + # of managed devices, and in such cases will provide a + # Proxy-State attribute. For those systems, you _must_ look + # at the actual packets to determine what to do. It may be + # that the only way to fix the vulnerability is to upgrade + # the WLC, and set "require_message_authenticator" to "yes". # # * "auto" - Automatically determine the value of the flag, # based on the first packet received from that client.