From aa8d278c9cc05d8b6848ec150ae6eea61d1f9295 Mon Sep 17 00:00:00 2001 From: Carlos Matos Date: Wed, 23 Oct 2024 09:54:57 -0400 Subject: [PATCH] feat(sensor_download): adds lock files to prevent collision when downloading 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 | 5 + changelogs/fragments/fix-truthy-564.yml | 2 +- plugins/modules/sensor_download.py | 147 +++++++++++++++------ roles/falcon_install/README.md | 6 +- roles/falcon_install/defaults/main.yml | 9 +- roles/falcon_install/tasks/api.yml | 24 +++- roles/falcon_install/tasks/install.yml | 17 +-- roles/falcon_install/tasks/preinstall.yml | 21 +-- roles/falcon_install/tasks/win_install.yml | 22 +-- roles/falcon_install/tasks/win_url.yml | 4 +- 10 files changed, 155 insertions(+), 102 deletions(-) create mode 100644 changelogs/fragments/add-locking.yml diff --git a/changelogs/fragments/add-locking.yml b/changelogs/fragments/add-locking.yml new file mode 100644 index 00000000..c5ed0e1b --- /dev/null +++ b/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) diff --git a/changelogs/fragments/fix-truthy-564.yml b/changelogs/fragments/fix-truthy-564.yml index b4b672d3..a0254d04 100644 --- a/changelogs/fragments/fix-truthy-564.yml +++ b/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) diff --git a/plugins/modules/sensor_download.py b/plugins/modules/sensor_download.py index 3c7e81bc..5ffd32e4 100644 --- a/plugins/modules/sensor_download.py +++ b/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( diff --git a/roles/falcon_install/README.md b/roles/falcon_install/README.md index 1ad05086..a809d56d 100644 --- a/roles/falcon_install/README.md +++ b/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***) diff --git a/roles/falcon_install/defaults/main.yml b/roles/falcon_install/defaults/main.yml index e6d465c8..4c6aea68 100644 --- a/roles/falcon_install/defaults/main.yml +++ b/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 # diff --git a/roles/falcon_install/tasks/api.yml b/roles/falcon_install/tasks/api.yml index b82bde53..f4264b95 100644 --- a/roles/falcon_install/tasks/api.yml +++ b/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) diff --git a/roles/falcon_install/tasks/install.yml b/roles/falcon_install/tasks/install.yml index 4351580f..b387a2f3 100644 --- a/roles/falcon_install/tasks/install.yml +++ b/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) diff --git a/roles/falcon_install/tasks/preinstall.yml b/roles/falcon_install/tasks/preinstall.yml index 0f6664e6..11dee2c6 100644 --- a/roles/falcon_install/tasks/preinstall.yml +++ b/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 }}" diff --git a/roles/falcon_install/tasks/win_install.yml b/roles/falcon_install/tasks/win_install.yml index 8d950853..d231f765 100644 --- a/roles/falcon_install/tasks/win_install.yml +++ b/roles/falcon_install/tasks/win_install.yml @@ -4,7 +4,7 @@ path: "{{ falcon_sensor_pkg }}" state: present creates_service: csfalconservice - arguments: '/install /quiet CID={{ falcon_cid }} {{ falcon_windows_install_args }}' + arguments: "/install /quiet CID={{ falcon_cid }} {{ falcon_windows_install_args }}" when: - ansible_facts['os_family'] == "Windows" register: falcon_installed @@ -12,26 +12,8 @@ delay: "{{ falcon_windows_install_delay }}" until: falcon_installed is success -- name: CrowdStrike Falcon | Expand tmp install directory objects path (Windows) - ansible.windows.win_stat: - path: "{{ falcon_windows_tmp_dir }}" - register: falcon_windows_expanded_path - -- name: CrowdStrike Falcon | Gather tmp install directory objects (Windows) - ansible.windows.win_find: - paths: "{{ falcon_windows_expanded_path.stat.path }}" - patterns: "ansible.*falcon" - file_type: directory - register: falcon_tmp_dir_objects - when: falcon_windows_tmp_dir | length > 0 - changed_when: no - - name: CrowdStrike Falcon | Remove tmp install directory (Windows) ansible.windows.win_file: - path: "{{ item.path }}" + path: "{{ falcon_windows_tmp_dir_stat.stat.path }}" state: absent - loop: "{{ falcon_tmp_dir_objects.files }}" - when: - - falcon_windows_tmp_dir | length > 0 - - falcon_tmp_dir_objects is defined and falcon_tmp_dir_objects.files | length > 0 changed_when: no diff --git a/roles/falcon_install/tasks/win_url.yml b/roles/falcon_install/tasks/win_url.yml index 312db570..9149da3b 100644 --- a/roles/falcon_install/tasks/win_url.yml +++ b/roles/falcon_install/tasks/win_url.yml @@ -2,12 +2,12 @@ - name: CrowdStrike Falcon | Downloading Installation Package from URL (Windows) ansible.windows.win_get_url: url: "{{ falcon_download_url }}" - dest: "{{ falcon_install_win_temp_directory.path }}" + dest: "{{ falcon_windows_tmp_dir_stat.stat.path }}" url_username: "{{ falcon_download_url_username | default(omit) }}" url_password: "{{ falcon_download_url_password | default(omit) }}" when: - falcon_download_url - - falcon_install_win_temp_directory + - falcon_windows_tmp_dir_stat.stat.path register: falcon_sensor_download retries: "{{ falcon_retries }}" delay: "{{ falcon_delay }}"