Skip to content

Commit 36eb37d

Browse files
julienloizeletjulienloizelet
authored
feat(appsec): Handle multipart request (#25)
* feat(appsec): Handle raw body for uploads * test(end to end): Add test for AppSec upload * test(end to end): Add test for AppSec too big body post * style(*): Pass through code format tools * feat(appsec): Use lib helper to build raw body from superglobals * feat(*): Prepare release 1.3.0
1 parent 9041750 commit 36eb37d

File tree

18 files changed

+348
-9
lines changed

18 files changed

+348
-9
lines changed

‎.github/workflows/coding-standards.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,5 +114,5 @@ jobs:
114114
if: github.event.inputs.coverage_report == 'true'
115115
run: |
116116
ddev xdebug
117-
ddev exec XDEBUG_MODE=coverage BOUNCER_KEY=${{ env.BOUNCER_KEY }} AGENT_TLS_PATH=/var/www/html/cfssl LAPI_URL=https://crowdsec:8080 MEMCACHED_DSN=memcached://memcached:11211 REDIS_DSN=redis://redis:6379 /usr/bin/php ./${{env.EXTENSION_PATH}}/tools/coding-standards/vendor/bin/phpunit --configuration ./${{env.EXTENSION_PATH}}/tools/coding-standards/phpunit/phpunit.xml --coverage-text=./${{env.EXTENSION_PATH}}/coding-standards/phpunit/code-coverage/report.txt
117+
ddev exec XDEBUG_MODE=coverage BOUNCER_KEY=${{ env.BOUNCER_KEY }} APPSEC_URL=http://crowdsec:7422 AGENT_TLS_PATH=/var/www/html/cfssl LAPI_URL=https://crowdsec:8080 MEMCACHED_DSN=memcached://memcached:11211 REDIS_DSN=redis://redis:6379 /usr/bin/php ./${{env.EXTENSION_PATH}}/tools/coding-standards/vendor/bin/phpunit --configuration ./${{env.EXTENSION_PATH}}/tools/coding-standards/phpunit/phpunit.xml --coverage-text=./${{env.EXTENSION_PATH}}/coding-standards/phpunit/code-coverage/report.txt
118118
cat ${{env.EXTENSION_PATH}}/coding-standards/phpunit/code-coverage/report.txt

‎.github/workflows/test-suite.yml

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -503,6 +503,38 @@ jobs:
503503
exit 1
504504
fi
505505
506+
- name: Run "AppSec upload" test
507+
run: |
508+
cd ${{ github.workspace }}/${{env.EXTENSION_PATH}}
509+
# Set block as AppSec action
510+
sed -i 's/\x27appsec_body_size_exceeded_action\x27 => \x27headers_only\x27/\x27appsec_body_size_exceeded_action\x27 => \x27block\x27/g' scripts/settings.php
511+
cat scripts/settings.php
512+
cd ${{ github.workspace }}/${{env.EXTENSION_PATH}}/tests/end-to-end/
513+
./__scripts__/run-tests.sh ci "./__tests__/12-appsec-upload.js"
514+
PENDING_TESTS=$(grep -oP '"numPendingTests":\K(.*),"numRuntimeErrorTestSuites"' .test-results.json | sed 's/,"numRuntimeErrorTestSuites"//g')
515+
if [[ $PENDING_TESTS == "0" ]]
516+
then
517+
echo "No pending tests: OK"
518+
else
519+
echo "There are pending tests: $PENDING_TESTS (KO)"
520+
exit 1
521+
fi
522+
523+
- name: Run "AppSec POST too big body" test
524+
run: |
525+
cd ${{ github.workspace }}/${{env.EXTENSION_PATH}}
526+
sed -i 's/\x27appsec_max_body_size_kb\x27 => 1024/\x27appsec_max_body_size_kb\x27 => 1/g' scripts/settings.php
527+
cat scripts/settings.php
528+
cd ${{ github.workspace }}/${{env.EXTENSION_PATH}}/tests/end-to-end/
529+
./__scripts__/run-tests.sh ci "./__tests__/11-appsec-max-body-ban.js"
530+
PENDING_TESTS=$(grep -oP '"numPendingTests":\K(.*),"numRuntimeErrorTestSuites"' .test-results.json | sed 's/,"numRuntimeErrorTestSuites"//g')
531+
if [[ $PENDING_TESTS == "0" ]]
532+
then
533+
echo "No pending tests: OK"
534+
else
535+
echo "There are pending tests: $PENDING_TESTS (KO)"
536+
exit 1
537+
fi
506538
507539
- name: Run "AppSec with timeout (captcha fallback) and file_get_contents" test
508540
run: |

‎.gitignore

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,7 @@ scripts/settings.php
1313
.cache
1414

1515
# MaxMind databases
16-
*.mmdb
16+
*.mmdb
17+
18+
# Uploads test
19+
tests/scripts/public/uploads/*

‎CHANGELOG.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,14 @@ protected methods, properties and constants belonging to the `src` folder and of
1010

1111
---
1212

13+
## [1.3.0](https://github.com/crowdsecurity/cs-standalone-php-bouncer/releases/tag/v1.3.0) - 2024-10-23
14+
[_Compare with previous release_](https://github.com/crowdsecurity/cs-standalone-php-bouncer/compare/v1.2.0...v1.3.0)
15+
16+
### Added
17+
- Add multipart request support for AppSec
18+
19+
---
20+
1321
## [1.2.0](https://github.com/crowdsecurity/cs-standalone-php-bouncer/releases/tag/v1.2.0) - 2024-10-04
1422
[_Compare with previous release_](https://github.com/crowdsecurity/cs-standalone-php-bouncer/compare/v1.1.0...v1.2.0)
1523

‎composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@
4141
],
4242
"require": {
4343
"php": ">=7.2.5",
44-
"crowdsec/bouncer": "^3.0.0"
44+
"crowdsec/bouncer": "^3.2.0"
4545
},
4646
"require-dev": {
4747
"phpunit/phpunit": "^8.5.30 || ^9.3",

‎scripts/settings.php.dist

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,23 @@ return [
5050
* If true, the AppSec URL must be set in the 'appsec_url' setting below.
5151
*/
5252
'use_appsec' => false,
53+
/**
54+
* Maximum size of request body (in kilobytes) which, if exceeded,
55+
* will trigger the action defined by the "appsec_body_size_exceeded_action" setting below.
56+
*
57+
* This setting is not required. By default, the maximum body size is set to 1024KB (1MB).
58+
*/
59+
'appsec_max_body_size_kb' => Constants::APPSEC_DEFAULT_MAX_BODY_SIZE,
60+
/**
61+
* Action to take when the request body size exceeds the maximum size defined by the "appsec_max_body_size_kb" setting above.
62+
*
63+
* Select from:
64+
* - `headers_only` (recommended and default value): only the headers of the original request are forwarded to AppSec,
65+
* not the body.
66+
* - `allow` (not recommended): the request is considered as safe and a bypass remediation is returned, without calling AppSec.
67+
* - `block`: the request is considered as malicious and a ban remediation is returned, without calling AppSec.
68+
*/
69+
'appsec_body_size_exceeded_action' => Constants::APPSEC_ACTION_HEADERS_ONLY,
5370
/**
5471
* array of URIs that will not be bounced.
5572
*/

‎src/Bouncer.php

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,13 @@ public function getRequestHeaders(): array
9595
$allHeaders = getallheaders();
9696
// @codeCoverageIgnoreEnd
9797
} else {
98+
$this->logger->warning(
99+
'getallheaders() function is not available',
100+
[
101+
'type' => 'GETALLHEADERS_NOT_AVAILABLE',
102+
'message' => 'Resulting headers may not be accurate',
103+
]
104+
);
98105
foreach ($_SERVER as $name => $value) {
99106
if ('HTTP_' == substr($name, 0, 5)) {
100107
$name = str_replace(' ', '-', ucwords(strtolower(str_replace('_', ' ', substr($name, 5)))));
@@ -104,8 +111,6 @@ public function getRequestHeaders(): array
104111
}
105112
}
106113
}
107-
// Remove Content-Length header for AppSec
108-
unset($allHeaders['Content-Length']);
109114

110115
return $allHeaders;
111116
}
@@ -118,12 +123,9 @@ public function getRequestHost(): string
118123
return $_SERVER['HTTP_HOST'] ?? '';
119124
}
120125

121-
/**
122-
* Get current request raw body.
123-
*/
124126
public function getRequestRawBody(): string
125127
{
126-
return file_get_contents('php://input');
128+
return $this->buildRequestRawBody(fopen('php://input', 'rb'));
127129
}
128130

129131
/**

‎tests/Integration/IpVerificationTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
* @covers \CrowdSecStandalone\Bouncer::getRequestHost
3131
* @covers \CrowdSecStandalone\Bouncer::getRequestRawBody
3232
* @covers \CrowdSecStandalone\Bouncer::getRequestUserAgent
33+
*
3334
*/
3435
final class IpVerificationTest extends TestCase
3536
{

‎tests/end-to-end/.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,4 @@ node_modules
44
*.lock
55
*.log
66
*.jpg
7+
!assets/*.jpg

‎tests/end-to-end/__scripts__/run-tests.sh

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ CLEAN_CACHE_DURATION=$(ddev exec grep -E "'clean_ip_cache_duration'.*,$" /var/ww
4949
STREAM_MODE=$(ddev exec grep -E "'stream_mode'.*,$" /var/www/html/my-code/standalone-bouncer/scripts/settings.php | sed 's/stream_mode//g' | sed -e 's|[=>,"'\'']||g' | sed s/'\s'//g)
5050
APPSEC_ENABLED=$(ddev exec grep -E "'use_appsec'.*,$" /var/www/html/my-code/standalone-bouncer/scripts/settings.php | sed 's/use_appsec//g' | sed -e 's|[=>,"'\'']||g' | sed s/'\s'//g)
5151
APPSEC_FALLBACK=$(ddev exec grep "'appsec_fallback_remediation'" /var/www/html/my-code/standalone-bouncer/scripts/settings.php | tail -1 | sed 's/appsec_fallback_remediation//g' | sed -e 's|[=>,"'\'']||g' | sed s/'\s'//g)
52+
APPSEC_ACTION=$(ddev exec grep "'appsec_body_size_exceeded_action'" /var/www/html/my-code/standalone-bouncer/scripts/settings.php | tail -1 | sed 's/appsec_body_size_exceeded_action//g' | sed -e 's|[=>,"'\'']||g' | sed s/'\s'//g)
53+
APPSEC_MAX_BODY_SIZE=$(ddev exec grep "'appsec_max_body_size_kb'" /var/www/html/my-code/standalone-bouncer/scripts/settings.php | tail -1 | sed 's/appsec_max_body_size_kb//g' | sed -e 's|[=>,"'\'']||g' | sed s/'\s'//g)
5254
DEBUG_MODE=$(ddev exec grep -E "'debug_mode'.*,$" /var/www/html/my-code/standalone-bouncer/scripts/settings.php | sed 's/debug_mode//g' | sed -e 's|[=>,"'\'']||g' | sed s/'\s'//g)
5355
JEST_PARAMS="--bail=true --runInBand --verbose"
5456
# If FAIL_FAST, will exit on first individual test fail
@@ -117,6 +119,8 @@ PROXY_IP="$PROXY_IP" \
117119
GEOLOC_ENABLED="$GEOLOC_ENABLED" \
118120
APPSEC_ENABLED="$APPSEC_ENABLED" \
119121
APPSEC_FALLBACK="$APPSEC_FALLBACK" \
122+
APPSEC_ACTION="$APPSEC_ACTION" \
123+
APPSEC_MAX_BODY_SIZE="$APPSEC_MAX_BODY_SIZE" \
120124
STREAM_MODE="$STREAM_MODE" \
121125
CLEAN_CACHE_DURATION="$CLEAN_CACHE_DURATION" \
122126
DEBUG_MODE="$DEBUG_MODE" \

‎tests/end-to-end/__tests__/11-appsec-max-body-ban.js

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
const {
2+
goToPublicPage,
3+
removeAllDecisions,
4+
runCacheAction,
5+
computeCurrentPageRemediation,
6+
fillInput,
7+
clickById,
8+
getTextById,
9+
wait,
10+
} = require("../utils/helpers");
11+
const {
12+
APPSEC_ENABLED,
13+
APPSEC_MAX_BODY_SIZE,
14+
APPSEC_ACTION,
15+
STREAM_MODE,
16+
CLEAN_CACHE_DURATION,
17+
} = require("../utils/constants");
18+
19+
describe(`Should work with ban as max body`, () => {
20+
beforeAll(async () => {
21+
await removeAllDecisions();
22+
await runCacheAction("clear");
23+
});
24+
25+
it("Should have correct settings", async () => {
26+
if (!APPSEC_ENABLED) {
27+
const errorMessage = `AppSec must be enabled for this test`;
28+
console.error(errorMessage);
29+
throw new Error(errorMessage);
30+
}
31+
if (STREAM_MODE) {
32+
const errorMessage = `Stream mode must be disabled for this test`;
33+
console.error(errorMessage);
34+
throw new Error(errorMessage);
35+
}
36+
if (CLEAN_CACHE_DURATION !== "3") {
37+
const errorMessage = `clean_ip_cache_duration setting must be exactly 3 for this test (current is ${CLEAN_CACHE_DURATION})`;
38+
console.error(errorMessage);
39+
throw new Error(errorMessage);
40+
}
41+
if (APPSEC_ACTION !== "block") {
42+
const errorMessage = `AppSec action must be "block" for this test (got ${APPSEC_ACTION})`;
43+
console.error(errorMessage);
44+
throw new Error(errorMessage);
45+
}
46+
});
47+
48+
it("Should ban when access home page page with POST and too big body", async () => {
49+
await goToPublicPage();
50+
const remediation = await computeCurrentPageRemediation();
51+
await expect(remediation).toBe("bypass");
52+
53+
let appsecResult = await getTextById("appsec-result");
54+
await expect(appsecResult).toBe("INITIAL STATE");
55+
56+
await fillInput(
57+
"request-body",
58+
"a".repeat(APPSEC_MAX_BODY_SIZE * 1024 + 1),
59+
);
60+
await clickById("appsec-post-button");
61+
await wait(1000);
62+
63+
appsecResult = await getTextById("appsec-result");
64+
await expect(appsecResult).toBe("Response status: 403");
65+
});
66+
67+
it("Should bypass when access home page page with POST and short body", async () => {
68+
await goToPublicPage();
69+
const remediation = await computeCurrentPageRemediation();
70+
await expect(remediation).toBe("bypass");
71+
72+
let appsecResult = await getTextById("appsec-result");
73+
await expect(appsecResult).toBe("INITIAL STATE");
74+
75+
await fillInput(
76+
"request-body",
77+
"a".repeat(APPSEC_MAX_BODY_SIZE * 1024),
78+
);
79+
await clickById("appsec-post-button");
80+
await wait(1000);
81+
82+
appsecResult = await getTextById("appsec-result");
83+
await expect(appsecResult).toBe("Response status: 200");
84+
});
85+
});

‎tests/end-to-end/__tests__/12-appsec-upload.js

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
const {
2+
goToPublicPage,
3+
removeAllDecisions,
4+
runCacheAction,
5+
computeCurrentPageRemediation,
6+
getHtmlById,
7+
clickById,
8+
getTextById,
9+
wait,
10+
} = require("../utils/helpers");
11+
const {
12+
APPSEC_ENABLED,
13+
APPSEC_MAX_BODY_SIZE,
14+
APPSEC_ACTION,
15+
STREAM_MODE,
16+
CLEAN_CACHE_DURATION,
17+
APPSEC_UPLOAD_TEST_URL,
18+
} = require("../utils/constants");
19+
20+
describe(`Should work with ban as max body`, () => {
21+
beforeAll(async () => {
22+
await removeAllDecisions();
23+
await runCacheAction("clear");
24+
});
25+
26+
it("Should have correct settings", async () => {
27+
if (!APPSEC_ENABLED) {
28+
const errorMessage = `AppSec must be enabled for this test`;
29+
console.error(errorMessage);
30+
throw new Error(errorMessage);
31+
}
32+
if (STREAM_MODE) {
33+
const errorMessage = `Stream mode must be disabled for this test`;
34+
console.error(errorMessage);
35+
throw new Error(errorMessage);
36+
}
37+
if (CLEAN_CACHE_DURATION !== "3") {
38+
const errorMessage = `clean_ip_cache_duration setting must be exactly 3 for this test (current is ${CLEAN_CACHE_DURATION})`;
39+
console.error(errorMessage);
40+
throw new Error(errorMessage);
41+
}
42+
if (APPSEC_ACTION !== "block") {
43+
const errorMessage = `AppSec action must be "block" for this test (got ${APPSEC_ACTION})`;
44+
console.error(errorMessage);
45+
throw new Error(errorMessage);
46+
}
47+
if (APPSEC_MAX_BODY_SIZE > 1024) {
48+
const errorMessage = `AppSec max size must greater than "1024" for this test (got ${APPSEC_MAX_BODY_SIZE})`;
49+
console.error(errorMessage);
50+
throw new Error(errorMessage);
51+
}
52+
});
53+
54+
it("Should ban when upload a too big image", async () => {
55+
await goToPublicPage(APPSEC_UPLOAD_TEST_URL);
56+
const remediation = await computeCurrentPageRemediation("Image Upload");
57+
await expect(remediation).toBe("bypass");
58+
59+
let appsecResult = await getTextById("appsec-result");
60+
await expect(appsecResult).toBe("INITIAL STATE");
61+
62+
await page
63+
.locator('input[name="image"]')
64+
.setInputFiles("./assets/too-big.jpg");
65+
await clickById("imageUpload");
66+
await wait(2000);
67+
68+
appsecResult = await getTextById("appsec-result");
69+
await expect(appsecResult).toBe("403");
70+
});
71+
72+
it("Should bypass when upload a small enough image", async () => {
73+
await goToPublicPage(APPSEC_UPLOAD_TEST_URL);
74+
const remediation = await computeCurrentPageRemediation("Image Upload");
75+
await expect(remediation).toBe("bypass");
76+
77+
let appsecResult = await getTextById("appsec-result");
78+
await expect(appsecResult).toBe("INITIAL STATE");
79+
80+
await page
81+
.locator('input[name="image"]')
82+
.setInputFiles("./assets/small-enough.jpg");
83+
await clickById("imageUpload");
84+
await wait(2000);
85+
86+
appsecResult = await getHtmlById("appsec-result");
87+
await expect(appsecResult).toBe(
88+
'<img src="uploads/small-enough.jpg" alt="Uploaded Image">',
89+
);
90+
});
91+
});

‎tests/end-to-end/assets/small-enough.jpg

80 KB
Loading

‎tests/end-to-end/assets/too-big.jpg

1.13 MB
Loading

‎tests/end-to-end/settings/base.php.dist

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ return [
3030
'excluded_uris' => ['/favicon.ico'],
3131
'fallback_remediation' => Constants::REMEDIATION_CAPTCHA,
3232
'use_appsec' => false,
33+
'appsec_max_body_size_kb' => 1024,
34+
'appsec_body_size_exceeded_action' => 'headers_only',
3335
'appsec_fallback_remediation' => Constants::REMEDIATION_CAPTCHA,
3436
'trust_ip_forward_array' => ['REPLACE_PROXY_IP'],
3537
// Cache

‎tests/end-to-end/utils/constants.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ const PUBLIC_URL =
22
"/my-code/standalone-bouncer/tests/scripts/public/protected-page.php";
33
const APPSEC_TEST_URL =
44
"/my-code/standalone-bouncer/tests/scripts/public/testappsec.php";
5+
const APPSEC_UPLOAD_TEST_URL =
6+
"/my-code/standalone-bouncer/tests/scripts/public/testappsec-upload.php";
57
const APPSEC_MALICIOUS_BODY = "class.module.classLoader.resources.";
68
const FORCED_TEST_FORWARDED_IP =
79
process.env.FORCED_TEST_FORWARDED_IP !== ""
@@ -19,6 +21,8 @@ const WATCHER_PASSWORD = "watcherPassword";
1921
const {
2022
BOUNCER_KEY,
2123
APPSEC_FALLBACK,
24+
APPSEC_ACTION,
25+
APPSEC_MAX_BODY_SIZE,
2226
DEBUG,
2327
CURRENT_IP,
2428
LAPI_URL_FROM_PLAYWRIGHT,
@@ -35,9 +39,12 @@ const DEBUG_LOG_PATH = `${AGENT_TLS_PATH}/../my-code/standalone-bouncer/scripts/
3539

3640
module.exports = {
3741
APPSEC_TEST_URL,
42+
APPSEC_UPLOAD_TEST_URL,
3843
APPSEC_ENABLED,
3944
APPSEC_MALICIOUS_BODY,
4045
APPSEC_FALLBACK,
46+
APPSEC_ACTION,
47+
APPSEC_MAX_BODY_SIZE,
4148
PHP_URL,
4249
BOUNCER_KEY,
4350
CLEAN_CACHE_DURATION,

0 commit comments

Comments
 (0)