[RFE]: Ability to execute commands without a shell environment #3858

mqus · 2024-10-15T23:46:10Z

Feature request type

I feel really unsafe letting "attackers" insert strings into my commands while a shell is also trying to interpret them. Of course, I don't mean to imply that the included regexes are vulnerable but I would feel much safer if there would be a way to just execute a command directly, without a shell looking for $, ``` , \\, etcpp. (and this is just when using double quotes). I would really like the additional line of defense. Not needing to invoke a shell also provides a small speed boost.

For me the main use case would be ignorecommand so I can do stuff like

ignorecommand = test '<F-USER>' = specialuser

without worrying that I will write an imperfect regex some time in the future or forgot the quotes. But I imagine the same applies to other command options.

I also don't think the in-shell execution should be turned off or get replaced, there is clearly a use case for it and it is powerful.

Description

I would imagine some kind of toggle (or alternative options)
where commands are interpreted in a very simple way:

split command string on every non-escaped whitespace (maybe backslash as the escape character)
For each part of the array, replace the placeholders with actual values (after splitting so spaces in placeholders don't get "interpreted")
Run the command with shell=False

I saw that Utils.executeCommand already has a shell=False option, but afaict it is not accessible from config files and it also does not do substitution before splitting (does it actually split? not sure)

Considered alternatives

Just keeping the current state might also be fine. But I would really welcome if there was at least a line like "this will execute in a shell but don't worry, we took care of securing it" or "this will execute in a shell, make sure to quote all placeholders with single-ticks" in the man page.
Right now, I was really surprised it executes in a shell at all (I'm trying out fail2ban for the first time today).

Any additional information

As written before, I'm a fail2ban noob. I might have made wrong assumptions so please tell me if I am wrong somewhere.

PS: I grepped the jail.conf man page for "shell" for good measure and found that placeholders do get escaped at least. This weakens my point a bit but I still think that leaving out the shell would be a great option to have.

The text was updated successfully, but these errors were encountered:

sebres · 2024-10-16T10:35:46Z

I would feel much safer if there would be a way to just execute a command directly, without a shell looking for $, ``` , \\, etcpp. (and this is just when using double quotes).

It is fully safe to puts fail2ban tags in double quotes (it is even mandatory if the string could contain some special chars), because fail2ban uses safe escape internally - it would wrap the value to a variable and then put $f2bV_... instead of the value to the command line.

See

fail2ban/fail2ban/tests/actiontestcase.py

Lines 467 to 482 in 0bf1106

    
           def testExecuteWithVars(self): 
        
           	self.assertTrue(self.__action.executeCmd( 
        
           		r'''printf %b "foreign input:\n''' 
        
           		r''' -- $f2bV_A --\n''' 
        
           		r''' -- $f2bV_B --\n''' 
        
           		r''' -- $(echo -n $f2bV_C) --''' # echo just replaces \n to test it as single line 
        
           		r'''"''',  
        
           		varsDict={ 
        
           		'f2bV_A': 'I\'m a hacker; && $(echo $f2bV_B)',  
        
           		'f2bV_B': 'I"m very bad hacker',  
        
           		'f2bV_C': '`Very | very\n$(bad & worst hacker)`' 
        
           	})) 
        
           	self.assertLogged(r"""foreign input:""", 
        
           		' -- I\'m a hacker; && $(echo $f2bV_B) --', 
        
           		' -- I"m very bad hacker --', 
        
           		' -- `Very | very $(bad & worst hacker)` --', all=True)

for several tests covering that.

So for a username with value like I'm a hacker; && $(rm -rf ...), fail2ban will wrap:

- test "<F-USER>"
+ test "$f2bV_F_USER"; # f2bV_F_USER variable contains `I'm a hacker; && $(rm -rf ...)` by invocation

Even with single quotes it would be be safe, just...

- test '<F-USER>'
+ test '$f2bV_F_USER'

just with single quotes, it'd not be substituted in the shell, so you'd get $f2bV_F_USER instead of value for first parameter of the command. The thing works single-quoted (mostly for backwards-compat), only as long as string value is safe (doesn't contain special characters).

Also note that fail2ban supports pythonic actions (invoking python instead of shell)... However there is no such pythonic facility for ignorecommand at the moment.

mqus · 2024-10-16T14:05:42Z

So I actually cannot use single quotes for this currently? Is this documented somewhere because I think this is a massive surprise. And I actually have to use double quotes or else whitespace in the matched expressions will destroy my commands, right?

Is there a reason this is not just a simple string replacement?

mqus · 2024-10-16T14:07:14Z

All of these "surprises" to me show even clearer why the simple variant is necessary imho

sebres · 2024-10-16T16:07:21Z

So I actually cannot use single quotes for this currently?

You can, but for simple plain values (without special characters). If you don't know the content or know that it'd contain special chars, then use double quotes.

The special chars are currently - \ # & ; ` | * ? ~ < > ^ ( ) [ ] { } $ ' " NL CR.
See the RE matching them:

fail2ban/fail2ban/server/action.py

Line 700 in 0bf1106

ESCAPE_CRE = re.compile(r"""[\\#&;`|*?~<>^()\[\]{}$'"\n\r]""")

And I actually have to use double quotes or else whitespace in the matched expressions will destroy my commands, right?

No. Neither whitespace (excepting NL and CR) is special char in this sense, nor it would destroy something, if it gets "escaped" as variable (by values with some special chars) - enclosed in single quotes it would simply get a unsubstituted $f2bV_... instead of value (stored in variable). With other words you'd get a variable name as value not the value.
Typical example is #2310 and similar.

Is there a reason this is not just a simple string replacement?

Because it is almost impossible to implement it correctly or else fully safe, especially without to know how it gets included in the shell script (inclusive mixed quotation, e. g. mixed double and single quotes). Here is the PR #1726 for that.
If you rather mean a sanitized replacement (e. g. $ -> DOLLAR), then please read #1726 (comment)

All of these "surprises" to me show even clearer why the simple variant is necessary imho

I'm not against this RFE (otherwise I had simply close it)... Just... What you're calling "simple variant" - is not so simple, neither in the action (interpolation of command line from string command with variadic arguments as pythonic array), nor in the suitable way in the configuration reader... However surely possible.

mqus added the enhancement label Oct 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RFE]: Ability to execute commands without a shell environment #3858

[RFE]: Ability to execute commands without a shell environment #3858

mqus commented Oct 15, 2024 •

edited

Loading

sebres commented Oct 16, 2024 •

edited

Loading

mqus commented Oct 16, 2024

mqus commented Oct 16, 2024

sebres commented Oct 16, 2024

[RFE]: Ability to execute commands without a shell environment #3858

[RFE]: Ability to execute commands without a shell environment #3858

Comments

mqus commented Oct 15, 2024 • edited Loading

Feature request type

Description

Considered alternatives

Any additional information

sebres commented Oct 16, 2024 • edited Loading

mqus commented Oct 16, 2024

mqus commented Oct 16, 2024

sebres commented Oct 16, 2024

mqus commented Oct 15, 2024 •

edited

Loading

sebres commented Oct 16, 2024 •

edited

Loading