search

crowdsecurity / cs-firewall-bouncer

Crowdsec bouncer written in golang for firewalls
MIT License
119 stars 43 forks source link

Installation fails with JSON output configured as default #322

Closed pascal-zarrad closed 1 year ago

pascal-zarrad commented 1 year ago

I just stumbled upon some really weird bug regarding the installation of this bouncer while writing an Ansible role. The installation fails when cscli.output is configured and set to json for the CrowdSec engine before the bouncer is installed.

If this issue is approved, I'd implement the solution mentioned below and open a PR.

Reproduce

  1. Setup CrowdSec repository and install the engine according to the official documentation
  2. Either create config.yaml.local or edit the existing config.yaml and set cscli.output to json
  3. Install crowdsec-firewall-bouncer-iptables using apt

Expected

Installation of the bouncer completes as expected and the bouncer is started.

The api_url for the bouncer is configured like this:

...
api_url: http://127.0.0.1:8080
...

Actual

The bouncer uses the cscli to get the address for the local API.

For the formats, the output looks like this:

The quotation marks in the JSON output cause the value in the generated crowdsec-firewall-bouncer.yaml to look like this:

...
api_url: http://127.0.0.1:8080"/
...

Solution

Firstly, the workaround is to just stick to human or raw for cscli.output to ensure the installation works as expected.

The long term solution would be to enforce raw output on the commands in _bouncer.sh, like it is already used in this line. This is for the bouncers add command, however the --output option is also available for config show.

Example with cscli.output: json in config.yaml:

$ cscli config show --key "Config.API.Server.ListenURI"
"127.0.0.1:8080"
$ cscli config -oraw show --key "Config.API.Server.ListenURI"
127.0.0.1:8080

The -oraw should be added to the following two lines to ensure JSON doesn't break:

I choose raw instead of human as I think it matches the use case here best.

System Info

OS: Debian 12 Source: APT

LaurenceJJones commented 1 year ago

we should just enforce the output format we expect, however, this issue will be across all bouncer repos as the same script is deployed across all of them. Assigned mmetc to have visibility.

mmetc commented 1 year ago

Done, and thanks for reporting!

Although I wouldn't recommend a global setting for the cscli output, it's too easy to break scripts by accident.