Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
openSUSE:Evergreen:11.2:Test
systemtap
systemtap-CVE-2009-4273.diff
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File systemtap-CVE-2009-4273.diff of Package systemtap
Subject: VUL-0: systemtap-server code exec References: bnc#574243 Signed-Off-By: Tony Jones <tonyj@suse.de> 'stap-client -p1 -B\;ai2' will print the usage help followed by a message similar to /usr/local/bin/stap-server: line 340: ai2: command not found which indicates that server tried to run the 'ai2' command. cases such as stap-client -B CC=/bin/rm CFLAGS=/ ... where the execution is done by make Comprises the following commits. commit 148297a5de2231e50262ec28a400b89b2d7c21f4 Author: Dave Brolley <brolley@redhat.com> Date: Tue Dec 15 15:13:13 2009 -0500 Remove unused variable. commit 5d4ea4cc1d1f7531fb0ff4d0498957e0333a61eb Author: Dave Brolley <brolley@redhat.com> Date: Wed Dec 2 16:40:13 2009 -0500 No need to pass -c option to the server. commit a0626e2e2ea13b6fc974157fb71fe6d48f4c7ec0 Author: Dave Brolley <brolley@redhat.com> Date: Thu Jan 7 13:58:11 2010 -0500 Client argument handling: Pass partial options to the server instead of complaining about them in the client. Update known failures from buildok in server.exp. commit 12091330be193cd0836d48c525bab015fcec2c75 Author: Dave Brolley <brolley@redhat.com> Date: Thu Jan 7 17:10:30 2010 -0500 Take care when echoing something that could start with a -. commit ed03894041aedf79811d5ad5c41caedbf90052cd Author: Dave Brolley <brolley@redhat.com> Date: Fri Jan 8 16:25:59 2010 -0500 New test suite for client/server argument handling. commit 3c07041760dccbb3151ef21602b8bc5da4b32197 Author: Dave Brolley <brolley@redhat.com> Date: Mon Jan 11 14:34:27 2010 -0500 Filter options for unprivileged use after --stap-client is seen. commit a0ace4915e5d963c28fa3b54f87afef34b82b6a5 Author: Dave Brolley <brolley@redhat.com> Date: Mon Jan 11 20:13:40 2010 -0500 Rework filtering of client options. Add testsuite. commit 75c2a31ddbe337af46ec4dad8a4dbbbd790c47b7 Author: Frank Ch. Eigler <fche@elastic.org> Date: Tue Jan 12 14:12:04 2010 -0500 tweak stap argument checking * main.cxx (checkOptions): Inline into main(), abeam other option checks. commit 5f03ebf5b2acccb652c9135627184479bc8d7d47 Author: Dave Brolley <brolley@redhat.com> Date: Mon Jan 11 20:19:54 2010 -0500 Invalid casess can be tested for 'make check'. commit f2aadddae0d01fa5a676404e49c6c36825b40512 Author: Dave Brolley <brolley@redhat.com> Date: Mon Jan 11 22:14:36 2010 -0500 Add some additional test cases. commit f73d5cad4e9aa5baa0a763a76cf4516721d29b2a Author: Dave Brolley <brolley@redhat.com> Date: Wed Jan 13 15:07:52 2010 -0500 Test newline characters as part of fuzzing argument strings. commit 622fa74aa720b3eda55c81530d458e3ea7792bb2 Author: Dave Brolley <brolley@redhat.com> Date: Thu Jan 14 15:44:09 2010 -0500 Allow / as a random argyment character when fuzzing. commit e4d80588594a7495a3efedbd3a4281df13ff253b Author: Dave Brolley <brolley@redhat.com> Date: Fri Jan 15 00:47:32 2010 -0500 PR11105: stap-client wire protocol change commit cf4a6df840531c1b30f8cfa7d10981d071911b98 Author: Frank Ch. Eigler <fche@elastic.org> Date: Fri Jan 15 03:06:52 2010 -0500 PR11105: robustify stap-server * main.cxx (main): Always downgrade client-provided -p5 to -p4. * stap-client (unpack_response): Sanitize stdout due to same. * stap-server-connect.c: Eliminate a bunch of globals. (handle_connection): Make things locals instead. Base tmp files on $TMPDIR. (spawn_and_wait): New helper function. (handleRequest): New monster function to inline rest of old stap-server-request. commit 36d1c134edc4bd4ee20225003041188c13b7f36f Author: Frank Ch. Eigler <fche@elastic.org> Date: Fri Jan 15 03:12:53 2010 -0500 testsuite: fix wording of invalid-entry test group commit 86f99ad8206574dc6400d48563db58341cb50f52 Author: Frank Ch. Eigler <fche@elastic.org> Date: Fri Jan 15 03:27:34 2010 -0500 PR11105: remove extraneous \n from localized foo.stp script file name commit 2a1c9b5db533fe7d2d2d4bac572195c490de62fb Author: Frank Ch. Eigler <fche@elastic.org> Date: Fri Jan 15 12:34:39 2010 -0500 PR11105: support default unset --prefix * configure.ac (STAP_PREFIX): Map NONE -> /usr/local. commit b75067caf1bb416af21473e40c917d953531e9f9 Author: Dave Brolley <brolley@redhat.com> Date: Mon Jan 18 11:56:13 2010 -0500 Correct client-side quoting issues discovered by fche during the server-side reimplementation. Also add the test cases to the test suite. commit 4240be911e37b817727ecb33f321f0ea389ede61 Author: Dave Brolley <brolley@redhat.com> Date: Mon Feb 15 15:01:28 2010 -0500 Don't pass client-only options to the server. Also correct parsing of the --server option. commit 241443ad36a5a2cacb9e8e6f12f808d304835f2a Author: Dave Brolley <brolley@redhat.com> Date: Tue Feb 2 08:26:01 2010 -0500 PR 11105: Remaining client-side problems: stap-client: Correct handling of embedded newlines in arguments. server_args.exp: Add additional cases discovered by fche and by fuzzing. commit c0d1b5a004b9949bb455b7dbe17b335b7cab9ead Author: Frank Ch. Eigler <fche@elastic.org> Date: Fri Feb 12 10:25:43 2010 -0500 PR11105 part 2: tighten constraints on stap-server parameters passed to make * util.h, util.cxx (assert_match_regexp): New function. * main.cxx (main): Constrain -R, -r, -a, -D, -S, -q, -B flags. * stap-serverd (listen): Harden stap-server-connect with ulimit/loop. * testsuite/systemtap.server/{client,server}_args.exp: Revised. commit cc9e5488d82b728e568bca1f8d6094856fc8e641 Author: Frank Ch. Eigler <fche@elastic.org> Date: Fri Feb 12 10:39:58 2010 -0500 PR11105 part 2a, fix buggy \\. in -r option regexp --- configure | 4 configure.ac | 7 main.cxx | 100 +++-- stap-client | 326 ++++++++++--------- stap-server-connect.c | 496 +++++++++++++++++++++++------ stap-serverd | 22 - testsuite/systemtap.server/client_args.exp | 119 ++++++ testsuite/systemtap.server/hello.stp | 2 testsuite/systemtap.server/server.exp | 4 testsuite/systemtap.server/server_args.exp | 182 ++++++++++ testsuite/systemtap.server/test.stp | 1 util.cxx | 36 ++ util.h | 2 13 files changed, 994 insertions(+), 307 deletions(-) --- a/configure +++ b/configure @@ -8937,9 +8937,11 @@ done fi +stap_prefix=$prefix +test "$stap_prefix" = NONE && stap_prefix=$ac_default_prefix cat >>confdefs.h <<_ACEOF -#define STAP_PREFIX "$prefix" +#define STAP_PREFIX "$stap_prefix" _ACEOF --- a/configure.ac +++ b/configure.ac @@ -576,8 +576,11 @@ fi dnl This is here mainly to make sure that configure --prefix=... changes dnl the config.h files so files depending on it are recompiled dnl prefix is passed through indirectly in the Makefile.am AM_CPPFLAGS. -dnl Don't use this directly (when not given it is set to NONE). -AC_DEFINE_UNQUOTED(STAP_PREFIX, "$prefix", [configure prefix location]) +dnl Formerly: Don't use this directly (when not given it is set to NONE). +dnl Currently: inline autoconf's later defaulting +stap_prefix=$prefix +test "$stap_prefix" = NONE && stap_prefix=$ac_default_prefix +AC_DEFINE_UNQUOTED(STAP_PREFIX, "$stap_prefix", [configure prefix location]) AC_CONFIG_HEADERS([config.h:config.in]) AC_CONFIG_FILES(Makefile doc/Makefile doc/SystemTap_Tapset_Reference/Makefile grapher/Makefile stap.1 stapprobes.3stap stapfuncs.3stap stapvars.3stap stapex.3stap staprun.8 stap-server.8 man/stapprobes.iosched.3stap man/stapprobes.netdev.3stap man/stapprobes.nfs.3stap man/stapprobes.nfsd.3stap man/stapprobes.pagefault.3stap man/stapprobes.kprocess.3stap man/stapprobes.rpc.3stap man/stapprobes.scsi.3stap man/stapprobes.signal.3stap man/stapprobes.socket.3stap man/stapprobes.tcp.3stap man/stapprobes.udp.3stap man/stapprobes.snmp.3stap initscript/systemtap) --- a/main.cxx +++ b/main.cxx @@ -1,5 +1,5 @@ // systemtap translator/driver -// Copyright (C) 2005-2009 Red Hat Inc. +// Copyright (C) 2005-2010 Red Hat Inc. // Copyright (C) 2005 IBM Corp. // Copyright (C) 2006 Intel Corporation. // @@ -56,7 +56,7 @@ version () << "SystemTap translator/driver " << "(version " << VERSION << "/" << dwfl_version (NULL) << " " << GIT_MESSAGE << ")" << endl - << "Copyright (C) 2005-2009 Red Hat, Inc. and others" << endl + << "Copyright (C) 2005-2010 Red Hat, Inc. and others" << endl << "This is free software; see the source for copying conditions." << endl; } @@ -362,43 +362,6 @@ setup_kernel_release (systemtap_session s.kernel_build_tree = "/lib/modules/" + s.kernel_release + "/build"; } } - -static void -checkOptions (systemtap_session &s) -{ - bool optionsConflict = false; - - if((s.cmd != "") && (s.target_pid)) - { - cerr << "You can't specify -c and -x options together." <<endl; - optionsConflict = true; - } - - if (s.unprivileged) - { - if (s.guru_mode) - { - cerr << "You can't specify -g and --unprivileged together." << endl; - optionsConflict = true; - } - } - - if (!s.kernel_symtab_path.empty()) - { - if (s.consult_symtab) - { - cerr << "You can't specify --kelf and --kmap together." << endl; - optionsConflict = true; - } - s.consult_symtab = true; - if (s.kernel_symtab_path == PATH_TBD) - s.kernel_symtab_path = string("/boot/System.map-") + s.kernel_release; - } - - if (optionsConflict) - usage (s, 1); -} - int main (int argc, char * const argv []) { @@ -467,6 +430,8 @@ main (int argc, char * const argv []) s.load_only = false; s.skip_badvars = false; s.unprivileged = false; + bool client_options = false; + string client_options_disallowed; // Location of our signing certificate. // If we're root, use the database in SYSCONFDIR, otherwise @@ -538,7 +503,6 @@ main (int argc, char * const argv []) setup_kernel_release(s, s_kr); } - while (true) { int long_opt; @@ -550,6 +514,7 @@ main (int argc, char * const argv []) #define LONG_OPT_VERBOSE_PASS 5 #define LONG_OPT_SKIP_BADVARS 6 #define LONG_OPT_UNPRIVILEGED 7 +#define LONG_OPT_CLIENT_OPTIONS 8 // NB: also see find_hash(), usage(), switch stmt below, stap.1 man page static struct option long_options[] = { { "kelf", 0, &long_opt, LONG_OPT_KELF }, @@ -559,6 +524,7 @@ main (int argc, char * const argv []) { "skip-badvars", 0, &long_opt, LONG_OPT_SKIP_BADVARS }, { "vp", 1, &long_opt, LONG_OPT_VERBOSE_PASS }, { "unprivileged", 0, &long_opt, LONG_OPT_UNPRIVILEGED }, + { "client-options", 0, &long_opt, LONG_OPT_CLIENT_OPTIONS }, { NULL, 0, NULL, 0 } }; int grc = getopt_long (argc, argv, "hVvtp:I:e:o:R:r:a:m:kgPc:x:D:bs:uqwl:d:L:FS:B:", @@ -599,6 +565,8 @@ main (int argc, char * const argv []) break; case 'I': + if (client_options) + client_options_disallowed += client_options_disallowed.empty () ? "-I" : ", -I"; s.include_path.push_back (string (optarg)); break; @@ -621,16 +589,21 @@ main (int argc, char * const argv []) break; case 'o': + // NB: client_options not a problem, since pass 1-4 does not use output_file. s.output_file = string (optarg); break; case 'R': + if (client_options) { cerr << "ERROR: -R invalid with --client-options" << endl; usage(s,1); } s.runtime_path = string (optarg); break; case 'm': + if (client_options) + client_options_disallowed += client_options_disallowed.empty () ? "-m" : ", -m"; s.module_name = string (optarg); save_module = true; + // XXX: convert to assert_regexp_match() { string::size_type len = s.module_name.length(); @@ -675,10 +648,13 @@ main (int argc, char * const argv []) break; case 'r': + if (client_options) // NB: no paths! + assert_regexp_match("-r parameter from client", optarg, "^[a-z0-9_.-]+$"); setup_kernel_release(s, optarg); break; case 'a': + assert_regexp_match("-a parameter", optarg, "^[a-z0-9_-]+$"); s.architecture = string(optarg); break; @@ -726,14 +702,19 @@ main (int argc, char * const argv []) break; case 'D': + assert_regexp_match ("-D parameter", optarg, "^[a-z_][a-z_0-9]*(=[a-z_0-9]+)?$"); + if (client_options) + client_options_disallowed += client_options_disallowed.empty () ? "-D" : ", -D"; s.macros.push_back (string (optarg)); break; case 'S': + assert_regexp_match ("-S parameter", optarg, "^[0-9]+(,[0-9]+)?$"); s.size_option = string (optarg); break; case 'q': + if (client_options) { cerr << "ERROR: -q invalid with --client-options" << endl; usage(s,1); } s.tapset_compile_coverage = true; break; @@ -764,6 +745,7 @@ main (int argc, char * const argv []) break; case 'B': + if (client_options) { cerr << "ERROR: -B invalid with --client-options" << endl; usage(s,1); } s.kbuildflags.push_back (string (optarg)); break; @@ -816,6 +798,11 @@ main (int argc, char * const argv []) break; case LONG_OPT_UNPRIVILEGED: s.unprivileged = true; + /* NB: for server security, it is essential that once this flag is + set, no future flag be able to unset it. */ + break; + case LONG_OPT_CLIENT_OPTIONS: + client_options = true; break; default: cerr << "Internal error parsing command arguments." << endl; @@ -830,8 +817,37 @@ main (int argc, char * const argv []) } // Check for options conflicts. - checkOptions (s); + if (client_options && s.last_pass > 4) + { + s.last_pass = 4; /* Quietly downgrade. Server passed through -p5 naively. */ + } + if (client_options && s.unprivileged && ! client_options_disallowed.empty ()) + { + cerr << "You can't specify " << client_options_disallowed << " when --unprivileged is specified." << endl; + usage (s, 1); + } + if ((s.cmd != "") && (s.target_pid)) + { + cerr << "You can't specify -c and -x options together." << endl; + usage (s, 1); + } + if (s.unprivileged && s.guru_mode) + { + cerr << "You can't specify -g and --unprivileged together." << endl; + usage (s, 1); + } + if (!s.kernel_symtab_path.empty()) + { + if (s.consult_symtab) + { + cerr << "You can't specify --kelf and --kmap together." << endl; + usage (s, 1); + } + s.consult_symtab = true; + if (s.kernel_symtab_path == PATH_TBD) + s.kernel_symtab_path = string("/boot/System.map-") + s.kernel_release; + } // Warn in case the target kernel release doesn't match the running one. if (s.last_pass > 4 && (string(buf.release) != s.kernel_release || @@ -1279,6 +1295,8 @@ pass_5: else { if (s.keep_tmpdir) + // NB: the format of this message needs to match the expectations + // of stap-server-connect.c. clog << "Keeping temporary directory \"" << s.tmpdir << "\"" << endl; else { --- a/stap-client +++ b/stap-client @@ -2,7 +2,7 @@ # Compile server client for systemtap # -# Copyright (C) 2008, 2009 Red Hat Inc. +# Copyright (C) 2008-2010 Red Hat Inc. # # This file is part of systemtap, and is free software. You can # redistribute it and/or modify it under the terms of the GNU General @@ -57,7 +57,6 @@ function initialization { p_phase=5 v_level=0 keep_temps=0 - b_specified=0 m_name= module_name=stap_$$ uname_r="`uname -r`" @@ -82,52 +81,51 @@ function initialization { # output. # function parse_options { - cmdline= - cmdline1= - cmdline2= + # Each command line argument will be written to its own file within the + # request package. + argc=1 + packed_options='-' + arg_subst= while test $# != 0 do - advance_p=0 + advance=0 dash_seen=0 + client_arg=0 # Start of a new token. - first_token=$1 - until test $advance_p != 0 + first_token="$1" + until test $advance != 0 do # Identify the next option - first_char=`expr "$first_token" : '\(.\).*'` + first_char="${first_token:0:1}" second_char= if test $dash_seen = 0; then if test "$first_char" = "-"; then if test "$first_token" != "-"; then # It's not a lone dash, so it's an option. # Is it a long option (i.e. --option)? - second_char=`expr "$first_token" : '.\(.\).*'` + second_char="${first_token:1:1}" if test "X$second_char" = "X-"; then - long_option=`expr "$first_token" : '--\(.*\)=.*'` - test "X$long_option" != "X" || long_option=`expr "$first_token" : '--\(.*\)'` - case $long_option in - ssl) - process_ssl $first_token + case "$first_token" in + --ssl=*) + process_ssl "$first_token" ;; - server) - process_server $first_token + --server=*) + process_server "$first_token" ;; *) - # An unknown or unimportant option. - # Ignore it, but pass it on to the server. - cmdline2="$cmdline2 $first_token" + # An unknown or unimportant option. Ignore it. ;; esac - advance_p=$(($advance_p + 1)) + advance=$(($advance + 1)) break fi # It's not a lone dash, or a long option, so it's a short option string. # Remove the dash. - first_token=`expr "$first_token" : '-\(.*\)'` + first_token="${first_token:1}" dash_seen=1 - first_char=`expr "$first_token" : '\(.\).*'` + first_char="${first_token:0:1}" fi fi if test $dash_seen = 0; then @@ -137,14 +135,9 @@ function parse_options { # then it could be the name of the script file. if test "X$e_script" = "X" -a "X$script_file" = "X"; then script_file="$first_token" - cmdline1="$cmdline2" - cmdline2= - elif test "$first_char" != "'"; then - cmdline2="$cmdline2 '$first_token'" - else - cmdline2="$cmdline2 $first_token" + script_file_argc=$argc fi - advance_p=$(($advance_p + 1)) + advance=$(($advance + 1)) break fi fi @@ -152,115 +145,129 @@ function parse_options { # We are at the start of an option. Look at the first character. case $first_char in a) - get_arg $first_token $2 - process_a $stap_arg + get_arg "$first_token" "$2" + process_a "$stap_arg" ;; - b) - b_specified=1 - ;; B) - get_arg $first_token $2 - cmdline2="${cmdline2} -$first_char '$stap_arg'" + get_arg "$first_token" "$2" ;; c) - get_arg $first_token "$2" + get_arg "$first_token" "$2" process_c "$stap_arg" ;; D) - get_arg $first_token $2 - cmdline2="${cmdline2} -$first_char '$stap_arg'" + get_arg "$first_token" "$2" ;; e) - get_arg $first_token "$2" + get_arg "$first_token" "$2" process_e "$stap_arg" ;; I) - get_arg $first_token $2 - process_I $stap_arg + get_arg "$first_token" "$2" + process_I "$stap_arg" ;; k) keep_temps=1 ;; l) - get_arg $first_token $2 - cmdline2="${cmdline2} -$first_char '$stap_arg'" + get_arg "$first_token" "$2" p_phase=2 ;; L) - get_arg $first_token $2 - cmdline2="${cmdline2} -$first_char '$stap_arg'" + get_arg "$first_token" "$2" p_phase=2 ;; m) - get_arg $first_token $2 - process_m $stap_arg + get_arg "$first_token" "$2" + process_m "$stap_arg" ;; o) - get_arg $first_token $2 - process_o $stap_arg + get_arg "$first_token" "$2" + process_o "$stap_arg" ;; p) - get_arg $first_token $2 - process_p $stap_arg + get_arg "$first_token" "$2" + process_p "$stap_arg" ;; r) - get_arg $first_token $2 - process_r $stap_arg + get_arg "$first_token" "$2" + process_r "$stap_arg" ;; R) - get_arg $first_token $2 - process_R $stap_arg + get_arg "$first_token" "$2" + process_R "$stap_arg" ;; s) - get_arg $first_token $2 - cmdline2="${cmdline2} -$first_char '$stap_arg'" + get_arg "$first_token" "$2" ;; S) - get_arg $first_token $2 - cmdline2="${cmdline2} -$first_char '$stap_arg'" + get_arg "$first_token" "$2" ;; v) v_level=$(($v_level + 1)) ;; x) - get_arg $first_token $2 - cmdline2="${cmdline2} -$first_char '$stap_arg'" + get_arg "$first_token" "$2" ;; *) - # An unknown or unimportant flag. Ignore it, but pass it on to the server. + # An unknown or unimportant flag. ;; esac - if test $advance_p = 0; then + if test $advance = 0; then # Just another flag character. Consume it. - cmdline2="$cmdline2 -$first_char" - first_token=`expr "$first_token" : '.\(.*\)'` + first_token="${first_token:1}" if test "X$first_token" = "X"; then - advance_p=$(($advance_p + 1)) + advance=$(($advance + 1)) fi fi done # Consume the arguments we just processed. - while test $advance_p != 0 - do + while test $advance != 0; do + # Does the final argument file contain a client-side file + # name which must be changed to a server-side name? + local arg + if test "X$arg_subst" != "X" -a $advance = 1; then + arg="$arg_subst" + arg_subst= + else + arg="$1" + fi + + # If it's not client-only argument, + # place the argument in a numbered file within our temp + # directory. + # o We don't write a newline at the end, since newline could be + # part of the argument. + # o We add an X to the beginning of the file + # in order to avoid having 'echo' interpret the output as + # its own option. We then remove the X. + # There must be a better way. + if test $client_arg = 0; then + echo -n "X$arg" > "$tmpdir_client/argv$argc" + sed -i "s|^X||" "$tmpdir_client/argv$argc" + argc=$(($argc + 1)) + fi + + # Get the next argument. shift - advance_p=$(($advance_p - 1)) + advance=$(($advance - 1)) + packed_options='-' done done # If the script file was given and it's not '-', then replace it with its - # client-temp-name in the command string. + # client-temp-name in its argument file. if test "X$script_file" != "X"; then local local_name if test "$script_file" != "-"; then - local_name=`generate_client_temp_name "$script_file"` + generate_client_temp_name "$script_file" + local_name="$client_temp_name" else - local_name="$script_file" + local_name="-" fi - cmdline="$cmdline1 'script/$local_name' $cmdline2" - else - cmdline="$cmdline2" + echo -n "script/$local_name" > "$tmpdir_client/argv$script_file_argc" fi # Processing based on final options settings @@ -270,12 +277,6 @@ function parse_options { # We must have at least one usable certificate database. test "X$local_ssl_dbs" != "X " -o "X$public_ssl_dbs" != "X" || \ fatal "No usable certificate databases found" - - # We can use any server if the phase is less than 4 - # But don't for now.... - #if test $p_phase -lt 4; then - # find_all="--all" - #fi } # function: get_arg FIRSTWORD SECONDWORD @@ -283,33 +284,31 @@ function parse_options { # Collect an argument to the given option function get_arg { # Remove first character. - local opt=`expr "$1" : '\(.\).*'` - local first=`expr "$1" : '.\(.*\)'` + local opt="${1:0:1}" + local first="${1:1}" + packed_options="${packed_options}$opt" # Advance to the next token, if the first one is exhausted. if test "X$first" = "X"; then - shift - advance_p=$(($advance_p + 1)) - first=$1 + advance=$(($advance + 1)) + first="$2" fi - test "X$first" != "X" || \ - fatal "Missing argument to -$opt" - stap_arg="$first" - advance_p=$(($advance_p + 1)) + test "X$first" != "X" && advance=$(($advance + 1)) } # function: process_ssl ARGUMENT # # Process the --ssl option. function process_ssl { - local db=`expr "$1" : '--ssl=\(.*\)'` + client_arg=1 + local db="${1:6}" test "X$db" != "X" || \ fatal "Missing argument to --ssl" - check_db $db || return + check_db "$db" || return additional_local_ssl_dbs="$additional_local_ssl_dbs $db" } @@ -318,7 +317,8 @@ function process_ssl { # # Process the --server option. function process_server { - local spec=`expr "$1" : '--server=\(.*\)'` + client_arg=1 + local spec="${1:9}" test "X$spec" != "X" || \ fatal "Missing argument to --server" @@ -331,7 +331,6 @@ function process_server { # Process the -c flag. function process_c { c_cmd="$1" - cmdline2="${cmdline2} -c '$1'" } # function: process_e ARGUMENT @@ -342,22 +341,23 @@ function process_e { # which may have already been identified. if test "X$e_script" = "X"; then e_script="$1" - if test "X$script_file" != "X"; then - cmdline2="$cmdline1 '$script_file' $cmdline2" - cmdline1= - script_file= - fi + script_file= fi - cmdline2="${cmdline2} -e '$1'" } -# function: process_I ARGUMENT +# function: process_I ARGUMENT ORIGINAL_ARGUMENT # # Process the -I flag. function process_I { - local local_name=`include_file_or_directory tapsets $1` - test "X$local_name" != "X" || return - cmdline2="${cmdline2} -I 'tapsets/$local_name'" + test "X$1" = "X" && return + test "${1:0:1}" = " +" && return + include_file_or_directory tapsets "$1" + if test $advance = 1; then + arg_subst="${packed_options}tapsets/$included_name" + else + arg_subst="tapsets/$included_name" + fi } # function: process_m ARGUMENT @@ -366,7 +366,6 @@ function process_I { function process_m { module_name="$1" m_name="$1" - cmdline2="${cmdline2} -m '$1'" } # function: process_o ARGUMENT @@ -374,40 +373,38 @@ function process_m { # Process the -o flag. function process_o { stdout_redirection="$1" - cmdline2="${cmdline2} -o '$1'" } # function: process_p ARGUMENT # # Process the -p flag. function process_p { - p_phase=$1 - cmdline2="${cmdline2} -p '$1'" + p_phase="$1" } # function: process_r ARGUMENT # # Process the -r flag. function process_r { - local first_char=`expr "$1" : '\(.\).*'` + local first_char="${1:0:1}" if test "$first_char" = "/"; then # fully specified path - kernel_build_tree=$1 + kernel_build_tree="$1" version_file_name="$kernel_build_tree/include/config/kernel.release" # The file include/config/kernel.release within the # build tree is used to pull out the version information - release=`cat $version_file_name 2>/dev/null` + release=`cat "$version_file_name" 2>/dev/null` if test "X$release" = "X"; then fatal "Missing $version_file_name" return fi else # kernel release specified directly - release=$1 + release="$1" fi if test "X$release" != "X$uname_r"; then - uname_r=$release + uname_r="$release" find_all="--all" fi } @@ -417,47 +414,65 @@ function process_r { # Process the -a flag. function process_a { if test "X$1" != "X$arch"; then - arch=$1 + arch="$1" find_all="--all" fi } -# function: process_R ARGUMENT +# function: process_R ARGUMENT ORIGINAL_ARGUMENT # # Process the -R flag. function process_R { - local local_name=`include_file_or_directory runtime $1` - test "X$local_name" != "X" || return - cmdline2="${cmdline2} -R 'runtime/$local_name'" + test "X$1" = "X" && return + test "${1:0:1}" = " +" && return + include_file_or_directory runtime "$1" + if test $advance = 1; then + arg_subst="${packed_options}runtime/$included_name" + else + arg_subst="runtime/$included_name" + fi } # function: include_file_or_directory PREFIX NAME # # Include the given file or directory in the client's temporary -# tree to be sent to the server. +# tree to be sent to the server and save it's name in the variable +# included_name. We use a global variable instread of echoing the +# result since the use of `include_file_or_directory` loses a trailing +# newline. function include_file_or_directory { - # Add a symbolic link of the named file or directory to our temporary directory - local local_name=`generate_client_temp_name "$2"` + # Add a symbolic link of the named file or directory to our temporary + # directory, but only if the file or directory exists. + generate_client_temp_name "$2" + local local_name="$client_temp_name" + included_name="$local_name" + test -e "/$local_name" || return + local local_dirname=`dirname "$local_name"` mkdir -p "$tmpdir_client/$1/$local_dirname" || \ fatal "Could not create $tmpdir_client/$1/$local_dirname" ln -s "/$local_name" "$tmpdir_client/$1/$local_name" || \ fatal "Could not link $tmpdir_client/$1/$local_name to /$local_name" - echo "$local_name" } # function: generate_client_temp_name NAME # # Generate the name to be used for the given file/directory relative to the -# client's temporary directory. +# client's temporary directory and stores it in the variable +# client_temp_name. We use a global variable instread of echoing the +# result since the use of `generate_client_temp_name` loses a trailing +# newline. function generate_client_temp_name { # Transform the name into a fully qualified path name - local full_name=`echo "$1" | sed "s,^\\\([^/]\\\),$wd/\\\\1,"` + local full_name="$1" + test "${full_name:0:1}" != "/" && full_name="$wd/$full_name" # The same name without the initial / or trailing / - local local_name=`echo "$full_name" | sed 's,^/\(.*\),\1,'` - local_name=`echo "$local_name" | sed 's,\(.*\)/$,\1,'` - echo "$local_name" + local local_name="${full_name:1}" + test "${local_name: -1:1}" = "/" && local_name="${local_name:0:$((${#local_name}-1))}" + + client_temp_name="$local_name" } # function: create_request @@ -474,12 +489,11 @@ function create_request { fatal "Cannot create temporary directory " $tmpdir_client/script cat > "$tmpdir_client/script/$script_file" else - include_file_or_directory script "$script_file" > /dev/null + include_file_or_directory script "$script_file" fi fi # Add the necessary info to special files in our temporary directory. - echo "cmdline: $cmdline" > cmdline echo "sysinfo: `client_sysinfo`" > sysinfo } @@ -502,7 +516,8 @@ function package_request { zip_client=$tmpdir_env/`mktemp $tmpdir_client_base.zip.XXXXXX` || \ fatal "Cannot create temporary file " $zip_client - (rm $zip_client && zip -r $zip_client $tmpdir_client_base > /dev/null) || \ + cd $tmpdir_client + (rm -f $zip_client && zip -r $zip_client * > /dev/null) || \ fatal "zip of request tree, $tmpdir_client, failed" } @@ -518,23 +533,12 @@ function unpack_response { unzip -d $tmpdir_server $zip_server > /dev/null || \ fatal "Cannot unpack server response, $zip_server" - # Check the contents of the expanded directory. It should contain a - # single directory whose name matches $stap_tmpdir_prefix_server.?????? - local num_files=`ls $tmpdir_server | wc -l` - test $num_files = 1 || \ - fatal "Wrong number of files in server's temp directory" - test -d $tmpdir_server/$stap_tmpdir_prefix_server.?????? || \ - fatal "`ls $tmpdir_server` does not match the expected name or is not a directory" - # Move the contents of the directory down one level. - mv $tmpdir_server/$stap_tmpdir_prefix_server.??????/* $tmpdir_server - rm -fr $tmpdir_server/$stap_tmpdir_prefix_server.?????? - # Check the contents of the directory. It should contain: # 1) a file called stdout # 2) a file called stderr # 3) a file called rc # 4) optionally a directory named to match stap?????? - num_files=`ls $tmpdir_server | wc -l` + local num_files=`ls $tmpdir_server | wc -l` test $num_files = 4 -o $num_files = 3 || \ fatal "Wrong number of files in server's temp directory" test -f $tmpdir_server/stdout || \ @@ -544,7 +548,8 @@ function unpack_response { test -f $tmpdir_server/rc || \ fatal "`pwd`/$tmpdir_server/rc does not exist or is not a regular file" - # See if there is a systemtap temp directory + # See if there is a systemtap temp directory. There should be at least an empty one. + # ls -l $tmpdir_server tmpdir_stap=`cd $tmpdir_server && ls | grep stap......\$ 2>/dev/null` if test "X$tmpdir_stap" != "X"; then test -d $tmpdir_server/$tmpdir_stap || \ @@ -567,6 +572,16 @@ function unpack_response { test $EUID = 0 && chown $EUID:$EUID $tmpdir_stap fi fi + + if test $keep_temps = 0; then + # Remove the output line due to the synthetic server-side -k + sed -i "/^Keeping temporary directory.*/ d" $tmpdir_server/stderr + fi + + if test $p_phase = 5; then + # Remove the output line due to the synthetic server-side -p4 + sed -i "/^.*\.ko$/ d" $tmpdir_server/stdout + fi } # function: find_and_connect_to_server @@ -652,16 +667,21 @@ function find_and_connect_to_server { # Remember which ssl certificate database was used to authenticate the chosen # server. ssl_db=`${stap_exec_prefix}stap-find-servers $find_all | choose_server` - test "X$ssl_db" != "X" && return + if test "X$ssl_db" != "X"; then + rm -f $tmpdir_client/connect + return + fi num_servers=`${stap_exec_prefix}stap-find-servers $find_all | wc -l` fi if test $num_servers = 0; then + rm -f $tmpdir_client/connect fatal "Unable to find a server" fi cat $tmpdir_client/connect >&2 + rm -f $tmpdir_client/connect fatal "Unable to connect to a server" } @@ -703,8 +723,8 @@ function choose_server { # echo the name of the ssl certificate database used to successfully authenticate # the server. function send_receive { - local server=$1 - local port=$2 + local server="$1" + local port="$2" # The server must match the dns name on the certificate # and must be 'localhost' if the server is on the local host. @@ -838,7 +858,7 @@ function maybe_call_staprun { # Run it in the background and wait for it. This # way any signals sent to us can be caught. if test $v_level -ge 2; then - echo "running `which staprun` $staprun_opts $module_name.ko" >&2 + echo "running `staprun_PATH` $staprun_opts $module_name.ko" >&2 fi eval `staprun_PATH` "$staprun_opts" $module_name.ko rc=$? @@ -883,9 +903,9 @@ function staprun_PATH { # # Check the security of the given database directory. function check_db { - local dir=$1 - local euid=$2 - local user=$3 + local dir="$1" + local euid="$2" + local user="$3" local rc=0 # Check that we have been given a directory @@ -960,7 +980,7 @@ function check_db { # # Check the security of the given database file. function check_db_file { - local file=$1 + local file="$1" local rc=0 # Check that we have been given a file --- a/stap-server-connect.c +++ b/stap-server-connect.c @@ -1,9 +1,9 @@ /* SSL server program listens on a port, accepts client connection, reads - the data into a temporary file, calls the systemtap server script and - then transmits the resulting fileback to the client. + the data into a temporary file, calls the systemtap translator and + then transmits the resulting file back to the client. - Copyright (C) 2008, 2009 Red Hat Inc. + Copyright (C) 2008-2010 Red Hat Inc. This file is part of systemtap, and is free software. You can redistribute it and/or modify it under the terms of the GNU General Public @@ -23,25 +23,36 @@ #include <stdio.h> #include <stdlib.h> #include <string.h> +#include <unistd.h> #include <errno.h> +#include <spawn.h> +#include <fcntl.h> +#include <glob.h> +#include <sys/types.h> +#include <sys/stat.h> +#include <sys/wait.h> +#include <wordexp.h> +#include <sys/param.h> #include <ssl.h> #include <nspr.h> #include <plgetopt.h> #include <nss.h> #include <pk11func.h> +#include "config.h" -#define READ_BUFFER_SIZE (60 * 1024) /* Global variables */ static char *password = NULL; static CERTCertificate *cert = NULL; static SECKEYPrivateKey *privKey = NULL; static char *dbdir = NULL; -static char requestFileName[] = "/tmp/stap.server.client.zip.XXXXXX"; -static char responseDirName[] = "/tmp/stap.server.XXXXXX"; -static char responseZipName[] = "/tmp/stap.server.XXXXXX.zip.XXXXXX"; -static const char *stapOptions = ""; +static const char *stapOptions = ""; + + +static PRStatus spawn_and_wait (char **argv, + const char* fd0, const char* fd1, const char* fd2, const char *pwd); + static void Usage(const char *progName) @@ -71,24 +82,27 @@ exitErr(char *function) exit(1); } + + + /* Function: readDataFromSocket() * * Purpose: Read data from the socket into a temporary file. * */ -static SECStatus -readDataFromSocket(PRFileDesc *sslSocket) +static SECStatus readDataFromSocket(PRFileDesc *sslSocket, const char *requestFileName) { PRFileDesc *local_file_fd; PRFileInfo info; PRInt32 numBytesRead; PRInt32 numBytesWritten; PRInt32 totalBytes; +#define READ_BUFFER_SIZE 4096 char buffer[READ_BUFFER_SIZE]; /* Open the output file. */ local_file_fd = PR_Open(requestFileName, PR_WRONLY | PR_CREATE_FILE | PR_TRUNCATE, - PR_IRUSR | PR_IWUSR | PR_IRGRP | PR_IWGRP | PR_IROTH); + PR_IRUSR | PR_IWUSR); if (local_file_fd == NULL) { fprintf (stderr, "could not open output file %s\n", requestFileName); @@ -96,6 +110,7 @@ readDataFromSocket(PRFileDesc *sslSocket } /* Read the number of bytes to be received. */ + /* XXX: impose a limit to prevent disk space consumption DoS */ numBytesRead = PR_Read(sslSocket, & info.size, sizeof (info.size)); if (numBytesRead == 0) /* EOF */ { @@ -122,10 +137,11 @@ readDataFromSocket(PRFileDesc *sslSocket /* Write to stdout */ numBytesWritten = PR_Write(local_file_fd, buffer, numBytesRead); - if (numBytesWritten < 0) - fprintf (stderr, "could not write to output file %s\n", requestFileName); - if (numBytesWritten != numBytesRead) - fprintf (stderr, "could not write to output file %s\n", requestFileName); + if (numBytesWritten < 0 || (numBytesWritten != numBytesRead)) + { + fprintf (stderr, "could not write to output file %s\n", requestFileName); + break; + } #if DEBUG fprintf(stderr, "***** Connection read %d bytes.\n", numBytesRead); #if 0 @@ -316,27 +332,15 @@ authenticateSocket(PRFileDesc *sslSocket * */ static SECStatus -writeDataToSocket(PRFileDesc *sslSocket) +writeDataToSocket(PRFileDesc *sslSocket, const char *responseFileName) { int numBytes; PRFileDesc *local_file_fd; - PRFileInfo info; - PRStatus prStatus; - /* Try to open the local file named. - * If successful, then write it to the client. - */ - prStatus = PR_GetFileInfo(responseZipName, &info); - if (prStatus != PR_SUCCESS || info.type != PR_FILE_FILE || info.size < 0) - { - fprintf (stderr, "Input file %s not found\n", responseZipName); - return SECFailure; - } - - local_file_fd = PR_Open(responseZipName, PR_RDONLY, 0); + local_file_fd = PR_Open(responseFileName, PR_RDONLY, 0); if (local_file_fd == NULL) { - fprintf (stderr, "Could not open input file %s\n", responseZipName); + fprintf (stderr, "Could not open input file %s\n", responseFileName); return SECFailure; } @@ -356,7 +360,7 @@ writeDataToSocket(PRFileDesc *sslSocket) #if DEBUG /* Transmitted bytes successfully. */ fprintf(stderr, "PR_TransmitFile wrote %d bytes from %s\n", - numBytes, responseZipName); + numBytes, responseFileName); #endif PR_Close(local_file_fd); @@ -364,10 +368,303 @@ writeDataToSocket(PRFileDesc *sslSocket) return SECSuccess; } + +/* Run the translator on the data in the request directory, and produce output + in the given output directory. */ +static void handleRequest (const char* requestDirName, const char* responseDirName) +{ + char stapstdout[PATH_MAX]; + char stapstderr[PATH_MAX]; + char staprc[PATH_MAX]; +#define MAXSTAPARGC 1000 /* sorry, too lazy to dynamically allocate */ + char* stapargv[MAXSTAPARGC]; + int stapargc=0; + int rc; + wordexp_t words; + int i; + FILE* f; + int unprivileged = 0; + int stapargv_freestart = 0; + + stapargv[stapargc++]= STAP_PREFIX "/bin/stap"; + + /* Transcribe stapOptions. We use plain wordexp(3), since these + options are coming from the local trusted user, so malicious + content is not a concern. */ + + rc = wordexp (stapOptions, & words, WRDE_NOCMD|WRDE_UNDEF); + if (rc) + { + errWarn("cannot parse -s stap options"); + return; + } + if (words.we_wordc+10 >= MAXSTAPARGC) /* 10: padding for literal entries */ + { + errWarn("too many -s options; MAXSTAPARGC"); + return; + } + + for (i=0; i<words.we_wordc; i++) + stapargv[stapargc++] = words.we_wordv[i]; + + stapargv[stapargc++] = "-k"; /* Need to keep temp files in order to package them up again. */ + + /* Process the saved command line arguments. Avoid quoting/unquoting errors by + transcribing literally. */ + stapargv[stapargc++] = "--client-options"; + stapargv_freestart = stapargc; + + for (i=1 ; ; i++) + { + char stapargfile[PATH_MAX]; + FILE* argfile; + struct stat st; + char *arg; + + if (stapargc >= MAXSTAPARGC) + { + errWarn("too many stap options; MAXSTAPARGC"); + return; + } + + snprintf (stapargfile, PATH_MAX, "%s/argv%d", requestDirName, i); + + rc = stat(stapargfile, & st); + if (rc) break; + + arg = malloc (st.st_size+1); + if (!arg) + { + errWarn("stap arg malloc"); + return; + } + + argfile = fopen(stapargfile, "r"); + if (! argfile) + { + errWarn("stap arg fopen"); + return; + } + + rc = fread(arg, 1, st.st_size, argfile); + if (rc != st.st_size) + { + errWarn("stap arg fread"); + return; + } + + arg[st.st_size] = '\0'; + stapargv[stapargc++] = arg; /* freed later */ + fclose (argfile); + } + + snprintf (stapstdout, PATH_MAX, "%s/stdout", responseDirName); + snprintf (stapstderr, PATH_MAX, "%s/stderr", responseDirName); + + stapargv[stapargc] = NULL; /* spawn_and_wait expects NULL termination */ + + /* Check for the unprivileged flag; we need this so that we can decide to sign the module. */ + for (i=0; i<stapargc; i++) + if (strcmp (stapargv[i], "--unprivileged") == 0) + unprivileged=1; + /* NB: but it's not that easy! What if an attacker passes + --unprivileged as some sort of argument-parameter, so that the + translator does not interpret it as an --unprivileged mode flag, + but something else? Then it could generate unrestrained modules, + but silly we might still sign it, and let the attacker get away + with murder. And yet we don't want to fully getopt-parse the + args here for duplication of effort. + + So let's do a hack: forcefully add --unprivileged to stapargv[] + near the front in this case, something which a later option + cannot undo. */ + if (unprivileged) + { + if (stapargc+1 >= MAXSTAPARGC) + { + errWarn("too many stap options; MAXSTAPARGC"); + return; + } + + /* Shift all stapargv[] entries up one, including the NULL. */ + for (i=stapargc; i>=1; i--) + stapargv[i+1]=stapargv[i]; + stapargv_freestart ++; /* adjust for shift */ + + stapargv[1]="--unprivileged"; /* better not be resettable by later option */ + } + + /* All ready, let's run the translator! */ + rc = spawn_and_wait (stapargv, "/dev/null", stapstdout, stapstderr, requestDirName); + + /* Save the RC */ + snprintf (staprc, PATH_MAX, "%s/rc", responseDirName); + f = fopen(staprc, "w"); + if (f) + { + /* best effort basis */ + fprintf(f, "%d", rc); + fclose(f); + } + + /* Parse output to extract the -k-saved temprary directory. + XXX: bletch. */ + f = fopen(stapstderr, "r"); + if (!f) + { + errWarn("stap stderr open"); + return; + } + + while (1) + { + char line[PATH_MAX]; + char *l = fgets(line, PATH_MAX, f); /* NB: normally includes \n at end */ + if (!l) break; + char key[]="Keeping temporary directory \""; + + /* Look for line from main.cxx: s.keep_tmpdir */ + if (strncmp(l, key, strlen(key)) == 0 && + l[strlen(l)-2] == '"') /* "\n */ + { + /* Move this directory under responseDirName. We don't have to + preserve the exact stapXXXXXX suffix part, since stap-client + will accept anything ("stap......" regexp), and rewrite it + to a client-local string. + + We don't just symlink because then we'd have to + remember to delete it later anyhow. */ + char *mvargv[10]; + char *orig_staptmpdir = & l[strlen(key)]; + char new_staptmpdir[PATH_MAX]; + + orig_staptmpdir[strlen(orig_staptmpdir)-2] = '\0'; /* Kill the closing "\n */ + snprintf(new_staptmpdir, PATH_MAX, "%s/stap000000", responseDirName); + mvargv[0]="mv"; + mvargv[1]=orig_staptmpdir; + mvargv[2]=new_staptmpdir; + mvargv[3]=NULL; + rc = spawn_and_wait (mvargv, NULL, NULL, NULL, NULL); + if (rc != PR_SUCCESS) + errWarn("stap tmpdir move"); + + /* In unprivileged mode, if we have a module built, we need to + sign the sucker. */ + if (unprivileged) + { + glob_t globber; + char pattern[PATH_MAX]; + snprintf (pattern,PATH_MAX,"%s/*.ko", new_staptmpdir); + rc = glob (pattern, GLOB_ERR, NULL, &globber); + if (rc) + errWarn("stap tmpdir .ko glob"); + else if (globber.gl_pathc != 1) + errWarn("stap tmpdir too many .ko globs"); + else + { + char *signargv [10]; + signargv[0] = STAP_PREFIX "/libexec/systemtap/stap-sign-module"; + signargv[1] = globber.gl_pathv[0]; + signargv[2] = dbdir; + signargv[3] = NULL; + rc = spawn_and_wait (signargv, NULL, NULL, NULL, NULL); + if (rc != PR_SUCCESS) + errWarn("stap-sign-module"); + } + } + } + + /* XXX: What about uprobes.ko? */ + } + + /* Free up all the arg string copies. Note that the first few were alloc'd + by wordexp(), which wordfree() frees; others were hand-set to literal strings. */ + for (i= stapargv_freestart; i<stapargc; i++) + free (stapargv[i]); + wordfree (& words); + + /* Sorry about the inconvenience. C string/file processing is such a pleasure. */ +} + + +/* A frontend for posix_spawnp that waits for the child process and + returns overall success or failure. */ +static PRStatus spawn_and_wait (char ** argv, + const char* fd0, const char* fd1, const char* fd2, const char *pwd) +{ + pid_t pid; + int rc; + int status; + extern char** environ; + posix_spawn_file_actions_t actions; + int dotfd = -1; + +#define CHECKRC(msg) do { if (rc) { errWarn(msg); return PR_FAILURE; } } while (0) + + rc = posix_spawn_file_actions_init (& actions); + CHECKRC ("spawn file actions ctor"); + if (fd0) { + rc = posix_spawn_file_actions_addopen(& actions, 0, fd0, O_RDONLY, 0600); + CHECKRC ("spawn file actions fd0"); + } + if (fd1) { + rc = posix_spawn_file_actions_addopen(& actions, 1, fd1, O_WRONLY|O_CREAT, 0600); + CHECKRC ("spawn file actions fd1"); + } + if (fd2) { + rc = posix_spawn_file_actions_addopen(& actions, 2, fd2, O_WRONLY|O_CREAT, 0600); + CHECKRC ("spawn file actions fd2"); + } + + /* change temporarily to a directory if requested */ + if (pwd) + { + dotfd = open (".", O_RDONLY); + if (dotfd < 0) + { + errWarn ("spawn getcwd"); + return PR_FAILURE; + } + + rc = chdir (pwd); + CHECKRC ("spawn chdir"); + } + + rc = posix_spawnp (& pid, argv[0], & actions, NULL, argv, environ); + /* NB: don't react to rc!=0 right away; need to chdir back first. */ + + if (pwd && dotfd >= 0) + { + int subrc; + subrc = fchdir (dotfd); + subrc |= close (dotfd); + if (subrc) + errWarn("spawn unchdir"); + } + + CHECKRC ("spawn"); + + rc = waitpid (pid, &status, 0); + if ((rc!=pid) || !WIFEXITED(status)) + { + errWarn ("waitpid"); + return PR_FAILURE; + } + + rc = posix_spawn_file_actions_destroy (&actions); + CHECKRC ("spawn file actions dtor"); + + return WEXITSTATUS(status) ? PR_FAILURE : PR_SUCCESS; +#undef CHECKRC +} + + + /* Function: int handle_connection() * - * Purpose: Handle a connection to a socket. - * + * Purpose: Handle a connection to a socket. Copy in request zip + * file, process it, copy out response. Temporary directories are + * created & destroyed here. */ static SECStatus handle_connection(PRFileDesc *tcpSocket) @@ -376,11 +673,16 @@ handle_connection(PRFileDesc *tcpSocket) SECStatus secStatus = SECFailure; PRStatus prStatus; PRSocketOptionData socketOption; - PRFileInfo info; - char *cmdline; - char *stap_server_prefix; int rc; char *rc1; + char tmpdir[PATH_MAX]; + char requestFileName[PATH_MAX]; + char requestDirName[PATH_MAX]; + char responseDirName[PATH_MAX]; + char responseFileName[PATH_MAX]; + char *argv[10]; /* we use fewer than these in all the posix_spawn's below. */ + + tmpdir[0]='\0'; /* prevent cleanup-time /bin/rm of uninitialized directory */ /* Make sure the socket is blocking. */ socketOption.option = PR_SockOpt_Nonblocking; @@ -409,50 +711,49 @@ handle_connection(PRFileDesc *tcpSocket) goto cleanup; } - /* Create a temporary files and directories. */ - memcpy (requestFileName + sizeof (requestFileName) - 1 - 6, "XXXXXX", 6); - rc = mkstemp(requestFileName); - if (rc == -1) + snprintf(tmpdir, PATH_MAX, "%s/stap-server.XXXXXX", getenv("TMPDIR") ?: "/tmp"); + rc1 = mkdtemp(tmpdir); + if (! rc1) { - fprintf (stderr, "Could not create temporary file %s\n", requestFileName); + fprintf (stderr, "Could not create temporary directory %s\n", tmpdir); perror (""); secStatus = SECFailure; + tmpdir[0]=0; /* prevent /bin/rm */ goto cleanup; } - memcpy (responseDirName + sizeof (responseDirName) - 1 - 6, "XXXXXX", 6); - rc1 = mkdtemp(responseDirName); - if (! rc1) + /* Create a temporary files names and directories. */ + snprintf (requestFileName, PATH_MAX, "%s/request.zip", tmpdir); + + snprintf (requestDirName, PATH_MAX, "%s/request", tmpdir); + rc = mkdir(requestDirName, 0700); + if (rc) { - fprintf (stderr, "Could not create temporary directory %s\n", responseDirName); + fprintf (stderr, "Could not create temporary directory %s\n", requestDirName); perror (""); secStatus = SECFailure; goto cleanup; } - memcpy (responseZipName, responseDirName, sizeof (responseDirName) - 1); - memcpy (responseZipName + sizeof (responseZipName) - 1 - 6, "XXXXXX", 6); - rc = mkstemp(responseZipName); - if (rc == -1) + snprintf (responseDirName, PATH_MAX, "%s/response", tmpdir); + rc = mkdir(responseDirName, 0700); + if (rc) { - fprintf (stderr, "Could not create temporary file %s\n", responseZipName); + fprintf (stderr, "Could not create temporary directory %s\n", responseDirName); perror (""); secStatus = SECFailure; - - /* Remove this so that the other temp files will get removed in cleanup. */ - prStatus = PR_RmDir (responseDirName); - if (prStatus != PR_SUCCESS) - errWarn ("PR_RmDir"); goto cleanup; } + snprintf (responseFileName, PATH_MAX, "%s/response.zip", tmpdir); + /* Read data from the socket. * If the user is requesting/requiring authentication, authenticate * the socket. */ #if DEBUG fprintf(stdout, "\nReading data from socket...\n\n"); #endif - secStatus = readDataFromSocket(sslSocket); + secStatus = readDataFromSocket(sslSocket, requestFileName); if (secStatus != SECSuccess) goto cleanup; @@ -466,32 +767,42 @@ handle_connection(PRFileDesc *tcpSocket) } #endif - /* Call the stap-server script. */ - stap_server_prefix = getenv("SYSTEMTAP_SERVER_SCRIPTS") ?: BINDIR; - cmdline = PORT_Alloc(strlen (stap_server_prefix) + sizeof ("/stap-server") + 1 + - sizeof (requestFileName) + 1 + - sizeof (responseDirName) + 1 + - sizeof (responseZipName) + 1 + - strlen (dbdir) + 1 + - 1 + strlen (stapOptions) + 1 + - 1); - if (! cmdline) { - errWarn ("PORT_Alloc"); - secStatus = SECFailure; - goto cleanup; - } - - sprintf (cmdline, "%s/stap-server %s %s %s %s '%s'", stap_server_prefix, - requestFileName, responseDirName, responseZipName, dbdir, - stapOptions); - rc = system (cmdline); + /* Unzip the request. */ + argv[0]="unzip"; + argv[1]="-d"; + argv[2]=requestDirName; + argv[3]=requestFileName; + rc = spawn_and_wait(argv, NULL, NULL, NULL, NULL); + if (rc != PR_SUCCESS) + { + errWarn ("request unzip"); + secStatus = SECFailure; + goto cleanup; + } - PR_Free (cmdline); + /* Handle the request zip file. An error therein should still result + in a response zip file (containing stderr etc.) so we don't have to + have a result code here. */ + handleRequest(requestDirName, responseDirName); + + /* Zip the response. */ + argv[0]="zip"; + argv[1]="-r"; + argv[2]=responseFileName; + argv[3]="."; + argv[4]=NULL; + rc = spawn_and_wait(argv, NULL, NULL, NULL, responseDirName); + if (rc != PR_SUCCESS) + { + errWarn ("response zip"); + secStatus = SECFailure; + goto cleanup; + } #if DEBUG fprintf(stdout, "\nWriting data to socket...\n\n"); #endif - secStatus = writeDataToSocket(sslSocket); + secStatus = writeDataToSocket(sslSocket, responseFileName); cleanup: /* Close down the socket. */ @@ -499,17 +810,16 @@ cleanup: if (prStatus != PR_SUCCESS) errWarn("PR_Close"); - /* Attempt to remove temporary files, unless the temporary directory was - not deleted by the server script. */ - prStatus = PR_GetFileInfo(responseDirName, &info); - if (prStatus != PR_SUCCESS) + if (tmpdir[0]) { - prStatus = PR_Delete (requestFileName); - if (prStatus != PR_SUCCESS) - errWarn ("PR_Delete"); - prStatus = PR_Delete (responseZipName); - if (prStatus != PR_SUCCESS) - errWarn ("PR_Delete"); + /* Remove the whole tmpdir and all that lies beneath. */ + argv[0]="rm"; + argv[1]="-r"; + argv[2]=tmpdir; + argv[3]=NULL; + rc = spawn_and_wait(argv, NULL, NULL, NULL, NULL); + if (rc != PR_SUCCESS) + errWarn ("tmpdir cleanup"); } return secStatus; @@ -544,17 +854,11 @@ accept_connection(PRFileDesc *listenSock break; } + /* XXX: alarm() or somesuch to set a timeout. */ + /* XXX: fork() or somesuch to handle concurrent requests. */ + /* Accepted the connection, now handle it. */ /*result =*/ handle_connection (tcpSocket); -#if 0 /* Not necessary */ - if (result != SECSuccess) - { - prStatus = PR_Close(tcpSocket); - if (prStatus != PR_SUCCESS) - exitErr("PR_Close"); - break; - } -#endif } #if DEBUG --- a/stap-serverd +++ b/stap-serverd @@ -320,11 +320,19 @@ function advertise_presence { function listen { # The stap-server-connect program will listen forever # accepting requests. - ${stap_exec_prefix}stap-server-connect \ - -p $port -n $nss_cert -d $ssl_db -w $nss_pw \ - -s "$stap_options" \ - 2>&1 & - wait '%${stap_exec_prefix}stap-server-connect' >/dev/null 2>&1 + # CVE-2009-4273 ... or at least, until resource limits fire + while true; do # NB: loop to avoid DoS by deliberate rlimit-induced halt + # NB: impose resource limits in case of mischevious data inducing + # too much / long computation + (ulimit -f 50000 -s 1000 -t 60 -u 20 -v 500000; + exec ${stap_pkglibexecdir}stap-server-connect \ + -p $port -n $nss_cert -d $ssl_db -w $nss_pw \ + -s "$stap_options") & + stap_server_connect_pid=$! + wait + # NB: avoid superfast spinning in case of a ulimit or other failure + sleep 1 + done 2>&1 } # function: check_db DBNAME @@ -562,8 +570,8 @@ function terminate { wait '%avahi-publish-service' >/dev/null 2>&1 # Kill any running 'stap-server-connect' job. - kill -s SIGTERM '%${stap_exec_prefix}stap-server-connect' 2> /dev/null - wait '%${stap_exec_prefix}stap-server-connect' >/dev/null 2>&1 + kill -s SIGTERM $stap_server_connect_pid >/dev/null 2>&1 + wait $stap_server_connect_pid >/dev/null 2>&1 exit } --- /dev/null +++ b/testsuite/systemtap.server/client_args.exp @@ -0,0 +1,119 @@ +set test "Invalid Server Client Arguments" + +# Test that stap on the server side will correctly accept/reject certain +# arguments in unprivileged mode. +set test_file $srcdir/systemtap.server/test.stp + +# Test invalid combinations. +set error_regexp ".*(ERROR)|(You can't specify .* when --unprivileged is specified).*" + +set invalid_options [list \ + "--unprivileged --client-options -B X=Y" \ + "--unprivileged --client-options -D X=Y" \ + "--unprivileged --client-options -I /tmp" \ + "--unprivileged --client-options -m test" \ + "--unprivileged --client-options -R /tmp" \ + "--unprivileged --client-options -B X=Y -D X=Y -I /tmp -m test -R /tmp -r [exec uname -r]" \ + "--client-options --unprivileged -B X=Y" \ + "--client-options --unprivileged -D X=Y" \ + "--client-options --unprivileged -I /tmp" \ + "--client-options --unprivileged -m test" \ + "--client-options --unprivileged -R /tmp" \ + "--client-options --unprivileged -B X=Y -D X=Y -I /tmp -m test -R /tmp -r [exec uname -r]" \ + "--client-options -B X=Y --unprivileged" \ + "--client-options -D X=Y --unprivileged" \ + "--client-options -I /tmp --unprivileged" \ + "--client-options -m test --unprivileged" \ + "--client-options -R /tmp --unprivileged" \ + "--client-options -B X=Y -D X=Y -I /tmp -m test -R /tmp -r [exec uname -r] --unprivileged" \ + "--client-options -R /path" \ + "-D \"foo;bar\"" \ + "-D 2=4" \ + "-D \"foo;bar\"" \ + "--client-options -r /path" \ + "-S /path" \ + "--client-options -q" \ +] + +foreach options $invalid_options { + verbose -log "eval exec stap $options" + catch {eval exec stap $test_file -p1 $options} res_stap + verbose -log $res_stap + + if {[regexp $error_regexp $res_stap]} { + pass "$test: $options" + } else { + fail "$test: $options" + } +} + +# Test valid combinations +# stap_run_exact (used below) only works for 'make installcheck' +if {[info procs installtest_p] != "" && ![installtest_p]} { untested $test; return } + +set test "Valid Server Client Arguments" + +set no_error_result "# parse tree dump +# file $test_file +probe begin{ +exit() +} + +" + +set valid_options [list \ + "-a i386" \ + "-B X=Y" \ + "-D X=Y" \ + "-I /tmp" \ + "-m test" \ + "-r [exec uname -r]" \ + "-a i386 -B X=Y -D X=Y -I /tmp -m test -r [exec uname -r]" \ + "--unprivileged" \ + "--unprivileged -a i386" \ + "--unprivileged -B X=Y" \ + "--unprivileged -D X=Y" \ + "--unprivileged -I /tmp" \ + "--unprivileged -m test" \ + "--unprivileged -R /tmp" \ + "--unprivileged -r [exec uname -r]" \ + "--unprivileged -a i386 -B X=Y -D X=Y -I /tmp -m test -R /tmp -r [exec uname -r]" \ + "--client-options" \ + "--client-options -a i386" \ + "--client-options -D X=Y" \ + "--client-options -I /tmp" \ + "--client-options -m test" \ + "--client-options -r [exec uname -r]" \ + "--client-options -a i386 -D X=Y -I /tmp -m test -r [exec uname -r]" \ + "--unprivileged --client-options" \ + "--client-options --unprivileged" \ + "--unprivileged -a i386 --client-options" \ + "--unprivileged -B X=Y --client-options" \ + "--unprivileged -D X=Y --client-options" \ + "--unprivileged -I /tmp --client-options" \ + "--unprivileged -m test --client-options" \ + "--unprivileged -R /tmp --client-options" \ + "--unprivileged -r [exec uname -r] --client-options" \ + "--unprivileged -a i386 -B X=Y -D X=Y -I /tmp -m test -R /tmp -r [exec uname -r] --client-options" \ + "-a i386 --unprivileged --client-options" \ + "-B X=Y --unprivileged --client-options" \ + "-D X=Y --unprivileged --client-options" \ + "-I /tmp --unprivileged --client-options" \ + "-m test --unprivileged --client-options" \ + "-R /tmp --unprivileged --client-options" \ + "-r [exec uname -r] --unprivileged --client-options" \ + "-a i386 -B X=Y -D X=Y -I /tmp -m test -R /tmp -r [exec uname -r] --unprivileged --client-options" \ + "-a i386 --client-options --unprivileged" \ + "-B X=Y --client-options --unprivileged" \ + "-D X=Y --client-options --unprivileged" \ + "-I /tmp --client-options --unprivileged" \ + "-m test --client-options --unprivileged" \ + "-R /tmp --client-options --unprivileged" \ + "-r [exec uname -r] --client-options --unprivileged" \ + "-a i386 -B X=Y -D X=Y -I /tmp -m test -R /tmp -r [exec uname -r] --client-options --unprivileged" \ +] + +set ::result_string "$no_error_result" +foreach options $valid_options { + eval stap_run_exact {"$test: $options"} $test_file -p1 $options +} --- a/testsuite/systemtap.server/hello.stp +++ b/testsuite/systemtap.server/hello.stp @@ -1,4 +1,4 @@ -#! stap +#! stap -p5 probe begin { --- a/testsuite/systemtap.server/server.exp +++ b/testsuite/systemtap.server/server.exp @@ -16,11 +16,7 @@ foreach file [lsort [glob -nocomplain $s # some tests are known to fail. switch $test { "buildok/perfmon01.stp with server" {setup_kfail 909 *-*-*} - "buildok/twentyseven.stp with server" {setup_kfail 4166 *-*-*} - "buildok/sched_test.stp with server" {setup_kfail 1155 *-*-*} - "buildok/process_test.stp with server" {setup_kfail 1155 *-*-*} "buildok/rpc-all-probes.stp with server" {setup_kfail 4413 *-*-*} - "buildok/nfs-all-probes.stp with server" {setup_kfail 4413 *-*-*} } if {$rc == 0} { pass $test } else { fail $test } } --- /dev/null +++ b/testsuite/systemtap.server/server_args.exp @@ -0,0 +1,182 @@ +set test "Server Argument Test" + +# Each test will take 3 seconds or more due to the use of avahi to browse for +# servers, so limit iterations to some small but reasonable number. +set iterations 10 + +# Allow seeding the random number generator using an environment variable in +# order to facilitate reproducing a given series of tests. Otherwise seed it +# using the current time. +if [info exists env(STAP_RANDOM_SEED)] { + set random_seed $env(STAP_RANDOM_SEED) +} else { + set random_seed [clock seconds] +} +verbose -log "Random seed is $random_seed" +expr srand($random_seed) + +proc stap_direct_and_with_client {stap stap_client options} { + # tcl's 'eval' creates a string containing the arguments and + # recursively passes it to the tcl interpreter. Special + # characters need to be quoted. + regsub -all "\[\"\\\\;\]" $options {\\\0} options + regsub -all "\[\n\]" $options {\\n} options + + verbose -log "eval exec $stap $options" + catch {eval exec $stap $options} res_stap + verbose -log $res_stap + + # Now run it against stap-client + verbose -log "eval exec $stap_client $options" + catch {eval exec $stap_client $options} res_stap_client + verbose -log $res_stap_client + + # Now check the output + set n 0 + set expected [split $res_stap "\n"] + set received [split $res_stap_client "\n"] + foreach line $received { + set expected_line [lindex $expected $n] + # Some messages contain the names of files or directories + # and will be prefixed for the client. + if {[regexp "^ (.*)" $expected_line match data]} { + # Special characters in the regexp need to be quoted. + regsub -all "\[\"\\\\;\]" $data {\\\0} data + if {[regexp "^ tapsets.*/$data" $line]} { + incr n + continue + } + if {[regexp "^ runtime.*/$data" $line]} { + incr n + continue + } + # Some messages contain the name of the module based on the + # process id. + if {[regexp "^ stap_\[0-9\]*" $expected_line] && + [regexp "^ stap_\[0-9\]*" $line]} { + incr n + continue + } + } else { + if {[regexp "^Input file '(.*)' is empty or missing." $expected_line match data]} { + # Special characters in the regexp need to be quoted. + regsub -all "\[\"\\\\;\]" $data {\\\0} data + if {[regexp "^Input file 'script.*/$data' is empty or missing." $line]} { + incr n + continue + } + } + } + + # Otherwise the output must be identical. + if {! [string equal $line $expected_line]} { + break + } + incr n + } + if {$n == [llength $expected] && [llength $expected] == [llength $received]} { + # Test passes + return 1 + } + # Test fails + send_log "line [expr $n + 1]:\n" + send_log "Expected: \"[lindex $expected $n]\"\n" + send_log "Got : \"$line\"\n" + return 0 +} + +# ************ Start of mainline ************* + +# Don't attempt these tests if the client/server are not available +# Start a systemtap server, if one is not already started. +if {! [use_server_p]} then { + if {! [setup_server]} then { + untested "$test" + return + } +} + +# Make sure we call the correct instances of stap and stap-client. +# There is a copy of 'stap-client' on the PATH as 'stap'. +if {[installtest_p]} then { + # For 'make installcheck', use both from the location of the actual + # stap-client on the PATH. + set stap_client [exec which stap-client] + set stap [exec dirname $stap_client]/stap +} else { + # For 'make check' use stap-client from the source tree and use stap from + # the build tree. + set stap_client $srcdir/../stap-client + set stap [exec pwd]/../stap +} + +# Test some argument strings which have failed in the past. This is useful +# for debugging a currently failing case and helps to ensure that previously +# fixed cases do not regress. +set previously_fixed [list \ + "-p1 -I8o\\2ie -e'1\\ -D\n\" -c" \ + "-p1 -Ira\\3;c g -e0fle'qq -Dr/316k\\o8 -cjyoc\n3" \ + "-p1 -I6p3 -elc -Dqgsgv' -c" \ + "-p1 -I\"vyv;z -ej\"/3 -D/ 01qck\n -c3u55zut" \ + "-p1 -I1 -eo9e\nx047q -D9xyefk0a -cvl98/x1'i" \ + "-p1 -c; test.stp" \ + "-p1 -I4hgy96 -e5oo39p -Ddx8v -c4;" \ + "-p1 -I -esq3wors -Dz -c*eibz8h2e" \ + "-p1 -I a -em339db5 -Du2;c0ps -ch9o\\" \ + "-p1 -Ipfjps4 -ebug4dc -Du8vd fvkl -c" \ + "-p1 -I0\"nspzjyf -e5r3up8h -Dmi;ojp9m -cx;a2fat" \ + "-p1 -Iu -ek7;r -Dcu\"; -c\"hc" \ + "-p1 -Icd4fidq m40mv -edn -D;8ha\\cjr -c1*vnq" \ + "-p1 -I;3 -er8e -D -cb6k29z" \ + "-p1 -Ircj -e -D -c\\vmww" \ + "-p1 -Illc5 -e65wof9 qr*=x7x5 -D -cgx;" \ + "-p1 -Iyaj420=3 -e\" -D -cd'5mi" \ + "-p1 -Ir -e -D29\\ -cj2szt;4" \ + "-p1 -Ibno3=b4sk -e' -Dg2-j;e -c2ijx'" \ + "-p1 -I285v7pl -eo5\\0 -D86s -c-c*v" \ +] + +set i 0 +foreach options $previously_fixed { + if {[stap_direct_and_with_client $stap $stap_client $options]} { + pass "$test $i" + } else { + fail "$test $i" + } + incr i +} + +# Generate semi-random arguments containing with potential problem characters. +# Check that running systemtap with the client/server generates output +# comparable to running stap directly. +set dangerous_options [list "-I" "-e" "-D" "-c" "-S"] +# NB: Other options could be candidates here, like -r and -B, but +# there stap-server imposes more restrictions than local stap, so +# this simple error-matching test cannot use them. +set argchars "0123456789/;*'=-\\\"\n abcdefghijklmnopqrstuvwxyz" + +for {set i 0} {$i < $iterations} {incr i} { + verbose -log "Iteration $i" + + # First generate an argument string + set options "-p1" + foreach option $dangerous_options { + set options "$options $option" + set limit [expr int(rand() * 10)] + for {set c 0} {$c < $limit} {incr c} { + set options "$options[string index $argchars [expr int(rand() * [string length $argchars])]]" + } + } + + # Now test it against stap and stap-client + if {[stap_direct_and_with_client $stap $stap_client $options]} { + pass "Fuzzing $test $i" + } else { + fail "Fuzzing $test $i" + } +} + +# Shudown the server, if we started it. +if {! [use_server_p]} then { + shutdown_server +} --- /dev/null +++ b/testsuite/systemtap.server/test.stp @@ -0,0 +1 @@ +probe begin { exit (); } --- a/util.cxx +++ b/util.cxx @@ -1,5 +1,5 @@ // Copyright (C) Andrew Tridgell 2002 (original file) -// Copyright (C) 2006, 2009 Red Hat Inc. (systemtap changes) +// Copyright (C) 2006-2010 Red Hat Inc. (systemtap changes) // // This program is free software; you can redistribute it and/or // modify it under the terms of the GNU General Public License as @@ -19,6 +19,8 @@ #include "sys/sdt.h" #include <stdexcept> #include <cerrno> +#include <map> +#include <string> extern "C" { #include <fcntl.h> @@ -30,6 +32,7 @@ extern "C" { #include <sys/types.h> #include <sys/wait.h> #include <unistd.h> +#include <regex.h> } using namespace std; @@ -374,4 +377,35 @@ kill_stap_spawn(int sig) return spawned_pid ? kill(spawned_pid, sig) : 0; } + +void assert_regexp_match (const string& name, const string& value, const string& re) +{ + typedef map<string,regex_t*> cache; + static cache compiled; + cache::iterator it = compiled.find (re); + regex_t* r = 0; + if (it == compiled.end()) + { + r = new regex_t; + int rc = regcomp (r, re.c_str(), REG_ICASE|REG_NOSUB|REG_EXTENDED); + if (rc) { + cerr << "regcomp " << re << " (" << name << ") error rc=" << rc << endl; + exit(1); + } + compiled[re] = r; + } + else + r = it->second; + + // run regexec + int rc = regexec (r, value.c_str(), 0, 0, 0); + if (rc) + { + cerr << "ERROR: Safety pattern mismatch for " << name + << " ('" << value << "' vs. '" << re << "') rc=" << rc << endl; + exit(1); + } +} + + /* vim: set sw=2 ts=8 cino=>4,n-2,{2,^-2,t0,(0,u0,w1,M1 : */ --- a/util.h +++ b/util.h @@ -18,7 +18,7 @@ const std::string cmdstr_quoted(const st std::string git_revision(const std::string& path); int stap_system(int verbose, const std::string& command); int kill_stap_spawn(int sig); - +void assert_regexp_match (const std::string& name, const std::string& value, const std::string& re); // stringification generics
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
