commit 6ee2598e23e10a11ed98f4f23cf63638c8524104
author Lorenzo Colitti <lorenzo@google.com> Tue Jul 18 21:44:04 2017 +0900
committer Lorenzo Colitti <lorenzo@google.com> Tue Jul 18 21:47:33 2017 +0900
tree 73d698a7613c4329071bb935ca82fc6c34d9fc7a
parent 28e4da0438e22cc5169bf987385305fa6ecbcf99

Move the last StrictController command to iptables-restore

Bug: 28362720
Test: unit tests pass
Change-Id: I8a4d2b8ea66799c6c3205b00f04ee1999fc7c68b

server/StrictControllerTest.cpp

diff --git a/server/StrictControllerTest.cpp b/server/StrictControllerTest.cpp
index 3783c30..82d0cda 100644
--- a/server/StrictControllerTest.cpp
+++ b/server/StrictControllerTest.cpp
@@ -29,7 +29,6 @@
 class StrictControllerTest : public IptablesBaseTest {
 public:
     StrictControllerTest() {
-        StrictController::execIptables = fakeExecIptables;
         StrictController::execIptablesRestore = fakeExecIptablesRestore;
     }
     StrictController mStrictCtrl;
@@ -125,28 +124,38 @@
 
 TEST_F(StrictControllerTest, TestSetUidCleartextPenalty) {
     std::vector<std::string> acceptCommands = {
-        "-D st_OUTPUT -m owner --uid-owner 12345 -j st_clear_detect",
-        "-D st_clear_caught -m owner --uid-owner 12345 -j st_penalty_log",
-        "-D st_clear_caught -m owner --uid-owner 12345 -j st_penalty_reject",
+        "*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"
+        "COMMIT\n"
     };
     std::vector<std::string> logCommands = {
-        "-I st_OUTPUT -m owner --uid-owner 12345 -j st_clear_detect",
-        "-I st_clear_caught -m owner --uid-owner 12345 -j st_penalty_log",
+        "*filter\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"
+        "COMMIT\n"
     };
     std::vector<std::string> rejectCommands = {
-        "-I st_OUTPUT -m owner --uid-owner 12345 -j st_clear_detect",
-        "-I st_clear_caught -m owner --uid-owner 12345 -j st_penalty_reject",
+        "*filter\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"
+        "COMMIT\n"
     };
 
     mStrictCtrl.setUidCleartextPenalty(12345, LOG);
-    expectIptablesCommands(logCommands);
+    expectIptablesRestoreCommands(logCommands);
 
     mStrictCtrl.setUidCleartextPenalty(12345, ACCEPT);
-    expectIptablesCommands(acceptCommands);
+    expectIptablesRestoreCommands(acceptCommands);
 
+    // StrictController doesn't keep any state and it is not correct to call its methods in the
+    // wrong order (e.g., to go from LOG to REJECT without passing through ACCEPT).
+    // NetworkManagementService does keep state (not just to ensure correctness, but also so it can
+    // reprogram the rules when netd crashes).
     mStrictCtrl.setUidCleartextPenalty(12345, REJECT);
-    expectIptablesCommands(rejectCommands);
+    expectIptablesRestoreCommands(rejectCommands);
 
     mStrictCtrl.setUidCleartextPenalty(12345, ACCEPT);
-    expectIptablesCommands(acceptCommands);
+    expectIptablesRestoreCommands(acceptCommands);
 }