horms at verge
Aug 6, 2008, 2:16 AM
Post #1 of 1
(424 views)
Permalink
|
|
[patch] safely create tempoary files
|
|
After my previous post, I realised that there are lots of
other places in the code where mktemp is used and more importantly,
alternate unsafe code is used. So I have had a crack at cleaning things up.
------------------------------------------------------------------------
There are currently at least two problems with maketempfile.
Firstly, there is a race in the following constrct:
rm -f "$F"; touch "$F"
As an attacker could potitinally create a symlink to "$F" between
the call to rm and the call to touch.
Secondly the use of $RANDOM appears to be a bashism.
On dash its usage in BasicSanityCheck appears to evaluate to
the empty string. See Debian Bug #489607, http://bugs.debian.org/489607
This patch takes the approach of using mkdir, which is atomic,
to create a safe place to store the logfile the tempoary directory
that is used by the filesystem check.
The patch also makes sure that the return value is checked
and the script exits cleanly if an error occurs.
This code is always used, there seems no point is providing
a robust fallback to mktemp - if its robust, it can be used :-)
For a discussion of creating tempoary files in shell see
http://www.linuxsecurity.com/content/view/115462/81/
Signed-off-by: Simon Horman <horms [at] verge>
---
Please note that this patch adds the inclusion of ha.d/shellfuncs to
tsa_plugin/linuxha-adapter.in. It would also benifit from
doing the same to BasicSanityCheck.
Index: heartbeat/heartbeat/lib/BasicSanityCheck.in
===================================================================
--- heartbeat.orig/heartbeat/lib/BasicSanityCheck.in 2008-08-06 18:08:05.000000000 +1000
+++ heartbeat/heartbeat/lib/BasicSanityCheck.in 2008-08-06 18:50:19.000000000 +1000
@@ -49,7 +49,6 @@ CRMTEST="@PYTHON@ $SCRIPTDIR/cts/CTSlab.
SNMPAGENTTEST=$SCRIPTDIR/SNMPAgentSanityCheck
BASE64_MD5_TEST=$HBLIB/base64_md5_test
MALLOC_CHECK_=2; export MALLOC_CHECK_
-MKTEMP=@MKTEMP@
TESTPROG=@TEST@
#
IDENTSTRING="Linux-HA TEST configuration file - REMOVEME!!"
@@ -64,35 +63,28 @@ SIGLIST="0 1 2 3 6 15"
errcount=0
-# Make temp files the paranoid way...
-maketempfile() {
-#
-# Use mktemp if we have it, otherwise...
-#
-# Construct a difficult-to-guess filename if we don't
-# Make sure non-mktemp files can't be subverted
-# $RANDOM is not strictly necessary, but nice to have...
-#
- if
- test "x$MKTEMP" != "x" \
- && F=`$MKTEMP /tmp/lha-XXXXXX` && [ ! -z "$F" -a -f "$F" ]
- then
- echo $F
- else
- while
- echo >/dev/null &
- F=/tmp/lha-${RANDOM}-$$-$!
- rm -f "$F"; touch "$F"
- # Try again if we don't own it, or it's a symlink
- # Or somehow not a regular file...
- $TESTPROG ! -O "$F" -o -L "$F" -o ! -f "$F"
- do
- : Try again...
- done
- echo $F
- fi
+# Make a safe place to store logs
+maketempdir()
+{
+ local i=0
+ local tmp
+
+ while [ $i -gt 0 ]; do
+ tmp="/tmp/lha-dir-$$-$i"
+ if (umask 077 && mkdir "$tmp"); then
+ echo "$tmp"
+ return 0
+ fi
+ i=$((i+1))
+ done
+
+ echo "Could not create tempoary directory to store logs" >& 2
+ return 1
}
-LOGFILE=`maketempfile`
+
+LOGDIR=`maketempdir` || exit 1
+LOGFILE="$LOGDIR/log"
+touch "$LOGFILE"
cd $HADIR
ulimit -c unlimited
@@ -328,6 +320,7 @@ SaveLog() {
SAVELOG=/tmp/linux-ha.testlog
chmod a+r $LOGFILE
mv $LOGFILE $SAVELOG
+ rm -rf "$LOGDIR"
echo "$errcount errors. Log file is stored in $SAVELOG"
}
@@ -985,7 +978,8 @@ TestRA() {
fi
if [ `uname -s` = 'Linux' ]; then
- MNT_DIR=`@MKTEMP@ -d /tmp/lha-dir-XXXXXXXXXXX`
+ MNT_DIR="$LOGDIR/mnt"
+ mkdir "$MNT_DIR"
echo "Testing RA: Filesystem" | tee -a $LOGFILE
$OCF_TESTER -o device=/dev/null -o fstype=proc -o directory=$MNT_DIR \
-n DemoFS $RADIR/Filesystem >>$LOGFILE 2>&1
Index: heartbeat/heartbeat/shellfuncs.in
===================================================================
--- heartbeat.orig/heartbeat/shellfuncs.in 2008-08-06 18:08:05.000000000 +1000
+++ heartbeat/heartbeat/shellfuncs.in 2008-08-06 18:08:18.000000000 +1000
@@ -30,7 +30,6 @@ export HA_DIR HA_RCDIR HA_FIFO HA_BIN
export HA_DEBUGLOG HA_LOGFILE HA_LOGFACILITY
export HA_DATEFMT HA_RESOURCEDIR HA_DOCDIR
-MKTEMP=@MKTEMP@
TESTPROG=@TEST@
PATH=$HA_BIN:${HA_SBIN_DIR}:@HA_NOARCHDATAHBDIR@:$PATH
PATH=`echo $PATH | sed -e 's%::%%' -e 's%:\.:%:%' -e 's%^:%%' -e 's%^\.:%%'`
@@ -226,33 +225,98 @@ ha_pseudo_resource()
esac
}
-# Make temp files the paranoid way...
-maketempfile() {
-#
-# Use mktemp if we have it, otherwise...
-#
-# Construct a difficult-to-guess filename if we don't
-# Make sure non-mktemp files can't be subverted
-# $RANDOM is not strictly necessary, but nice to have...
-#
- if
- test "x$MKTEMP" != "x" \
- && F=`$MKTEMP /tmp/lha-XXXXXX` && [ ! -z "$F" -a -f "$F" ]
- then
- echo $F
- else
- while
- echo >/dev/null &
- F=/tmp/lha-${RANDOM}-$$-$!
- rm -f "$F"; touch "$F"
- # Try again if we don't own it, or it's a symlink
- # Or somehow not a regular file...
- $TESTPROG ! -O "$F" -o -L "$F" -o ! -f "$F"
- do
- : Try again...
- done
- echo $F
- fi
+# usage: dirname DIR
+dirname()
+{
+ local a
+ local b
+
+ [ $# = 1 ] || return 1
+ a="$1"
+ while [ 1 ]; do
+ b="${a%/}"
+ [ "$a" = "$b" ] && break
+ a="$b"
+ done
+ b=${a%/*}
+ [ -z "$b" -o "$a" = "$b" ] && b="."
+
+ echo "$b"
+ return 0
+}
+
+# usage: maketempdir
+maketempdir()
+{
+ local i=1
+ local tmp
+
+ # The loop allows for the cases where
+ # A process has multiple tempfiles or dirs or;
+ # A previous process with the same PID that had tempfiles or
+ # directories did not exit cleanly
+ while [ $i -gt 0 ]; do
+ tmp="/tmp/lha-dir-$$-$i"
+ if (umask 077 && mkdir "$tmp" 2>/dev/null); then
+ echo "$tmp"
+ return 0
+ fi
+ i=$((i+1))
+ done
+
+ echo "Could not create tempoary directory to store logs" >& 2
+ return 1
+}
+
+# usage: rmtempdir TMPDIR
+rmtempdir()
+{
+ [ $# = 1 ] || return 1
+ if [ -e "$1" ]; then
+ rmdir "$1" || return 1
+ fi
+ return 0
+}
+
+# usage: maketempfile [-d]
+maketempfile()
+{
+ local dir="no"
+ local name
+
+ if [ $# = 1 -a "$1" = "-d" ]; then
+ dir="yes"
+ elif [ $# != 0 ]; then
+ return 1
+ fi
+
+ name=$(maketempdir) || return $?
+
+ if [ "$dir" = "yes" ]; then
+ echo "$name"
+ return 0
+ fi
+
+ name="$name/tmpfile"
+
+ if ! touch "$name"; then
+ rmtempdir
+ return 1
+ fi
+
+ echo "$name"
+ return 0
+}
+
+# usage: rmtempfile TMPFILE
+rmtempfile ()
+{
+ [ $# = 1 ] || return 1
+ if [ -e "$1" ]; then
+ rm "$1" || return 1
+ fi
+ rmtempdir $(dirname "$1") || return 1
+ return 0
}
# The 'ping' command takes highly OS-dependent arguments, so this
Index: heartbeat/lib/plugins/stonith/external/hmchttp.in
===================================================================
--- heartbeat.orig/lib/plugins/stonith/external/hmchttp.in 2008-08-06 18:08:05.000000000 +1000
+++ heartbeat/lib/plugins/stonith/external/hmchttp.in 2008-08-06 18:42:42.000000000 +1000
@@ -28,9 +28,8 @@
#set -x
hostlist=`echo $hostlist | tr ',' ' '`
-COOKIEFILE=`maketempfile`
-#trap "cat $COOKIEFILE >&2; rm -f $COOKIEFILE" EXIT
-trap "rm -f $COOKIEFILE" EXIT
+trap '[ ! -e "$COOKIEFILE" ] || rmtempfile "$COOKIEFILE"' 0
+COOKIEFILE=`maketempfile` || exit 1
: ${CURLBIN="/usr/bin/curl"}
: ${user=admin}
Index: heartbeat/lib/plugins/stonith/external/ibmrsa.in
===================================================================
--- heartbeat.orig/lib/plugins/stonith/external/ibmrsa.in 2008-08-06 18:08:05.000000000 +1000
+++ heartbeat/lib/plugins/stonith/external/ibmrsa.in 2008-08-06 18:44:27.000000000 +1000
@@ -28,8 +28,8 @@
#echo "started with: $@" | debug
. @HA_HBCONF_DIR@/shellfuncs
-outf=`maketempfile`
-trap "cat $outf >&2; rm -f $outf" EXIT
+trap 'if [ -n "$outf" ]; then cat "$outf" >&2; rmtempfile "$outf"; fi' 0
+outf=`maketempfile` || exit 1
chkmpcli() {
test -x /opt/IBMmpcli/bin/MPCLI.sh
Index: heartbeat/resources/OCF/WAS
===================================================================
--- heartbeat.orig/resources/OCF/WAS 2008-08-06 18:08:05.000000000 +1000
+++ heartbeat/resources/OCF/WAS 2008-08-06 18:45:20.000000000 +1000
@@ -325,7 +325,8 @@ WAS_report_status() {
# This is actually faster than WAS_status above...
#
WAS_monitor() {
- tmpfile=`maketempfile`
+ trap '[ -z "$tmpfile" ] || rmtempfile "$tmpfile"' 0
+ tmpfile=`maketempfile` || return 1
SnoopPort=`GetWASSnoopPort $1`
output=`$WGET -nv -O$tmpfile http://localhost:$SnoopPort/servlet/snoop 2>&1`
rc=$?
@@ -344,7 +345,6 @@ WAS_monitor() {
ocf_log "err" "WAS: $1: wget failure: $output"
rc=$OCF_ERR_GENERIC
fi
- rm -fr $tmpfile
return $rc
}
Index: heartbeat/resources/OCF/WAS6
===================================================================
--- heartbeat.orig/resources/OCF/WAS6 2008-08-06 18:08:05.000000000 +1000
+++ heartbeat/resources/OCF/WAS6 2008-08-06 18:46:54.000000000 +1000
@@ -294,7 +294,8 @@ WAS_report_status() {
# This is actually faster than WAS_status above...
#
WAS_monitor() {
- tmpfile=`maketempfile`
+ trap '[. -z "$tmpfile" || rmtempfile "$tmpfile"' 0
+ tmpfile=`maketempfile` || exit 1
SnoopPort=`GetWASSnoopPort $1`
output=`$WGET -nv -O$tmpfile http://localhost:$SnoopPort/snoop 2>&1`
rc=$?
@@ -313,7 +314,6 @@ WAS_monitor() {
ocf_log "err" "WAS: $1: wget failure: $output"
rc=$OCF_ERR_GENERIC
fi
- rm -fr $tmpfile
return $rc
}
Index: heartbeat/tools/hb_report.in
===================================================================
--- heartbeat.orig/tools/hb_report.in 2008-08-06 18:08:05.000000000 +1000
+++ heartbeat/tools/hb_report.in 2008-08-06 18:47:53.000000000 +1000
@@ -399,6 +399,7 @@ checklogs() {
logs=$(find $1 -name $HALOG_F;
for l in $EXTRA_LOGS; do find $1/* -name `basename $l`; done)
[ "$logs" ] || return
+ trap '[ -z "$patfiles" ] || rmtempfile "$patfiles"]' 0
pattfile=`maketempfile` ||
fatal "cannot create temporary files"
for p in $LOG_PATTERNS; do
@@ -409,7 +410,8 @@ checklogs() {
for n in `getnodes`; do
cat $logs | grep -f $pattfile
done
- rm -f $pattfile
+ rmtempfile $patfiles
+ trap "" 0
}
#
Index: heartbeat/tools/utillib.sh
===================================================================
--- heartbeat.orig/tools/utillib.sh 2008-08-06 18:08:06.000000000 +1000
+++ heartbeat/tools/utillib.sh 2008-08-06 18:48:40.000000000 +1000
@@ -203,6 +203,12 @@ touchfile() {
perl -e "\$file=\"$t\"; \$tm=$1;" -e 'utime $tm, $tm, $file;' &&
echo $t
}
+find_files_clean() {
+ [ -z "$to_stamp" ] || rmtempfile "$to_stamp"
+ to_stamp=""
+ [ -z "$from_stamp" ] || rmtempfile "$from_stamp"
+ from_stamp=""
+}
find_files() {
dir=$1
from_time=$2
@@ -211,14 +217,24 @@ find_files() {
warning "sorry, can't find files based on time if you don't supply time"
return
}
- from_stamp=`touchfile $from_time`
+ trap find_files_clean 0
+ if ! from_stamp=`touchfile $from_time`; then
+ warning "sorry, can't create temoary file for find_files"
+ return
+ fi
findexp="-newer $from_stamp"
if isnumber "$to_time" && [ "$to_time" -gt 0 ]; then
- to_stamp=`touchfile $to_time`
+ if ! to_stamp=`touchfile $to_time`; then
+ warning "sorry, can't create temoary file for" \
+ "find_files"
+ find_files_clean
+ return
+ fi
findexp="$findexp ! -newer $to_stamp"
fi
find $dir -type f $findexp
- rm -f $from_stamp $to_stamp
+ find_files_clean
+ trap "" 0
}
#
@@ -311,6 +327,12 @@ sanitize_hacf() {
{print}
'
}
+sanitize_one_clean() {
+ [ -z "$tmp" ] || rmtempfile "$tmp"
+ tmp=""
+ [ -z "$ref" ] || rmtempfile "$ref"
+ ref=""
+}
sanitize_one() {
file=$1
compress=""
@@ -322,8 +344,13 @@ sanitize_one() {
compress=cat
decompress=cat
fi
- tmp=`maketempfile` && ref=`maketempfile` ||
+ trap sanitize_one_clean 0
+ tmp=`maketempfile`
+ ref=`maketempfile`
+ if [ -z "$tmp" -o -z "$ref" ]; then
+ sanitize_one_clean
fatal "cannot create temporary files"
+ fi
touch -r $file $ref # save the mtime
if [ "`basename $file`" = ha.cf ]; then
sanitize_hacf
@@ -331,8 +358,10 @@ sanitize_one() {
$decompress | sanitize_xml_attrs | $compress
fi < $file > $tmp
mv $tmp $file
+ tmp=""
touch -r $ref $file
- rm -f $ref
+ sanitize_one_clean
+ trap "" 0
}
#
Index: heartbeat/resources/OCF/.ocf-binaries.in
===================================================================
--- heartbeat.orig/resources/OCF/.ocf-binaries.in 2008-08-06 18:08:17.000000000 +1000
+++ heartbeat/resources/OCF/.ocf-binaries.in 2008-08-06 18:08:32.000000000 +1000
@@ -11,7 +11,6 @@ export PATH
: ${EGREP:="@EGREP@"}
: ${IFCONFIG_A_OPT:="@IFCONFIG_A_OPT@"}
: ${MAILCMD:=@MAILCMD@}
-: ${MKTEMP:=@MKTEMP@}
: ${PING:=@PING@}
: ${RPM:=@RPM@}
: ${SH:=@SHELL@}
@@ -48,7 +47,6 @@ export PATH
: ${SCP:=scp}
: ${SSH:=ssh}
: ${SWIG:=swig}
-: ${MKTEMP:=mktemp}
: ${GZIP_PROG:=gzip}
: ${TAR:=tar}
: ${MD5:=md5}
Index: heartbeat/tsa_plugin/linuxha-adapter.in
===================================================================
--- heartbeat.orig/tsa_plugin/linuxha-adapter.in 2008-08-06 18:08:44.000000000 +1000
+++ heartbeat/tsa_plugin/linuxha-adapter.in 2008-08-06 18:49:12.000000000 +1000
@@ -17,6 +17,8 @@ MSG_NOJAVA='Cannot find Java - exit.'
UNAME=`uname -s`
+. @sysconfdir@/ha.d/shellfuncs
+
function logit ()
{
if [ $LOG_LVL -ge $1 ]
@@ -114,22 +116,14 @@ function startAdapter ()
RC=$?
if [ $RC -eq $OFFLINE ] ; then
if [ "$UNAME" = AIX ] ; then
- MKTEMP=''
LSOF=''
else #linux
- MKTEMP=`whereis mktemp | cut -d' ' -f2 | cut -d' ' -f3`
LSOF=`whereis lsof | cut -d' ' -f2 | cut -d' ' -f3`
fi
rm -f /tmp/$adapter.*
# Create a tempfile to capture the first few lines of the output
- if [ -n "$MKTEMP" ] ; then
- TMPFILE=`$MKTEMP -q /tmp/$adapter.XXXXXX`
- else
- # It's better to use mktemp, but we don't have it, we make it cryptic
- RNDM=$(echo $RANDOM)
- TMPFILE=/tmp/$adapter.XXXXXX-RNDM
- touch $TMPFILE
- fi
+ trap '[ -z "$TMPFILE" ] || rmtempfile "$TMPFILE"' 0
+ TMPFILE=`maketempfile` || exit 1
# set LIBRAY path for jni parts
NATIVE_LIBPATH=/lib:/usr/lib
if [ "$UNAME" = "AIX" ] ; then
@@ -163,7 +157,7 @@ function startAdapter ()
fi
# cat the output to the screen and remove the tempfile
- cat $TMPFILE && rm -f $TMPFILE
+ cat $TMPFILE
echo "$adapter has started."
logit 0 "$adapter has started."
RC=0
Index: heartbeat/configure.in
===================================================================
--- heartbeat.orig/configure.in 2008-08-06 18:18:30.000000000 +1000
+++ heartbeat/configure.in 2008-08-06 18:19:47.000000000 +1000
@@ -588,20 +588,14 @@ AC_PATH_PROGS(PKGCONFIG, pkg-config)
dnl ************************************************************************
dnl Check whether non-root user can chown.
dnl ************************************************************************
-AC_PATH_PROGS(MKTEMP, mktemp)
if test -n "$WHOAMI"; then
IAM=`$WHOAMI`
fi
AC_MSG_CHECKING(if chown works for non-root)
-dnl Prefer "mktemp" command. But some OSes lack it; they can "touch".
-if test -n "$MKTEMP"; then
- F=`$MKTEMP "./.chown-testXXXXX"`
-else
- F="./.chown-test.$$"
- touch $F
-fi
+F="./.chown-test.$$"
+touch $F
if
case "$IAM" in
_______________________________________________________
Linux-HA-Dev: Linux-HA-Dev [at] lists
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/
|
