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 <m...@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 <tstcl...@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 <m...@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 <zmil...@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 );
