[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#688945: unblock: condor/7.8.2~dfsg.1-1+deb7u1



On Thu, Sep 27, 2012 at 09:33:26AM +0200, Michael Hanke wrote:
> Added patches are attached.

Now they are.


-- 
Michael Hanke
http://mih.voxindeserto.de
>From 94e84ce4ff93ea071ca17bcf823918432749c868 Mon Sep 17 00:00:00 2001
From: Matthew Farrellee <matt@redhat.com>
Date: Fri, 10 Aug 2012 12:36:44 -0400
Subject: [PATCH] Check setuid return value (7.6 version), #3165

Signed-off-by: Timothy St. Clair <tstclair@redhat.com>
---
 src/condor_utils/my_popen.cpp |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/src/condor_utils/my_popen.cpp
+++ b/src/condor_utils/my_popen.cpp
@@ -397,7 +397,7 @@
 		seteuid( 0 );
 		setgroups( 1, &egid );
 		setgid( egid );
-		setuid( euid );
+		if( setuid( euid ) ) _exit(ENOEXEC); // Unsafe?
 
 			/* before we exec(), clear the signal mask and reset SIGPIPE
 			   to SIG_DFL
@@ -677,7 +677,7 @@
 		seteuid( 0 );
 		setgroups( 1, &egid );
 		setgid( egid );
-		setuid( euid );
+		if( setuid( euid ) ) _exit(ENOEXEC); // Unsafe?
 
 			/* Now it's safe to exec whatever we were given */
 		execv( cmd, const_cast<char *const*>(argv) );
diff --git a/src/condor_schedd.V6/schedd.cpp b/src/condor_schedd.V6/schedd.cpp
index 74e2a9e..e59ddf8 100644
--- a/src/condor_schedd.V6/schedd.cpp
+++ b/src/condor_schedd.V6/schedd.cpp
@@ -2961,79 +2961,6 @@ Scheduler::WriteAttrChangeToUserLog( const char* job_id_str, const char* attr,
 
 
 int
-Scheduler::abort_job(int, Stream* s)
-{
-	PROC_ID	job_id;
-	int nToRemove = -1;
-
-	// First grab the number of jobs to remove/hold
-	if ( !s->code(nToRemove) ) {
-		dprintf(D_ALWAYS,"abort_job() can't read job count\n");
-		return FALSE;
-	}
-
-	if ( nToRemove > 0 ) {
-		// We are being told how many and which jobs to abort
-
-		dprintf(D_FULLDEBUG,"abort_job: asked to abort %d jobs\n",nToRemove);
-
-		while ( nToRemove > 0 ) {
-			if( !s->code(job_id) ) {
-				dprintf( D_ALWAYS, "abort_job() can't read job_id #%d\n",
-					nToRemove);
-				return FALSE;
-			}
-			abort_job_myself(job_id, JA_REMOVE_JOBS, false, true );
-			nToRemove--;
-		}
-		s->end_of_message();
-	} else {
-		// We are being told to scan the queue ourselves and abort
-		// any jobs which have a status = REMOVED or HELD
-		ClassAd *job_ad;
-		static bool already_removing = false;	// must be static!!!
-		char constraint[120];
-
-		// This could take a long time if the queue is large; do the
-		// end_of_message first so condor_rm does not timeout. We do not
-		// need any more info off of the socket anyway.
-		s->end_of_message();
-
-		dprintf(D_FULLDEBUG,"abort_job: asked to abort all status REMOVED/HELD jobs\n");
-
-		// if already_removing is true, it means the user sent a second condor_rm
-		// command before the first condor_rm command completed, and we are
-		// already in the below job scan/removal loop in a different stack frame.
-		// so we should just return here.
-		if ( already_removing ) {
-			return TRUE;
-		}
-
-		snprintf(constraint,120,"%s == %d || %s == %d",ATTR_JOB_STATUS,REMOVED,
-				 ATTR_JOB_STATUS,HELD);
-
-		job_ad = GetNextJobByConstraint(constraint,1);
-		if ( job_ad ) {
-			already_removing = true;
-		}
-		while ( job_ad ) {
-			if ( (job_ad->LookupInteger(ATTR_CLUSTER_ID,job_id.cluster) == 1) &&
-				 (job_ad->LookupInteger(ATTR_PROC_ID,job_id.proc) == 1) ) {
-
-				 abort_job_myself(job_id, JA_REMOVE_JOBS, false, true );
-
-			}
-			FreeJobAd(job_ad);
-
-			job_ad = GetNextJobByConstraint(constraint,0);
-		}
-		already_removing = false;
-	}
-
-	return TRUE;
-}
-
-int
 Scheduler::transferJobFilesReaper(int tid,int exit_status)
 {
 	ExtArray<PROC_ID> *jobs = NULL;
@@ -10706,9 +10633,6 @@ Scheduler::Register()
 	 daemonCore->Register_Command( RESCHEDULE, "RESCHEDULE", 
 			(CommandHandlercpp)&Scheduler::reschedule_negotiator, 
 			"reschedule_negotiator", this, WRITE);
-	 daemonCore->Register_CommandWithPayload(KILL_FRGN_JOB, "KILL_FRGN_JOB", 
-			(CommandHandlercpp)&Scheduler::abort_job, 
-			"abort_job", this, WRITE);
 	 daemonCore->Register_CommandWithPayload(ACT_ON_JOBS, "ACT_ON_JOBS", 
 			(CommandHandlercpp)&Scheduler::actOnJobs, 
 			"actOnJobs", this, WRITE, D_COMMAND,
diff --git a/src/condor_schedd.V6/scheduler.h b/src/condor_schedd.V6/scheduler.h
index 863189e..842b81f 100644
--- a/src/condor_schedd.V6/scheduler.h
+++ b/src/condor_schedd.V6/scheduler.h
@@ -301,9 +301,6 @@ class Scheduler : public Service
 		// requires a new round of negotiation
 	void            needReschedule();
 
-	// job managing
-	int				abort_job(int, Stream *);
-
 	// [IPV6] These two functions are never called by others.
 	// It is non-IPv6 compatible, though.
 	void			send_all_jobs(ReliSock*, struct sockaddr_in*);
>From 1db67805b2f9ec0f20548b0307c176666cc1eb1f Mon Sep 17 00:00:00 2001
From: Matthew Farrellee <matt@redhat.com>
Date: Mon, 30 Jul 2012 15:31:37 -0700
Subject: [PATCH] FS authentication requires authentication directory to be
 mode=0700, #3166

Signed-off-by: Zach Miller <zmiller@cs.wisc.edu>
---
 src/condor_io/condor_auth_fs.cpp |   15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/src/condor_io/condor_auth_fs.cpp b/src/condor_io/condor_auth_fs.cpp
index c246f24..b055a17 100644
--- a/src/condor_io/condor_auth_fs.cpp
+++ b/src/condor_io/condor_auth_fs.cpp
@@ -332,12 +332,25 @@ int Condor_Auth_FS::authenticate(const char * /* remoteHost */, CondorError* err
 				// making a conservative change in 7.6.5 that allows
 				// the link count to be 1 or 2.  -Dan 11/11
 
+				// The CREATE_LOCKS_ON_LOCAL_DISK code relies on
+				// creating directories with mode=0777 that may be
+				// owned by any user who happens to need a lock. This
+				// is especially true for users with running
+				// jobs. Because local lock code uses a tree of these
+				// directories, it is possibly for a malicious user to
+				// satify FS authentication requirements by renamed a
+				// target user's lock directory into /tmp. To avoid
+				// this attack, the server side must also fail
+				// authentication for any directories whose mode is
+				// not 0700. -matt July 2012
+
 				// assume it is wrong until we see otherwise
 				bool stat_is_okay = false;
 
 				if ((stat_buf.st_nlink == 1 || stat_buf.st_nlink == 2) &&
 					(!S_ISLNK(stat_buf.st_mode)) && // check for soft link
-					(S_ISDIR(stat_buf.st_mode)) )   // check for directory
+					(S_ISDIR(stat_buf.st_mode)) &&      // check for directory
+					((stat_buf.st_mode & 07777) == 0700)) // check for mode=0700
 				{
 					// client created a directory as instructed
 					stat_is_okay = true;
-- 
1.7.10.4

diff --git a/src/condor_startd.V6/command.cpp b/src/condor_startd.V6/command.cpp
index fd3dd5a..0615350 100644
--- a/src/condor_startd.V6/command.cpp
+++ b/src/condor_startd.V6/command.cpp
@@ -624,50 +624,6 @@ command_match_info( Service*, int cmd, Stream* stream )
 
 
 int
-command_give_request_ad( Service*, int, Stream* stream) 
-{
-	int pid = -1;  // Starter sends it's pid so we know what
-				   // resource's request ad to send 
-	Claim*		claim;
-	ClassAd*	cp;
-
-	if( ! stream->code(pid) ) {
-		dprintf( D_ALWAYS, "give_request_ad: Can't read pid\n" );
-		stream->encode();
-		stream->end_of_message();
-		return FALSE;
-	}
-	if( ! stream->end_of_message() ) {
-		dprintf( D_ALWAYS, "give_request_ad: Can't read eom\n" );
-		stream->encode();
-		stream->end_of_message();
-		return FALSE;
-	}
-	claim = resmgr->getClaimByPid( pid );
-	if( !claim ) {
-		dprintf( D_ALWAYS, 
-				 "give_request_ad: Can't find starter with pid %d\n",
-				 pid ); 
-		stream->encode();
-		stream->end_of_message();
-		return FALSE;
-	}
-	cp = claim->ad();
-	if( !cp ) {
-		claim->dprintf( D_ALWAYS, 
-						"give_request_ad: current claim has NULL classad.\n" );
-		stream->encode();
-		stream->end_of_message();
-		return FALSE;
-	}
-	stream->encode();
-	cp->put( *stream );
-	stream->end_of_message();
-	return TRUE;
-}
-
-
-int
 command_query_ads( Service*, int, Stream* stream) 
 {
 	ClassAd queryAd;
diff --git a/src/condor_startd.V6/startd_main.cpp b/src/condor_startd.V6/startd_main.cpp
index 1d41db3..445f8d0 100644
--- a/src/condor_startd.V6/startd_main.cpp
+++ b/src/condor_startd.V6/startd_main.cpp
@@ -267,9 +267,6 @@ main_init( int, char* argv[] )
 								  "GIVE_TOTALS_CLASSAD",
 								  (CommandHandler)command_give_totals_classad,
 								  "command_give_totals_classad", 0, READ );
-	daemonCore->Register_Command( GIVE_REQUEST_AD, "GIVE_REQUEST_AD",
-								  (CommandHandler)command_give_request_ad,
-								  "command_give_request_ad", 0, READ );
 	daemonCore->Register_Command( QUERY_STARTD_ADS, "QUERY_STARTD_ADS",
 								  (CommandHandler)command_query_ads,
 								  "command_query_ads", 0, READ );

Reply to: