client: improve file checking on task start

client/app_start.cpp

@@ -540,7 +540,6 @@ int ACTIVE_TASK::start(bool test) {
     char cmdline[80000];    // 64KB plus some extra
     unsigned int i;
     FILE_REF fref;
-    FILE_INFO* fip;
     int retval;
     APP_INIT_DATA aid;
 #ifdef _WIN32
@@ -567,21 +566,17 @@ int ACTIVE_TASK::start(bool test) {
     if (app_version->avg_ncpus < 1) high_priority = true;
     if (app_version->is_wrapper) high_priority = true;
 
-    if (wup->project->verify_files_on_app_start) {
-        fip=0;
-        retval = gstate.input_files_available(result, true, &fip);
-        if (retval) {
-            if (fip) {
-                snprintf(
-                    buf, sizeof(buf),
-                    "Input file %s missing or invalid: %s",
-                    fip->name, boincerror(retval)
-                );
-            } else {
-                safe_strcpy(buf, "Input file missing or invalid");
-            }
-            goto error;
-        }
+    // make sure the task files exist
+    //
+    FILE_INFO* fip = 0;
+    retval = gstate.task_files_present(result, true, &fip);
+    if (retval) {
+        snprintf(
+            buf, sizeof(buf),
+            "Task file %s: %s",
+            fip->name, boincerror(retval)
+        );
+        goto error;
     }
 
     current_cpu_time = checkpoint_cpu_time;
@@ -1144,17 +1139,21 @@ int ACTIVE_TASK::start(bool test) {
     set_task_state(PROCESS_EXECUTING, "start");
     return 0;
 
-    // go here on error; "buf" contains error message, "retval" is nonzero
-    //
 error:
+    // here on error; "buf" contains error message, "retval" is nonzero
+    //
     if (test) {
         return retval;
     }
 
-    // if something failed, it's possible that the executable was munged.
-    // Verify it to trigger another download.
+    // if failed to run program, it's possible that the executable was munged.
+    // Verify the app version files to detect this
+    // and trigger another download if it's the case
     //
-    gstate.input_files_available(result, true);
+    if (retval == ERR_EXEC) {
+        gstate.verify_app_version_files(result);
+    }
+
     char err_msg[4096];
     snprintf(err_msg, sizeof(err_msg), "couldn't start app: %.256s", buf);
     gstate.report_result_error(*result, err_msg);

client/client_state.cpp

@@ -1802,7 +1802,7 @@ bool CLIENT_STATE::update_results() {
             break;
 #ifndef SIM
         case RESULT_FILES_DOWNLOADING:
-            if (input_files_available(rp, false) == 0) {
+            if (task_files_present(rp, false) == 0) {
                 if (rp->avp->app_files.size()==0) {
                     // if this is a file-transfer app, start the upload phase
                     //

client/client_state.h

@@ -358,7 +358,8 @@ struct CLIENT_STATE {
 
 // --------------- cs_apps.cpp:
     double get_fraction_done(RESULT* result);
-    int input_files_available(RESULT*, bool, FILE_INFO** f=0);
+    int task_files_present(RESULT*, bool check_size, FILE_INFO** f=0);
+    int verify_app_version_files(RESULT*);
     ACTIVE_TASK* lookup_active_task_by_result(RESULT*);
     int n_usable_cpus;
         // number of usable CPUs

client/client_types.h

@@ -153,6 +153,7 @@ struct FILE_INFO {
     void failure_message(std::string&);
     int merge_info(FILE_INFO&);
     int verify_file(bool, bool, bool);
+    int check_size();
     bool verify_file_certs();
     int gzip();
         // gzip file and add .gz to name

client/cs_apps.cpp

@@ -215,62 +215,79 @@ int CLIENT_STATE::app_finished(ACTIVE_TASK& at) {
     return 0;
 }
 
-// Returns zero iff all the input files for a result are present
-// (both WU and app version)
-// Called from CLIENT_STATE::update_results (with verify=false)
-// to transition result from DOWNLOADING to DOWNLOADED.
-// Called from ACTIVE_TASK::start() (with verify=true)
-// when project has verify_files_on_app_start set.
+// Check whether the input and app version files for a result are
+// marked as FILE_PRESENT.
+// If check_size is set, also check whether the exist and have right size.
+// Called from:
+// CLIENT_STATE::update_results (with check_existence=false)
+//      to transition result from DOWNLOADING to DOWNLOADED.
+// ACTIVE_TASK::start() (with check_existence=true)
+//      to check files before running a task
 //
 // If fipp is nonzero, return a pointer to offending FILE_INFO on error
 //
-int CLIENT_STATE::input_files_available(
-    RESULT* rp, bool verify_contents, FILE_INFO** fipp
+int CLIENT_STATE::task_files_present(
+    RESULT* rp, bool check_size, FILE_INFO** fipp
 ) {
     WORKUNIT* wup = rp->wup;
     FILE_INFO* fip;
     unsigned int i;
-    APP_VERSION* avp;
-    FILE_REF fr;
-    PROJECT* project = rp->project;
-    int retval;
+    APP_VERSION* avp = rp->avp; 
+    int retval, ret = 0;
 
-    avp = rp->avp;
     for (i=0; i<avp->app_files.size(); i++) {
-        fr = avp->app_files[i];
-        fip = fr.file_info;
+        fip = avp->app_files[i].file_info;
         if (fip->status != FILE_PRESENT) {
             if (fipp) *fipp = fip;
-            return ERR_FILE_MISSING;
-        }
-
-        // don't verify app files if using anonymous platform
-        //
-        if (verify_contents && !project->anonymous_platform) {
-            retval = fip->verify_file(true, true, false);
+            ret = ERR_FILE_MISSING;
+        } else if (check_size) {
+            retval = fip->check_size();
             if (retval) {
                 if (fipp) *fipp = fip;
-                return retval;
+                ret = retval;
             }
         }
     }
 
     for (i=0; i<wup->input_files.size(); i++) {
+        if (wup->input_files[i].optional) continue;
         fip = wup->input_files[i].file_info;
         if (fip->status != FILE_PRESENT) {
-            if (wup->input_files[i].optional) continue;
             if (fipp) *fipp = fip;
-            return ERR_FILE_MISSING;
-        }
-        if (verify_contents) {
-            retval = fip->verify_file(true, true, false);
+            ret = ERR_FILE_MISSING;
+        } else if (check_size) {
+            retval = fip->check_size();
             if (retval) {
                 if (fipp) *fipp = fip;
-                return retval;
+                ret = retval;
             }
         }
     }
-    return 0;
+    return ret;
+}
+
+// The app for the given result failed to start.
+// Verify the app version files; maybe one of them was corrupted.
+//
+int CLIENT_STATE::verify_app_version_files(RESULT* rp) {
+    int ret = 0;
+    FILE_INFO* fip;
+    PROJECT* project = rp->project;
+
+    if (project->anonymous_platform) return 0;
+    APP_VERSION* avp = rp->avp; 
+    for (unsigned int i=0; i<avp->app_files.size(); i++) {
+        fip = avp->app_files[i].file_info;
+        int retval = fip->verify_file(true, true, false);
+        if (retval && log_flags.task_debug) {
+            msg_printf(fip->project, MSG_INFO,
+                "app version file %s: bad contents",
+                fip->name
+            );
+            ret = retval;
+        }
+    }
+    return ret;
 }
 
 inline double force_fraction(double f) {

client/cs_files.cpp

@@ -235,8 +235,8 @@ int FILE_INFO::verify_file(
                 name, nbytes, size
             );
         }
-        status = ERR_WRONG_SIZE;
-        return ERR_WRONG_SIZE;
+        status = ERR_FILE_WRONG_SIZE;
+        return ERR_FILE_WRONG_SIZE;
     }
 
     if (!verify_contents) return 0;
@@ -466,6 +466,34 @@ bool CLIENT_STATE::create_and_delete_pers_file_xfers() {
 }
 #endif
 
+// check whether file exists and has the right size
+//
+int FILE_INFO::check_size() {
+    char path[MAXPATHLEN];
+    get_pathname(this, path, sizeof(path));
+    double size;
+    int retval = file_size(path, size);
+    if (retval) {
+        delete_project_owned_file(path, true);
+        status = FILE_NOT_PRESENT;
+        msg_printf(project, MSG_INFO, "File %s not found", path);
+        return ERR_FILE_MISSING;
+    }
+    if (gstate.global_prefs.dont_verify_images && is_image_file(path)) {
+        return 0;
+    }
+    if (nbytes && (size != nbytes)) {
+        delete_project_owned_file(path, true);
+        status = FILE_NOT_PRESENT;
+        msg_printf(project, MSG_INFO,
+            "File %s has wrong size: expected %.0f, got %.0f",
+            path, nbytes, size
+        );
+        return ERR_FILE_WRONG_SIZE;
+    }
+    return 0;
+}
+
 // for each FILE_INFO (i.e. each project file the client knows about)
 // check that the file exists and is of the right size.
 // Called at startup.
@@ -487,22 +515,8 @@ void CLIENT_STATE::check_file_existence() {
         }
         if (cc_config.dont_check_file_sizes) continue;
         if (fip->status == FILE_PRESENT) {
-            get_pathname(fip, path, sizeof(path));
-            double size;
-            int retval = file_size(path, size);
-            if (retval) {
-                delete_project_owned_file(path, true);
-                fip->status = FILE_NOT_PRESENT;
-                msg_printf(fip->project, MSG_INFO, "File %s not found", path);
-            } else if (fip->nbytes && (size != fip->nbytes)) {
-                if (gstate.global_prefs.dont_verify_images && is_image_file(path)) continue;
-                delete_project_owned_file(path, true);
-                fip->status = FILE_NOT_PRESENT;
-                msg_printf(fip->project, MSG_INFO,
-                    "File %s has wrong size: expected %.0f, got %.0f",
-                    path, fip->nbytes, size
-                );
-            }
+            fip->check_size();
+
             // If an output file disappears before it's uploaded,
             // flag the job as an error.
             //

client/project.cpp

@@ -87,7 +87,6 @@ void PROJECT::init() {
     disk_share = 0.0;
     anonymous_platform = false;
     non_cpu_intensive = false;
-    verify_files_on_app_start = false;
     report_results_immediately = false;
     pwf.reset(this);
     send_time_stats_log = 0;
@@ -246,7 +245,6 @@ int PROJECT::parse_state(XML_PARSER& xp) {
         if (xp.parse_bool("send_full_workload", send_full_workload)) continue;
         if (xp.parse_bool("dont_use_dcf", dont_use_dcf)) continue;
         if (xp.parse_bool("non_cpu_intensive", non_cpu_intensive)) continue;
-        if (xp.parse_bool("verify_files_on_app_start", verify_files_on_app_start)) continue;
         if (xp.parse_bool("suspended_via_gui", suspended_via_gui)) continue;
         if (xp.parse_bool("dont_request_more_work", dont_request_more_work)) continue;
         if (xp.parse_bool("detach_when_done", detach_when_done)) continue;
@@ -423,7 +421,7 @@ int PROJECT::write_state(MIOFILE& out, bool gui_rpc) {
         "    <njobs_error>%d</njobs_error>\n"
         "    <elapsed_time>%f</elapsed_time>\n"
         "    <last_rpc_time>%f</last_rpc_time>\n"
-        "%s%s%s%s%s%s%s%s%s%s%s%s%s%s",
+        "%s%s%s%s%s%s%s%s%s%s%s%s%s",
         master_url,
         project_name,
         symstore,
@@ -467,7 +465,6 @@ int PROJECT::write_state(MIOFILE& out, bool gui_rpc) {
         send_full_workload?"    <send_full_workload/>\n":"",
         dont_use_dcf?"    <dont_use_dcf/>\n":"",
         non_cpu_intensive?"    <non_cpu_intensive/>\n":"",
-        verify_files_on_app_start?"    <verify_files_on_app_start/>\n":"",
         suspended_via_gui?"    <suspended_via_gui/>\n":"",
         dont_request_more_work?"    <dont_request_more_work/>\n":"",
         detach_when_done?"    <detach_when_done/>\n":"",
@@ -609,7 +606,6 @@ void PROJECT::copy_state_fields(PROJECT& p) {
     send_time_stats_log = p.send_time_stats_log;
     send_job_log = p.send_job_log;
     non_cpu_intensive = p.non_cpu_intensive;
-    verify_files_on_app_start = p.verify_files_on_app_start;
     suspended_via_gui = p.suspended_via_gui;
     dont_request_more_work = p.dont_request_more_work;
     detach_when_done = p.detach_when_done;

client/project.h

@@ -170,10 +170,6 @@ struct PROJECT : PROJ_AM {
     bool non_cpu_intensive;
         // All this project's apps are non-CPU-intensive.
         // Apps can also be individually marked as NCI
-    bool verify_files_on_app_start;
-        // Check app version and input files on app startup,
-        // to make sure they haven't been tampered with.
-        // This provides only the illusion of security.
     bool use_symlinks;
     bool report_results_immediately;
     bool sched_req_no_work[MAX_RSC];

client/scheduler_op.cpp

@@ -586,7 +586,6 @@ int SCHEDULER_REPLY::parse(FILE* in, PROJECT* project) {
     MIOFILE mf;
     XML_PARSER xp(&mf);
     string delete_file_name;
-    bool verify_files_on_app_start = false;
     bool non_cpu_intensive = false;
     bool ended = false;
 
@@ -649,7 +648,6 @@ int SCHEDULER_REPLY::parse(FILE* in, PROJECT* project) {
             // boolean project attributes.
             // If the scheduler reply didn't specify them, they're not set.
             //
-            project->verify_files_on_app_start = verify_files_on_app_start;
             project->non_cpu_intensive = non_cpu_intensive;
             project->ended = ended;
             return 0;
@@ -882,8 +880,6 @@ int SCHEDULER_REPLY::parse(FILE* in, PROJECT* project) {
                 handle_no_rsc_apps(buf, project, true);
             }
             continue;
-        } else if (xp.parse_bool("verify_files_on_app_start", verify_files_on_app_start)) {
-            continue;
         } else if (xp.parse_bool("send_full_workload", send_full_workload)) {
             continue;
         } else if (xp.parse_bool("dont_use_dcf", dont_use_dcf)) {

lib/boinc_stdio.h

@@ -66,7 +66,7 @@ namespace boinc {
 #ifdef _USING_FCGI_
         int i=FCGI_vfprintf(fp,format,va);
 #else
-        int i=::fprintf(fp,format,va);
+        int i=::vfprintf(fp,format,va);
 #endif
         va_end(va);
         return i;

lib/error_numbers.h

@@ -195,6 +195,7 @@
 #define ERR_TRUNCATE        -218
 #define ERR_WRONG_URL       -219
 #define ERR_DUP_NAME        -220
+#define ERR_FILE_WRONG_SIZE -221
 #define ERR_GETGRNAM        -222
 #define ERR_CHOWN           -223
 #define ERR_HTTP_PERMANENT  -224

lib/str_util.cpp

@@ -572,7 +572,7 @@ const char* boincerror(int which_error) {
         case ERR_ABORTED_VIA_GUI: return "result aborted via GUI";
         case ERR_INSUFFICIENT_RESOURCE: return "insufficient resources";
         case ERR_RETRY: return "retry";
-        case ERR_WRONG_SIZE: return "wrong size";
+        case ERR_WRONG_SIZE: return "wrong buffer size";
         case ERR_USER_PERMISSION: return "user permission";
         case ERR_BAD_EMAIL_ADDR: return "bad email address";
         case ERR_BAD_PASSWD: return "bad password";
@@ -593,6 +593,7 @@ const char* boincerror(int which_error) {
         case ERR_TRUNCATE: return "truncate() failed";
         case ERR_WRONG_URL: return "wrong URL";
         case ERR_DUP_NAME: return "coprocs with duplicate names detected";
+        case ERR_FILE_WRONG_SIZE: return "file has the wrong size";
         case ERR_GETGRNAM: return "getgrnam() failed";
         case ERR_CHOWN: return "chown() failed";
         case ERR_HTTP_PERMANENT: return "permanent HTTP error";