From efc428a3080a8c562d8b4b79820ad1f868a7303d Mon Sep 17 00:00:00 2001 From: Tudor TIMCU Date: Wed, 13 Nov 2024 17:24:49 +0200 Subject: [PATCH 1/3] Drop zend_execute and move the IP allowlist blocking to GetBlockingStatus --- docs/should_block_request.md | 7 +++- lib/php-extension/Aikido.cpp | 1 - lib/php-extension/Hooks.cpp | 30 ---------------- lib/php-extension/PhpLifecycle.cpp | 1 - lib/php-extension/include/Hooks.h | 4 --- .../handle_blocking_request.go | 34 ++++++++++++------- .../handle_request_metadata.go | 26 -------------- 7 files changed, 27 insertions(+), 76 deletions(-) diff --git a/docs/should_block_request.md b/docs/should_block_request.md index 78127f0f..f7642216 100644 --- a/docs/should_block_request.md +++ b/docs/should_block_request.md @@ -86,7 +86,12 @@ class ZenBlockDecision if ($decision->block) { if ($decision->type == "blocked") { - return response('Your user is blocked!', 403); + if ($decision->trigger == "user") { + return response('Your user is blocked!', 403); + } + else if ($decision->trigger == "ip") { + return response("Your IP is not allowed to access this endpoint!", 403); + } } else if ($decision->type == "ratelimited") { if ($decision->trigger == "user") { diff --git a/lib/php-extension/Aikido.cpp b/lib/php-extension/Aikido.cpp index ddd34f22..1299a081 100644 --- a/lib/php-extension/Aikido.cpp +++ b/lib/php-extension/Aikido.cpp @@ -18,7 +18,6 @@ PHP_MINIT_FUNCTION(aikido) { HookFunctions(); HookMethods(); - HookExecute(); /* If SAPI name is "cli" run in "simple" mode */ if (AIKIDO_GLOBAL(sapi_name) == "cli") { diff --git a/lib/php-extension/Hooks.cpp b/lib/php-extension/Hooks.cpp index 81afbf7a..858b1f24 100644 --- a/lib/php-extension/Hooks.cpp +++ b/lib/php-extension/Hooks.cpp @@ -126,33 +126,3 @@ void UnhookMethods() { it.second.original_handler = nullptr; } } - -static void (*original_zend_execute_ex)(zend_execute_data *execute_data) = nullptr; - -void aikido_zend_execute_ex(zend_execute_data *execute_data) { - if (action.Exit()) { - AIKIDO_LOG_INFO("Current request is marked for exit. Bailing out...\n"); - zend_bailout(); - } - if (original_zend_execute_ex) { - original_zend_execute_ex(execute_data); - } -} - -void HookExecute() { - if (original_zend_execute_ex) { - AIKIDO_LOG_WARN("Function zend_execute_ex already hooked (original handler %p)!\n", original_zend_execute_ex); - return; - } - original_zend_execute_ex = zend_execute_ex; - zend_execute_ex = aikido_zend_execute_ex; -} - -void UnhookExecute() { - if (!original_zend_execute_ex) { - AIKIDO_LOG_WARN("Cannot unhook zend_execute_ex without an original handler (was not hooked before)!\n"); - return; - } - zend_execute_ex = original_zend_execute_ex; - original_zend_execute_ex = nullptr; -} diff --git a/lib/php-extension/PhpLifecycle.cpp b/lib/php-extension/PhpLifecycle.cpp index 03e3aba9..9915d5a5 100644 --- a/lib/php-extension/PhpLifecycle.cpp +++ b/lib/php-extension/PhpLifecycle.cpp @@ -26,7 +26,6 @@ void PhpLifecycle::ModuleShutdown() { AIKIDO_LOG_INFO("Unhooking functions...\n"); UnhookFunctions(); UnhookMethods(); - UnhookExecute(); AIKIDO_LOG_INFO("Uninitializing Aikido Agent...\n"); AIKIDO_GLOBAL(agent).Uninit(); } else { diff --git a/lib/php-extension/include/Hooks.h b/lib/php-extension/include/Hooks.h index dd532905..b134f181 100644 --- a/lib/php-extension/include/Hooks.h +++ b/lib/php-extension/include/Hooks.h @@ -39,7 +39,3 @@ void UnhookFunctions(); void HookMethods(); void UnhookMethods(); - -void HookExecute(); - -void UnhookExecute(); diff --git a/lib/request-processor/handle_blocking_request.go b/lib/request-processor/handle_blocking_request.go index c3c28359..1c8594b1 100644 --- a/lib/request-processor/handle_blocking_request.go +++ b/lib/request-processor/handle_blocking_request.go @@ -9,10 +9,26 @@ import ( "time" ) +func GetStoreAction(actionType, trigger, ip string) string { + actionMap := map[string]interface{}{ + "action": "store", + "type": actionType, + "trigger": trigger, + } + if trigger == "ip" { + actionMap["ip"] = ip + } + actionJson, err := json.Marshal(actionMap) + if err != nil { + return "" + } + return string(actionJson) +} + func OnGetBlockingStatus() string { userId := context.GetUserId() if utils.IsUserBlocked(userId) { - return `{"action": "store", "type": "blocked", "trigger": "user"}` + return GetStoreAction("blocked", "user", "") } method := context.GetMethod() @@ -35,23 +51,15 @@ func OnGetBlockingStatus() string { // do a sync call via gRPC to see if the request should be blocked or not rateLimitingStatus := grpc.GetRateLimitingStatus(method, route, userId, ip, 10*time.Millisecond) if rateLimitingStatus != nil && rateLimitingStatus.Block { - action := map[string]interface{}{ - "action": "store", - "type": "ratelimited", - "trigger": rateLimitingStatus.Trigger, - } - if rateLimitingStatus.Trigger == "ip" { - action["ip"] = ip - } - actionJson, err := json.Marshal(action) - if err == nil { - return string(actionJson) - } + return GetStoreAction("ratelimited", rateLimitingStatus.Trigger, ip) } } else { log.Infof("IP \"%s\" is bypassed for rate limiting!", ip) } } + if !utils.IsIpAllowed(endpointData.AllowedIPAddresses, ip) { + return GetStoreAction("blocked", "ip", ip) + } return "" } diff --git a/lib/request-processor/handle_request_metadata.go b/lib/request-processor/handle_request_metadata.go index d93475b7..67d22ca2 100644 --- a/lib/request-processor/handle_request_metadata.go +++ b/lib/request-processor/handle_request_metadata.go @@ -1,7 +1,6 @@ package main import ( - "fmt" "main/api_discovery" "main/context" "main/grpc" @@ -12,31 +11,6 @@ import ( func OnPreRequest() string { context.Clear() - - method := context.GetMethod() - route := context.GetParsedRoute() - if method == "" || route == "" { - return "" - } - - log.Infof("Got request metadata: %s %s", method, route) - - endpointData, err := utils.GetEndpointConfig(method, route) - if err != nil { - log.Debugf("Method+route in not configured in endpoints! Skipping checks...") - return "" - } - - ip := context.GetIp() - - if !utils.IsIpAllowed(endpointData.AllowedIPAddresses, ip) { - message := "Your IP address is not allowed to access this resource!" - if ip != "" { - message += fmt.Sprintf(" (Your IP: %s)", ip) - } - return fmt.Sprintf(`{"action": "exit", "message": "%s", "response_code": 403}`, message) - } - return "" } From 09673a16c6483373c75677c55411a7ae72a725b9 Mon Sep 17 00:00:00 2001 From: Tudor TIMCU Date: Wed, 13 Nov 2024 17:30:18 +0200 Subject: [PATCH 2/3] Updated docs and tests --- docs/should_block_request.md | 11 ++++++++--- lib/request-processor/attack/attack.go | 16 +++++++++++++++- tests/server/test_allowed_ip/index.php | 10 +++++++++- tests/server/test_allowed_ip/test.py | 4 ++-- 4 files changed, 34 insertions(+), 7 deletions(-) diff --git a/docs/should_block_request.md b/docs/should_block_request.md index f7642216..2f08fb47 100644 --- a/docs/should_block_request.md +++ b/docs/should_block_request.md @@ -32,9 +32,14 @@ if (extension_loaded('aikido')) { if ($decision->block) { if ($decision->type == "blocked") { - // If the user is blocked, return a 403 status code + // If the user/ip is blocked, return a 403 status code http_response_code(403); - echo "Your user is blocked!"; + if ($decision->trigger == "user") { + echo "Your user is blocked!"; + } + else if ($decision->trigger == "ip") { + echo "Your IP address is not allowed to access this endpoint! (Your IP: {$decision->ip})"; + } } else if ($decision->type == "ratelimited") { // If the rate limit is exceeded, return a 429 status code @@ -90,7 +95,7 @@ class ZenBlockDecision return response('Your user is blocked!', 403); } else if ($decision->trigger == "ip") { - return response("Your IP is not allowed to access this endpoint!", 403); + return response("Your IP address is not allowed to access this endpoint! (Your IP: {$decision->ip})", 403); } } else if ($decision->type == "ratelimited") { diff --git a/lib/request-processor/attack/attack.go b/lib/request-processor/attack/attack.go index 6ba6f3ba..00e863d9 100644 --- a/lib/request-processor/attack/attack.go +++ b/lib/request-processor/attack/attack.go @@ -1,6 +1,7 @@ package attack import ( + "encoding/json" "fmt" "main/context" "main/grpc" @@ -64,8 +65,21 @@ func BuildAttackDetectedMessage(result utils.InterceptorResult) string { utils.EscapeHTML(result.PathToPayload)) } +func GetThrowAction(message string, code int) string { + actionMap := map[string]interface{}{ + "action": "throw", + "message": message, + "code": code, + } + actionJson, err := json.Marshal(actionMap) + if err != nil { + return "" + } + return string(actionJson) +} + func GetAttackDetectedAction(result utils.InterceptorResult) string { - return fmt.Sprintf(`{"action": "throw", "message": "%s", "code": -1}`, BuildAttackDetectedMessage(result)) + return GetThrowAction(BuildAttackDetectedMessage(result), -1) } func ReportAttackDetected(res *utils.InterceptorResult) string { diff --git a/tests/server/test_allowed_ip/index.php b/tests/server/test_allowed_ip/index.php index ffeba7e6..2dbd5d89 100755 --- a/tests/server/test_allowed_ip/index.php +++ b/tests/server/test_allowed_ip/index.php @@ -1,5 +1,13 @@ block && $decision->type == "blocked" && $decision->trigger == "ip") { + http_response_code(403); + echo "Your IP address is not allowed to access this endpoint! (Your IP: {$decision->ip})"; + exit(); + } +} ?> diff --git a/tests/server/test_allowed_ip/test.py b/tests/server/test_allowed_ip/test.py index a6281e8f..2ecac733 100755 --- a/tests/server/test_allowed_ip/test.py +++ b/tests/server/test_allowed_ip/test.py @@ -14,7 +14,7 @@ def run_test(): response = php_server_get("/somethingVerySpecific") assert_response_code_is(response, 403) assert_response_header_contains(response, "Content-Type", "text") - assert_response_body_contains(response, "Your IP address is not allowed to access this resource! (Your IP: ") + assert_response_body_contains(response, "Your IP address is not allowed to access this endpoint! (Your IP: ") apply_config("change_config_remove_allowed_ip.json") @@ -27,7 +27,7 @@ def run_test(): response = php_server_get("/somethingVerySpecific") assert_response_code_is(response, 403) assert_response_header_contains(response, "Content-Type", "text") - assert_response_body_contains(response, "Your IP address is not allowed to access this resource! (Your IP: ") + assert_response_body_contains(response, "Your IP address is not allowed to access this endpoint! (Your IP: ") if __name__ == "__main__": From 487b0b02edef81ee1cc3fba78fe711d8c8a11821 Mon Sep 17 00:00:00 2001 From: Tudor TIMCU Date: Wed, 13 Nov 2024 17:51:01 +0200 Subject: [PATCH 3/3] Fix test --- tests/server/test_allowed_ip/index.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/server/test_allowed_ip/index.php b/tests/server/test_allowed_ip/index.php index 2dbd5d89..a5baaa7b 100755 --- a/tests/server/test_allowed_ip/index.php +++ b/tests/server/test_allowed_ip/index.php @@ -10,4 +10,6 @@ } } +echo "Something"; + ?>