From: czichy <thoralf.czichy@nokia.com>
Date: Mon, 11 Dec 2023 15:24:44 +0000 (+0200)
Subject: fixing RMR messages with negative size
X-Git-Tag: 4.9.4~4
X-Git-Url: https://gerrit.o-ran-sc.org/r/gitweb?a=commitdiff_plain;h=9819507472caa5906a7047b38f50a94d1931ea26;hp=472609280e241527e51128781603f0a340ffe9cc;p=ric-plt%2Flib%2Frmr.git

fixing RMR messages with negative size

This fixes an issue in RMR when receiving messages that in their
header have a negative length. We now mark the connection
as to be reset, i.e, it becomes unusable once there was a
message sent with a wrong header length.

Issue-ID: RIC-989

Change-Id: I1e6e958d1f3977b2caf15477d8381aeda3c77822
Signed-off-by: czichy <thoralf.czichy@nokia.com>
---

diff --git a/ci/ci_build.ksh b/ci/ci_build.ksh
index 8c3f699..9ee710d 100644
--- a/ci/ci_build.ksh
+++ b/ci/ci_build.ksh
@@ -112,7 +112,7 @@ mkdir -p .build
 	cd test                 		# execute tests
 	ksh unit_test.ksh				# unit tests first
 	cd app_test
-	ksh run_all.ksh					# application based tests if units pass
+	ksh run_all.sh					# application based tests if units pass
 )
 
 printf "---\nfiles:\n" >$yaml_file 	# initialise the yaml file
diff --git a/src/rmr/si/src/mt_call_si_static.c b/src/rmr/si/src/mt_call_si_static.c
index b523c45..78a1039 100644
--- a/src/rmr/si/src/mt_call_si_static.c
+++ b/src/rmr/si/src/mt_call_si_static.c
@@ -199,8 +199,13 @@ static int mt_data_cb( void* vctx, int fd, char* buf, int buflen ) {
 			river->accum = (char *) malloc( river->nbytes );
 			river->ipt = 0;
 		} else {
-			// future -- sync to next marker
-			river->ipt = 0;						// insert point
+			if( river->state == RS_RESET ) {
+				// future -- reset not implemented
+				return SI_RET_OK;
+			} else {
+				// future -- sync to next marker
+				river->ipt = 0;						// insert point
+			}
 		}
 	}
 
@@ -238,6 +243,12 @@ static int mt_data_cb( void* vctx, int fd, char* buf, int buflen ) {
 			} else {
 				river->msg_size = extract_mlen( &buf[bidx] );			// pull from buf as it's all there; it will copy later
 			}
+
+                        if( river->msg_size < 0) { // addressing RIC-989
+                                river->state=RS_RESET;
+                        	return SI_RET_OK;
+                        }
+
 			if( DEBUG ) rmr_vlog( RMR_VL_DEBUG, "data callback setting msg size: %d\n", river->msg_size );
 
 			if( river->msg_size > river->nbytes ) {						// message bigger than app max size; grab huge buffer
diff --git a/test/app_test/create_invalid_request.sh b/test/app_test/create_invalid_request.sh
new file mode 100644
index 0000000..50e7837
--- /dev/null
+++ b/test/app_test/create_invalid_request.sh
@@ -0,0 +1,33 @@
+#!/bin/bash
+
+# byte number 1-4 define the size of the RMR package
+# we define here a negativbe size as we had a bug for this (RIC-989)
+
+echo -n -e \
+"\xa2\x01\xFF\xFF\x00\x00\x01\xa2\x24\x00\x00\x00\x00\x7f" \
+"\x00\x00\xf0\x70\x71\x32\x65\x55\x00\x00\xf0\x70\x71\x32\x65\x55" \
+"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\xff\xff\x7f\x00" \
+"\x00\x01\x00\x18\x00\x00\x00\x00\x00\x00\x00\x49\x00\x00\x00\x03" \
+"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" \
+"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" \
+"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" \
+"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" \
+"\x6f\x73\x63\x33\x3a\x34\x33\x30\x38\x36\x00\x00\x00\x00\x00\x00" \
+"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" \
+"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" \
+"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" \
+"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" \
+"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" \
+"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" \
+"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x18\x00\x00\x00\x0b" \
+"\x00\x00\x00\x04\x00\x00\x00\x00\x00\x00\x00\x00\x31\x39\x32\x2e" \
+"\x31\x36\x38\x2e\x30\x2e\x31\x38\x3a\x34\x33\x30\x38\x36\x00\x00" \
+"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" \
+"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" \
+"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x31\x37\x30\x32" \
+"\x32\x39\x33\x39\x34\x31\x00\x00\x00\x00\x00\x31\x31\x30\x20\x35" \
+"\x33\x7c\x63\x6f\x75\x6e\x74\x3d\x30\x20\x74\x72\x3d\x31\x37\x30" \
+"\x32\x32\x39\x33\x39\x34\x31\x20\x31\x38\x30\x34\x32\x38\x39\x33" \
+"\x38\x33\x20\x73\x74\x61\x6e\x64\x20\x75\x70\x20\x61\x6e\x64\x20" \
+"\x63\x68\x65\x65\x72\x21\x3e\x6f\x73\x63\x33\x2d\x33\x38\x33\x35" \
+"\x31\x30\x36\x00" >/tmp/invalid_data.bin
diff --git a/test/app_test/create_valid_request.sh b/test/app_test/create_valid_request.sh
new file mode 100644
index 0000000..daad39f
--- /dev/null
+++ b/test/app_test/create_valid_request.sh
@@ -0,0 +1,33 @@
+#!/bin/bash
+
+# this should be a valid RMR request
+#
+
+echo -n -e \
+"\xa2\x01\x00\x00\x00\x00\x01\xa2\x24\x00\x00\x00\x00\x7f" \
+"\x00\x00\xf0\x70\x71\x32\x65\x55\x00\x00\xf0\x70\x71\x32\x65\x55" \
+"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\xff\xff\x7f\x00" \
+"\x00\x01\x00\x18\x00\x00\x00\x00\x00\x00\x00\x49\x00\x00\x00\x03" \
+"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" \
+"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" \
+"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" \
+"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" \
+"\x6f\x73\x63\x33\x3a\x34\x33\x30\x38\x36\x00\x00\x00\x00\x00\x00" \
+"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" \
+"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" \
+"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" \
+"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" \
+"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" \
+"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" \
+"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x18\x00\x00\x00\x0b" \
+"\x00\x00\x00\x04\x00\x00\x00\x00\x00\x00\x00\x00\x31\x39\x32\x2e" \
+"\x31\x36\x38\x2e\x30\x2e\x31\x38\x3a\x34\x33\x30\x38\x36\x00\x00" \
+"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" \
+"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" \
+"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x31\x37\x30\x32" \
+"\x32\x39\x33\x39\x34\x31\x00\x00\x00\x00\x00\x31\x31\x30\x20\x35" \
+"\x33\x7c\x63\x6f\x75\x6e\x74\x3d\x30\x20\x74\x72\x3d\x31\x37\x30" \
+"\x32\x32\x39\x33\x39\x34\x31\x20\x31\x38\x30\x34\x32\x38\x39\x33" \
+"\x38\x33\x20\x73\x74\x61\x6e\x64\x20\x75\x70\x20\x61\x6e\x64\x20" \
+"\x63\x68\x65\x65\x72\x21\x3e\x6f\x73\x63\x33\x2d\x33\x38\x33\x35" \
+"\x31\x30\x36\x00" >/tmp/valid_data.bin
diff --git a/test/app_test/run_all.sh b/test/app_test/run_all.sh
index c905140..63c347a 100644
--- a/test/app_test/run_all.sh
+++ b/test/app_test/run_all.sh
@@ -162,6 +162,9 @@ then
 	build=""
 fi
 
+echo "----- testing RIC-989 --------"
+run_test run_neg_rmr_size_test.sh -v $installed $build
+
 echo "----- multi ------------------"
 run_test run_multi_test.sh  $build
 
diff --git a/test/app_test/run_neg_rmr_size_test.sh b/test/app_test/run_neg_rmr_size_test.sh
new file mode 100644
index 0000000..da69f27
--- /dev/null
+++ b/test/app_test/run_neg_rmr_size_test.sh
@@ -0,0 +1,260 @@
+#!/bin/bash
+#/usr/bin/env ksh
+# vim: ts=4 sw=4 noet :
+#==================================================================================
+#    Copyright (c) 2019-2021 Nokia
+#    Copyright (c) 2018-2021 AT&T Intellectual Property.
+#
+#   Licensed under the Apache License, Version 2.0 (the "License");
+#   you may not use this file except in compliance with the License.
+#   You may obtain a copy of the License at
+#
+#       http://www.apache.org/licenses/LICENSE-2.0
+#
+#   Unless required by applicable law or agreed to in writing, software
+#   distributed under the License is distributed on an "AS IS" BASIS,
+#   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#   See the License for the specific language governing permissions and
+#   limitations under the License.
+#==================================================================================
+#
+
+# ---------------------------------------------------------------------------------
+#	Mnemonic:	run_app_test.ksh
+#	Abstract:	This is a simple script to set up and run the basic send/receive
+#				processes for some library validation on top of nng.
+#				It should be possible to clone the repo, switch to this directory
+#				and execute  'ksh run -B'  which will build RMr, make the sender and
+#				recevier then  run the basic test.
+#
+#				Example command line:
+#					ksh ./run_app_test.ksh		# default 20 messages at 2 msg/sec
+#					ksh ./run_app_test.ksh -d 100 -n 10000 # send 10k messages with 100ms delay between
+#
+#	Date:		22 April 2019
+#	Author:		E. Scott Daniels
+# ---------------------------------------------------------------------------------
+
+
+# The sender and receiver are run asynch. Their exit statuses are captured in a
+# file in order for the 'main' to pick them up easily.
+#
+function run_sender {
+
+#	first we send an invalid message with a negative message length 
+#	as per RIC-989 / CVE-2023-40998 processes used to  crash for such 
+#	invalid size data. So this test case would fail with a crash
+#	if for some reason the bug comes back.
+#
+  bash ./create_invalid_request.sh
+  cat /tmp/invalid_data.bin |netcat -w 2 127.0.0.1 4560
+	
+	echo $? >/tmp/PID$$.src		# must communicate state back via file b/c asynch
+
+  
+#      now let's send still one valid message over another TCP 
+#      connection
+  bash ./create_valid_request.sh
+  cat /tmp/valid_data.bin|netcat -w 2 127.0.0.1 4560
+
+}
+
+function run_rcvr {
+	export RMR_LOG_VLEVEL=5
+	if (( mt_receiver ))
+	then
+		echo "<TEST> testing with mt-receiver" >&2
+		./mt_receiver${si} $nmsg
+	else
+		./receiver${si} $nmsg
+	fi
+	echo $? >/tmp/PID$$.rrc
+}
+
+# snarf the first v4 IP (not the loopback) that belongs to this box/container/guest
+function snarf_ip {
+	ip addr| sed -e '/inet /b c; d' -e ':c' -e '/127.0.0.1/d; s!/.*!!; s!^.* !!; q'
+}
+
+#	Drop a contrived route table in. This table should add a reference to our 
+#	local IP to an entry to ensure that the route table collector code in RMr
+#	is removing 'hairpin' loops. If RMr isn't removing the references to our
+#	hostname and IP address when it builds the endpoint lists, the sender will
+#	send messages to itself some of the time, causing the receiver to come up 
+#	short when comparing messages received with expected count and thus failing.
+#
+function set_rt {
+	typeset port=4560			# port the receiver listens on by default
+
+	cat <<endKat >app_test.rt
+		newrt | start
+			mse | 0 |  0 | 127.0.0.1:$port,$my_ip:43086
+			mse | 1 | 10 | 127.0.0.1:$port,${my_host//.*/}:43086
+			mse | 2 | 20 | 127.0.0.1:$port
+			rte | 3 | 127.0.0.1:$port
+			mse | 3 | 100 | 127.0.0.1:$port	# special test to ensure that this does not affect previous entry
+			rte | 4 | 127.0.0.1:$port
+			rte | 5 | 127.0.0.1:$port
+			rte | 6 | 127.0.0.1:$port
+			rte | 7 | 127.0.0.1:$port
+			rte | 8 | 127.0.0.1:$port
+			rte | 9 | 127.0.0.1:$port
+			rte | 10 | 127.0.0.1:$port
+			rte | 11 | 127.0.0.1:$port
+			rte | 12 | 127.0.0.1:$port
+			rte | 13 | 127.0.0.1:$port
+		newrt | end
+
+head -3 app_test.rt
+
+endKat
+
+}
+
+# ---------------------------------------------------------
+
+nmsg=1						# total number of messages to be exchanged (-n value changes)
+							# need two sent to each receiver to ensure hairpin entries were removed (will fail if they were not)
+delay=100000				# microsec sleep between msg 1,000,000 == 1s
+wait=1
+rebuild=0
+nopull=""					# -b sets so that build does not pull
+verbose=0
+use_installed=0
+my_ip=$(snarf_ip)			# get an ip to insert into the route table
+keep=0
+mt_receiver=0				# -m sets in order to test with multi-threaded receive stuff
+force_make=0
+
+echo --------------------
+echo running test for negative message size
+echo --------------------
+
+
+while [[ $1 == -* ]]
+do
+	case $1 in
+		-B)	rebuild=1;;						# build with pull first
+		-b)	rebuild=1; nopull="nopull";;	# buld without pull
+		-d)	delay=$2; shift;;
+		-k) keep=1;;
+		-i) use_installed=1;;
+		-M)	force_make=1;;
+		-m)	mt_receiver=1;;
+		-n)	nmsg=$2; shift;;
+		-N)	;;							# ignored for back compat; NNG not supported; all binaries are si95
+		-S)	;;							# ignored for back compat; all binaries are si95 and no _si suffix is used
+		-v)	verbose=1;;
+
+		*)	echo "unrecognised option: $1"
+			echo "usage: $0 [-B] [-d micor-sec-delay] [-i] [-k] [-M] [-m] [-n num-msgs] [-S]"
+			echo "  -B forces an RMR rebuild which will use .build"
+			echo "  -i causes the installd libraries (/usr/local) to be referenced; -B is ignored if supplied"
+			echo "  -k keeps the route table"
+			echo "  -M force make on test applications"
+			echo "  -m test with mt-receive mode"
+			echo "  -S build/test SI95 based binaries"
+			echo ""
+			echo "total number of messages must > 20 to correctly test hairpin loop removal"
+			exit 1
+			;;
+	esac
+
+	shift
+done
+
+if [[ ! -f app_test.rt ]]		# we need the real host name in the local.rt; build one from mask if not there
+then
+	my_host=$(hostname)
+	set_rt
+	if (( verbose ))
+	then
+		cat app_test.rt
+	fi
+fi
+
+if (( verbose ))
+then
+	echo "4" >.verbose
+	export RMR_VCTL_FILE=".verbose"
+fi
+
+
+if (( use_installed ))			# point at installed library rather than testing the build
+then
+	export LD_LIBRARY_PATH=/usr/local/lib
+	export LIBRARY_PATH=$LD_LIBRARY_PATH
+else
+	if (( rebuild ))
+	then
+		set -e
+		$SHELL ./rebuild.ksh $nopull | read build_path
+		set +e
+	else
+		build_path=${BUILD_PATH:-"../../.build"}	# we prefer .build at the root level, but allow user option
+	
+		if [[ ! -d $build_path ]]
+		then
+			echo "[FAIL] cannot find build in: $build_path"
+			echo "[FAIL] either create, and then build RMr, or set BUILD_PATH as an evironment var before running this"
+			exit 1
+		fi
+	fi
+
+	if [[ -z $LD_LIBRARY_PATH ]]		# the cmake test environment will set this; we must use if set
+	then
+		if [[ -d $build_path/lib64 ]]
+		then
+			export LD_LIBRARY_PATH=$build_path:$build_path/lib64
+		else
+			export LD_LIBRARY_PATH=$build_path:$build_path/lib
+		fi
+	fi
+	export LIBRARY_PATH=$LD_LIBRARY_PATH
+fi
+
+export RMR_SEED_RT=${RMR_SEED_RT:-./app_test.rt}		# allow easy testing with different rt
+
+if (( force_make )) || [[ ! -f ./sender${si} || ! -f ./receiver${si} ]]
+then
+	if ! make -B sender${si} receiver${si} >/tmp/PID$$.clog 2>&1
+	then
+		echo "[FAIL] cannot find sender${si} and/or receiver${si} binary, and cannot make them.... humm?"
+		echo "[INFO] ------------- PATH settings -----------------"
+		env | grep PATH
+		echo "[INFO] ------------- compiler output (head) -----------------"
+		head -50 /tmp/PID$$.clog
+		rm -fr /tmp/PID$$.*
+		exit 1
+	fi
+fi
+
+#exit
+run_rcvr &
+sleep 2				# if sender starts faster than rcvr we can drop msgs, so pause a bit
+netstat -nptl|grep receiver 
+netstat -nptl|grep sender 
+
+run_sender &
+
+wait
+head -1 /tmp/PID$$.rrc | read rrc
+head -1 /tmp/PID$$.src | read src
+
+if (( !! (src + rrc) ))
+then
+	echo "[FAIL] sender rc=$src  receiver rc=$rrc"
+else
+	echo "[PASS] sender rc=$src  receiver rc=$rrc"
+fi
+
+if (( ! keep )) 
+then
+	rm app_test.rt
+fi
+
+rm /tmp/PID$$.*
+#rm -f .verbose
+
+exit $(( !! (src + rrc) ))
+