-
Notifications
You must be signed in to change notification settings - Fork 1.7k
ip collection does not seem to work properly with latest ModSecurity #3394
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
Comments
I have tried to capture some log output and this is what I found: Occurence of 205.166.94.38:
So setting the ip address to the collection seems to have no effect? |
HI @ne20002, thanks for reporting this issue, and for the detailed explanation. Let me check this soon and give a feedback. |
I have done a logging session with debug level 9. It creates a rather big log file. From what I see there is no other log statement related to collection. I would have expected at least something according to storing the collection. But .. nothing like that. @airween, if should do some special tests or can help in any other way ... |
sorry for the late reply, finally I was able to take a look at this issue. I tried your setup and got the same result - especially this line in debug.log:
I think this line is generated by this part of rc = getaddrinfo(host.c_str(), NULL, NULL, &info);
if (rc != 0) {
if (info != NULL) {
freeaddrinfo(info);
}
ms_dbg_a(t, 5, "RBL lookup of " + ipStr + " failed.");
return false;
} The used I suggest you to first try to use the IP collection with some other way, eg. use a static list with @ipMatch operator. |
Thank you @airween for the answer. Actually the lookup of The inverse number order is how this is handled by the rbl systems (they all use it) and a query to But the This is what I wanted to achieve:
If the ip is not known as spammer...
If the ip has not been tested before...
So I expect for the second and following requests from an ip address (within the 3600s timeframe) that it is
The last part is the problem. From what I see is, that the checks on the ip collection (for The interessting part is, that it has worked before. So I believe the problem is not the rbl check but the collection. |
hi @ne20002, ah, thanks for the clarification, now it's clear. I see the problem.
Do you remember in which version was that this worked as well? |
I'm using the With a previous version ( I also read the documentation, and based on that:
So I don't think it is the collection code except for (based on what I see happening here):
This is also why I looked for the persistant storage file. And I haven't found one (so this e.g. might be a container problem). So I'm limited in options to dig deeper but still I'm willing to try or test whatever you need. |
CRS's Docker uses LMDB (based on this configuration part), which means you have to have a file inside the container with name Here you can find a Gist, with that you can read the content of the LMDB file (which is a "database": keys and values). Could you try to read that file (and share the relevant content here)? |
I think I don't have that file.
|
Can you help me with defining (a few) rules that should result in writing a lmdb file? Maybe by setting dummy values? This way we may get info if writing a lmdb is working at all or not with the container. |
Today I investigated this issue and found a few problems. I will write a summary later. |
Here is what I found:
I changed your rules and set the keys to upper case format, then I get this:
As you can see, in the second request the rule Other sad fact, that this does not work in case of in-memory backend. In that case the collection handling does not work to me at all - this needs more investigations. Workaround Even though I changed the code and put the database to under
After the engine started and received the first request, the files were filled with data, so this could work in your case too. I don't know how to solve it permanently in Docker. @theseion do you have some idea? Note that if the engine runs and uses those files, you can open and look its content with this version of |
Thank you very much @airween I updated my config to fix the lower-/uppercase issue (all lowercase now). According to the documentation the storage path for the persistant collections should be I tried to create the files as u did with touch by mounting a shell script into the I have now created those two files on my host and mounted them rw to ogw to the root directory inside the container. The files seems to be used as the size increased. According to the debug log it now works. :) I believe the best option to solve the issue would be to fix the creation of the persistent storage to use SecDataDir as according to the documentation. If so, no change to the docker image would be required. And also an error log message should be written if the attempt to create or write to the persistent store fails. |
I think that has no effect, because as you wrote you don't have the database files...
Vainly, because - as your wrote too -
May be @theseion can help with this.
I think Nginx shouldn't start if a necessary file does not exists... Anyway, I think this is another important part of the engine that we have to care about and fix the issues. Moreover, if we deal with it, we should do some other missing things. Eg. it always bothered that a user have to choose in build time which backend want they use for collections (in-memory or LMDB). There is no option to build LMDB and choose in-memory if it's better for the requirements. I was thinking to build two separated packages from the library (for Debian and Ubuntu systems): once without LMDB and another one with it. Debian developers helped me, ahd suggested to solve this issue inside the library, and don't split the package into more parts. There we can add some code which controls the database path. |
I agree with the idea to fix a few more things then. For me, with mounting the files into the docker container in its root directory, it is working now. Thank you very much for the investigation, I wouldn't have expected this as the reason for the issue. By documentation SecDataDir should be the place for the persitent storage. So this should be implemented anyway. An option to choose for memory or lmdb would still be a good idea (but this is also not available in v2, am I right?) For the docker image it would be an option to create/provide the two files writeable in the root directory by default. So persistent collection would work and other user won't run into this problem. But it would break the read-only feature in case someone uses the collections. If I remember correctly, collections are used in PL2 so this can currently be considered broken for the docker image. |
you're welcome!
I have to investigate this in case of v2, but I have some concerns regarding to use this as the path. For eg. what if a user has several vhost contexts, and set this value as a different locations...? Now I can't tell yo what could we expect.
V2 does not support in-memory collections (only the
Yes, but as I wrote, if a user chooses in-memory collection then it does not work at all... we have to inspect that too. |
That's on purpose :) We could probably create the files as symlinks to a writable location. I'm not sure it's worth the effort though. @ne20002 you're the first with this issue that I've seen and we now have a workaround. I'd rather wait for a proper working solution in ModSecurity than having to document how this is broken and how the workaround works for the container image. |
Uh oh!
There was an error while loading. Please reload this page.
I am using
@rbl xbl.spamhaus.org.
rule for protecting my Fediverse server and I face the issue that with latest version of either ModSecurity or the docker.io/owasp/modsecurity-crs:nginx container the collection handling for the IP collection seems to not longer work properly.In my pi-hole I see a huge number of queries to .xbl.spamhaus.org for all the ip addresses checked and with queries to the same ip based query many times where I would expect that this is not the case due to how I set up the ModSecurity rules.
Based on what I see I would see it as if
According to modsecurity.conf the SecTmpDir is set to /tmp/modsecurity/tmp (as data and upload directory are also located in /tmp/modsecurity).
This directory is empty. Wouldn't this be the directory for persistent storage and shouldn't there be a file for the IP collection database?
This is the rules I use:
This is part of my setup.conf before including the CRS rules. As said, it worked much better (but also not perfect) with the older docker image mentioned above.
The text was updated successfully, but these errors were encountered: