diff options
| -rw-r--r-- | server/StrictController.cpp | 24 | ||||
| -rw-r--r-- | server/StrictControllerTest.cpp | 13 |
2 files changed, 26 insertions, 11 deletions
diff --git a/server/StrictController.cpp b/server/StrictController.cpp index ea25b2ea..04c1bfec 100644 --- a/server/StrictController.cpp +++ b/server/StrictController.cpp @@ -157,6 +157,13 @@ int StrictController::disableStrict(void) { } int StrictController::setUidCleartextPenalty(uid_t uid, StrictPenalty penalty) { + // When a penalty is set, we don't know what penalty the UID previously had. In order to be able + // to clear the previous penalty without causing an iptables error by deleting rules that don't + // exist, put each UID's rules in a chain specific to that UID. That way, the commands we need + // to run to clear the previous penalty don't depend on what the penalty actually was - all we + // need to do is clear the chain. + std::string perUidChain = StringPrintf("st_clear_caught_%u", uid); + std::vector<std::string> commands; if (penalty == ACCEPT) { // Clean up any old rules @@ -165,21 +172,24 @@ int StrictController::setUidCleartextPenalty(uid_t uid, StrictPenalty penalty) { StringPrintf("-D %s -m owner --uid-owner %d -j %s", LOCAL_OUTPUT, uid, LOCAL_CLEAR_DETECT), StringPrintf("-D %s -m owner --uid-owner %d -j %s", - LOCAL_CLEAR_CAUGHT, uid, LOCAL_PENALTY_LOG), - StringPrintf("-D %s -m owner --uid-owner %d -j %s", - LOCAL_CLEAR_CAUGHT, uid, LOCAL_PENALTY_REJECT), + LOCAL_CLEAR_CAUGHT, uid, perUidChain.c_str()), + StringPrintf("-F %s", perUidChain.c_str()), + StringPrintf("-X %s", perUidChain.c_str()), }; } else { // Always take a detour to investigate this UID commands.push_back("*filter"); + commands.push_back(StringPrintf(":%s -", perUidChain.c_str())); commands.push_back(StringPrintf("-I %s -m owner --uid-owner %d -j %s", LOCAL_OUTPUT, uid, LOCAL_CLEAR_DETECT)); + commands.push_back(StringPrintf("-I %s -m owner --uid-owner %d -j %s", + LOCAL_CLEAR_CAUGHT, uid, perUidChain.c_str())); + if (penalty == LOG) { - commands.push_back(StringPrintf("-I %s -m owner --uid-owner %d -j %s", - LOCAL_CLEAR_CAUGHT, uid, LOCAL_PENALTY_LOG)); + commands.push_back(StringPrintf("-A %s -j %s", perUidChain.c_str(), LOCAL_PENALTY_LOG)); } else if (penalty == REJECT) { - commands.push_back(StringPrintf("-I %s -m owner --uid-owner %d -j %s", - LOCAL_CLEAR_CAUGHT, uid, LOCAL_PENALTY_REJECT)); + commands.push_back(StringPrintf("-A %s -j %s", perUidChain.c_str(), + LOCAL_PENALTY_REJECT)); } } commands.push_back("COMMIT\n"); diff --git a/server/StrictControllerTest.cpp b/server/StrictControllerTest.cpp index 82d0cdaf..9770352c 100644 --- a/server/StrictControllerTest.cpp +++ b/server/StrictControllerTest.cpp @@ -126,20 +126,25 @@ TEST_F(StrictControllerTest, TestSetUidCleartextPenalty) { std::vector<std::string> acceptCommands = { "*filter\n" "-D st_OUTPUT -m owner --uid-owner 12345 -j st_clear_detect\n" - "-D st_clear_caught -m owner --uid-owner 12345 -j st_penalty_log\n" - "-D st_clear_caught -m owner --uid-owner 12345 -j st_penalty_reject\n" + "-D st_clear_caught -m owner --uid-owner 12345 -j st_clear_caught_12345\n" + "-F st_clear_caught_12345\n" + "-X st_clear_caught_12345\n" "COMMIT\n" }; std::vector<std::string> logCommands = { "*filter\n" + ":st_clear_caught_12345 -\n" "-I st_OUTPUT -m owner --uid-owner 12345 -j st_clear_detect\n" - "-I st_clear_caught -m owner --uid-owner 12345 -j st_penalty_log\n" + "-I st_clear_caught -m owner --uid-owner 12345 -j st_clear_caught_12345\n" + "-A st_clear_caught_12345 -j st_penalty_log\n" "COMMIT\n" }; std::vector<std::string> rejectCommands = { "*filter\n" + ":st_clear_caught_12345 -\n" "-I st_OUTPUT -m owner --uid-owner 12345 -j st_clear_detect\n" - "-I st_clear_caught -m owner --uid-owner 12345 -j st_penalty_reject\n" + "-I st_clear_caught -m owner --uid-owner 12345 -j st_clear_caught_12345\n" + "-A st_clear_caught_12345 -j st_penalty_reject\n" "COMMIT\n" }; |