-
-
Notifications
You must be signed in to change notification settings - Fork 391
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
Anti-virus support without need to run external programs #1986
Conversation
Hey @azurit, thank you for this contribution. I can not get it to work so far, but I have some preliminary feedback:
|
Thanks for reviewing! Strange, it's fully working for me. Also see comments below.
Fixed.
Done.
I was planning to add support also for other anti-virus software and allow activation of multiple anti-viruses simultaneously - it makes sense as various AVs are able to detect different viruses.
Now i noticed that Probably a modsecurity bug, I will look into this.
I already added few but was not pushing it only because of this. But ok, i will add more.
Added.
This is not supposed to work.
It's working for me, modsec2, apache 2.4, lua 5.1. What version of modsec are you testing on? Which web server software?
Yes, do you find this not correct? I can add additional check.
This is ok.
It's a special modsecurity Lua library shipped with modsecurity and always available to all Lua scripts executed internally, using it, Lua scripts has access to modsecurity internals (for example |
Thank you for petting me. I have lua 5.3 installed. Does it work for you on lua 5.3 as well? Definitely no "m" to load / require here. :( |
Are you sure your modsecurity is compiled with Lua 5.3? Is it modsec 2 or 3? The whole modsecurity 2.x documentation is mentioning only Lua version 5.1. Newer Lua versions are available only with modsec 3 (as far as i understand it). |
Ah, my bad. Have not worked with Lua in quite a long time. No problem during compilation, but if that is a constraint, I need to downgrade and try it out anew. |
I just pushed mentioned fixies. |
Were you able to make it work? |
Have not tried it yet. |
Meeting decision March: #2008 (comment): |
Hey @airween: You promised to review this PR. Did you have the time to look into it? |
I reviewed this PR, see my comments between lines. There is one more note: libmodsecurity3 has a known bug: variable
These FP's always triggered, no matter the anti-virus feature is on or off, but the anomaly score value will be distorted. And finally: in case of Apache2 + mod_security everything is fine. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my comments between the lines.
# setvar:'tx.crs_anti-virus_clamav_port=3310',\ | ||
# setvar:'tx.crs_anti-virus_clamav_chunk_size=4096',\ | ||
# setvar:'tx.crs_anti-virus_clamav_network_timeout_seconds=2',\ | ||
# setvar:'tx.crs_anti-virus_clamav_max_file_size_bytes=1048576'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid the FP's in case of libmodsecurity3 (because the REQUEST_BODY
is always presented), we should add these actions:
ctl:ruleRemoveById=920272,\
ctl:ruleRemoveById=920273"
or we can combine these exceptions with checking of CT header.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would not count on CT as it can be faked to bypass anti-virus check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be we should discuss this on monthly chat. I tried this rule in crs-setup.conf:
SecRule REQUEST_HEADERS:Content-Type "(?:multipart/form-data)" \
"id:'900801',\
phase:1,\
t:none,t:lowercase,\
pass,\
nolog,\
ctl:ruleRemoveTargetById=920272;REQUEST_BODY,\
ctl:ruleRemoveTargetById=920273;REQUEST_BODY"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will keep this open, i think we need to discuss it with others.
Simple way of testing anti-virus: |
@airween Thanks A LOT for a detailed review! |
Just a note: I will rewrite this into plugin after plugin infrastructure is merged. |
|
Try this one:
|
|
That's really weird, everything is fully working with same versions of the software on Debian 10. What about this:
|
i don't understand it too. the lua-socket module is from epel repository.
|
Can you point me to the repository page of the lua-socket you are using? Looks like you have something different than i have. You need this library: |
here the link to the source package. last update was 2018 |
@azurit you use v3.0 too right? |
@emphazer Yes, 3.0 rc1, same as you. |
well, i could recompile the src package without the 3 included patches and see if it works... |
No, wait with that please, i will try to look at it again probably tomorrow (i'm out of energy for today). Thank you! |
no problemo ;-) |
I don't know why, but looks like the RH and for eg. Debian Luasocket are different (I've tested both of them). Here is a snippet which worked on both system: socket = require("socket")
socket.unix = require("socket.unix")
c = socket.unix()
local ok = c:connect("/var/run/clamav/clamd.ctl")
print(ok) Also it's interesting that on Debian the result will |
yeeeeeehaaaa, work perfect! :-) |
Does ANYONE see any problems or have feature requests? |
I think it's really nice work, congratulation! I'd check the whole cumulated patch again both on Apache and Nginx, but may be I just can do that tomorrow. |
This is coming along nicely. Thank you all for your work and looking fwd to @airween's review. |
@azurit, @emphazer - can you help me how can I emulate the After the first tests, this implementation works with the "regular" file upload:
and PUT method:
But the rule 925120 didn't triggered with the request was like this:
And my 2 cents: in case of first case (regular file upload) there are two blocking rules. It's no problem, just saying. |
It works for me. Is that modsec3? Have you set
modsec3? |
Can you share with me the
yes, and yes :) But I also tested with mod_security2 with same result.
yes, but in case of mod_security2, the |
I used the same command as you: |
Apache or nginx? |
I'm using Apache2 with mod_security2, and Nginx with libmodsecurity3. Here is the debug log from Apache+mod_security2 and the request what you shared:
Of course, I upgraded the But looks like
Thoughts? |
I reviewed the whole PR again both on Apache2 + mod_security2 and Nginx and libmodsecurity3. There were two files: I tested the engines with these requests:
the requests:
All request were catched by both engine, the one and only side-effect is when I uploaded the zipped file to Nginx, it logged the both rule (925110 and 925120) - the reason is known ( I asked @azurit that fix the prerequisites in case of v3 and nginx in comment - every other part of this PR is perfect :) |
@airween Thank you very much! Can you be so kind and test again - this time as a plugin, see: Don't forget to apply changes in includes needs for plugins to work: Almost nothing changed so do only quick test mainly on Nginx and modsec3 (i already tested modsec2 + apache). |
sure, I'll check it again soon. |
Closing in favour of official Antivirus plugin, see: Thanks to everyone! |
Adding anti-virus support implemented in pure Lua. The only external requirement is lua-socket library (should be part of all major distros). Currently, only ClamAV is supported. All configuration can be done in crs-setup.conf. This feature is disabled by default.
Feature is implemented using
INSTREAM
ClamAV command, so:Thing to consider: Requests containing virus should be hard-blocked, current implementation is only adding score (CRITICAL severity).
ClamAV signatures for PHP malware (tested, both are good):
https://www.rfxn.com/projects/linux-malware-detect/ (free of charge)
https://malware.expert/signatures/ (commercial)