{"action":"created","issue":{"url":"https://api.github.com/repos/SpiderLabs/owasp-modsecurity-crs/issues/335","repository_url":"https://api.github.com/repos/SpiderLabs/owasp-modsecurity-crs","labels_url":"https://api.github.com/repos/SpiderLabs/owasp-modsecurity-crs/issues/335/labels{/name}","comments_url":"https://api.github.com/repos/SpiderLabs/owasp-modsecurity-crs/issues/335/comments","events_url":"https://api.github.com/repos/SpiderLabs/owasp-modsecurity-crs/issues/335/events","html_url":"https://github.com/SpiderLabs/owasp-modsecurity-crs/pull/335","id":149872165,"number":335,"title":"Adding stricter sibling for 912100, cleanup","user":{"login":"dune73","id":618722,"avatar_url":"https://avatars.githubusercontent.com/u/618722?v=3","gravatar_id":"","url":"https://api.github.com/users/dune73","html_url":"https://github.com/dune73","followers_url":"https://api.github.com/users/dune73/followers","following_url":"https://api.github.com/users/dune73/following{/other_user}","gists_url":"https://api.github.com/users/dune73/gists{/gist_id}","starred_url":"https://api.github.com/users/dune73/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dune73/subscriptions","organizations_url":"https://api.github.com/users/dune73/orgs","repos_url":"https://api.github.com/users/dune73/repos","events_url":"https://api.github.com/users/dune73/events{/privacy}","received_events_url":"https://api.github.com/users/dune73/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":{"login":"dune73","id":618722,"avatar_url":"https://avatars.githubusercontent.com/u/618722?v=3","gravatar_id":"","url":"https://api.github.com/users/dune73","html_url":"https://github.com/dune73","followers_url":"https://api.github.com/users/dune73/followers","following_url":"https://api.github.com/users/dune73/following{/other_user}","gists_url":"https://api.github.com/users/dune73/gists{/gist_id}","starred_url":"https://api.github.com/users/dune73/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dune73/subscriptions","organizations_url":"https://api.github.com/users/dune73/orgs","repos_url":"https://api.github.com/users/dune73/repos","events_url":"https://api.github.com/users/dune73/events{/privacy}","received_events_url":"https://api.github.com/users/dune73/received_events","type":"User","site_admin":false},"milestone":null,"comments":12,"created_at":"2016-04-20T20:09:04Z","updated_at":"2016-06-02T20:46:14Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/SpiderLabs/owasp-modsecurity-crs/pulls/335","html_url":"https://github.com/SpiderLabs/owasp-modsecurity-crs/pull/335","diff_url":"https://github.com/SpiderLabs/owasp-modsecurity-crs/pull/335.diff","patch_url":"https://github.com/SpiderLabs/owasp-modsecurity-crs/pull/335.patch"},"body":"I've looked at the anti-automation / dos protection rules in detail to come up with a stricter sibling.\r\n\r\nDetails of this pull request:\r\n - Re-formatted all rules for 3.0\r\n - Added a detailed description of the mechanics\r\n - Added a summary for every variable in use\r\n - Added a base check for the setting of the\r\n   necessary parameters. Skip everything if unset.\r\n - Fix a bug with ip.dos_block_counter\r\n   (used to count every blocked request twice)\r\n - Added a stricter sibling for paranoia level 2\r\n   which triggers a block on the first detected\r\n   burst of requests per IP (The standard rule \r\n   triggers only on the 2nd occurrence of a \r\n   burst).\r\n\r\nI do not really know why the original rule waits for the 2nd burst to happen before a blocking is issued.\r\nWhatever the intention was, it allows for a stricter sibling in PL 2 without changing the default behaviour.\r\n\r\nIf you want to this, then it may be useful to use the following\r\nrule to monitor the behaviour:\r\n\r\n```\r\nSecRule REQUEST_URI \"@beginsWith /index.html\" \"id:912001,phase:1,pass,log,msg:'\\\r\nI:DB %{ip.dos_block} \\\r\nI:DBLC %{ip.dos_block_counter} \\\r\nI:DBF %{ip.dos_block_flag} \\\r\nI:DBUC %{ip.dos_burst_counter} \\\r\nI:DC %{ip.dos_counter} \\\r\nT:DBLC %{tx.dos_block_counter} | \\\r\nT:DBT %{tx.dos_block_timeout} \\\r\nT:DCT %{tx.dos_counter_threshold} \\\r\nT:DBTS %{tx.dos_burst_time_slice} \\\r\nT:SR %{tx.static_resources}'\r\n```\r\n"},"comment":{"url":"https://api.github.com/repos/SpiderLabs/owasp-modsecurity-crs/issues/comments/223416941","html_url":"https://github.com/SpiderLabs/owasp-modsecurity-crs/pull/335#issuecomment-223416941","issue_url":"https://api.github.com/repos/SpiderLabs/owasp-modsecurity-crs/issues/335","id":223416941,"user":{"login":"lifeforms","id":3464056,"avatar_url":"https://avatars.githubusercontent.com/u/3464056?v=3","gravatar_id":"","url":"https://api.github.com/users/lifeforms","html_url":"https://github.com/lifeforms","followers_url":"https://api.github.com/users/lifeforms/followers","following_url":"https://api.github.com/users/lifeforms/following{/other_user}","gists_url":"https://api.github.com/users/lifeforms/gists{/gist_id}","starred_url":"https://api.github.com/users/lifeforms/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lifeforms/subscriptions","organizations_url":"https://api.github.com/users/lifeforms/orgs","repos_url":"https://api.github.com/users/lifeforms/repos","events_url":"https://api.github.com/users/lifeforms/events{/privacy}","received_events_url":"https://api.github.com/users/lifeforms/received_events","type":"User","site_admin":false},"created_at":"2016-06-02T20:46:13Z","updated_at":"2016-06-02T20:46:13Z","body":"I love that you are cleaning up these rules and commenting the principles. These rules were always a bit mysterious and this should help people get comfortable to try them out!\r\n\r\nMy comments are as follows:\r\n\r\n----\r\n\r\n### Breakage introduced by skip rule phase\r\n\r\nThe PR breaks DoS protection for me with the default configuration.\r\n\r\nOn v3.0.0-rc1 branch, in paranoia level 1, requests start getting blocked at counter 203 which is as expected (two bursts).\r\n\r\nHowever when I use paranoia-pull-912100 branch, the only thing I see in the audit log is request no. 202 hitting the burst limit:\r\n\r\n`Message: Warning. Operator GE matched 2 at IP:dos_burst_counter. [file \"/usr/local/etc/apache24/security2/activated_rules/REQUEST-12-DOS-PROTECTION.conf\"] [line \"210\"] [id \"912100\"] [msg \"Potential Denial of Service (DoS) Attack from 192.168.0.2 - # of Request Bursts: 2\"] [tag \"application-multi\"] [tag \"language-multi\"] [tag \"platform-multi\"] [tag \"attack-dos\"]`\r\n\r\nbut there is no blocking going on anymore.\r\n\r\nIt was quite a challenge to debug. The cause seems that 'skipping rule' 912021 executes in `phase:1`, but rule 900015 executes in `phase:request` and its variables are not yet set at point of execution of rule 912021. I can work around the problem by changing rule 900015 to:\r\n\r\n```\r\nSecAction \\\r\n \"id:'900015',\\\r\n  phase:1,\\\r\n```\r\n\r\nThen blocking is working again.\r\n\r\nAre our setups different? I include the setup conf before the CRS rule files in my Apache configuration. The default `-DREQUEST_EARLY` is set.\r\n\r\nI used the following script to create requests:\r\n\r\n```sh\r\n#!/bin/sh\r\n\r\nbase_url='http://sim/?count='\r\n\r\ni=0\r\nwhile true; do\r\n\ti=$(($i + 1))\r\n\turl=${base_url}${i}\r\n\tres=`curl -s -o /dev/null -w '%{http_code}' $url`\r\n\techo $i: $res\r\ndone\r\n```\r\n\r\n----\r\n\r\n### setup.conf comment\r\n\r\nThe setup.conf should lower the bar for trying this feature, I think we should keep a short description of what the user can expect from this feature and as a minimum explain the variables, though we can do better than the previous version. What about this:\r\n\r\n```\r\n# -- [[ Anti-Automation / DoS Protection ]] ----------------------------------------------\r\n#\r\n# Optional DoS protection against clients making requests too quickly.\r\n#\r\n# When a client making more than 100 requests (excluding static files) within 60 seconds,\r\n# this is considered a 'burst'. After two bursts, the client is blocked for 600 seconds.\r\n# Requests to static files are unlimited; they are listed in 'tx.static_resources' setting.\r\n#\r\n# To enable the DoS protection rules, uncomment the following rule.\r\n# For a detailed description, see rule file REQUEST-12-DOS-PROTECTION.conf.\r\n#\r\n#SecAction \\\r\n \"id:'900015',\\\r\n```\r\n\r\n----\r\n\r\n### tx.static_resources location and comment\r\n\r\nAt this moment, `tx.static_resources` in setup.conf is only used for the DoS rule, but it's set somewhere else in the file. This is not too ergonomic and it's a bit of YAGNI to create a global variable in a  remote place. To not cause too much work, we could leave it as is, but it would be nice to improve that rule's comment in that case, it currently lies a bit.\r\n\r\n----\r\n\r\n### tx.static_resources contents\r\n\r\nI suggest to add `svg` and `webp` to `tx.static_resources` in setup.conf."}}
{"actor":{"display_login":"lifeforms"}}