Redmine #6027: Directories randomly changed to files.

This is quite nasty and border case scenario. For recursive file
copying if connection was timeouted recourse copying was continued.
In some cases timeouted package was sent by the server (usually under
heavy load) and was wrongly interpreted as response for next request.

Usually (if not always) directories were created first and then files,
so most often directory was replaced with a file and content of the file
was corrupted protocol leftovers (t 74OK: 0 432 0 0 20010 1302 140197032)
which is CFEngine copy file protocol transaction header and data.

The fix is to drpo connection immediately after experiencing timeouts in
all cases where files and/or directories are copied recursively in loop.

Changelog: Redmine #6027 Directories should no more be changed randomly
into files.

(cherry picked from commit 94ea8cd)

cf-agent/verify_files_utils.c

@@ -823,7 +823,16 @@ static PromiseResult SourceSearchAndCopy(EvalContext *ctx, const char *from, cha
         /* This sends 1st STAT command. */
         if (!ConsiderAbstractFile(dirp->d_name, from, attr.copy, conn))
         {
-            continue;
+            if (conn != NULL && conn->conn_info->is_broken)
+            {
+                cfPS(ctx, LOG_LEVEL_INFO, PROMISE_RESULT_INTERRUPTED, pp,
+                     attr, "connection timeout");
+                return PROMISE_RESULT_INTERRUPTED;
+            }
+            else
+            {
+                continue;
+            }
         }
 
         if (attr.copy.purge)    /* Purge this file */
@@ -854,15 +863,34 @@ static PromiseResult SourceSearchAndCopy(EvalContext *ctx, const char *from, cha
             if (cf_stat(newfrom, &sb, attr.copy, conn) == -1)
             {
                 Log(LOG_LEVEL_VERBOSE, "Can't stat '%s'. (cf_stat: %s)", newfrom, GetErrorStr());
-                continue;                                       /* TODO FAIL? */
+                if (conn != NULL && conn->conn_info->is_broken)
+                {
+                    cfPS(ctx, LOG_LEVEL_INFO, PROMISE_RESULT_INTERRUPTED, pp,
+                         attr, "connection timeout");
+                    return PROMISE_RESULT_INTERRUPTED;
+                }
+                else
+                {
+                    continue;
+                }
             }
         }
         else
         {
             if (cf_lstat(newfrom, &sb, attr.copy, conn) == -1)
             {
                 Log(LOG_LEVEL_VERBOSE, "Can't stat '%s'. (cf_stat: %s)", newfrom, GetErrorStr());
-                continue;                                       /* TODO FAIL? */
+                if (conn != NULL && conn->conn_info->is_broken)
+                {
+                    cfPS(ctx, LOG_LEVEL_INFO,
+                         PROMISE_RESULT_INTERRUPTED, pp, attr,
+                         "connection timeout");
+                    return PROMISE_RESULT_INTERRUPTED;
+                }
+                else
+                {
+                    continue;
+                }
             }
         }
 
@@ -1032,7 +1060,16 @@ static PromiseResult VerifyCopy(EvalContext *ctx,
             if (!ConsiderAbstractFile(dirp->d_name, sourcedir,
                                       attr.copy, conn))
             {
-                continue;
+                if (conn != NULL && conn->conn_info->is_broken)
+                {
+                    cfPS(ctx, LOG_LEVEL_INFO, PROMISE_RESULT_INTERRUPTED,
+                         pp, attr, "connection timeout");
+                    return PROMISE_RESULT_INTERRUPTED;
+                }
+                else
+                {
+                    continue;
+                }
             }
 
             strcpy(sourcefile, sourcedir);

libcfnet/client_code.c

@@ -754,7 +754,7 @@ int CopyRegularFileNet(const char *source, const char *dest, off_t size,
     {
         int toget = MIN(size - n_read_total, buf_size);
 
-        assert(toget != 0);
+        assert(toget > 0);
 
         /* Stage C1 - receive */
         int n_read;

libcfnet/connection_info.h

@@ -56,6 +56,7 @@ struct ConnectionInfo {
     socklen_t ss_len;
     struct sockaddr_storage ss;
     bool is_call_collect;       /* Maybe replace with a bitfield later ... */
+    bool is_broken; /* used to propagate connection errors up in function calls */
 };
 
 typedef struct ConnectionInfo ConnectionInfo;

libcfnet/net.c

@@ -127,7 +127,7 @@ int SendTransaction(const ConnectionInfo *conn_info,
  *  @TODO shutdown() the connection in all cases were this function returns -1,
  *        in order to protect against future garbage reads.
  */
-int ReceiveTransaction(const ConnectionInfo *conn_info, char *buffer, int *more)
+int ReceiveTransaction(ConnectionInfo *conn_info, char *buffer, int *more)
 {
     char proto[CF_INBAND_OFFSET + 1] = { 0 };
     int ret;
@@ -151,6 +151,13 @@ int ReceiveTransaction(const ConnectionInfo *conn_info, char *buffer, int *more)
      * closed. Connection has been finalised. */
     if (ret == -1 || ret == 0)
     {
+        /* We are experiencing problems with receiving data from server.
+         * This might lead to packages being not delivered in correct
+         * order and unexpected issues like directories being replaced
+         * with files.
+         * In order to make sure that file transfer is reliable we have to
+         * close connection to avoid broken packages being received. */
+        conn_info->is_broken = true;
         return ret;
     }
     else if (ret != CF_INBAND_OFFSET)
@@ -160,6 +167,7 @@ int ReceiveTransaction(const ConnectionInfo *conn_info, char *buffer, int *more)
         Log(LOG_LEVEL_ERR,
             "ReceiveTransaction: bogus short header (%d bytes: '%s')",
             ret, proto);
+        conn_info->is_broken = true;
         return -1;
     }
 
@@ -173,18 +181,21 @@ int ReceiveTransaction(const ConnectionInfo *conn_info, char *buffer, int *more)
     {
         Log(LOG_LEVEL_ERR,
             "ReceiveTransaction: bogus header: %s", proto);
+        conn_info->is_broken = true;
         return -1;
     }
     if (status != CF_MORE && status != CF_DONE)
     {
         Log(LOG_LEVEL_ERR,
             "ReceiveTransaction: bogus header (more='%c')", status);
+        conn_info->is_broken = true;
         return -1;
     }
     if (len > CF_BUFSIZE - CF_INBAND_OFFSET)
     {
         Log(LOG_LEVEL_ERR,
             "ReceiveTransaction: packet too long (len=%d)", len);
+        conn_info->is_broken = true;
         return -1;
     }
     else if (len <= 0)
@@ -193,6 +204,7 @@ int ReceiveTransaction(const ConnectionInfo *conn_info, char *buffer, int *more)
          * ReceiveTransaction() == 0 currently means connection closed. */
         Log(LOG_LEVEL_ERR,
             "ReceiveTransaction: packet too short (len=%d)", len);
+        conn_info->is_broken = true;
         return -1;
     }
 
@@ -227,8 +239,9 @@ int ReceiveTransaction(const ConnectionInfo *conn_info, char *buffer, int *more)
         ret = -1;
     }
 
-    if (ret == -1 || ret == 0)
+    if (ret <= 0)
     {
+        conn_info->is_broken = true;
         return ret;
     }
     else if (ret != len)
@@ -244,6 +257,7 @@ int ReceiveTransaction(const ConnectionInfo *conn_info, char *buffer, int *more)
         Log(LOG_LEVEL_ERR,
             "Partial transaction read %d != %d bytes!",
             ret, len);
+        conn_info->is_broken = true;
         return -1;
     }
 

libcfnet/net.h

@@ -35,7 +35,7 @@ extern uint32_t bwlimit_kbytes;
 
 
 int SendTransaction(const ConnectionInfo *conn_info, const char *buffer, int len, char status);
-int ReceiveTransaction(const ConnectionInfo *conn_info, char *buffer, int *more);
+int ReceiveTransaction(ConnectionInfo *conn_info, char *buffer, int *more);
 
 int SetReceiveTimeout(int fd, unsigned long ms);