[PATCH] net: tftp: fix option validation as per RFCs

[Ravik Hasija]

RFC2348, RFC2349:
- Option string is case in-sensitive.
- Client must generate ERR pkt in case option value mismatch in server OACK
- Fix debug print for options

Signed-off-by: Ravik Hasija <rahasij at linux.microsoft.com>
---

 net/tftp.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 52 insertions(+), 7 deletions(-)

diff --git a/net/tftp.c b/net/tftp.c
index be24e63075..14621e2441 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -68,6 +68,7 @@ enum {
 	TFTP_ERR_UNEXPECTED_OPCODE   = 4,
 	TFTP_ERR_UNKNOWN_TRANSFER_ID  = 5,
 	TFTP_ERR_FILE_ALREADY_EXISTS = 6,
+	TFTP_ERR_OPTION_NEGOTIATION = 8,
 };
 
 static struct in_addr tftp_remote_ip;
@@ -111,6 +112,7 @@ static int	tftp_put_final_block_sent;
 #define STATE_OACK	5
 #define STATE_RECV_WRQ	6
 #define STATE_SEND_WRQ	7
+#define STATE_INVALID_OPTION	8
 
 /* default TFTP block size */
 #define TFTP_BLOCK_SIZE		512
@@ -313,6 +315,7 @@ static void tftp_send(void)
 	uchar *xp;
 	int len = 0;
 	ushort *s;
+	bool err_pkt = false;
 
 	/*
 	 *	We will always be sending some sort of packet, so
@@ -383,6 +386,7 @@ static void tftp_send(void)
 		strcpy((char *)pkt, "File too large");
 		pkt += 14 /*strlen("File too large")*/ + 1;
 		len = pkt - xp;
+		err_pkt = true;
 		break;
 
 	case STATE_BAD_MAGIC:
@@ -394,11 +398,28 @@ static void tftp_send(void)
 		strcpy((char *)pkt, "File has bad magic");
 		pkt += 18 /*strlen("File has bad magic")*/ + 1;
 		len = pkt - xp;
+		err_pkt = true;
+		break;
+
+	case STATE_INVALID_OPTION:
+		xp = pkt;
+		s = (ushort *)pkt;
+		*s++ = htons(TFTP_ERROR);
+		*s++ = htons(TFTP_ERR_OPTION_NEGOTIATION);
+		pkt = (uchar *)s;
+		strcpy((char *)pkt, "Option Negotiation Failed");
+		/* strlen("Option Negotiation Failed") + NULL*/
+		pkt += 25 + 1;
+		len = pkt - xp;
+		err_pkt = true;
 		break;
 	}
 
 	net_send_udp_packet(net_server_ethaddr, tftp_remote_ip,
 			    tftp_remote_port, tftp_our_port, len);
+
+	if (err_pkt)
+		net_set_state(NETLOOP_FAIL);
 }
 
 #ifdef CONFIG_CMD_TFTPPUT
@@ -419,6 +440,7 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
 	__be16 proto;
 	__be16 *s;
 	int i;
+	u16 timeout_val_rcvd;
 
 	if (dest != tftp_our_port) {
 			return;
@@ -475,8 +497,14 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
 #endif
 
 	case TFTP_OACK:
-		debug("Got OACK: %s %s\n",
-		      pkt, pkt + strlen((char *)pkt) + 1);
+		debug("Got OACK: ");
+		for (i = 0; i < len; i++) {
+			if (pkt[i] == '\0')
+				debug(" ");
+			else
+				debug("%c", pkt[i]);
+		}
+		debug("\n");
 		tftp_state = STATE_OACK;
 		tftp_remote_port = src;
 		/*
@@ -485,15 +513,32 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
 		 * something like "len-8" may give a *huge* number
 		 */
 		for (i = 0; i+8 < len; i++) {
-			if (strcmp((char *)pkt + i, "blksize") == 0) {
+			if (strcasecmp((char *)pkt + i, "blksize") == 0) {
 				tftp_block_size = (unsigned short)
 					simple_strtoul((char *)pkt + i + 8,
 						       NULL, 10);
-				debug("Blocksize ack: %s, %d\n",
+				debug("Blocksize oack: %s, %d\n",
 				      (char *)pkt + i + 8, tftp_block_size);
+				if (tftp_block_size > tftp_block_size_option) {
+					printf("Invalid blk size(=%d)\n",
+					       tftp_block_size);
+					tftp_state = STATE_INVALID_OPTION;
+				}
+			}
+			if (strcasecmp((char *)pkt + i, "timeout") == 0) {
+				timeout_val_rcvd = (unsigned short)
+					simple_strtoul((char *)pkt + i + 8,
+						       NULL, 10);
+				debug("Timeout oack: %s, %d\n",
+				      (char *)pkt + i + 8, timeout_val_rcvd);
+				if (timeout_val_rcvd != (timeout_ms / 1000)) {
+					printf("Invalid timeout val(=%d s)\n",
+					       timeout_val_rcvd);
+					tftp_state = STATE_INVALID_OPTION;
+				}
 			}
 #ifdef CONFIG_TFTP_TSIZE
-			if (strcmp((char *)pkt+i, "tsize") == 0) {
+			if (strcasecmp((char *)pkt + i, "tsize") == 0) {
 				tftp_tsize = simple_strtoul((char *)pkt + i + 6,
 							   NULL, 10);
 				debug("size = %s, %d\n",
@@ -502,7 +547,7 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
 #endif
 		}
 #ifdef CONFIG_CMD_TFTPPUT
-		if (tftp_put_active) {
+		if (tftp_put_active && tftp_state == STATE_OACK) {
 			/* Get ready to send the first block */
 			tftp_state = STATE_DATA;
 			tftp_cur_block++;
@@ -519,7 +564,7 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
 		update_block_number();
 
 		if (tftp_state == STATE_SEND_RRQ)
-			debug("Server did not acknowledge timeout option!\n");
+			debug("Server did not acknowledge any options!\n");
 
 		if (tftp_state == STATE_SEND_RRQ || tftp_state == STATE_OACK ||
 		    tftp_state == STATE_RECV_WRQ) {
-- 
2.17.1