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 (file)
--- 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 (file)
--- 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 (file)
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 (file)
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 (file)
--- 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 (file)
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) ))
+