NAT: Disable FORWARD by default and set postroute before FORWARD.
[Adaptation from https://android-review.googlesource.com/#/c/50223 ]
For NAT table, only the first packet of data flow
hits the rules in this table. If forward is
enabled before postrouting in NAT, packet may be
forwarded without hit rules in NAT table. Phone
will not get responses for such packets because
source IP address is not translated.
How to reproduce:
1) Enable usb tethering;
2) Start ping test on PC;
3) Disable data on Phone;
4) NAT is disabled since data service is lost;
5) Enable data on Phone;
6) Ping can not recover on PC;
7) Capture tcpdump data on phone, source IP
Address is not translated for ICMP packets.
Test result:
Run steps 1-5 in "How to reproduce", Ping session
on PC can be recovered after data is enabled.
CQ00027191
Change-Id: I1c3bcbb3d69eb7e2f61d720fa17086ee0da22fa0
diff --git a/NatController.cpp b/NatController.cpp
index 46ac01c..140cb36 100644
--- a/NatController.cpp
+++ b/NatController.cpp
@@ -67,6 +67,7 @@
int NatController::setDefaults() {
struct CommandsAndArgs defaultCommands[] = {
{{IPTABLES_PATH, "-F", "natctrl_FORWARD",}, 1},
+ {{IPTABLES_PATH, "-A", "natctrl_FORWARD", "-j", "DROP"}, 1},
{{IPTABLES_PATH, "-t", "nat", "-F", "natctrl_nat_POSTROUTING"}, 1},
{{IP_PATH, "rule", "flush"}, 0},
{{IP_PATH, "-6", "rule", "flush"}, 0},
@@ -123,7 +124,6 @@
int NatController::enableNat(const int argc, char **argv) {
int i;
int addrCount = atoi(argv[4]);
- int ret = 0;
const char *intIface = argv[2];
const char *extIface = argv[3];
int tableNumber;
@@ -139,10 +139,42 @@
errno = EINVAL;
return -1;
}
- ret = routesOp(true, intIface, extIface, argv, addrCount);
- if (ret != 0 || setForwardRules(true, intIface, extIface) != 0) {
+ if (routesOp(true, intIface, extIface, argv, addrCount)) {
+ ALOGE("Error setting route rules");
+ routesOp(false, intIface, extIface, argv, addrCount);
+ errno = ENODEV;
+ return -1;
+ }
+
+ // add this if we are the first added nat
+ if (natCount == 0) {
+ const char *cmd[] = {
+ IPTABLES_PATH,
+ "-t",
+ "nat",
+ "-A",
+ "natctrl_nat_POSTROUTING",
+ "-o",
+ extIface,
+ "-j",
+ "MASQUERADE"
+ };
+ if (runCmd(ARRAY_SIZE(cmd), cmd)) {
+ ALOGE("Error seting postroute rule: iface=%s", extIface);
+ // unwind what's been done, but don't care about success - what more could we do?
+ routesOp(false, intIface, extIface, argv, addrCount);
+ setDefaults();
+ return -1;
+ }
+ }
+
+
+ if (setForwardRules(true, intIface, extIface) != 0) {
ALOGE("Error setting forward rules");
routesOp(false, intIface, extIface, argv, addrCount);
+ if (natCount == 0) {
+ setDefaults();
+ }
errno = ENODEV;
return -1;
}
@@ -165,30 +197,7 @@
};
runCmd(ARRAY_SIZE(cmd2), cmd2);
-
natCount++;
- // add this if we are the first added nat
- if (natCount == 1) {
- const char *cmd[] = {
- IPTABLES_PATH,
- "-t",
- "nat",
- "-A",
- "natctrl_nat_POSTROUTING",
- "-o",
- extIface,
- "-j",
- "MASQUERADE"
- };
- if (runCmd(ARRAY_SIZE(cmd), cmd)) {
- ALOGE("Error seting postroute rule: iface=%s", extIface);
- // unwind what's been done, but don't care about success - what more could we do?
- routesOp(false, intIface, extIface, argv, addrCount);
- setDefaults();
- return -1;
- }
- }
-
return 0;
}
