[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH] CVE-2010-2242 Apply a source port mapping to virtual network masquerading



For

  https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2010-2242

IPtables will seek to preserve the source port unchanged when
doing masquerading, if possible. NFS has a pseudo-security
option where it checks for the source port <= 1023 before
allowing a mount request. If an admin has used this to make the
host OS trusted for mounts, the default iptables behaviour will
potentially allow NAT'd guests access too. This needs to be
stopped.

With this change, the iptables -t nat -L -n -v rules for the
default network will be

Chain POSTROUTING (policy ACCEPT 95 packets, 9163 bytes)
 pkts bytes target     prot opt in     out     source               destination
   14   840 MASQUERADE  tcp  --  *      *       192.168.122.0/24    !192.168.122.0/24    masq ports: 1024-65535
   75  5752 MASQUERADE  udp  --  *      *       192.168.122.0/24    !192.168.122.0/24    masq ports: 1024-65535
    0     0 MASQUERADE  all  --  *      *       192.168.122.0/24    !192.168.122.0/24

* src/network/bridge_driver.c: Add masquerade rules for TCP
  and UDP protocols
* src/util/iptables.c, src/util/iptables.c: Add source port
  mappings for TCP & UDP protocols when masquerading.
---
 src/network/bridge_driver.c |   73 ++++++++++++++++++++++++++++++++++++++++--
 src/util/iptables.c         |   70 +++++++++++++++++++++++++++++------------
 src/util/iptables.h         |    6 ++-
 3 files changed, 122 insertions(+), 27 deletions(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 72255c1..80ed57a 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -638,18 +638,74 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
         goto masqerr2;
     }
 
-    /* enable masquerading */
+    /*
+     * Enable masquerading.
+     *
+     * We need to end up with 3 rules in the table in this order
+     *
+     *  1. protocol=tcp with sport mapping restricton
+     *  2. protocol=udp with sport mapping restricton
+     *  3. generic any protocol
+     *
+     * The sport mappings are required, because default IPtables
+     * MASQUERADE is maintain port number unchanged where possible.
+     *
+     * NFS can be configured to only "trust" port numbers < 1023.
+     *
+     * Guests using NAT thus need to be prevented from having port
+     * numbers < 1023, otherwise they can bypass the NFS "security"
+     * check on the source port number.
+     *
+     * Since we use '--insert' to add rules to the header of the
+     * chain, we actually need to add them in the reverse of the
+     * order just mentioned !
+     */
+
+    /* First the generic masquerade rule for other protocols */
     if ((err = iptablesAddForwardMasquerade(driver->iptables,
                                             network->def->network,
-                                            network->def->forwardDev))) {
+                                            network->def->forwardDev,
+                                            NULL))) {
         virReportSystemError(err,
                              _("failed to add iptables rule to enable masquerading to '%s'"),
                              network->def->forwardDev ? network->def->forwardDev : NULL);
         goto masqerr3;
     }
 
+    /* UDP with a source port restriction */
+    if ((err = iptablesAddForwardMasquerade(driver->iptables,
+                                            network->def->network,
+                                            network->def->forwardDev,
+                                            "udp"))) {
+        virReportSystemError(err,
+                             _("failed to add iptables rule to enable UDP masquerading to '%s'"),
+                             network->def->forwardDev ? network->def->forwardDev : NULL);
+        goto masqerr4;
+    }
+
+    /* TCP with a source port restriction */
+    if ((err = iptablesAddForwardMasquerade(driver->iptables,
+                                            network->def->network,
+                                            network->def->forwardDev,
+                                            "tcp"))) {
+        virReportSystemError(err,
+                             _("failed to add iptables rule to enable TCP masquerading to '%s'"),
+                             network->def->forwardDev ? network->def->forwardDev : NULL);
+        goto masqerr5;
+    }
+
     return 1;
 
+ masqerr5:
+    iptablesRemoveForwardMasquerade(driver->iptables,
+                                    network->def->network,
+                                    network->def->forwardDev,
+                                    "udp");
+ masqerr4:
+    iptablesRemoveForwardMasquerade(driver->iptables,
+                                    network->def->network,
+                                    network->def->forwardDev,
+                                    NULL);
  masqerr3:
     iptablesRemoveForwardAllowRelatedIn(driver->iptables,
                                  network->def->network,
@@ -814,8 +870,17 @@ networkRemoveIptablesRules(struct network_driver *driver,
     if (network->def->forwardType != VIR_NETWORK_FORWARD_NONE) {
         if (network->def->forwardType == VIR_NETWORK_FORWARD_NAT) {
             iptablesRemoveForwardMasquerade(driver->iptables,
-                                                network->def->network,
-                                                network->def->forwardDev);
+                                            network->def->network,
+                                            network->def->forwardDev,
+                                            "tcp");
+            iptablesRemoveForwardMasquerade(driver->iptables,
+                                            network->def->network,
+                                            network->def->forwardDev,
+                                            "udp");
+            iptablesRemoveForwardMasquerade(driver->iptables,
+                                            network->def->network,
+                                            network->def->forwardDev,
+                                            NULL);
             iptablesRemoveForwardAllowRelatedIn(driver->iptables,
                                                 network->def->network,
                                                 network->def->bridge,
diff --git a/src/util/iptables.c b/src/util/iptables.c
index d06b857..f63e8c6 100644
--- a/src/util/iptables.c
+++ b/src/util/iptables.c
@@ -692,25 +692,49 @@ iptablesRemoveForwardRejectIn(iptablesContext *ctx,
  */
 static int
 iptablesForwardMasquerade(iptablesContext *ctx,
-                       const char *network,
-                       const char *physdev,
-                       int action)
+                          const char *network,
+                          const char *physdev,
+                          const char *protocol,
+                          int action)
 {
-    if (physdev && physdev[0]) {
-        return iptablesAddRemoveRule(ctx->nat_postrouting,
-                                     action,
-                                     "--source", network,
-                                     "!", "--destination", network,
-                                     "--out-interface", physdev,
-                                     "--jump", "MASQUERADE",
-                                     NULL);
+    if (protocol && protocol[0]) {
+        if (physdev && physdev[0]) {
+            return iptablesAddRemoveRule(ctx->nat_postrouting,
+                                         action,
+                                         "--source", network,
+                                         "-p", protocol,
+                                         "!", "--destination", network,
+                                         "--out-interface", physdev,
+                                         "--jump", "MASQUERADE",
+                                         "--to-ports", "1024-65535",
+                                         NULL);
+        } else {
+            return iptablesAddRemoveRule(ctx->nat_postrouting,
+                                         action,
+                                         "--source", network,
+                                         "-p", protocol,
+                                         "!", "--destination", network,
+                                         "--jump", "MASQUERADE",
+                                         "--to-ports", "1024-65535",
+                                         NULL);
+        }
     } else {
-        return iptablesAddRemoveRule(ctx->nat_postrouting,
-                                     action,
-                                     "--source", network,
-                                     "!", "--destination", network,
-                                     "--jump", "MASQUERADE",
-                                     NULL);
+        if (physdev && physdev[0]) {
+            return iptablesAddRemoveRule(ctx->nat_postrouting,
+                                         action,
+                                         "--source", network,
+                                         "!", "--destination", network,
+                                         "--out-interface", physdev,
+                                         "--jump", "MASQUERADE",
+                                         NULL);
+        } else {
+            return iptablesAddRemoveRule(ctx->nat_postrouting,
+                                         action,
+                                         "--source", network,
+                                         "!", "--destination", network,
+                                         "--jump", "MASQUERADE",
+                                         NULL);
+        }
     }
 }
 
@@ -719,6 +743,7 @@ iptablesForwardMasquerade(iptablesContext *ctx,
  * @ctx: pointer to the IP table context
  * @network: the source network name
  * @physdev: the physical input device or NULL
+ * @protocol: the network protocol or NULL
  *
  * Add rules to the IP table context to allow masquerading
  * network @network on @physdev. This allow the bridge to
@@ -729,9 +754,10 @@ iptablesForwardMasquerade(iptablesContext *ctx,
 int
 iptablesAddForwardMasquerade(iptablesContext *ctx,
                              const char *network,
-                             const char *physdev)
+                             const char *physdev,
+                             const char *protocol)
 {
-    return iptablesForwardMasquerade(ctx, network, physdev, ADD);
+    return iptablesForwardMasquerade(ctx, network, physdev, protocol, ADD);
 }
 
 /**
@@ -739,6 +765,7 @@ iptablesAddForwardMasquerade(iptablesContext *ctx,
  * @ctx: pointer to the IP table context
  * @network: the source network name
  * @physdev: the physical input device or NULL
+ * @protocol: the network protocol or NULL
  *
  * Remove rules from the IP table context to stop masquerading
  * network @network on @physdev. This stops the bridge from
@@ -749,7 +776,8 @@ iptablesAddForwardMasquerade(iptablesContext *ctx,
 int
 iptablesRemoveForwardMasquerade(iptablesContext *ctx,
                                 const char *network,
-                                const char *physdev)
+                                const char *physdev,
+                                const char *protocol)
 {
-    return iptablesForwardMasquerade(ctx, network, physdev, REMOVE);
+    return iptablesForwardMasquerade(ctx, network, physdev, protocol, REMOVE);
 }
diff --git a/src/util/iptables.h b/src/util/iptables.h
index 7d55a6d..b47d854 100644
--- a/src/util/iptables.h
+++ b/src/util/iptables.h
@@ -85,9 +85,11 @@ int              iptablesRemoveForwardRejectIn   (iptablesContext *ctx,
 
 int              iptablesAddForwardMasquerade    (iptablesContext *ctx,
                                                   const char *network,
-                                                  const char *physdev);
+                                                  const char *physdev,
+                                                  const char *protocol);
 int              iptablesRemoveForwardMasquerade (iptablesContext *ctx,
                                                   const char *network,
-                                                  const char *physdev);
+                                                  const char *physdev,
+                                                  const char *protocol);
 
 #endif /* __QEMUD_IPTABLES_H__ */
-- 
1.6.6.1


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]