fixes #485, #482 Implemented proper tests for subinterfaces according to the "linux interfaces.pdf" diagram. Vlan interfaces can be bridge ports.

build_num

@@ -1 +1 @@
-#define BUILD_NUM 1523
+#define BUILD_NUM 1524

src/compiler_lib/CompilerDriver.cpp

@@ -44,6 +44,8 @@
 
 
 #include "CompilerDriver.h"
+#include "interfaceProperties.h"
+#include "interfacePropertiesObjectFactory.h"
 
 #include "fwbuilder/FWObject.h"
 #include "fwbuilder/FWObjectDatabase.h"
@@ -358,6 +360,7 @@ void CompilerDriver::commonChecks2(Cluster *cluster, Firewall *fw)
             }
         } else
         {
+            // Regular interface (should have an ip address)
             bool no_addr_ok = false;
             if (iface->getOptionsObject()->getBool("cluster_interface"))
             {
@@ -403,6 +406,62 @@ void CompilerDriver::commonChecks2(Cluster *cluster, Firewall *fw)
                 }
             }
         }
+
+        FWObject *parent = iface->getParent();
+
+        if (Interface::isA(parent))
+        {
+            Resources* os_res = Resources::os_res[fw->getStr("host_OS")];
+            string os_family = fw->getStr("host_OS");
+            if (os_res!=NULL)
+                os_family = os_res->getResourceStr("/FWBuilderResources/Target/family");
+
+            std::auto_ptr<interfaceProperties> int_prop(
+                interfacePropertiesObjectFactory::getInterfacePropertiesObject(
+                    os_family));
+
+            QString err;
+            if (!int_prop->validateInterface(parent, iface, true, err))
+                abort(fw, NULL, NULL, err.toStdString());
+
+            string interface_type = iface->getOptionsObject()->getStr("type");
+            if (interface_type.empty()) interface_type = "ethernet";
+
+            string parent_interface_type =
+                Interface::cast(parent)->getOptionsObject()->getStr("type");
+
+            if (parent_interface_type == "bridge" &&
+                interface_type == "ethernet" &&
+                int_prop->looksLikeVlanInterface(iface->getName().c_str()))
+            {
+                // if vlan interface is used as a bridge port, it
+                // should be a copy of the top-level interface object
+                // with the same name
+                bool have_top_level_copy = false;
+                for (list<FWObject*>::iterator i2=interfaces.begin();
+                     i2!=interfaces.end(); ++i2)
+                {
+                    Interface *in = Interface::cast(*i2);
+                    assert(in);
+                    if (in == iface) continue;
+                    if (in->getName() == iface->getName())
+                    {
+                        have_top_level_copy = true;
+                        break;
+                    }
+                }
+                if (!have_top_level_copy)
+                {
+                    QString err("Interface %1 looks like Vlan interface and is "
+                                "used as a bridge port. This configuration "
+                                "is only allowed if this object is a copy of another "
+                                "top-level interface with the same name"
+                    );
+                    abort(fw, NULL, NULL,
+                          err.arg(iface->getName().c_str()).toStdString());
+                }
+            }
+        }
     }
 }
 

src/compiler_lib/interfaceProperties.cpp

@@ -30,6 +30,8 @@
 #include "fwbuilder/Interface.h"
 #include "fwbuilder/IPv4.h"
 #include "fwbuilder/IPv6.h"
+#include "fwbuilder/Firewall.h"
+#include "fwbuilder/Cluster.h"
 #include "fwbuilder/Resources.h"
 #include "fwbuilder/FailoverClusterGroup.h"
 
@@ -122,8 +124,6 @@ bool interfaceProperties::manageIpAddresses(Interface *intf,
 
         getListOfAddresses(intf, update_addresses);
 
-        FWObject *parent_fw = intf->getParentHost();
-
         list<FWObject*> interfaces = fw->getByTypeDeep(Interface::TYPENAME);
         list<FWObject*>::iterator i;
         for (i=interfaces.begin(); i!=interfaces.end(); ++i )
@@ -152,4 +152,123 @@ bool interfaceProperties::manageIpAddresses(Interface *intf,
     return true;
 }
 
+bool interfaceProperties::validateInterface(FWObject *parent,
+                                            FWObject *intf,
+                                            bool check_types,
+                                            QString &err)
+{
+    if (Interface::cast(parent) && Interface::cast(intf))
+    {
+        if (!Interface::cast(parent)->validateChild(intf))
+        {
+            // See Interface::validateChild(). Currently the only
+            // condition when interface can not become a child of
+            // another interface is when interface has subinterfaces
+            // of its own.
+            err = QObject::tr("Interface %1 can not become subinterface of %2 "
+                              "because only one level of subinterfaces is allowed.")
+                .arg(intf->getName().c_str())
+                .arg(parent->getName().c_str());
+            return false;
+        }
+
+        if (check_types)
+        {
+            // We check types when this method is called from a compiler
+            string parent_interface_type =
+                Interface::cast(parent)->getOptionsObject()->getStr("type");
+            if (parent_interface_type.empty()) parent_interface_type = "ethernet";
+
+            FWObject *fw = Interface::cast(parent)->getParentHost();
+            QString host_os = fw->getStr("host_OS").c_str();
+            Resources* os_res = Resources::os_res[host_os.toStdString()];
+            list<string> interface_type_pairs;
+            os_res->getResourceStrList(
+                "/FWBuilderResources/Target/subinterfaces/" + parent_interface_type,
+                interface_type_pairs);
+            list<string> interface_types;
+            for (list<string>::iterator it=interface_type_pairs.begin();
+                 it!=interface_type_pairs.end(); ++it)
+            {
+                QString p = it->c_str();
+                interface_types.push_back(p.split(",")[0].toStdString());
+            }
+
+            // Implement interface type checks here
+            string interface_type = Interface::cast(intf)->getOptionsObject()->getStr("type");
+            if (interface_type.empty()) interface_type = "ethernet";
+
+            if (std::find(interface_types.begin(), interface_types.end(),
+                          interface_type) == interface_types.end())
+            {
+                err = QObject::tr("Interface %1 (type '%2') can not be a child "
+                                  "of interface %3 (type '%4')")
+                    .arg(intf->getName().c_str())
+                    .arg(interface_type.c_str())
+                    .arg(parent->getName().c_str())
+                    .arg(parent_interface_type.c_str());
+
+                return false;
+            }
+        }
+    }
+    return validateInterface(intf->getParent(), intf->getName().c_str(), err);
+}
+
+/**
+ * this method implements policy for the interface hierarchy, that is,
+ * can given interface be a child of a cluster or a firewall or
+ * another interface.
+ */
+bool interfaceProperties::validateInterface(FWObject *parent,
+                                            const QString &interface_name,
+                                            QString &err)
+{
+    if (Firewall::cast(parent))
+    {
+        if (looksLikeVlanInterface(interface_name))
+        {
+            QString parent_name = parent->getName().c_str();
+            if (Cluster::isA(parent))
+            {
+                // cluster is allowed to have top-level vlan interfaces,
+                // therefore we do not need to check the name of the
+                // interface against the name of the parent. This is
+                // signalled to isValidVlanInterfaceName() by passing
+                // empty string as parent_interface
+                parent_name = "";
+            }
+            return isValidVlanInterfaceName(interface_name, parent_name, err);
+        }
+        return true;
+    }
+
+    if (Interface::cast(parent))
+    {
+        string parent_interface_type =
+            Interface::cast(parent)->getOptionsObject()->getStr("type");
+        // check vlan conditions as well
+        if (looksLikeVlanInterface(interface_name))
+        {
+            // vlan interface can be a child of a bridge, in which
+            // case its base name does not match the
+            // parent. Perform other checks except this, pass ""
+            // as parent name argument to isValidVlanInterfaceName()
+            if (parent_interface_type == "bridge")
+                return isValidVlanInterfaceName(interface_name, "", err);
+
+            QString parent_name = parent->getName().c_str();
+            return isValidVlanInterfaceName(interface_name, parent_name, err);
+        }
+
+        // interface_name is not a vlan
+        // regular interface can only be a child of bond or bridge
+        return (parent_interface_type == "bridge" || parent_interface_type == "bonding");
+    }
+
+    // parent is not firewall (cluster) and not interface
+    err = QObject::tr("Interface can not be a child object of %1").arg(
+        parent->getTypeName().c_str());
+    return false;
+}
 

src/compiler_lib/interfaceProperties.h

@@ -77,6 +77,14 @@ public:
     virtual bool looksLikeVlanInterface(libfwbuilder::InterfaceData*) { return false; }
     virtual bool looksLikeVlanInterface(const QString&) { return false; }
 
+    virtual bool validateInterface(libfwbuilder::FWObject *parent,
+                                   const QString &inetrface_name,
+                                   QString &err);
+    virtual bool validateInterface(libfwbuilder::FWObject *parent,
+                                   libfwbuilder::FWObject *intf,
+                                   bool check_types,
+                                   QString &err);
+    
     /**
      * for the given interface return list of its ip addresses that we
      * should manage using update_addresses shell function and list of

src/gui/InterfaceDialog.cpp

@@ -364,32 +364,18 @@ void InterfaceDialog::validate(bool *res)
         os_family = os_res->getResourceStr("/FWBuilderResources/Target/family");
 
     interfaceProperties *int_prop =
-        interfacePropertiesObjectFactory::getInterfacePropertiesObject(
-            os_family);
-    if (int_prop->looksLikeVlanInterface(obj_name))
+        interfacePropertiesObjectFactory::getInterfacePropertiesObject(os_family);
+    QString err;
+    if ( ! int_prop->validateInterface(obj->getParent(), obj_name, err))
     {
-        QString parent_name = obj->getParent()->getName().c_str();
-        if (Cluster::isA(obj->getParent()))
-        {
-            // cluster is allowed to have top-level vlan interfaces,
-            // therefore we do not need to change the name of the
-            // interface against the name of the parent. This is
-            // signalled to isValidVlanInterfaceName() by passing
-            // empty string as parent_interface
-            parent_name = "";
-        }
-        QString err;
-        if ( ! int_prop->isValidVlanInterfaceName(obj_name, parent_name, err))
-        {
-            *res = false;
-            QMessageBox::critical(
-                this,"Firewall Builder",
-                err,
-                tr("&Continue"), QString::null,QString::null,
-                0, 1 );
-
-        }
+        *res = false;
+        QMessageBox::critical(
+            this,"Firewall Builder",
+            err,
+            tr("&Continue"), QString::null,QString::null,
+            0, 1 );
     }
+
     delete int_prop;
 }
 

src/gui/ObjectManipulator.cpp

@@ -2038,31 +2038,27 @@ bool ObjectManipulator::validateForPaste(FWObject *target, FWObject *obj,
     Host *hst = Host::cast(ta);
     Firewall *fw = Firewall::cast(ta);
     Interface *intf = Interface::cast(ta);
+    FWObject *parent_fw = ta;
+    while (parent_fw && Firewall::cast(parent_fw)==NULL) parent_fw = parent_fw->getParent();
+
+    if (parent_fw && Interface::isA(obj))
+    {
+        Resources* os_res = Resources::os_res[parent_fw->getStr("host_OS")];
+        string os_family = parent_fw->getStr("host_OS");
+        if (os_res!=NULL)
+            os_family = os_res->getResourceStr("/FWBuilderResources/Target/family");
+
+        std::auto_ptr<interfaceProperties> int_prop(
+            interfacePropertiesObjectFactory::getInterfacePropertiesObject(
+                os_family));
+
+        return int_prop->validateInterface(ta, obj, false, err);
+    }
 
     if (fw!=NULL)
     {
         // inserting some object into firewall or cluster
         if (!fw->validateChild(obj)) return false;
-        if (Interface::isA(obj))
-        {
-            // check if obj is vlan interface
-            Resources* os_res = Resources::os_res[fw->getStr("host_OS")];
-            string os_family = fw->getStr("host_OS");
-            if (os_res!=NULL)
-                os_family = os_res->getResourceStr("/FWBuilderResources/Target/family");
-
-            std::auto_ptr<interfaceProperties> int_prop(
-                interfacePropertiesObjectFactory::getInterfacePropertiesObject(
-                    os_family));
-            QString obj_name = obj->getName().c_str();
-            if (int_prop->looksLikeVlanInterface(obj_name))
-            {
-                err = QObject::tr("'%1' looks like a name of a vlan interface; "
-                                  "vlan can only be a subinterface of another interface, "
-                                  "it can not be a top-level interface.").arg(obj_name);
-                return false;
-            }
-        }
         return true;
     }
 
@@ -2070,50 +2066,7 @@ bool ObjectManipulator::validateForPaste(FWObject *target, FWObject *obj,
 
     if (intf!=NULL)
     {
-        // inserting some object into interface
-        if (Interface::isA(obj))
-        {
-            if (!intf->validateChild(obj))
-            {
-                // See Interface::validateChild(). Currently the only
-                // condition when interface can not become a child of
-                // another interface is when interface has subinterfaces
-                // of its own.
-                err = QObject::tr("Interface %1 can not become subinterface of %2 "
-                                  "because only one level of subinterfaces is allowed.")
-                    .arg(obj->getName().c_str())
-                    .arg(ta->getName().c_str());
-                return false;
-            }
-            // check vlan conditions as well
-            FWObject *f = intf->getParentHost();
-
-            Resources* os_res = Resources::os_res[f->getStr("host_OS")];
-            string os_family = f->getStr("host_OS");
-            if (os_res!=NULL)
-                os_family = os_res->getResourceStr("/FWBuilderResources/Target/family");
-
-            std::auto_ptr<interfaceProperties> int_prop(
-                interfacePropertiesObjectFactory::getInterfacePropertiesObject(
-                    os_family));
-            QString obj_name = obj->getName().c_str();
-            if (int_prop->looksLikeVlanInterface(obj_name))
-            {
-                // vlan interface can be a child of a bridge, in which
-                // case its base name does not match the
-                // parent. Perform other checks except this, pass ""
-                // as parent name argument to isValidVlanInterfaceName()
-                if (Interface::cast(ta)->getOptionsObject()->getStr("type") ==
-                    "bridge")
-                    return int_prop->isValidVlanInterfaceName(obj_name, "", err);
-
-                QString parent_name = ta->getName().c_str();
-                return int_prop->isValidVlanInterfaceName(obj_name, parent_name, err);
-            }
-        } else
-        {
-            if (!intf->validateChild(obj)) return false;
-        }
+        if (!intf->validateChild(obj)) return false;
         return true;
     }