Daniel P. Berrange e3a592
From 112a309bc7839e95c558b535143f855ce89cca8c Mon Sep 17 00:00:00 2001
Daniel P. Berrange e3a592
From: Daniel P. Berrange <berrange@redhat.com>
Daniel P. Berrange e3a592
Date: Thu, 10 Jun 2010 12:50:38 -0400
Daniel P. Berrange e3a592
Subject: [PATCH] CVE-2010-2242 Apply a source port mapping to virtual network masquerading
Daniel P. Berrange e3a592
Daniel P. Berrange e3a592
IPtables will seek to preserve the source port unchanged when
Daniel P. Berrange e3a592
doing masquerading, if possible. NFS has a pseudo-security
Daniel P. Berrange e3a592
option where it checks for the source port <= 1023 before
Daniel P. Berrange e3a592
allowing a mount request. If an admin has used this to make the
Daniel P. Berrange e3a592
host OS trusted for mounts, the default iptables behaviour will
Daniel P. Berrange e3a592
potentially allow NAT'd guests access too. This needs to be
Daniel P. Berrange e3a592
stopped.
Daniel P. Berrange e3a592
Daniel P. Berrange e3a592
With this change, the iptables -t nat -L -n -v rules for the
Daniel P. Berrange e3a592
default network will be
Daniel P. Berrange e3a592
Daniel P. Berrange e3a592
Chain POSTROUTING (policy ACCEPT 95 packets, 9163 bytes)
Daniel P. Berrange e3a592
 pkts bytes target     prot opt in     out     source               destination
Daniel P. Berrange e3a592
   14   840 MASQUERADE  tcp  --  *      *       192.168.122.0/24    !192.168.122.0/24    masq ports: 1024-65535
Daniel P. Berrange e3a592
   75  5752 MASQUERADE  udp  --  *      *       192.168.122.0/24    !192.168.122.0/24    masq ports: 1024-65535
Daniel P. Berrange e3a592
    0     0 MASQUERADE  all  --  *      *       192.168.122.0/24    !192.168.122.0/24
Daniel P. Berrange e3a592
Daniel P. Berrange e3a592
* src/network/bridge_driver.c: Add masquerade rules for TCP
Daniel P. Berrange e3a592
  and UDP protocols
Daniel P. Berrange e3a592
* src/util/iptables.c, src/util/iptables.c: Add source port
Daniel P. Berrange e3a592
  mappings for TCP & UDP protocols when masquerading.
Daniel P. Berrange e3a592
---
Daniel P. Berrange e3a592
 src/network/bridge_driver.c |   73 ++++++++++++++++++++++++++++++++++++++++--
Daniel P. Berrange e3a592
 src/util/iptables.c         |   70 +++++++++++++++++++++++++++++------------
Daniel P. Berrange e3a592
 src/util/iptables.h         |    6 ++-
Daniel P. Berrange e3a592
 3 files changed, 122 insertions(+), 27 deletions(-)
Daniel P. Berrange e3a592
Daniel P. Berrange e3a592
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
Daniel P. Berrange e3a592
index 72255c1..80ed57a 100644
Daniel P. Berrange e3a592
--- a/src/network/bridge_driver.c
Daniel P. Berrange e3a592
+++ b/src/network/bridge_driver.c
Daniel P. Berrange e3a592
@@ -638,18 +638,74 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
Daniel P. Berrange e3a592
         goto masqerr2;
Daniel P. Berrange e3a592
     }
Daniel P. Berrange e3a592
 
Daniel P. Berrange e3a592
-    /* enable masquerading */
Daniel P. Berrange e3a592
+    /*
Daniel P. Berrange e3a592
+     * Enable masquerading.
Daniel P. Berrange e3a592
+     *
Daniel P. Berrange e3a592
+     * We need to end up with 3 rules in the table in this order
Daniel P. Berrange e3a592
+     *
Daniel P. Berrange e3a592
+     *  1. protocol=tcp with sport mapping restricton
Daniel P. Berrange e3a592
+     *  2. protocol=udp with sport mapping restricton
Daniel P. Berrange e3a592
+     *  3. generic any protocol
Daniel P. Berrange e3a592
+     *
Daniel P. Berrange e3a592
+     * The sport mappings are required, because default IPtables
Daniel P. Berrange e3a592
+     * MASQUERADE is maintain port number unchanged where possible.
Daniel P. Berrange e3a592
+     *
Daniel P. Berrange e3a592
+     * NFS can be configured to only "trust" port numbers < 1023.
Daniel P. Berrange e3a592
+     *
Daniel P. Berrange e3a592
+     * Guests using NAT thus need to be prevented from having port
Daniel P. Berrange e3a592
+     * numbers < 1023, otherwise they can bypass the NFS "security"
Daniel P. Berrange e3a592
+     * check on the source port number.
Daniel P. Berrange e3a592
+     *
Daniel P. Berrange e3a592
+     * Since we use '--insert' to add rules to the header of the
Daniel P. Berrange e3a592
+     * chain, we actually need to add them in the reverse of the
Daniel P. Berrange e3a592
+     * order just mentioned !
Daniel P. Berrange e3a592
+     */
Daniel P. Berrange e3a592
+
Daniel P. Berrange e3a592
+    /* First the generic masquerade rule for other protocols */
Daniel P. Berrange e3a592
     if ((err = iptablesAddForwardMasquerade(driver->iptables,
Daniel P. Berrange e3a592
                                             network->def->network,
Daniel P. Berrange e3a592
-                                            network->def->forwardDev))) {
Daniel P. Berrange e3a592
+                                            network->def->forwardDev,
Daniel P. Berrange e3a592
+                                            NULL))) {
Daniel P. Berrange e3a592
         virReportSystemError(err,
Daniel P. Berrange e3a592
                              _("failed to add iptables rule to enable masquerading to '%s'"),
Daniel P. Berrange e3a592
                              network->def->forwardDev ? network->def->forwardDev : NULL);
Daniel P. Berrange e3a592
         goto masqerr3;
Daniel P. Berrange e3a592
     }
Daniel P. Berrange e3a592
 
Daniel P. Berrange e3a592
+    /* UDP with a source port restriction */
Daniel P. Berrange e3a592
+    if ((err = iptablesAddForwardMasquerade(driver->iptables,
Daniel P. Berrange e3a592
+                                            network->def->network,
Daniel P. Berrange e3a592
+                                            network->def->forwardDev,
Daniel P. Berrange e3a592
+                                            "udp"))) {
Daniel P. Berrange e3a592
+        virReportSystemError(err,
Daniel P. Berrange e3a592
+                             _("failed to add iptables rule to enable UDP masquerading to '%s'"),
Daniel P. Berrange e3a592
+                             network->def->forwardDev ? network->def->forwardDev : NULL);
Daniel P. Berrange e3a592
+        goto masqerr4;
Daniel P. Berrange e3a592
+    }
Daniel P. Berrange e3a592
+
Daniel P. Berrange e3a592
+    /* TCP with a source port restriction */
Daniel P. Berrange e3a592
+    if ((err = iptablesAddForwardMasquerade(driver->iptables,
Daniel P. Berrange e3a592
+                                            network->def->network,
Daniel P. Berrange e3a592
+                                            network->def->forwardDev,
Daniel P. Berrange e3a592
+                                            "tcp"))) {
Daniel P. Berrange e3a592
+        virReportSystemError(err,
Daniel P. Berrange e3a592
+                             _("failed to add iptables rule to enable TCP masquerading to '%s'"),
Daniel P. Berrange e3a592
+                             network->def->forwardDev ? network->def->forwardDev : NULL);
Daniel P. Berrange e3a592
+        goto masqerr5;
Daniel P. Berrange e3a592
+    }
Daniel P. Berrange e3a592
+
Daniel P. Berrange e3a592
     return 1;
Daniel P. Berrange e3a592
 
Daniel P. Berrange e3a592
+ masqerr5:
Daniel P. Berrange e3a592
+    iptablesRemoveForwardMasquerade(driver->iptables,
Daniel P. Berrange e3a592
+                                    network->def->network,
Daniel P. Berrange e3a592
+                                    network->def->forwardDev,
Daniel P. Berrange e3a592
+                                    "udp");
Daniel P. Berrange e3a592
+ masqerr4:
Daniel P. Berrange e3a592
+    iptablesRemoveForwardMasquerade(driver->iptables,
Daniel P. Berrange e3a592
+                                    network->def->network,
Daniel P. Berrange e3a592
+                                    network->def->forwardDev,
Daniel P. Berrange e3a592
+                                    NULL);
Daniel P. Berrange e3a592
  masqerr3:
Daniel P. Berrange e3a592
     iptablesRemoveForwardAllowRelatedIn(driver->iptables,
Daniel P. Berrange e3a592
                                  network->def->network,
Daniel P. Berrange e3a592
@@ -814,8 +870,17 @@ networkRemoveIptablesRules(struct network_driver *driver,
Daniel P. Berrange e3a592
     if (network->def->forwardType != VIR_NETWORK_FORWARD_NONE) {
Daniel P. Berrange e3a592
         if (network->def->forwardType == VIR_NETWORK_FORWARD_NAT) {
Daniel P. Berrange e3a592
             iptablesRemoveForwardMasquerade(driver->iptables,
Daniel P. Berrange e3a592
-                                                network->def->network,
Daniel P. Berrange e3a592
-                                                network->def->forwardDev);
Daniel P. Berrange e3a592
+                                            network->def->network,
Daniel P. Berrange e3a592
+                                            network->def->forwardDev,
Daniel P. Berrange e3a592
+                                            "tcp");
Daniel P. Berrange e3a592
+            iptablesRemoveForwardMasquerade(driver->iptables,
Daniel P. Berrange e3a592
+                                            network->def->network,
Daniel P. Berrange e3a592
+                                            network->def->forwardDev,
Daniel P. Berrange e3a592
+                                            "udp");
Daniel P. Berrange e3a592
+            iptablesRemoveForwardMasquerade(driver->iptables,
Daniel P. Berrange e3a592
+                                            network->def->network,
Daniel P. Berrange e3a592
+                                            network->def->forwardDev,
Daniel P. Berrange e3a592
+                                            NULL);
Daniel P. Berrange e3a592
             iptablesRemoveForwardAllowRelatedIn(driver->iptables,
Daniel P. Berrange e3a592
                                                 network->def->network,
Daniel P. Berrange e3a592
                                                 network->def->bridge,
Daniel P. Berrange e3a592
diff --git a/src/util/iptables.c b/src/util/iptables.c
Daniel P. Berrange e3a592
index d06b857..f63e8c6 100644
Daniel P. Berrange e3a592
--- a/src/util/iptables.c
Daniel P. Berrange e3a592
+++ b/src/util/iptables.c
Daniel P. Berrange e3a592
@@ -692,25 +692,49 @@ iptablesRemoveForwardRejectIn(iptablesContext *ctx,
Daniel P. Berrange e3a592
  */
Daniel P. Berrange e3a592
 static int
Daniel P. Berrange e3a592
 iptablesForwardMasquerade(iptablesContext *ctx,
Daniel P. Berrange e3a592
-                       const char *network,
Daniel P. Berrange e3a592
-                       const char *physdev,
Daniel P. Berrange e3a592
-                       int action)
Daniel P. Berrange e3a592
+                          const char *network,
Daniel P. Berrange e3a592
+                          const char *physdev,
Daniel P. Berrange e3a592
+                          const char *protocol,
Daniel P. Berrange e3a592
+                          int action)
Daniel P. Berrange e3a592
 {
Daniel P. Berrange e3a592
-    if (physdev && physdev[0]) {
Daniel P. Berrange e3a592
-        return iptablesAddRemoveRule(ctx->nat_postrouting,
Daniel P. Berrange e3a592
-                                     action,
Daniel P. Berrange e3a592
-                                     "--source", network,
Daniel P. Berrange e3a592
-                                     "!", "--destination", network,
Daniel P. Berrange e3a592
-                                     "--out-interface", physdev,
Daniel P. Berrange e3a592
-                                     "--jump", "MASQUERADE",
Daniel P. Berrange e3a592
-                                     NULL);
Daniel P. Berrange e3a592
+    if (protocol && protocol[0]) {
Daniel P. Berrange e3a592
+        if (physdev && physdev[0]) {
Daniel P. Berrange e3a592
+            return iptablesAddRemoveRule(ctx->nat_postrouting,
Daniel P. Berrange e3a592
+                                         action,
Daniel P. Berrange e3a592
+                                         "--source", network,
Daniel P. Berrange e3a592
+                                         "-p", protocol,
Daniel P. Berrange e3a592
+                                         "!", "--destination", network,
Daniel P. Berrange e3a592
+                                         "--out-interface", physdev,
Daniel P. Berrange e3a592
+                                         "--jump", "MASQUERADE",
Daniel P. Berrange e3a592
+                                         "--to-ports", "1024-65535",
Daniel P. Berrange e3a592
+                                         NULL);
Daniel P. Berrange e3a592
+        } else {
Daniel P. Berrange e3a592
+            return iptablesAddRemoveRule(ctx->nat_postrouting,
Daniel P. Berrange e3a592
+                                         action,
Daniel P. Berrange e3a592
+                                         "--source", network,
Daniel P. Berrange e3a592
+                                         "-p", protocol,
Daniel P. Berrange e3a592
+                                         "!", "--destination", network,
Daniel P. Berrange e3a592
+                                         "--jump", "MASQUERADE",
Daniel P. Berrange e3a592
+                                         "--to-ports", "1024-65535",
Daniel P. Berrange e3a592
+                                         NULL);
Daniel P. Berrange e3a592
+        }
Daniel P. Berrange e3a592
     } else {
Daniel P. Berrange e3a592
-        return iptablesAddRemoveRule(ctx->nat_postrouting,
Daniel P. Berrange e3a592
-                                     action,
Daniel P. Berrange e3a592
-                                     "--source", network,
Daniel P. Berrange e3a592
-                                     "!", "--destination", network,
Daniel P. Berrange e3a592
-                                     "--jump", "MASQUERADE",
Daniel P. Berrange e3a592
-                                     NULL);
Daniel P. Berrange e3a592
+        if (physdev && physdev[0]) {
Daniel P. Berrange e3a592
+            return iptablesAddRemoveRule(ctx->nat_postrouting,
Daniel P. Berrange e3a592
+                                         action,
Daniel P. Berrange e3a592
+                                         "--source", network,
Daniel P. Berrange e3a592
+                                         "!", "--destination", network,
Daniel P. Berrange e3a592
+                                         "--out-interface", physdev,
Daniel P. Berrange e3a592
+                                         "--jump", "MASQUERADE",
Daniel P. Berrange e3a592
+                                         NULL);
Daniel P. Berrange e3a592
+        } else {
Daniel P. Berrange e3a592
+            return iptablesAddRemoveRule(ctx->nat_postrouting,
Daniel P. Berrange e3a592
+                                         action,
Daniel P. Berrange e3a592
+                                         "--source", network,
Daniel P. Berrange e3a592
+                                         "!", "--destination", network,
Daniel P. Berrange e3a592
+                                         "--jump", "MASQUERADE",
Daniel P. Berrange e3a592
+                                         NULL);
Daniel P. Berrange e3a592
+        }
Daniel P. Berrange e3a592
     }
Daniel P. Berrange e3a592
 }
Daniel P. Berrange e3a592
 
Daniel P. Berrange e3a592
@@ -719,6 +743,7 @@ iptablesForwardMasquerade(iptablesContext *ctx,
Daniel P. Berrange e3a592
  * @ctx: pointer to the IP table context
Daniel P. Berrange e3a592
  * @network: the source network name
Daniel P. Berrange e3a592
  * @physdev: the physical input device or NULL
Daniel P. Berrange e3a592
+ * @protocol: the network protocol or NULL
Daniel P. Berrange e3a592
  *
Daniel P. Berrange e3a592
  * Add rules to the IP table context to allow masquerading
Daniel P. Berrange e3a592
  * network @network on @physdev. This allow the bridge to
Daniel P. Berrange e3a592
@@ -729,9 +754,10 @@ iptablesForwardMasquerade(iptablesContext *ctx,
Daniel P. Berrange e3a592
 int
Daniel P. Berrange e3a592
 iptablesAddForwardMasquerade(iptablesContext *ctx,
Daniel P. Berrange e3a592
                              const char *network,
Daniel P. Berrange e3a592
-                             const char *physdev)
Daniel P. Berrange e3a592
+                             const char *physdev,
Daniel P. Berrange e3a592
+                             const char *protocol)
Daniel P. Berrange e3a592
 {
Daniel P. Berrange e3a592
-    return iptablesForwardMasquerade(ctx, network, physdev, ADD);
Daniel P. Berrange e3a592
+    return iptablesForwardMasquerade(ctx, network, physdev, protocol, ADD);
Daniel P. Berrange e3a592
 }
Daniel P. Berrange e3a592
 
Daniel P. Berrange e3a592
 /**
Daniel P. Berrange e3a592
@@ -739,6 +765,7 @@ iptablesAddForwardMasquerade(iptablesContext *ctx,
Daniel P. Berrange e3a592
  * @ctx: pointer to the IP table context
Daniel P. Berrange e3a592
  * @network: the source network name
Daniel P. Berrange e3a592
  * @physdev: the physical input device or NULL
Daniel P. Berrange e3a592
+ * @protocol: the network protocol or NULL
Daniel P. Berrange e3a592
  *
Daniel P. Berrange e3a592
  * Remove rules from the IP table context to stop masquerading
Daniel P. Berrange e3a592
  * network @network on @physdev. This stops the bridge from
Daniel P. Berrange e3a592
@@ -749,7 +776,8 @@ iptablesAddForwardMasquerade(iptablesContext *ctx,
Daniel P. Berrange e3a592
 int
Daniel P. Berrange e3a592
 iptablesRemoveForwardMasquerade(iptablesContext *ctx,
Daniel P. Berrange e3a592
                                 const char *network,
Daniel P. Berrange e3a592
-                                const char *physdev)
Daniel P. Berrange e3a592
+                                const char *physdev,
Daniel P. Berrange e3a592
+                                const char *protocol)
Daniel P. Berrange e3a592
 {
Daniel P. Berrange e3a592
-    return iptablesForwardMasquerade(ctx, network, physdev, REMOVE);
Daniel P. Berrange e3a592
+    return iptablesForwardMasquerade(ctx, network, physdev, protocol, REMOVE);
Daniel P. Berrange e3a592
 }
Daniel P. Berrange e3a592
diff --git a/src/util/iptables.h b/src/util/iptables.h
Daniel P. Berrange e3a592
index 7d55a6d..b47d854 100644
Daniel P. Berrange e3a592
--- a/src/util/iptables.h
Daniel P. Berrange e3a592
+++ b/src/util/iptables.h
Daniel P. Berrange e3a592
@@ -85,9 +85,11 @@ int              iptablesRemoveForwardRejectIn   (iptablesContext *ctx,
Daniel P. Berrange e3a592
 
Daniel P. Berrange e3a592
 int              iptablesAddForwardMasquerade    (iptablesContext *ctx,
Daniel P. Berrange e3a592
                                                   const char *network,
Daniel P. Berrange e3a592
-                                                  const char *physdev);
Daniel P. Berrange e3a592
+                                                  const char *physdev,
Daniel P. Berrange e3a592
+                                                  const char *protocol);
Daniel P. Berrange e3a592
 int              iptablesRemoveForwardMasquerade (iptablesContext *ctx,
Daniel P. Berrange e3a592
                                                   const char *network,
Daniel P. Berrange e3a592
-                                                  const char *physdev);
Daniel P. Berrange e3a592
+                                                  const char *physdev,
Daniel P. Berrange e3a592
+                                                  const char *protocol);
Daniel P. Berrange e3a592
 
Daniel P. Berrange e3a592
 #endif /* __QEMUD_IPTABLES_H__ */
Daniel P. Berrange e3a592
-- 
Daniel P. Berrange e3a592
1.6.6.1
Daniel P. Berrange e3a592