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

Adding Modsecurity #13

Merged
merged 7 commits into from
Dec 11, 2018
Merged

Adding Modsecurity #13

merged 7 commits into from
Dec 11, 2018

Conversation

allupaku
Copy link
Contributor

Adding mod security module to nginx with default owas common rule set from https://github.com/SpiderLabs/owasp-modsecurity-crs/

@csandanov
Copy link
Member

Thank you for the PR. A few things to correct:

  1. You checkout modsecurity git repo branch v3 that I assume contains all changes for 3.x tags. Since we try to freeze all dependencies for stability sake, you should instead checkout the latest stable tag (move version to env vars)
  2. It's ok to have module enabled by default but I think it should be possible to disable it via something like $NGINX_DISABLE_MODSECURITY (add to README)
  3. The file /etc/nginx/modsec/main.conf isn't a template and probably should be generated during the build
  4. Add modsecurity module in README (mention OWASP) with the version

althafmohammed added 4 commits December 10, 2018 12:59
ModSecurity checked out from a stable tag.
Added NGINX_DISABLE_MODSECURITY var to disable loading of module
Removed and dynamically added the file /etc/nginx/modsec/main.conf
Updated Read me to mention the module and OWASP
@csandanov csandanov merged commit 31d1f2d into wodby:master Dec 11, 2018
@csandanov
Copy link
Member

I've just noticed you also install packages that are not deleted:

        doxygen \
        geoip \
        yajl \
        libstdc++ \
        sed \
        curl \
        libmaxminddb-dev; \

are those required runtime packages? do we really need all of them?

@allupaku
Copy link
Contributor Author

@csandanov Hi , Sorry that should have been moved to the other modsecurity build deps, which will get removed. I tested it and also added some more configuration options.
On another note : I think default behavior of mod security being on may break many installations without fine tuning the filtering capabilities. Hence i made some changes and pushed to my repository with documentation as well. Could you please have a look when your time permits ? here

@csandanov
Copy link
Member

Are you sure none of the packages needed in runtime? Could you please create a new PR and describe what are the cases when it breaks something and why, I know very little about mod_security configuration. Thank you

@allupaku allupaku mentioned this pull request Dec 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants