Replace get_url with shell and curl by william-stearns · Pull Request #58 · activecm/rita

william-stearns · 2025-07-02T19:47:19Z

Resolves #57 .

Co-Authored-By: William Stearns <[email protected]>

joelillo · 2025-07-03T19:46:17Z

Hey Bill! I'm curious if we could do something like the block I have included below. (Note that I haven't tested this yet - I need to head out for the weekend soon). I think this could maintain the current get_url functionality for ansible versions above a certain threshold, and fallback to the shell curl script if it get_url fails or if the version is below the threshold.

Any thoughts on something like that?

- name: "RITA Pre: Add Docker Ubuntu GPG apt key."
  # Note that apt-key is deprecated and that directly downloading the key to trusted.gpg.d WITH A .asc EXTENSION is the correct way now.
  get_url:
    url: https://download.docker.com/linux/ubuntu/gpg
    dest: /etc/apt/trusted.gpg.d/docker-ubuntu.asc
    mode: '0644'
    force: true
  when: ( ansible_distribution == 'Ubuntu' and ansible_version.full is version('2.10', '>='))
  register: get_url_result_ubuntu
  ignore_errors: yes
  tags:
    - packages
    - linux
    - linuxdeb

- name: Fallback to curl if get_url failed or Ansible < 2.10
  shell: curl -fsSL https://download.docker.com/linux/ubuntu/gpg -o /etc/apt/trusted.gpg.d/docker-ubuntu.asc
  when: >
    (ansible_version.full is version('2.10', '<')) or
    (get_url_result_ubuntu is failed)

william-stearns · 2025-07-03T20:01:02Z

I appreciate the idea, and doubly appreciate you writing it up. I'll admit to a preference for the "just curl" approach for two reasons: 1) I'm not aware of any additional features or functionality that get_url provides, and 2) having 2 code blocks means checking both in QA and being aware of both when someone writes in.
Your "do the second only if the first one fails" approach is enticing. If we can come up with an advantage to having get_url at all, I'd definitely do it the way you have above.

0x6d6f7468 · 2025-07-07T19:33:41Z

While @william-stearns' curl command works fine, I agree with @joelillo that we should always default to Ansible core built-in modules whenever possible and only fall back to shell commands in the event that an equivalent module does not exist (or, in this case, has a bug that causes said module to not function properly).

All of that said, I went ahead and tested/modified Bill's work and Joe's proposed solution somewhat, resulting in the following excerpt from rita_pre.yml:

#Add repositories
    - name: "RITA Pre: Add Docker Ubuntu GPG apt key."
      # Note that apt-key is deprecated and that directly downloading the key to trusted.gpg.d WITH A .asc EXTENSION is the correct way now.
      get_url:
        url: https://download.docker.com/linux/ubuntu/gpg
        dest: /etc/apt/trusted.gpg.d/docker-ubuntu.asc
        mode: '0644'
        force: true
      when: ( ansible_distribution == 'Ubuntu' )
      register: get_url_result_ubuntu
      ignore_errors: yes
      tags:
        - packages
        - linux
        - linuxdeb

    - name: "RITA Pre: Add Docker Debian GPG apt key."
      get_url:
        url: https://download.docker.com/linux/debian/gpg
        dest: /etc/apt/trusted.gpg.d/docker-debian.asc
        mode: '0644'
        force: true
      when: ( ansible_distribution == 'Debian' or ansible_distribution == 'Kali' or ansible_distribution == 'Pop!_OS' or ansible_distribution == 'Zorin OS' )
      register: get_url_result_debian
      ignore_errors: yes
      tags:
        - packages
        - linux
        - linuxdeb

    - name: "RITA Pre: Add Docker Ubuntu GPG apt key (Curl fallback due to get_url failure)"
      shell: curl -fsSL https://download.docker.com/linux/ubuntu/gpg -o /etc/apt/trusted.gpg.d/docker-ubuntu.asc
      when: >
        (ansible_distribution == 'Ubuntu') and
        (get_url_result_ubuntu is failed)

    - name: "RITA Pre: Add Docker Debian GPG apt key (Curl fallback due to get_url failure)"
      shell: curl -fsSL https://download.docker.com/linux/debian/gpg -o /etc/apt/trusted.gpg.d/docker-debian.asc
      when: >
        (ansible_distribution == 'Debian' or ansible_distribution == 'Kali' or ansible_distribution == 'Pop!_OS' or ansible_distribution == 'Zorin OS') and
        (get_url_result_ubuntu is failed)

Since it's hard to say if this is the only bug that we'll ever run into in the get_url module, and whether versions prior to 2.16.0 will be the only ones affected by this specific bug, I figure it's better to have the steps run and fail if they're going to, and then fall back to the curl alternative. The version condition does prevent the red error text from showing up, so if we're worried about that throwing users off we could keep that. Just thought I'd play with it a bit.

0x6d6f7468 · 2025-07-07T21:30:13Z

Something @joelillo suggested to me offline was the use of block/rescue instead of having two different steps. I played around with a sample he gave me and came up with the following:

# Add repositories
# Note that apt-key is deprecated and that directly downloading the key to trusted.gpg.d WITH A .asc EXTENSION is the correct way now.
    - name: "RITA Pre: Add Docker Ubuntu GPG apt key."
      block:
        - name: "RITA Pre: Download Docker Ubuntu GPG apt key with get_url module."

          get_url:
            url: https://download.docker.com/linux/ubuntu/gpg
            dest: /etc/apt/trusted.gpg.d/docker-ubuntu.asc
            mode: '0644'
            force: true
          register: get_url_result

      rescue:
        - name: "RITA Pre: Download failed with get_url module. Falling back to curl."
          shell: curl -fsSL https://download.docker.com/linux/ubuntu/gpg -o /etc/apt/trusted.gpg.d/docker-ubuntu.asc

      when: ( ansible_distribution == 'Ubuntu' )
      tags:
        - packages
        - linux
        - linuxdeb

I think this kind of structure would be beneficial for this case.

joelillo · 2025-07-07T21:36:36Z

Thanks for checking my work and testing this out @0x6d6f7468 😄

I like the idea of sticking to the Ansible modules when possible. The get_url module provides some nice features like check mode support, it detects if changes are actually needed and reports on it, clear logging...

It looks like the block/rescue pattern fits this scenario well. There is also the option to add additional logging if needed.

0x6d6f7468 · 2025-07-09T03:17:13Z

Bill and I chatted offline about this. While he still has some reservations about this change (specifically whether any benefits of using the get_url module outweigh the relative simplicity/ease of troubleshooting of curl), he agreed to let this change happen and gave me the OK to commit it myself.

Many thanks to @william-stearns for finding/reporting/investigating this issue and for providing the curl component of the solution.

Tested and working on all permutations of installation with Ubuntu, CentOS, and Rocky droplets, with both remote and local installations.

* Update on config structure, functionality, and tests Co-Authored-By: Naomi Kramer <[email protected]> * Extend subnet type to read/write from db, update tests Co-Authored-By: Liza Tsibur <[email protected]> * updated read file config test and subnet tests * fixed config and util tests, updated subnet related functions Co-Authored-By: Naomi Kramer <[email protected]> * Remove error return from GetDefaultConfig Co-Authored-By: Liza Tsibur <[email protected]> * added json tags to database struct * Updated beacon weights validation for config * updates to score thresholds validation tags * changes to config subnet validation and testing * Update subnet.go * Write missing host entries to http to populate http_proto * Updating some fields to uint64 * WIP update some field types * Update zeek count types and fix tests * Add clickhouse credentials * Misc fixes * Update pointer * Add ability to mark datasets as sample datasets * fix column name * Fix datasets exiting import if hour is empty * Fix zeek count parsing from TSV files * Remove storing dns conns in arrays, Fix historical first seen dns lag * Remove unused columns * Update config.hjson * Update config.hjson * updated impact category score functions to use float64 Co-Authored-By: Naomi Kramer <[email protected]> * Update subnet.go * Store import version in imports table * Fix duplicated SNI/IP long connections * Update subnet_test.go * Cleanup output * Rolling files updates (#39) * Limit number of days to import for rolling datasets * Fix breaking imports when import was interrupted * Remove debug output --------- Co-authored-by: Naomi Kramer <[email protected]> * Omit parts of env from output * Set max for threat intel datasize * Remove SELinux neutering for QA * Add network size column * Fix http_proto for missing host, update tests for missing host fixes * Add online feeds to default config * Update sshprep (#45) * Update sshprep Co-Authored-By: William Stearns <[email protected]> * Update sshprep Add Bradley's suggestion of using head -1 to limit to a single address. --------- Co-authored-by: Naomi Kramer <[email protected]> Co-authored-by: William Stearns <[email protected]> * Installer Behavior Tweaks (#41) * Add --yes flag to add-apt-repository command * Add missing sudo flags, make sure we're using the SUDO variable instead * Add ability to perform zone transfers (#48) * Store zone transfer records Co-Authored-By: moth <[email protected]> * Update config * Add tests * Tests, connectivity test * Update tests --------- Co-authored-by: moth <[email protected]> * Support RedHat/RHEL as a valid target (#47) * Update sshprep Co-Authored-By: William Stearns <[email protected]> * Supporrt RedHat/RHEL as a valid target --------- Co-authored-by: Naomi Kramer <[email protected]> Co-authored-by: William Stearns <[email protected]> Co-authored-by: moth <[email protected]> * Fix tests (#49) * Fix tests * Update WalkFiles to use UTC * fixed issue with rolling datasets over 24hours old not getting historical first seen timestamp set (#52) * Change values from float32 to float64 (#50) * Switch float32 to float64 * Update threat category calculation to match CalculateBucketedScore (#51) --------- Co-authored-by: Liza Tsibur <[email protected]> * Bump max query execution time default value * Use string instead of error for ZoneTransferConnectivityErrors struct fields (#61) * Upgrade Golang to version 1.24 (#59) (#60) * Replace get_url with shell and curl (#58) * Update sshprep Co-Authored-By: William Stearns <[email protected]> * Replace get_url with shell and curl * Use get_url by default, fall back to curl if it fails --------- Co-authored-by: Naomi Kramer <[email protected]> Co-authored-by: William Stearns <[email protected]> Co-authored-by: moth <[email protected]> * add automated log transfer, AC-Hunter issue 135 (#62) * Update sshprep Co-Authored-By: William Stearns <[email protected]> * add automated log transfer, PR135 * cron requires non-executable permission * Specify suggested YAML plugin and config in VSCode workspace * Linting and light cleanup * Update generate_installer.sh Download zeek_log_transport.sh to send to the sensor. * Create cron file if remote zeek installation * Only run zeek log import steps for remote sensor installations --------- Co-authored-by: Naomi Kramer <[email protected]> Co-authored-by: William Stearns <[email protected]> Co-authored-by: moth <[email protected]> * Temporarily disable RITA/Zeek log transport until installer is modular (#66) * Uniform -y flag usage for repo management/package installation; Uniform SUDO variable usage (#68) * Resolve Installer Side Effects and Formalize RHEL Support (#73) * Add missing necessary wildcards for RHEL versions * Remove Ansible task replacing python3-requests to avoid RHEL distro installation side effects * Update supported distros in README * Update scoring defaults * Resolve Ansible Reboot Errors (#75) * Clean up conditionals; Fix reboot step for Ubuntu * Suppress erroneous error output on RPM systems, ignore errors on reboot necessity checks * Ignore missing host rows for openhttp (#76) * Fix integration tests due to prevalence (#77) --------- Co-authored-by: Liza Tsibur <[email protected]> Co-authored-by: moth <[email protected]> Co-authored-by: William Stearns <[email protected]> Co-authored-by: William Stearns <[email protected]> Co-authored-by: moth <[email protected]>

caffeinatedpixel and others added 2 commits August 20, 2024 10:23

Update sshprep

93b2dc5

Co-Authored-By: William Stearns <[email protected]>

Replace get_url with shell and curl

415c3a8

william-stearns requested a review from a team July 2, 2025 19:47

william-stearns self-assigned this Jul 2, 2025

william-stearns added the bug Existing functionality is broken, incorrect, or not behaving as intended label Jul 2, 2025

0x6d6f7468 changed the base branch from main to config_update July 9, 2025 02:16

0x6d6f7468 added 2 commits July 8, 2025 20:17

Merge branch 'config_update' into wls_remove_get_url

1ef484d

Use get_url by default, fall back to curl if it fails

fafa6a5

0x6d6f7468 mentioned this pull request Jul 9, 2025

add automated log transfer, AC-Hunter issue 135 #62

Merged

lisaSW approved these changes Jul 9, 2025

View reviewed changes

lisaSW merged commit 2319c32 into config_update Jul 9, 2025
5 checks passed

lisaSW deleted the wls_remove_get_url branch July 9, 2025 17:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace get_url with shell and curl#58

Replace get_url with shell and curl#58
lisaSW merged 4 commits intoconfig_updatefrom
wls_remove_get_url

william-stearns commented Jul 2, 2025

Uh oh!

joelillo commented Jul 3, 2025

Uh oh!

william-stearns commented Jul 3, 2025 •

edited

Loading

Uh oh!

0x6d6f7468 commented Jul 7, 2025 •

edited

Loading

Uh oh!

0x6d6f7468 commented Jul 7, 2025

Uh oh!

joelillo commented Jul 7, 2025

Uh oh!

0x6d6f7468 commented Jul 9, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

william-stearns commented Jul 2, 2025

Uh oh!

joelillo commented Jul 3, 2025

Uh oh!

william-stearns commented Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0x6d6f7468 commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0x6d6f7468 commented Jul 7, 2025

Uh oh!

joelillo commented Jul 7, 2025

Uh oh!

0x6d6f7468 commented Jul 9, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

william-stearns commented Jul 3, 2025 •

edited

Loading

0x6d6f7468 commented Jul 7, 2025 •

edited

Loading