feat(sensor_download): adds lock files to prevent collision when down…

…loading similar sensors (#569)

* fix(falcon_install): add default sensor dl path to fix speed issues

Fixes #567

This PR fixes potential speed issues when working with large amounts of
hosts because for each host, it used to create a temp directory and
download the sensor. This of course is not very efficient, thus by
specifying a default such as /tmp - this should speed up download times
as hosts with the same sensor will not have to re-download every time.

* fix(falcon_install): make sure temp paths are static

This fixes an issue that will cause operations like copying the sensor
to create a new temp directory on the target host everytime it runs. For
example, in scenarios where something happens before the sensor is
cleaned, then running the role again will cause a new directory and
another full copy operation instead of checking to see if the sensor is
already copied.

* fix: cleaning up local sensor download dest should run once

* feat(sensor_download): adds the ability to lock files to prevent collision

This feature allows multiple hosts downloading the same file to not
overstep on each other as was the case before. This route improves
overall download times as the module will only download a sensor at most
once (assuming uniqueness) while other systems wait.

* chore: add sane retry_interval of 5 seconds

TBD - consider making this an option in the future?

* fix(falcon_install): win_temp_directory does not have path anymore

The win_file module does not return a path attribute. Instead of using
path, we'll just the variable instead as this is the path.

* chore: add delay to sensor_download flock

To prevent thundering herd!

* chore: add changelog

* lint: ignore bandit warning

* fix(falcon_install): update windows to use falcon_windows_tmp_dir variable

* fix(falcon_install): use win_stat to expand %SYSTEMROOT% properly

windows sucks... nuff said :(

* chore: changelog updates

* chore: add notes to sensor_download docs

Explain that a 0-byte lock file is created and can be safely deleted

* chore: revert back module arg 'name' changes

The refactoring was not needed and this makes more sense from an
organizational perspective.

changelogs/fragments/add-locking.yml

@@ -0,0 +1,5 @@
+minor_changes:
+  - sensor_download - adds the ability to lock files to prevent collision when downloading the sensor (https://github.com/CrowdStrike/ansible_collection_falcon/pull/569)
+
+bugfixes:
+  - falcon_install - fix issue with temp directories being random or non-existent (https://github.com/CrowdStrike/ansible_collection_falcon/pull/569)

changelogs/fragments/fix-truthy-564.yml

@@ -1,2 +1,2 @@
 bugfixes:
-- falcon_configure - Fix truthy condition for falcon_cid and falcon_provisioning_token (https://github.com/CrowdStrike/ansible_collection_falcon/pull/565)
+  - falcon_configure - Fix truthy condition for falcon_cid and falcon_provisioning_token (https://github.com/CrowdStrike/ansible_collection_falcon/pull/565)

plugins/modules/sensor_download.py

@@ -49,6 +49,12 @@
   - crowdstrike.falcon.credentials
   - crowdstrike.falcon.credentials.auth
 
+notes:
+  - This module implements file locking to ensure safe concurrent downloads by preventing multiple
+    instances from accessing the same file simultaneously. As a result, a temporary 0-byte .lock
+    file will be created in the same directory as the downloaded file. If needed, this lock file
+    can be safely removed in a subsequent task after the download completes.
+
 requirements:
   - Sensor download [B(READ)] API scope
 
@@ -83,8 +89,12 @@
   sample: /tmp/tmpzy7hn29t/falcon-sensor.deb
 """
 
+import errno
+import fcntl
 import traceback
 import os
+import time
+import random
 from tempfile import mkdtemp
 
 from ansible.module_utils.basic import AnsibleModule, missing_required_lib
@@ -127,6 +137,79 @@ def update_permissions(module, changed, path):
     return module.set_fs_attributes_if_different(file_args, changed=changed)
 
 
+def lock_file(file_path, exclusive=True, timeout=300, retry_interval=5):
+    """Lock a file for reading or writing."""
+    lock_file_path = file_path + ".lock"
+    # Ignore the pylint warning here as a with block will close the file handle immediately
+    # and we need to keep it open to maintain the lock
+    lock_file_handle = open(lock_file_path, 'w', encoding='utf-8')  # pylint: disable=consider-using-with
+    start_time = time.time()
+    # Implement a delay to prevent thundering herd
+    delay = random.random()  # nosec
+
+    while True:
+        try:
+            if exclusive:
+                fcntl.flock(lock_file_handle, fcntl.LOCK_EX | fcntl.LOCK_NB)
+            else:
+                fcntl.flock(lock_file_handle, fcntl.LOCK_SH | fcntl.LOCK_NB)
+            return lock_file_handle
+        except IOError as e:
+            if e.errno != errno.EAGAIN:
+                raise
+            if time.time() - start_time > timeout:
+                return None
+            time.sleep(delay + retry_interval)
+            delay = 0
+
+
+def unlock_file(locked_file):
+    """Unlock a file."""
+    fcntl.flock(locked_file, fcntl.LOCK_UN)
+    locked_file.close()
+
+
+def check_destination_path(module, dest):
+    """Check if the destination path is valid."""
+    if not os.path.isdir(dest):
+        module.fail_json(msg=f"Destination path does not exist or is not a directory: {dest}")
+
+    if not os.access(dest, os.W_OK):
+        module.fail_json(msg=f"Destination path is not writable: {dest}")
+
+
+def handle_existing_file(module, result, path, sensor_hash):
+    """Handle the case where the file already exists."""
+    # Compare sha256 hashes to see if any changes have been made
+    dest_hash = module.sha256(path)
+    if dest_hash == sensor_hash:
+        # File already exists and content is the same. Update permissions if needed.
+        msg = "File already exists and content is the same."
+
+        if update_permissions(module, result["changed"], path):
+            msg += " Permissions were updated."
+            result.update(changed=True)
+
+        module.exit_json(
+            msg=msg,
+            path=path,
+            **result,
+        )
+
+
+def download_sensor_installer(module, result, falcon, sensor_hash, path):
+    """Download the sensor installer."""
+    # Because this returns a binary, we need to handle errors differently
+    download = falcon.download_sensor_installer(id=sensor_hash)
+
+    if isinstance(download, dict):
+        # Error as download should not be a dict (from FalconPy)
+        module.fail_json(msg="Unable to download sensor installer", **result)
+
+    with open(path, "wb") as save_file:
+        save_file.write(download)
+
+
 def main():
     """Entry point for module execution."""
     module = AnsibleModule(
@@ -153,12 +236,7 @@ def main():
         tmp_dir = True
 
     # Make sure path exists and is a directory
-    if not os.path.isdir(dest):
-        module.fail_json(msg=f"Destination path does not exist or is not a directory: {dest}")
-
-    # Make sure path is writable
-    if not os.access(dest, os.W_OK):
-        module.fail_json(msg=f"Destination path is not writable: {dest}")
+    check_destination_path(module, dest)
 
     falcon = authenticate(module, SensorDownload)
 
@@ -175,51 +253,38 @@ def main():
             name = sensor_check["body"]["resources"][0]["name"]
 
         path = os.path.join(dest, name)
+        lock = None
+
+        try:
+            lock = lock_file(path, timeout=300, retry_interval=5)
+            if not lock:
+                module.fail_json(msg=f"Unable to acquire lock for file: {path} after 5 minutes.", **result)
 
-        # Check if the file already exists
-        if not tmp_dir and os.path.isfile(path):
-            # Compare sha256 hashes to see if any changes have been made
-            dest_hash = module.sha256(path)
-            if dest_hash == sensor_hash:
-                # File already exists and content is the same. Update permissions if needed.
-                msg = "File already exists and content is the same."
+            # Check if the file already exists
+            if not tmp_dir and os.path.isfile(path):
+                handle_existing_file(module, result, path, sensor_hash)
 
-                if update_permissions(module, result["changed"], path):
-                    msg += " Permissions were updated."
-                    result.update(changed=True)
+            # If we get here, the file either doesn't exist or has changed
+            result.update(changed=True)
 
+            if module.check_mode:
                 module.exit_json(
-                    msg=msg,
+                    msg=f"File would have been downloaded: {path}",
                     path=path,
                     **result,
                 )
 
-        # If we get here, the file either doesn't exist or has changed
-        result.update(changed=True)
-
-        if module.check_mode:
-            module.exit_json(
-                msg=f"File would have been downloaded: {path}",
-                path=path,
-                **result,
-            )
-
-        # Download the sensor installer
-        # Because this returns a binary, we need to handle errors differently
-        download = falcon.download_sensor_installer(id=sensor_hash)
-
-        if isinstance(download, dict):
-            # Error as download should not be a dict (from FalconPy)
-            module.fail_json(msg="Unable to download sensor installer", **result)
-
-        with open(path, "wb") as save_file:
-            save_file.write(download)
+            # Download the sensor installer
+            download_sensor_installer(module, result, falcon, sensor_hash, path)
 
-        # Set permissions on the file
-        update_permissions(module, result["changed"], path)
+            # Set permissions on the file
+            update_permissions(module, result["changed"], path)
 
-        result.update(path=path)
-        module.exit_json(**result)
+            result.update(path=path)
+            module.exit_json(**result)
+        finally:
+            if lock:
+                unlock_file(lock)
     else:
         # Should be caught by handle_return_errors, but just in case.
         module.fail_json(

roles/falcon_install/README.md

@@ -29,7 +29,7 @@ The following variables are currently supported:
 - `falcon_allow_downgrade` - Whether or not to allow downgrading the sensor version (bool, default: ***false***)
 - `falcon_gpg_key_check` - Whether or not to verify the Falcon sensor Linux based package (bool, default: ***true***)
   - :warning: When `falcon_install_method` is set to **api**, this value will be fetched by the API unless specified.
-- `falcon_install_tmp_dir` - Temporary Linux and MacOS installation directory for the Falson Sensor (string, default: ***/tmp***)
+- `falcon_install_tmp_dir` - Temporary Linux and MacOS installation directory for the Falson Sensor (string, default: ***/tmp/falcon-sensor***)
 - `falcon_retries` - Number of attempts to download the sensor (int, default: ***3***)
 - `falcon_delay` - Number of seconds before trying another download attempt (int, default: ***3***)
 
@@ -44,7 +44,7 @@ The following variables are currently supported:
     - **us-gov-1** -> api.laggar.gcw.crowdstrike.com
     - **eu-1** -> api.eu-1.crowdstrike.com
 - `falcon_api_enable_no_log` - Whether to enable or disable the logging of sensitive data being exposed in API calls (bool, default: ***true***)
-- `falcon_api_sensor_download_path` - Local directory path to download the sensor to (string, default: ***null***)
+- `falcon_api_sensor_download_path` - Local directory path to download the sensor to (string, default: ***/tmp/falcon-sensor***)
 - `falcon_api_sensor_download_mode` - The file permissions to set on the downloaded sensor (string, default: ***null***)
 - `falcon_api_sensor_download_owner` - The owner to set on the downloaded sensor (string, default: ***null***)
 - `falcon_api_sensor_download_group` - The group to set on the downloaded sensor (string, default: ***null***)
@@ -70,7 +70,7 @@ The following variables are currently supported:
 - `falcon_cid` - Specify CrowdStrike Customer ID with Checksum (string, default: ***null***)
 - `falcon_windows_install_retries` - Number of times to retry sensor install on windows (int, default: ***10***)
 - `falcon_windows_install_delay` - Number of seconds to wait to retry sensor install on windows in the event of a failure (int, default: ***120***)
-- `falcon_windows_tmp_dir` - Temporary Windows installation directory for the Falson Sensor (string, default: ***%SYSTEMROOT%\\Temp***)
+- `falcon_windows_tmp_dir` - Temporary Windows installation directory for the Falson Sensor (string, default: ***%SYSTEMROOT%\\Temp\\falcon-sensor***)
 - `falcon_windows_install_args` - Additional Windows install arguments (string, default: ***/norestart***)
 - `falcon_windows_uninstall_args` - Additional Windows uninstall arguments (string, default: ***/norestart***)
 - `falcon_windows_become` - Whether to become a privileged user on Windows (bool, default: ***true***)

roles/falcon_install/defaults/main.yml

@@ -23,10 +23,9 @@ falcon_install_method: api
 # The local directory path to download the sensor to.
 # This is in relation to the localhost running the role.
 #
-# If path is not specified, a temporary directory will be created using the system's
-# default temporary directory.
+# By default, this will be the temp OS filesystem
 #
-falcon_api_sensor_download_path:
+falcon_api_sensor_download_path: "/tmp/falcon-sensor"
 
 # The name to save the sensor file as.
 #
@@ -125,7 +124,7 @@ falcon_sensor_update_policy_name: ""
 # Where should the sensor file be copied to on Linux and MacOS systems?
 # By default, this will be the temp OS filesystem
 #
-falcon_install_tmp_dir: "/tmp"
+falcon_install_tmp_dir: "/tmp/falcon-sensor"
 
 # If the installation method is 'url', provide the url for the sensor to
 # be downloaded from.
@@ -160,7 +159,7 @@ falcon_windows_install_delay: 120
 #
 # For Windows, this can be "%SYSTEMROOT%\\Temp"
 #
-falcon_windows_tmp_dir: "%SYSTEMROOT%\\Temp"
+falcon_windows_tmp_dir: "%SYSTEMROOT%\\Temp\\falcon-sensor"
 
 # Additional install arguments beyond the default required
 #

roles/falcon_install/tasks/api.yml

@@ -40,13 +40,12 @@
       ansible.builtin.set_fact:
         falcon_sensor_version: "+version:'{{ falcon_sensor_update_policy_package_version }}'"
 
-
 - name: "CrowdStrike Falcon | Build API Sensor Query"
   ansible.builtin.set_fact:
-    falcon_os_query: "os:'{{ falcon_target_os }}'+os_version:'{{ falcon_os_version }}'\
+    falcon_os_query:
+      "os:'{{ falcon_target_os }}'+os_version:'{{ falcon_os_version }}'\
       {{ falcon_os_arch | default('') }}{{ falcon_sensor_version | default('') }}"
 
-
 - name: CrowdStrike Falcon | Get list of filtered Falcon sensors
   crowdstrike.falcon.sensor_download_info:
     auth: "{{ falcon.auth }}"
@@ -60,6 +59,15 @@
     msg: "No Falcon Sensor was found! If passing in falcon_sensor_version, ensure it is correct!"
   when: falcon_api_installer_list.installers[0] is not defined
 
+- name: CrowdStrike Falcon | Ensure download path exists (local)
+  ansible.builtin.file:
+    path: "{{ falcon_api_sensor_download_path }}"
+    state: directory
+    mode: "0755"
+  changed_when: false
+  delegate_to: localhost
+  run_once: true
+
 - name: CrowdStrike Falcon | Download Falcon Sensor Installation Package (local)
   crowdstrike.falcon.sensor_download:
     auth: "{{ falcon.auth }}"
@@ -85,18 +93,20 @@
 - name: CrowdStrike Falcon | Copy Sensor Installation Package to remote host (windows)
   ansible.windows.win_copy:
     src: "{{ falcon_sensor_download.path }}"
-    dest: "{{ falcon_install_win_temp_directory.path }}"
-    mode: 0640
+    dest: "{{ falcon_windows_tmp_dir_stat.stat.path }}"
   changed_when: false
   register: win_falcon_sensor_copied
   when: ansible_os_family == "Windows"
 
-- name: CrowdStrike Falcon | Remove Downloaded Sensor Installation Package (local)
+- name: CrowdStrike Falcon | Remove Downloaded Sensor Installation directory (local)
   ansible.builtin.file:
-    path: "{{ falcon_sensor_download.path }}"
+    path: "{{ item }}"
     state: absent
   changed_when: false
   delegate_to: localhost
+  loop:
+    - "{{ falcon_sensor_download.path }}"
+    - "{{ falcon_sensor_download.path + '.lock' }}"
   when: falcon_api_sensor_download_cleanup
 
 - name: CrowdStrike Falcon | Set full file download path (non-windows)

roles/falcon_install/tasks/install.yml

@@ -56,23 +56,10 @@
     manager: auto
   when: ansible_facts['distribution'] != "MacOSX"
 
-- name: CrowdStrike Falcon | Gather tmp install directory objects
-  ansible.builtin.find:
-    paths: "{{ falcon_install_tmp_dir }}"
-    patterns: "ansible.*falcon"
-    file_type: directory
-  register: falcon_tmp_dir_objects
-  when: falcon_install_tmp_dir | length > 0
-  changed_when: no
-
-- name: CrowdStrike Falcon | Remove tmp install directories
+- name: CrowdStrike Falcon | Remove tmp install directory
   ansible.builtin.file:
-    path: "{{ item.path }}"
+    path: "{{ falcon_install_tmp_dir }}"
     state: absent
-  loop: "{{ falcon_tmp_dir_objects.files }}"
-  when:
-    - falcon_install_tmp_dir | length > 0
-    - falcon_tmp_dir_objects is defined and falcon_tmp_dir_objects.files | length > 0
   changed_when: no
 
 - name: CrowdStrike Falcon | Remove Falcon Sensor Package (local file)

roles/falcon_install/tasks/preinstall.yml

@@ -80,28 +80,33 @@
     falcon_sensor_update_policy_platform: "{{ ansible_facts['os_family'] }}"
   when: ansible_facts['os_family'] == "Windows"
 
-- name: CrowdStrike Falcon | Verify Temporary Install Directory Exists (non-Windows)
-  ansible.builtin.tempfile:
+- name: CrowdStrike Falcon | Ensure Temporary Install Directory Exists (non-Windows)
+  ansible.builtin.file:
     path: "{{ falcon_install_tmp_dir }}"
     state: directory
-    suffix: falcon
+    mode: '0755'
   when:
     - ansible_facts['system'] == "Linux" or ansible_facts['system'] == "Darwin"
     - falcon_install_tmp_dir is defined
   register: falcon_install_temp_directory
   changed_when: no
 
-- name: CrowdStrike Falcon | Verify Temporary Install Directory Exists (Windows)
-  ansible.windows.win_tempfile:
+- name: CrowdStrike Falcon | Ensure Temporary Install Directory Exists (Windows)
+  ansible.windows.win_file:
     path: "{{ falcon_windows_tmp_dir }}"
     state: directory
-    suffix: falcon
   when:
     - ansible_facts['os_family'] == "Windows"
-    - falcon_windows_tmp_dir is defined
-  register: falcon_install_win_temp_directory
   changed_when: no
 
+- name: CrowdStrike Falcon | Validate Temporary install directory (Windows)
+  ansible.windows.win_stat:
+    path: "{{ falcon_windows_tmp_dir }}"
+  when:
+    - ansible_facts['os_family'] == "Windows"
+  register: falcon_windows_tmp_dir_stat
+  failed_when: false
+
 - name: CrowdStrike Falcon | Verify Falcon is not already installed (macOS)
   ansible.builtin.stat:
     path: "{{ falcon_path }}"