ENT-12414: Use librsync's Stream API in GET <FILENAME> requests

.github/workflows/acceptance_tests.yml

@@ -5,13 +5,13 @@ on:
 
 jobs:
   acceptance_tests:
-    runs-on: ubuntu-20.04
+    runs-on: ubuntu-24.04
     steps:
     - uses: actions/checkout@v3
       with:
         submodules: recursive
     - name: Install dependencies
-      run: sudo apt-get update -y && sudo apt-get install -y libssl-dev libpam0g-dev liblmdb-dev byacc curl libyaml-dev
+      run: sudo apt-get update -y && sudo apt-get install -y libssl-dev libpam0g-dev liblmdb-dev byacc curl libyaml-dev librsync-dev
     - name: Run autotools / configure
       run: ./autogen.sh --enable-debug
     - name: Compile and link (make)

.github/workflows/asan_unit_tests.yml

@@ -5,16 +5,16 @@ on:
 
 jobs:
   asan_unit_tests:
-    runs-on: ubuntu-20.04
+    runs-on: ubuntu-24.04
     steps:
     - uses: actions/checkout@v3
       with:
         submodules: recursive
     - name: Install dependencies
-      run: sudo apt-get update -y && sudo apt-get install -y libssl-dev libpam0g-dev liblmdb-dev byacc curl
+      run: sudo apt-get update -y && sudo apt-get install -y libssl-dev libpam0g-dev liblmdb-dev byacc curl librsync-dev
     - name: Run autotools / configure
       run: ./autogen.sh --enable-debug
     - name: Compile and link (make)
       run: make -j8 CFLAGS="-Werror -Wall -fsanitize=address" LDFLAGS="-fsanitize=address"
     - name: Run unit tests
-      run: make -C tests/unit CFLAGS="-fsanitize=address" LDFLAGS="-fsanitize=address" check
+      run: ASAN_OPTIONS=detect_odr_violation=0 make -C tests/unit CFLAGS="-fsanitize=address" LDFLAGS="-fsanitize=address" check

.github/workflows/codeql.yml

@@ -41,7 +41,7 @@ jobs:
 
       - name: Install dependencies (C)
         if: ${{ matrix.language == 'cpp' }}
-        run: sudo apt-get update -y && sudo apt-get install -y libssl-dev libpam0g-dev liblmdb-dev byacc curl
+        run: sudo apt-get update -y && sudo apt-get install -y libssl-dev libpam0g-dev liblmdb-dev byacc curl librsync-dev
 
       - name: Build (C)
         if: ${{ matrix.language == 'cpp' }}

.github/workflows/job-static-check.yml

@@ -5,7 +5,7 @@ on:
 
 jobs:
   static_check:
-    runs-on: ubuntu-latest
+    runs-on: ubuntu-24.04
     steps:
     - name: Checkout Core
       uses: actions/checkout@v3
@@ -41,7 +41,7 @@ jobs:
         sudo apt-get install -y dpkg-dev debhelper g++ libncurses6 pkg-config \
           build-essential libpam0g-dev fakeroot gcc make autoconf buildah \
           liblmdb-dev libacl1-dev libcurl4-openssl-dev libyaml-dev libxml2-dev \
-          libssl-dev libpcre2-dev
+          libssl-dev libpcre2-dev librsync-dev
 
     - name: Run Autogen
       run: NO_CONFIGURE=1 PROJECT=community ./buildscripts/build-scripts/autogen

.github/workflows/macos_unit_tests.yml

@@ -11,15 +11,15 @@ jobs:
       with:
         submodules: recursive
     - name: Install dependencies
-      run: brew install lmdb automake openssl pcre2 autoconf libtool
+      run: brew install lmdb automake openssl pcre2 autoconf libtool librsync
     - name: Check tools
       run: command -v libtool && command -v automake && command -v autoconf
     - name: Check tools versions
       run: libtool -V && automake --version && autoconf --version
     - name: Run autotools / configure
       run: >
-        LDFLAGS="-L`brew --prefix lmdb`/lib -L`brew --prefix openssl`/lib -L`brew --prefix pcre2`/lib"
-        CPPFLAGS="-I`brew --prefix lmdb`/include -I`brew --prefix openssl`/include -I`brew --prefix pcre2`/include"
+        LDFLAGS="-L`brew --prefix lmdb`/lib -L`brew --prefix openssl`/lib -L`brew --prefix pcre2`/lib -L`brew --prefix librsync`/lib"
+        CPPFLAGS="-I`brew --prefix lmdb`/include -I`brew --prefix openssl`/include -I`brew --prefix pcre2`/include -I`brew --prefix librsync`/include"
         PATH="/opt/homebrew/opt/libtool/libexec/gnubin:$PATH"
         ./autogen.sh --enable-debug
     - name: Compile and link

.github/workflows/shellcheck.yml

@@ -6,13 +6,13 @@ on:
 jobs:
   unit_tests:
     name: Run shellcheck on shell scripts
-    runs-on: ubuntu-20.04
+    runs-on: ubuntu-24.04
     steps:
     - uses: actions/checkout@v3
       with:
         submodules: recursive
     - name: Install dependencies
-      run: sudo apt-get update -y && sudo apt-get install -y libssl-dev libpam0g-dev liblmdb-dev byacc curl shellcheck
+      run: sudo apt-get update -y && sudo apt-get install -y libssl-dev libpam0g-dev liblmdb-dev byacc curl shellcheck librsync-dev
     - name: Run autotools / configure
       run: ./autogen.sh --enable-debug
     - name: Run shellcheck

.github/workflows/unit_tests.yml

@@ -6,13 +6,13 @@ on:
 jobs:
   unit_tests:
     name: Run Unit Tests
-    runs-on: ubuntu-20.04
+    runs-on: ubuntu-24.04
     steps:
     - uses: actions/checkout@v3
       with:
         submodules: recursive
     - name: Install dependencies
-      run: sudo apt-get update -y && sudo apt-get install -y libssl-dev libpam0g-dev liblmdb-dev byacc curl
+      run: sudo apt-get update -y && sudo apt-get install -y libssl-dev libpam0g-dev liblmdb-dev byacc curl librsync-dev
     - name: Run autotools / configure
       run: ./autogen.sh --enable-debug
     - name: Compile and link (make)

.github/workflows/valgrind.yml

@@ -5,7 +5,7 @@ on:
 
 jobs:
   valgrind_tests:
-    runs-on: ubuntu-22.04
+    runs-on: ubuntu-24.04
     defaults:
       run:
         working-directory: core
@@ -27,7 +27,7 @@ jobs:
         path: masterfiles
         submodules: recursive
     - name: Install dependencies
-      run: sudo apt-get update -y && sudo apt-get install -y libssl-dev libpam0g-dev liblmdb-dev byacc curl libyaml-dev valgrind
+      run: sudo apt-get update -y && sudo apt-get install -y libssl-dev libpam0g-dev liblmdb-dev byacc curl libyaml-dev valgrind librsync-dev
     - name: Run autotools / configure
       run: ./autogen.sh --enable-debug --with-systemd-service
     - name: Compile and link (make)

cf-agent/verify_files_utils.c

@@ -1551,7 +1551,7 @@ bool CopyRegularFile(EvalContext *ctx, const char *source, const char *dest, con
             return false;
         }
 
-        if (!CopyRegularFileNet(source, ToChangesPath(new),
+        if (!CopyRegularFileNet(source, dest, ToChangesPath(new),
                                 sstat->st_size, attr->copy.encrypt, conn, sstat->st_mode))
         {
             RecordFailure(ctx, pp, attr, "Failed to copy file '%s' from '%s'",

cf-serverd/server_common.c

@@ -54,6 +54,7 @@
 #include <mutex.h>                                 /* ThreadLock */
 #include <stat_cache.h>                            /* struct Stat */
 #include <unix.h>                                  /* GetUserID() */
+#include <file_stream.h>
 #include "server_access.h"
 
 
@@ -402,6 +403,9 @@
 
 void CfGetFile(ServerFileGetState *args)
 {
+    assert(args != NULL);
+    assert(args->conn != NULL);
+
     int fd;
     off_t n_read, total = 0, sendlen = 0, count = 0;
     char sendbuffer[CF_BUFSIZE + 256], filename[CF_BUFSIZE - 128];
@@ -421,12 +425,23 @@
 
     if (!TransferRights(args->conn, filename, &sb))
     {
+        const ProtocolVersion version = ConnectionInfoProtocolVersion(conn_info);
+        assert(ProtocolIsKnown(version));
+
         Log(LOG_LEVEL_INFO, "REFUSE access to file: %s", filename);
+
+        if (ProtocolSupportsFileStream(version)) {
+            Log(LOG_LEVEL_VERBOSE, "REFUSAL to user='%s' of request: %s",
+                NULL_OR_EMPTY(args->conn->username) ? "?" : args->conn->username,
+                args->replyfile);
+            FileStreamRefuse(args->conn->conn_info->ssl);
+            return;
+        }
+        /* Else then handle older protocols */
+
         RefuseAccess(args->conn, args->replyfile);
         snprintf(sendbuffer, CF_BUFSIZE, "%s", CF_FAILEDSTR);
 
-        const ProtocolVersion version = ConnectionInfoProtocolVersion(conn_info);
-        assert(ProtocolIsKnown(version));
         if (ProtocolIsClassic(version))
         {
             SendSocketStream(ConnectionInfoSocket(conn_info), sendbuffer, args->buf_size);
@@ -440,10 +455,16 @@
 
 /* File transfer */
 
+    const ProtocolVersion version = ConnectionInfoProtocolVersion(conn_info);
+    if (ProtocolSupportsFileStream(version)) {
+        FileStreamServe(conn_info->ssl, filename);
+        return;
+    }
+
     if ((fd = safe_open(filename, O_RDONLY)) == -1)
     {
         Log(LOG_LEVEL_ERR, "Open error of file '%s'. (open: %s)",
             filename, GetErrorStr());
         snprintf(sendbuffer, CF_BUFSIZE, "%s", CF_FAILEDSTR);
 
         const ProtocolVersion version = ConnectionInfoProtocolVersion(conn_info);
@@ -499,7 +520,7 @@
                 }
 
                 if (sb.st_size != savedlen)
                 {
                     snprintf(sendbuffer, CF_BUFSIZE, "%s%s: %s", CF_CHANGEDSTR1, CF_CHANGEDSTR2, filename);
 
                     const ProtocolVersion version = ConnectionInfoProtocolVersion(conn_info);
@@ -534,7 +555,7 @@
                     sendlen = (savedlen - total);
                 }
             }
 
             total += n_read;
 
             const ProtocolVersion version = ConnectionInfoProtocolVersion(conn_info);

configure.ac

@@ -521,6 +521,20 @@ CF3_WITH_LIBRARY(pcre2, [
   )]
 )
 
+dnl librsync
+
+AC_ARG_WITH([librsync], [AS_HELP_STRING([--with-librsync[[=PATH]]], [Specify librsync path])], [], [with_librsync=yes])
+
+if test "x$with_librsync" = "xno"; then
+   AC_MSG_ERROR([librsync is required])
+fi
+
+CF3_WITH_LIBRARY(librsync, [
+    AC_CHECK_HEADERS([librsync.h], [], AC_MSG_ERROR(Cannot find librsync))
+    AC_CHECK_LIB(rsync, rs_job_iter, [], [AC_MSG_ERROR(Cannot find librsync)])
+  ]
+)
+
 dnl defined for libntech
 
 AC_DEFINE(WITH_PCRE2, 1, [Define if PCRE2 is being used])
@@ -1317,10 +1331,10 @@ dnl ######################################################################
 dnl Collect all the options
 dnl ######################################################################
 
-CORE_CPPFLAGS="$LMDB_CPPFLAGS $TOKYOCABINET_CPPFLAGS $QDBM_CPPFLAGS $PCRE2_CPPFLAGS $OPENSSL_CPPFLAGS $SQLITE3_CPPFLAGS $LIBACL_CPPFLAGS $LIBCURL_CPPFLAGS $LIBYAML_CPPFLAGS $POSTGRESQL_CPPFLAGS $MYSQL_CPPFLAGS $LIBXML2_CPPFLAGS $CPPFLAGS $CFECOMPAT_CPPFLAGS"
-CORE_CFLAGS="$LMDB_CFLAGS $TOKYOCABINET_CFLAGS $QDBM_CFLAGS $PCRE2_CFLAGS $OPENSSL_CFLAGS $SQLITE3_CFLAGS $LIBACL_CFLAGS $LIBCURL_CFLAGS $LIBYAML_CFLAGS $POSTGRESQL_CFLAGS $MYSQL_CFLAGS $LIBXML2_CFLAGS $CFLAGS"
-CORE_LDFLAGS="$LMDB_LDFLAGS $TOKYOCABINET_LDFLAGS $QDBM_LDFLAGS $PCRE2_LDFLAGS $OPENSSL_LDFLAGS $SQLITE3_LDFLAGS $LIBACL_LDFLAGS $LIBCURL_LDFLAGS $LIBYAML_LDFLAGS $POSTGRESQL_LDFLAGS $MYSQL_LDFLAGS $LIBXML2_LDFLAGS $LDFLAGS"
-CORE_LIBS="$LMDB_LIBS $TOKYOCABINET_LIBS $QDBM_LIBS $PCRE2_LIBS $OPENSSL_LIBS $SQLITE3_LIBS $LIBACL_LIBS $LIBCURL_LIBS $LIBYAML_LIBS $POSTGRESQL_LIBS $MYSQL_LIBS $LIBXML2_LIBS $LIBS"
+CORE_CPPFLAGS="$LMDB_CPPFLAGS $TOKYOCABINET_CPPFLAGS $QDBM_CPPFLAGS $PCRE2_CPPFLAGS $OPENSSL_CPPFLAGS $SQLITE3_CPPFLAGS $LIBACL_CPPFLAGS $LIBCURL_CPPFLAGS $LIBRSYNC_CPPFLAGS $LIBYAML_CPPFLAGS $POSTGRESQL_CPPFLAGS $MYSQL_CPPFLAGS $LIBXML2_CPPFLAGS $CPPFLAGS $CFECOMPAT_CPPFLAGS"
+CORE_CFLAGS="$LMDB_CFLAGS $TOKYOCABINET_CFLAGS $QDBM_CFLAGS $PCRE2_CFLAGS $OPENSSL_CFLAGS $SQLITE3_CFLAGS $LIBACL_CFLAGS $LIBCURL_CFLAGS $LIBRSYNC_CFLAGS $LIBYAML_CFLAGS $POSTGRESQL_CFLAGS $MYSQL_CFLAGS $LIBXML2_CFLAGS $CFLAGS"
+CORE_LDFLAGS="$LMDB_LDFLAGS $TOKYOCABINET_LDFLAGS $QDBM_LDFLAGS $PCRE2_LDFLAGS $OPENSSL_LDFLAGS $SQLITE3_LDFLAGS $LIBACL_LDFLAGS $LIBCURL_LDFLAGS $LIBRSYNC_LDFLAGS $LIBYAML_LDFLAGS $POSTGRESQL_LDFLAGS $MYSQL_LDFLAGS $LIBXML2_LDFLAGS $LDFLAGS"
+CORE_LIBS="$LMDB_LIBS $TOKYOCABINET_LIBS $QDBM_LIBS $PCRE2_LIBS $OPENSSL_LIBS $SQLITE3_LIBS $LIBACL_LIBS $LIBCURL_LIBS $LIBRSYNC_LIBS $LIBYAML_LIBS $POSTGRESQL_LIBS $MYSQL_LIBS $LIBXML2_LIBS $LIBS"
 
 dnl ######################################################################
 dnl Make them available to subprojects.

libcfnet/Makefile.am

@@ -37,6 +37,7 @@ libcfnet_la_SOURCES = \
 	communication.c communication.h \
 	connection_info.c connection_info.h \
 	conn_cache.c conn_cache.h \
+	file_stream.c file_stream.h \
 	key.c key.h \
 	misc.c \
 	net.c net.h \

libcfnet/README.md

@@ -15,6 +15,7 @@ Names of protocol versions:
 1. `"classic"` - Legacy, pre-TLS, protocol. Not enabled or allowed by default.
 2. `"tls"` - TLS Protocol using OpenSSL. Encrypted and 2-way authentication.
 3. `"cookie"` - TLS Protocol with cookie command for duplicate host detection.
+3. `"filestream"` - Introduces a new streaming API for get file request (powered by librsync).
 
 Wanted protocol version can be specified from policy:
 
@@ -59,3 +60,30 @@ Both server and client will then set `conn_info->protocol` to `2`, and use proto
 There is currently no way to require a specific version number (only allow / disallow version 1).
 This is because version 2 and 3 are practically identical.
 Downgrade from version 3 to 2 happens seamlessly, but crucially, it doesn't downgrade to version 1 inside the TLS code.
+
+## Commands
+
+### `GET <FILENAME>` (protocol v4)
+
+The following is a description of the `GET <FILENAME>` command, modified in
+protocol version v4 (introduced in CFEngine 3.25).
+
+The initial motivation for creating a new protocol version `filestream` was
+due to a race condition found in the `GET <FILENAME>` request. It relied on the
+file size aquired by `STAT <FILENAME>`. However, if the file size increased
+between the two requests, the client would think that the remaining data at the
+offset of the aquired file size is a new protocol header. This situation would lead
+to undefined behaviour. Hence, we needed a new protocol to send files. Instead
+of reinventing the wheel, we decided to use librsync which utilizes the RSYNC
+protocol to transmit files.
+
+The server implementation is found in function
+[CfGet()](../cf-serverd/server_common.c). Client impementations are found in
+[CopyRegularFileNet()](client_code.c) and [ProtocolGet()](protocol.c)
+
+Similar to before, the client issues a `GET <FILENAME>` request. However,
+instead of continuing to execute the old protocol, the client immediately calls
+`FileStreamFetch()` from the "File Stream API". Upon receiving such a request,
+the server calls either `FileStreamRefuse()` (to refuse the request) or
+`FileStreamServe()` (to comply with the request). The internal workings of the
+File Stream API is well explained in [file_stream.h](file_stream.h).

libcfnet/client_code.c

@@ -45,6 +45,7 @@
 #include <misc_lib.h>                                   /* ProgrammingError */
 #include <printsize.h>                                         /* PRINTSIZE */
 #include <lastseen.h>                                            /* LastSaw */
+#include <file_stream.h>
 
 
 #define CFENGINE_SERVICE "cfengine"
@@ -749,9 +750,11 @@
 
 /* TODO finalise socket or TLS session in all cases that this function fails
  * and the transaction protocol is out of sync. */
-bool CopyRegularFileNet(const char *source, const char *dest, off_t size,
+bool CopyRegularFileNet(const char *source, const char *basis, const char *dest, off_t size,
                         bool encrypt, AgentConnection *conn, mode_t mode)
 {
+    assert(conn != NULL);
+
     char *buf, workbuf[CF_BUFSIZE], cfchangedstr[265];
     const int buf_size = 2048;
 
@@ -774,23 +777,12 @@
 
     unlink(dest);                /* To avoid link attacks */
 
-    int dd = safe_open_create_perms(dest, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL | O_BINARY, mode);
-    if (dd == -1)
-    {
-        Log(LOG_LEVEL_ERR,
-            "Copy from server '%s' to destination '%s' failed (open: %s)",
-            conn->this_server, dest, GetErrorStr());
-        unlink(dest);
-        return false;
-    }
-
     workbuf[0] = '\0';
     int tosend = snprintf(workbuf, CF_BUFSIZE, "GET %d %s", buf_size, source);
     if (tosend <= 0 || tosend >= CF_BUFSIZE)
     {
         Log(LOG_LEVEL_ERR, "Failed to compose GET command for file %s",
             source);
-        close(dd);
         return false;
     }
 
@@ -799,7 +791,21 @@
     if (SendTransaction(conn->conn_info, workbuf, tosend, CF_DONE) == -1)
     {
         Log(LOG_LEVEL_ERR, "Couldn't send GET command");
-        close(dd);
+        return false;
+    }
+
+    const ProtocolVersion version = ConnectionInfoProtocolVersion(conn->conn_info);
+    if (ProtocolSupportsFileStream(version)) {
+        return FileStreamFetch(conn->conn_info->ssl, basis, dest, mode);
+    }
+
+    int dd = safe_open_create_perms(dest, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL | O_BINARY, mode);
+    if (dd == -1)
+    {
+        Log(LOG_LEVEL_ERR,
+            "Copy from server '%s' to destination '%s' failed (open: %s)",
+            conn->this_server, dest, GetErrorStr());
+        unlink(dest);
         return false;
     }
 
@@ -817,7 +823,7 @@
         assert(toget > 0);
 
         /* Stage C1 - receive */
         int n_read;
 
         const ProtocolVersion version = conn->conn_info->protocol;
 

libcfnet/client_code.h

@@ -47,7 +47,7 @@ AgentConnection *ServerConnection(const char *server, const char *port, const Rl
 void DisconnectServer(AgentConnection *conn);
 
 bool CompareHashNet(const char *file1, const char *file2, bool encrypt, AgentConnection *conn);
-bool CopyRegularFileNet(const char *source, const char *dest, off_t size,
+bool CopyRegularFileNet(const char *source, const char *basis, const char *dest, off_t size,
                         bool encrypt, AgentConnection *conn, mode_t mode);
 Item *RemoteDirList(const char *dirname, bool encrypt, AgentConnection *conn);