Merge pull request #1151 from mattmundell/remove-default-pl

Remove default port list from CREATE_TARGET
greenbone · Jun 25, 2020 · effaa49 · effaa49
2 parents 8192e63 + 69998bb
commit effaa49
Show file tree

Hide file tree

Showing 9 changed files with 79 additions and 58 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -137,6 +137,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Add migrator to remove dead hosts [#1071](https://github.com/greenbone/gvmd/pull/1071)
 - Remove classic schedules elements from GMP [#1116](https://github.com/greenbone/gvmd/pull/1116) [#1121](https://github.com/greenbone/gvmd/pull/1121)
 - Remove parallel from target options. [#1119](https://github.com/greenbone/gvmd/pull/1119)
+- Remove default port list from CREATE_TARGET [#1151](https://github.com/greenbone/gvmd/pull/1151)
 
 [20.4]: https://github.com/greenbone/gvmd/compare/v9.0.0...master
 

diff --git a/src/gmp.c b/src/gmp.c
@@ -21849,6 +21849,13 @@ gmp_xml_handle_end_element (/* unused */ GMarkupParseContext* context,
                                     " 'snmp'"));
                 log_event_fail ("target", "Target", NULL, "created");
                 break;
+              case 12:
+                SEND_TO_CLIENT_OR_FAIL
+                 (XML_ERROR_SYNTAX ("create_target",
+                                    "One of PORT_LIST and PORT_RANGE are"
+                                    " required"));
+                log_event_fail ("target", "Target", NULL, "created");
+                break;
               case 99:
                 SEND_TO_CLIENT_OR_FAIL
                  (XML_ERROR_SYNTAX ("create_target",

diff --git a/src/manage_sql.c b/src/manage_sql.c
@@ -30853,7 +30853,7 @@ target_login_port (target_t target, const char* type)
  *         6 failed to find port list, 7 error in alive tests,
  *         8 invalid SSH credential type, 9 invalid SMB credential type,
  *         10 invalid ESXi credential type, 11 invalid SNMP credential type,
- *         99 permission denied, -1 error.
+ *         12 port range or port list required, 99 permission denied, -1 error.
  */
 int
 create_target (const char* name, const char* asset_hosts_filter,
@@ -30898,7 +30898,34 @@ create_target (const char* name, const char* asset_hosts_filter,
       sql_rollback ();
       return 1;
     }
-  quoted_name = sql_quote (name ?: "");
+
+  if (port_list_id)
+    {
+      if (find_port_list_with_permission (port_list_id, &port_list,
+                                          "get_port_lists")
+          || (port_list == 0))
+        {
+          sql_rollback ();
+          return 6;
+        }
+    }
+  else if (port_range == NULL)
+    {
+      sql_rollback ();
+      return 12;
+    }
+  else
+    {
+      port_list_comment = g_strdup_printf ("Autogenerated for target %s.", name);
+      ret = create_port_list_unique (name, port_list_comment, port_range,
+                                     &port_list);
+      g_free (port_list_comment);
+      if (ret)
+        {
+          sql_rollback ();
+          return ret;
+        }
+    }
 
   if (asset_hosts_filter)
     {
@@ -30943,15 +30970,13 @@ create_target (const char* name, const char* asset_hosts_filter,
   max = manage_count_hosts (clean, clean_exclude);
   if (max <= 0)
     {
-      g_free (quoted_name);
       g_free (clean);
       g_free (clean_exclude);
       sql_rollback ();
       return 2;
     }
   if (max > max_hosts)
     {
-      g_free (quoted_name);
       g_free (clean);
       g_free (clean_exclude);
       sql_rollback ();
@@ -30962,34 +30987,6 @@ create_target (const char* name, const char* asset_hosts_filter,
   g_free (clean);
   g_free (clean_exclude);
 
-  if (port_list_id)
-    {
-      if (find_port_list_with_permission (port_list_id, &port_list,
-                                          "get_port_lists")
-          || (port_list == 0))
-        {
-          g_free (quoted_name);
-          g_free (quoted_exclude_hosts);
-          g_free (quoted_hosts);
-          return 6;
-        }
-    }
-  else
-    {
-      port_list_comment = g_strdup_printf ("Autogenerated for target %s.", name);
-      ret = create_port_list_unique (name, port_list_comment, port_range,
-                                     &port_list);
-      g_free (port_list_comment);
-      if (ret)
-        {
-          g_free (quoted_name);
-          g_free (quoted_exclude_hosts);
-          g_free (quoted_hosts);
-          sql_rollback ();
-          return ret;
-        }
-    }
-
   if (ssh_credential)
     quoted_ssh_port = sql_insert (ssh_port ? ssh_port : "22");
   else
@@ -31004,6 +31001,8 @@ create_target (const char* name, const char* asset_hosts_filter,
   else
     reverse_lookup_unify = "1";
 
+  quoted_name = sql_quote (name ?: "");
+
   if (comment)
     quoted_comment = sql_quote (comment);
   else

diff --git a/src/manage_sql.h b/src/manage_sql.h
@@ -57,13 +57,6 @@
  */
 #define PERMISSION_UUID_SUPER_ADMIN_EVERYTHING "a9801074-6fe2-11e4-9d81-406186ea4fc5"
 
-/**
- * @brief UUID of 'OpenVAS Default' port list.
- *
- * Required for the default port list case of CREATE_PORT_LIST.
- */
-#define PORT_LIST_UUID_DEFAULT "c7e03b6c-3bbe-11e1-a057-406186ea4fc5"
-
 /**
  * @brief Predefined role UUID.
  */

diff --git a/src/manage_sql_port_lists.c b/src/manage_sql_port_lists.c
@@ -1077,7 +1077,7 @@ create_port_list_lock (const char *quoted_id, const char *quoted_name,
  *
  * @param[in]   name            Name of port list.
  * @param[in]   comment         Comment on port list.
- * @param[in]   port_range      GMP style port range list.  NULL for "default".
+ * @param[in]   port_range      GMP style port range list.
  * @param[out]  port_list       Created port list.
  *
  * @return 0 success, 4 error in port range.
@@ -1092,16 +1092,6 @@ create_port_list_unique (const char *name, const char *comment,
 
   assert (current_credentials.uuid);
 
-  if (port_range == NULL || (strcmp (port_range, "default") == 0))
-    {
-      if (find_port_list_with_permission (PORT_LIST_UUID_DEFAULT,
-                                          port_list,
-                                          "get_port_lists")
-          || (*port_list == 0))
-        return -1;
-      return 0;
-    }
-
   if (validate_port_range (port_range))
     return 4;
 

diff --git a/src/schema_formats/XML/GMP.xml.in b/src/schema_formats/XML/GMP.xml.in
@@ -5242,8 +5242,10 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
       <o><e>alive_tests</e></o>
       <o><e>reverse_lookup_only</e></o>
       <o><e>reverse_lookup_unify</e></o>
-      <o><e>port_range</e></o>
-      <o><e>port_list</e></o>
+      <or>
+        <e>port_range</e>
+        <e>port_list</e>
+      </or>
     </pattern>
     <ele>
       <name>name</name>
@@ -25968,6 +25970,23 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
   <!-- Compatibility changes between versions. -->
 
+  <change>
+    <command>CREATE_TARGET, RUN_WIZARD</command>
+    <summary>Default port list removed from CREATE_TARGET</summary>
+    <description>
+      <p>
+        CREATE_TARGET will no longer automatically add a port list to the
+        target if a port list or range is not given.  In other words, it
+        is now mandatory to supply either PORT_LIST or PORT_RANGE.
+      </p>
+      <p>
+        As a side effect wizards that use CREATE_TARGET now provide their
+        own default if no "port_list_id" param is given: "All IANA assigned
+        TCP", which has UUID 33d0cd82-57c6-11e1-8ed1-406186ea4fc5.
+      </p>
+    </description>
+    <version>20.08</version>
+  </change>
   <change>
     <command>CREATE_SCHEDULE, GET_SCHEDULES, MODIFY_SCHEDULE</command>
     <summary>Classic schedule elements removed</summary>

diff --git a/src/wizards/quick_auth_scan.xml b/src/wizards/quick_auth_scan.xml
@@ -127,6 +127,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
               </xsl:otherwise>
             </xsl:choose>
             <comment><xsl:value-of select="/wizard/params/param[name='comment']/value"/></comment>
+            <port_list id="33d0cd82-57c6-11e1-8ed1-406186ea4fc5"/>
           </create_target>
         </xsl:template>
       </xsl:stylesheet>

diff --git a/src/wizards/quick_first_scan.xml b/src/wizards/quick_first_scan.xml
@@ -120,9 +120,15 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
             </name>
             <hosts><xsl:value-of select="/wizard/params/param[name='hosts']/value"/></hosts>
 
-            <xsl:if test="/wizard/params/param[name='port_list_id']/value != ''">
-              <port_list id="{/wizard/params/param[name='port_list_id']/value}"/>
-            </xsl:if>
+            <xsl:choose>
+              <xsl:when test="/wizard/params/param[name='port_list_id']/value != ''">
+                <port_list id="{/wizard/params/param[name='port_list_id']/value}"/>
+              </xsl:when>
+              <xsl:otherwise>
+                <port_list id="33d0cd82-57c6-11e1-8ed1-406186ea4fc5"/>
+              </xsl:otherwise>
+            </xsl:choose>
+
             <xsl:if test="/wizard/params/param[name='ssh_credential']/value != ''">
               <ssh_lsc_credential id="{/wizard/params/param[name='ssh_credential']/value}">
                 <port>

diff --git a/src/wizards/quick_task.xml b/src/wizards/quick_task.xml
@@ -254,9 +254,14 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
             <comment>Automatically generated by wizard</comment>
             <hosts><xsl:value-of select="/wizard/params/param[name='target_hosts']/value"/></hosts>
 
-            <xsl:if test="/wizard/params/param[name='port_list_id']/value != ''">
-              <port_list id="{/wizard/params/param[name='port_list_id']/value}"/>
-            </xsl:if>
+            <xsl:choose>
+              <xsl:when test="/wizard/params/param[name='port_list_id']/value != ''">
+                <port_list id="{/wizard/params/param[name='port_list_id']/value}"/>
+              </xsl:when>
+              <xsl:otherwise>
+                <port_list id="33d0cd82-57c6-11e1-8ed1-406186ea4fc5"/>
+              </xsl:otherwise>
+            </xsl:choose>
 
             <xsl:if test="/wizard/params/param[name='ssh_credential']/value != ''">
               <ssh_lsc_credential id="{/wizard/params/param[name='ssh_credential']/value}">