-
Notifications
You must be signed in to change notification settings - Fork 53
test: add Docker-based build and smoke test environment #92
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
Open
fzipi
wants to merge
6
commits into
owasp-modsecurity:master
Choose a base branch
from
fzipi:test/docker-build-environment
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
84321bb
test: add Docker-based build and smoke test environment
fzipi 16d47bb
fix: use a standard HTTP status for the request-body test rule
fzipi 7d408a1
ci: add GitHub Actions workflow to build and smoke test the Dockerfile
fzipi 2dfc54a
fix: correct and slim the Docker test environment
fzipi 97436ec
docs: describe what the test environment actually does
fzipi 5c6d60c
docs: mark the sample output fence as text
fzipi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| name: Docker build | ||
|
|
||
| on: | ||
| push: | ||
| branches: [master] | ||
| paths: | ||
| - Dockerfile | ||
| - docker-compose.yml | ||
| - test-connector.sh | ||
| - src/** | ||
| - .github/workflows/docker-build.yml | ||
| pull_request: | ||
| paths: | ||
| - Dockerfile | ||
| - docker-compose.yml | ||
| - test-connector.sh | ||
| - src/** | ||
| - .github/workflows/docker-build.yml | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| concurrency: | ||
| group: ${{ github.workflow }}-${{ github.ref }} | ||
| cancel-in-progress: true | ||
|
|
||
| jobs: | ||
| build-and-smoke-test: | ||
| name: Build and smoke test | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 | ||
| with: | ||
| persist-credentials: false | ||
|
|
||
| - name: Build image | ||
| run: docker build -t modsec3-apache-test . | ||
|
|
||
| - name: Run container | ||
| run: docker run -d -p 8080:8080 --name modsec3-test modsec3-apache-test | ||
|
|
||
| - name: Run smoke tests | ||
| run: ./test-connector.sh | ||
|
|
||
| - name: Show container logs | ||
| if: always() | ||
| run: docker logs modsec3-test | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,122 @@ | ||
| # Docker Testing Guide for ModSecurity Apache Connector | ||
|
|
||
| This Docker setup tests the ModSecurity v3 Apache connector with all implemented fixes. | ||
|
|
||
| ## Quick Start | ||
|
|
||
| ```bash | ||
| # Build and run | ||
| docker build -t modsec3-apache-test . | ||
| docker run -d -p 8080:8080 --name modsec3-test modsec3-apache-test | ||
|
|
||
| # Or use docker-compose | ||
| docker-compose up -d | ||
|
|
||
| # Run automated tests | ||
| ./test-connector.sh | ||
| ``` | ||
|
|
||
| ## Manual Testing | ||
|
|
||
| ```bash | ||
| # Test 1: Normal request (should work - 200 OK) | ||
| curl http://localhost:8080/ | ||
|
|
||
| # Test 2: Query string rule (should be blocked - 403 Forbidden) | ||
| curl -v http://localhost:8080/?test=evil | ||
|
|
||
| # Test 3: Request body rule (should be blocked - 403 Forbidden) | ||
| curl -X POST http://localhost:8080/ -d "data=malicious" | ||
|
|
||
| # Test 4: Large POST - tests multi-bucket processing (should work - 200 OK) | ||
| curl -X POST http://localhost:8080/ -d "$(head -c 20000 /dev/zero | tr '\0' 'A')" | ||
|
|
||
| # Test 5: Large POST with evil content (should be blocked - 403) | ||
| # This specifically verifies the request body processing fix! | ||
| curl -X POST http://localhost:8080/ -d "A$(head -c 15000 /dev/zero | tr '\0' 'A')malicious" | ||
| ``` | ||
|
|
||
| ## Verifying the Fixes | ||
|
|
||
| ### ✅ Fix #1: Request Body Processing | ||
| **Issue**: Rules fired multiple times (once per ~8KB bucket) | ||
| **Fix**: Only call `msc_process_request_body()` once at EOS | ||
|
|
||
| **Test**: | ||
| ```bash | ||
| # Send large POST with "malicious" at the end | ||
| curl -v -X POST http://localhost:8080/ -d "$(head -c 20000 /dev/zero | tr '\0' 'A')malicious" | ||
| ``` | ||
| **Expected**: HTTP 403 (proves rules evaluated the complete body correctly) | ||
|
|
||
| ### ✅ Fix #2: Status Code Control | ||
| **Issue**: ModSecurity couldn't set status codes (missing `r->status`) | ||
| **Fix**: Added `f->r->status = status;` before `status_line` | ||
|
|
||
| **Test**: | ||
| ```bash | ||
| curl -v http://localhost:8080/?test=evil | ||
| ``` | ||
| **Expected**: `HTTP/1.1 403 Forbidden` (not 400 or other) | ||
|
|
||
| ### ✅ Fix #3: Filter Removal | ||
| **Issue**: Input filter called `ap_remove_output_filter()` | ||
| **Fix**: Changed to `ap_remove_input_filter()` | ||
|
|
||
| **Test**: Run all tests - no crashes | ||
|
|
||
| ### ✅ Fix #4: Error Handling | ||
| **Issue**: `apr_bucket_read()` return value not checked | ||
| **Fix**: Added error checking | ||
|
|
||
| **Test**: Normal operation should work without errors | ||
|
|
||
| ## Debugging | ||
|
|
||
| ```bash | ||
| # View live logs | ||
| docker logs -f modsec3-test | ||
|
|
||
| # Enter container | ||
| docker exec -it modsec3-test bash | ||
|
|
||
| # Check module loaded | ||
| /usr/local/apache2/bin/apachectl -M | grep security3 | ||
|
|
||
| # Check module dependencies | ||
| ldd /usr/local/apache2/modules/mod_security3.so | ||
|
|
||
| # View ModSecurity config | ||
| cat /etc/modsecurity/modsecurity.conf | ||
| cat /etc/modsecurity/test-rules.conf | ||
| ``` | ||
|
|
||
| ## Expected Results | ||
|
|
||
| All 6 tests should pass: | ||
| 1. ✅ Normal request - 200 OK | ||
| 2. ✅ Query string block - 403 Forbidden | ||
| 3. ✅ Request body block - 403 Forbidden | ||
| 4. ✅ Normal POST - 200 OK | ||
| 5. ✅ Large POST (multi-bucket) - 200 OK | ||
| 6. ✅ Large POST with evil - 403 Forbidden (verifies the fix!) | ||
|
|
||
| ## What's Included | ||
|
|
||
| - **libmodsecurity v3** (latest from v3/master branch) | ||
| - **Apache HTTP Server 2.4.62** | ||
| - **ModSecurity Apache Connector** with fixes: | ||
| - Request body processing (process once at EOS) | ||
| - Status code control (r->status properly set) | ||
| - Filter removal (correct function called) | ||
| - Error handling (return values checked) | ||
|
|
||
| ## Files Modified | ||
|
|
||
| The following files contain our fixes: | ||
| - `src/mod_security3.h` - Added `request_body_processed` flag | ||
| - `src/mod_security3.c` - Initialize flag | ||
| - `src/msc_filters.c` - Fixed request body processing, filter removal, error handling | ||
| - `src/msc_utils.c` - Fixed status code bug | ||
|
|
||
| See commit history or `/tmp/fixes_summary.md` for detailed changes. | ||
|
fzipi marked this conversation as resolved.
Outdated
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.