From: Alan T. DeKok <aland@freeradius.org>
Date: Thu, 8 Apr 2021 12:45:09 +0000 (-0400)
Subject: attributes with 'encrypt=2' can only appear in certain packets
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=68cd2a21cfb615a746f4945fea7f07402e9a76db;p=thirdparty%2Ffreeradius-server.git

attributes with 'encrypt=2' can only appear in certain packets
---

diff --git a/src/protocols/radius/base.c b/src/protocols/radius/base.c
index 80d0a9499e..41e8079f05 100644
--- a/src/protocols/radius/base.c
+++ b/src/protocols/radius/base.c
@@ -928,13 +928,14 @@ ssize_t fr_radius_encode_dbuff(fr_dbuff_t *dbuff, uint8_t const *original,
 	switch (code) {
 	case FR_RADIUS_CODE_ACCESS_REQUEST:
 	case FR_RADIUS_CODE_STATUS_SERVER:
+		packet_ctx.disallow_tunnel_passwords = true;
+
 		/*
-		 * Callers in these cases have preloaded the buffer with the authentication vector.
+		 *	Callers in these cases have preloaded the buffer with the authentication vector.
 		 */
 		FR_DBUFF_OUT_MEMCPY_RETURN(packet_ctx.vector, &work_dbuff, sizeof(packet_ctx.vector));
 		break;
 
-	case FR_RADIUS_CODE_ACCESS_ACCEPT:
 	case FR_RADIUS_CODE_ACCESS_REJECT:
 	case FR_RADIUS_CODE_ACCESS_CHALLENGE:
 	case FR_RADIUS_CODE_ACCOUNTING_RESPONSE:
@@ -943,6 +944,10 @@ ssize_t fr_radius_encode_dbuff(fr_dbuff_t *dbuff, uint8_t const *original,
 	case FR_RADIUS_CODE_DISCONNECT_ACK:
 	case FR_RADIUS_CODE_DISCONNECT_NAK:
 	case FR_RADIUS_CODE_PROTOCOL_ERROR:
+		packet_ctx.disallow_tunnel_passwords = true;
+		FALL_THROUGH;
+
+	case FR_RADIUS_CODE_ACCESS_ACCEPT:
 		if (!original) {
 			fr_strerror_const("Cannot encode response without request");
 			return -1;
@@ -952,8 +957,20 @@ ssize_t fr_radius_encode_dbuff(fr_dbuff_t *dbuff, uint8_t const *original,
 		break;
 
 	case FR_RADIUS_CODE_ACCOUNTING_REQUEST:
-	case FR_RADIUS_CODE_COA_REQUEST:
 	case FR_RADIUS_CODE_DISCONNECT_REQUEST:
+		packet_ctx.disallow_tunnel_passwords = true;
+		FALL_THROUGH;
+
+		/*
+		 *	Tunnel-Password encoded attributes are allowed
+		 *	in CoA-Request packets, by RFC 5176 Section
+		 *	3.6.  HOWEVER, the tunnel passwords are
+		 *	"encrypted" using the Request Authenticator,
+		 *	which is all zeros!  That makes them much
+		 *	easier to decrypt.  The only solution here is
+		 *	to say "don't do that!"
+		 */
+	case FR_RADIUS_CODE_COA_REQUEST:
 		memset(packet_ctx.vector, 0, sizeof(packet_ctx.vector));
 		FR_DBUFF_MEMSET_RETURN(&work_dbuff, 0, RADIUS_AUTH_VECTOR_LENGTH);
 		break;
diff --git a/src/protocols/radius/encode.c b/src/protocols/radius/encode.c
index fe9397e97c..f8bb7dd1a3 100644
--- a/src/protocols/radius/encode.c
+++ b/src/protocols/radius/encode.c
@@ -569,6 +569,11 @@ static ssize_t encode_value(fr_dbuff_t *dbuff,
 
 	case FLAG_TAGGED_TUNNEL_PASSWORD:
 	case FLAG_ENCRYPT_TUNNEL_PASSWORD:
+		if (packet_ctx->disallow_tunnel_passwords) {
+			fr_strerror_const("Attributes with 'encrypt=2' set cannot go into this packet.");
+			return PAIR_ENCODE_SKIPPED;
+		}
+
 		/*
 		 *	Always encode the tag even if it's zero.
 		 *
diff --git a/src/protocols/radius/radius.h b/src/protocols/radius/radius.h
index 025c703f2b..96f8b39975 100644
--- a/src/protocols/radius/radius.h
+++ b/src/protocols/radius/radius.h
@@ -172,7 +172,8 @@ typedef struct {
 	char const		*secret;		//!< shared secret.  MUST be talloc'd
 	fr_fast_rand_t		rand_ctx;		//!< for tunnel passwords
 	int			salt_offset;		//!< for tunnel passwords
-	bool 			tunnel_password_zeros;
+	bool 			tunnel_password_zeros;  //!< check for trailing zeros on decode
+	bool			disallow_tunnel_passwords; //!< not all packets can have tunnel passwords
 
 	uint8_t			tag;			//!< current tag for encoding
 	fr_radius_tag_ctx_t    	**tags;			//!< for decoding tagged attributes