From dbf9bab2f93607987561c6f696020a9815e8c914 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.munro@gmail.com>
Date: Sun, 4 Aug 2024 15:58:33 +1200
Subject: [PATCH v1 3/5] Mitigation for BlastRADIUS.

Add Message-Authenticator attribute (an HMAC-MD5 signature) to outgoing
Access-Request messages, and optionally require and verify them on
incoming Access-Accept and Access-Reject messages, as recommended to
mitigate CVE-2024-3596 and VU#456537 by:

  https://www.blastradius.fail/

No RADIUS server should be upset by the addition of our new
Message-Authenticator attribute in requests, so we always add that.

On the other hand, we can't require the attribute to be present in
responses yet because we can't assume that all sites are already doing
that.  For example, FreeRADIUS started sending them in 3.2.5, which
isn't yet in all distributions at the time of writing.  Therefore, users
have to opt in to this requirement with radiusrequirema=1 in pg_hba.conf
for now; the default could change in future.
---
 doc/src/sgml/client-auth.sgml |  17 +++
 src/backend/libpq/auth.c      | 208 +++++++++++++++++++++++++++++++++-
 src/backend/libpq/hba.c       |   8 ++
 src/include/libpq/hba.h       |   1 +
 4 files changed, 229 insertions(+), 5 deletions(-)

diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index 51343de7cad..6c49ba18346 100644
--- a/doc/src/sgml/client-auth.sgml
+++ b/doc/src/sgml/client-auth.sgml
@@ -2161,6 +2161,23 @@ host ... ldap ldapbasedn="dc=example,dc=net"
        </listitem>
       </varlistentry>
 
+      <varlistentry>
+       <term><literal>radiusrequirema</literal></term>
+       <listitem>
+        <para>
+         Whether to require a valid <literal>Message-Authenticator</literal>
+         attribute in messages received from RADIUS servers, and ignore messages
+         that don't contain it.  The default value
+         is <literal>0</literal>, but it can be set to <literal>1</literal>
+         to enable that requirement.
+         This setting does not affect requests sent by
+         <productname>PostgreSQL</productname> to the RADIUS server, which
+         always include a <literal>Message-Authenticator</literal> attribute
+         (but didn't in earlier releases).
+        </para>
+       </listitem>
+      </varlistentry>
+
      </variablelist>
    </para>
 
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 2b607c52704..40f0cabf3a9 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -24,6 +24,7 @@
 #include <unistd.h>
 
 #include "commands/user.h"
+#include "common/hmac.h"
 #include "common/ip.h"
 #include "common/md5.h"
 #include "libpq/auth.h"
@@ -198,7 +199,7 @@ static int	pg_SSPI_make_upn(char *accountname,
  *----------------------------------------------------------------
  */
 static int	CheckRADIUSAuth(Port *port);
-static int	PerformRadiusTransaction(const char *server, const char *secret, const char *portstr, const char *identifier, const char *user_name, const char *passwd);
+static int	PerformRadiusTransaction(const char *server, const char *secret, const char *portstr, const char *identifier, const char *user_name, const char *passwd, bool requirema);
 
 
 /*
@@ -2802,6 +2803,7 @@ typedef struct
 #define RADIUS_PASSWORD			2
 #define RADIUS_SERVICE_TYPE		6
 #define RADIUS_NAS_IDENTIFIER	32
+#define RADIUS_MESSAGE_AUTHENTICATOR 80
 
 /* RADIUS service types */
 #define RADIUS_AUTHENTICATE_ONLY	8
@@ -2809,7 +2811,7 @@ typedef struct
 /* Seconds to wait - XXX: should be in a config variable! */
 #define RADIUS_TIMEOUT 3
 
-static void
+static uint8 *
 radius_add_attribute(radius_packet *packet, uint8 type, const unsigned char *data, int len)
 {
 	radius_attribute *attr;
@@ -2825,7 +2827,7 @@ radius_add_attribute(radius_packet *packet, uint8 type, const unsigned char *dat
 		elog(WARNING,
 			 "adding attribute code %d with length %d to radius packet would create oversize packet, ignoring",
 			 type, len);
-		return;
+		return NULL;
 	}
 
 	attr = (radius_attribute *) ((unsigned char *) packet + packet->length);
@@ -2833,6 +2835,59 @@ radius_add_attribute(radius_packet *packet, uint8 type, const unsigned char *dat
 	attr->length = len + 2;		/* total size includes type and length */
 	memcpy(attr->data, data, len);
 	packet->length += attr->length;
+
+	/*
+	 * Return pointer into message, used to fill in Message-Authenticator
+	 * contents later.
+	 */
+	return attr->data;
+}
+
+/*
+ * Search for an attribute in a received message.  If the attribute is found,
+ * sets *value and *length (not including the header), and returns true.  If
+ * the attribute is not found but the message appears to be well-formed, sets
+ * *value to NULL and returns true.  If the message is found to be mal-formed,
+ * returns false.
+ */
+static bool
+radius_find_attribute(uint8 *packet,
+					  size_t packet_size,
+					  uint8 type,
+					  uint8 **value,
+					  uint8 *length)
+{
+	uint8	   *end = packet + packet_size;
+	radius_attribute *attr;
+
+	attr = (radius_attribute *) (packet + RADIUS_HEADER_LENGTH);
+
+	while ((uint8 *) attr < end)
+	{
+		/* Would this attribute overflow the buffer? */
+		if (&attr->length >= end || (uint8 *) attr + attr->length > end)
+			return false;
+
+		/* Would this attribute's length be shorter than its own header? */
+		if (attr->length < offsetof(radius_attribute, data))
+			return false;
+
+		/* Is this it? */
+		if (attr->attribute == type)
+		{
+			*value = attr->data;
+			*length = attr->length - offsetof(radius_attribute, data);
+			return true;
+		}
+
+		/* Nope, skip to the next one. */
+		attr = (radius_attribute *) ((uint8 *) attr + attr->length);
+	}
+
+	/* Well-formed, but not found. */
+	*value = NULL;
+	*length = 0;
+	return true;
 }
 
 static int
@@ -2890,7 +2945,8 @@ CheckRADIUSAuth(Port *port)
 												   radiusports ? lfirst(radiusports) : NULL,
 												   identifiers ? lfirst(identifiers) : NULL,
 												   port->user_name,
-												   passwd);
+												   passwd,
+												   port->hba->radiusrequirema);
 
 		/*------
 		 * STATUS_OK = Login OK
@@ -2931,8 +2987,13 @@ CheckRADIUSAuth(Port *port)
 }
 
 static int
-PerformRadiusTransaction(const char *server, const char *secret, const char *portstr, const char *identifier, const char *user_name, const char *passwd)
+PerformRadiusTransaction(const char *server, const char *secret, const char *portstr, const char *identifier, const char *user_name, const char *passwd, bool requirema)
 {
+	pg_hmac_ctx *hmac_context;
+	uint8		message_authenticator_key[RADIUS_VECTOR_LENGTH];
+	uint8		message_authenticator[RADIUS_VECTOR_LENGTH];
+	uint8	   *message_authenticator_location;
+	uint8		message_authenticator_size;
 	radius_packet radius_send_pack;
 	radius_packet radius_recv_pack;
 	radius_packet *packet = &radius_send_pack;
@@ -2993,6 +3054,18 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por
 		return STATUS_ERROR;
 	}
 	packet->id = packet->vector[0];
+
+	/*
+	 * Add Message-Authenticator attribute first, which for now holds zeroes.
+	 * We remember where it is in the message so that we can fill it in later.
+	 */
+	memset(message_authenticator, 0, lengthof(message_authenticator));
+	message_authenticator_location =
+		radius_add_attribute(packet,
+							 RADIUS_MESSAGE_AUTHENTICATOR,
+							 message_authenticator,
+							 lengthof(message_authenticator));
+
 	radius_add_attribute(packet, RADIUS_SERVICE_TYPE, (const unsigned char *) &service, sizeof(service));
 	radius_add_attribute(packet, RADIUS_USER_NAME, (const unsigned char *) user_name, strlen(user_name));
 	radius_add_attribute(packet, RADIUS_NAS_IDENTIFIER, (const unsigned char *) identifier, strlen(identifier));
@@ -3048,6 +3121,44 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por
 	packetlength = packet->length;
 	packet->length = pg_hton16(packet->length);
 
+	/*
+	 * We use the first 16 bytes of the shared secret, zero-padded if too
+	 * short, as an HMAC-MD5 key for creating and validating
+	 * Message-Authenticator attributes.
+	 */
+	memset(message_authenticator_key, 0, lengthof(message_authenticator_key));
+	memcpy(message_authenticator_key,
+		   secret,
+		   Min(lengthof(message_authenticator_key), strlen(secret)));
+
+	/*
+	 * Compute the Message-Authenticator for the whole message.  The
+	 * Message-Authenticator itself is one of the attributes, but it holds
+	 * zeroes at this point.
+	 */
+	hmac_context = pg_hmac_create(PG_MD5);
+	if (hmac_context == NULL ||
+		pg_hmac_init(hmac_context,
+					 message_authenticator_key,
+					 lengthof(message_authenticator_key)) < 0 ||
+		pg_hmac_update(hmac_context,
+					   (uint8 *) radius_buffer, packetlength) < 0 ||
+		pg_hmac_final(hmac_context,
+					  message_authenticator,
+					  lengthof(message_authenticator)) < 0)
+	{
+		ereport(LOG, (errmsg("could not compute RADIUS Message-Authenticator: %s",
+							 pg_hmac_error(hmac_context))));
+		pg_hmac_free(hmac_context);
+		pg_freeaddrinfo_all(hint.ai_family, serveraddrs);
+		return STATUS_ERROR;
+	}
+	pg_hmac_free(hmac_context);
+
+	/* Overwrite the attribute with the computed signature. */
+	memcpy(message_authenticator_location, message_authenticator,
+		   lengthof(message_authenticator));
+
 	sock = socket(serveraddrs[0].ai_family, SOCK_DGRAM, 0);
 	if (sock == PGINVALID_SOCKET)
 	{
@@ -3232,6 +3343,93 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por
 			continue;
 		}
 
+		/* Search for the Message-Authenticator attribute. */
+		if (!radius_find_attribute((uint8 *) receive_buffer,
+								   packetlength,
+								   RADIUS_MESSAGE_AUTHENTICATOR,
+								   &message_authenticator_location,
+								   &message_authenticator_size))
+		{
+			ereport(LOG,
+					(errmsg("RADIUS response from %s has malformed attributes",
+							server)));
+			continue;
+		}
+		else if (message_authenticator_location == NULL)
+		{
+			ereport(LOG,
+					(errmsg("RADIUS response from %s has no Message-Authenticator",
+							server)));
+
+			/* We'll ignore this message, unless pg_hba.conf told us not to. */
+			if (requirema)
+				continue;
+		}
+		else if (message_authenticator_size != RADIUS_VECTOR_LENGTH)
+		{
+			ereport(LOG,
+					(errmsg("RADIUS response from %s has unexpected Message-Authenticator size",
+							server)));
+			continue;
+		}
+		else
+		{
+			uint8		message_authenticator_copy[RADIUS_VECTOR_LENGTH];
+
+			/*
+			 * Save a copy of the received HMAC, and zero out the one in the
+			 * message so that we have input required to recompute it.
+			 */
+			memcpy(message_authenticator_copy,
+				   message_authenticator_location,
+				   RADIUS_VECTOR_LENGTH);
+			memset(message_authenticator_location,
+				   0,
+				   RADIUS_VECTOR_LENGTH);
+
+			/*
+			 * Compute the expected value.  Note that the HMAC for
+			 * Access-Accept and Access-Reject message uses the authenticator
+			 * from the original Access-Request message, so we have to do a
+			 * bit of splicing.
+			 */
+			hmac_context = pg_hmac_create(PG_MD5);
+			if (hmac_context == NULL ||
+				pg_hmac_init(hmac_context,
+							 message_authenticator_key,
+							 lengthof(message_authenticator_key)) < 0 ||
+				pg_hmac_update(hmac_context,
+							   (uint8 *) receive_buffer,
+							   offsetof(radius_packet, vector)) < 0 ||
+				pg_hmac_update(hmac_context,
+							   packet->vector,
+							   RADIUS_VECTOR_LENGTH) < 0 ||
+				pg_hmac_update(hmac_context,
+							   (uint8 *) receive_buffer + RADIUS_HEADER_LENGTH,
+							   packetlength - RADIUS_HEADER_LENGTH) < 0 ||
+				pg_hmac_final(hmac_context,
+							  message_authenticator,
+							  lengthof(message_authenticator)) < 0)
+			{
+				ereport(LOG, (errmsg("could not compute RADIUS Message-Authenticator: %s",
+									 pg_hmac_error(hmac_context))));
+				pg_hmac_free(hmac_context);
+				closesocket(sock);
+				return STATUS_ERROR;
+			}
+			pg_hmac_free(hmac_context);
+
+			/* Verify. */
+			if (memcmp(message_authenticator_copy,
+					   message_authenticator,
+					   RADIUS_VECTOR_LENGTH) != 0)
+			{
+				ereport(LOG, (errmsg("RADIUS response from %s has invalid Message-Authenticator",
+									 server)));
+				continue;
+			}
+		}
+
 		if (receivepacket->code == RADIUS_ACCESS_ACCEPT)
 		{
 			closesocket(sock);
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index 75d588e36a1..7ff8e1ca6e7 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -2446,6 +2446,14 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline,
 		hbaline->radiusidentifiers = parsed_identifiers;
 		hbaline->radiusidentifiers_s = pstrdup(val);
 	}
+	else if (strcmp(name, "radiusrequirema") == 0)
+	{
+		REQUIRE_AUTH_OPTION(uaRADIUS, "radiusrequirema", "radius");
+		if (strcmp(val, "1") == 0)
+			hbaline->radiusrequirema = true;
+		else
+			hbaline->radiusrequirema = false;
+	}
 	else
 	{
 		ereport(elevel,
diff --git a/src/include/libpq/hba.h b/src/include/libpq/hba.h
index 8ea837ae82a..139e4bd410a 100644
--- a/src/include/libpq/hba.h
+++ b/src/include/libpq/hba.h
@@ -135,6 +135,7 @@ typedef struct HbaLine
 	char	   *radiusidentifiers_s;
 	List	   *radiusports;
 	char	   *radiusports_s;
+	bool		radiusrequirema;
 } HbaLine;
 
 typedef struct IdentLine
-- 
2.39.3 (Apple Git-146)

