Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do more handholding for the configuration #300

Open
4 tasks
jvoisin opened this issue Jul 25, 2019 · 2 comments
Open
4 tasks

Do more handholding for the configuration #300

jvoisin opened this issue Jul 25, 2019 · 2 comments

Comments

@jvoisin
Copy link
Owner

jvoisin commented Jul 25, 2019

  • Suggest to use value when a value_r is unnecessarily used
  • Warn when groups are used in regexps, since it's almost always a bad idea to use them in Snuffleupagus, instead of writing better rules
  • Warn about stupid regexp, like ^(a+)+$
  • Provide more contextual info in case of a syntax error
    • "It seems that you forgot to close a ""
    • "Unrecognized keyword, did you mean to use XXX instead?", likely using a simple Levenstein distance computation
@WhiteWinterWolf
Copy link
Contributor

Warn when groups are used in regexps, since it's almost always a bad idea to use them in Snuffleupagus, instead of writing better rules

Just my personal feeling about this, but non capturing group should not necessarily be seen as evil, the real evilness is what the website regex101 detects as "catastrophic backtracking".

I don't know how difficult it may be to implement the same kind of control, but I can definitively see a warning (or even an error) upon detection of such situation as these most likely are bad rules which will cause performance or stability issues. As per my understanding however, such issue can be only detected live, by imposing a timeout or other limits to the regex execution, and can hardly be detected while parsing the conf (there is no regex to match poorly written regex ;) ).

If there would be a systematic warning simply because I consciously used a tool which requires a bit of care, I hope there would also be a way to disable this warning to confirm that due care has been taken :) .

@palaueb
Copy link

palaueb commented Mar 28, 2024

Hi, My 2 cents.
This proposal is better suited to be an external rule analysis tool rather than a feature inside the code. Anything that requires CPU usage just for checking should be an optional solution. It's preferable if it's outside the code and does not increase the complexity of the module's main purpose.
For example, there exists fail2ban-regex binary for fail2ban rule analysis.

@jvoisin jvoisin removed this from the 0.11.0 - Mastodon milestone Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants