Merge pull request #85 from AikidoSec/drop-zend-execute

Drop zend_execute and move the IP allowlist blocking to GetBlockingStatus

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
@@ -86,7 +91,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 address is not allowed to access this endpoint! (Your IP: {$decision->ip})", 403);
+                }
             }
             else if ($decision->type == "ratelimited") {
                 if ($decision->trigger == "user") {

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") {

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;
-}

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 {

lib/php-extension/include/Hooks.h

@@ -39,7 +39,3 @@ void UnhookFunctions();
 void HookMethods();
 
 void UnhookMethods();
-
-void HookExecute();
-
-void UnhookExecute();

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 {

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 ""
 }

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 ""
 }
 

tests/server/test_allowed_ip/index.php

@@ -1,5 +1,15 @@
 <?php
 
-echo "Something\n";
+if (extension_loaded('aikido')) {
+    $decision = \aikido\should_block_request();
+
+    if ($decision->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();
+    }
+}
+
+echo "Something";
 
 ?>

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__":