Merge pull request #2087 from GNS3/enhancement/2076

Checks for valid hostname on server side for Dynamips, IOU, Qemu and Docker nodes

gns3server/api/routes/compute/__init__.py

@@ -67,7 +67,7 @@ compute_api.state.controller_host = None
 
 
 @compute_api.exception_handler(ComputeError)
-async def controller_error_handler(request: Request, exc: ComputeError):
+async def compute_error_handler(request: Request, exc: ComputeError):
     log.error(f"Compute error: {exc}")
     return JSONResponse(
         status_code=409,
@@ -76,7 +76,7 @@ async def controller_error_handler(request: Request, exc: ComputeError):
 
 
 @compute_api.exception_handler(ComputeTimeoutError)
-async def controller_timeout_error_handler(request: Request, exc: ComputeTimeoutError):
+async def compute_timeout_error_handler(request: Request, exc: ComputeTimeoutError):
     log.error(f"Compute timeout error: {exc}")
     return JSONResponse(
         status_code=408,
@@ -85,7 +85,7 @@ async def controller_timeout_error_handler(request: Request, exc: ComputeTimeout
 
 
 @compute_api.exception_handler(ComputeUnauthorizedError)
-async def controller_unauthorized_error_handler(request: Request, exc: ComputeUnauthorizedError):
+async def compute_unauthorized_error_handler(request: Request, exc: ComputeUnauthorizedError):
     log.error(f"Compute unauthorized error: {exc}")
     return JSONResponse(
         status_code=401,
@@ -94,7 +94,7 @@ async def controller_unauthorized_error_handler(request: Request, exc: ComputeUn
 
 
 @compute_api.exception_handler(ComputeForbiddenError)
-async def controller_forbidden_error_handler(request: Request, exc: ComputeForbiddenError):
+async def compute_forbidden_error_handler(request: Request, exc: ComputeForbiddenError):
     log.error(f"Compute forbidden error: {exc}")
     return JSONResponse(
         status_code=403,
@@ -103,7 +103,7 @@ async def controller_forbidden_error_handler(request: Request, exc: ComputeForbi
 
 
 @compute_api.exception_handler(ComputeNotFoundError)
-async def controller_not_found_error_handler(request: Request, exc: ComputeNotFoundError):
+async def compute_not_found_error_handler(request: Request, exc: ComputeNotFoundError):
     log.error(f"Compute not found error: {exc}")
     return JSONResponse(
         status_code=404,
@@ -112,7 +112,7 @@ async def controller_not_found_error_handler(request: Request, exc: ComputeNotFo
 
 
 @compute_api.exception_handler(GNS3VMError)
-async def controller_error_handler(request: Request, exc: GNS3VMError):
+async def compute_gns3vm_error_handler(request: Request, exc: GNS3VMError):
     log.error(f"Compute GNS3 VM error: {exc}")
     return JSONResponse(
         status_code=409,

gns3server/api/routes/compute/dynamips_nodes.py

@@ -19,7 +19,6 @@ API routes for Dynamips nodes.
 """
 
 import os
-import sys
 
 from fastapi import APIRouter, WebSocket, Depends, Response, status
 from fastapi.encoders import jsonable_encoder
@@ -29,7 +28,6 @@ from uuid import UUID
 
 from gns3server.compute.dynamips import Dynamips
 from gns3server.compute.dynamips.nodes.router import Router
-from gns3server.compute.dynamips.dynamips_error import DynamipsError
 from gns3server import schemas
 
 responses = {404: {"model": schemas.ErrorMessage, "description": "Could not find project or Dynamips node"}}

gns3server/api/routes/compute/iou_nodes.py

@@ -20,7 +20,7 @@ API routes for IOU nodes.
 
 import os
 
-from fastapi import APIRouter, WebSocket, Depends, Body, Response, status
+from fastapi import APIRouter, WebSocket, Depends, Body, status
 from fastapi.encoders import jsonable_encoder
 from fastapi.responses import StreamingResponse
 from typing import Union

gns3server/compute/docker/docker_vm.py

@@ -33,6 +33,7 @@ from gns3server.utils.asyncio.raw_command_server import AsyncioRawCommandServer
 from gns3server.utils.asyncio import wait_for_file_creation
 from gns3server.utils.asyncio import monitor_process
 from gns3server.utils.get_resource import get_resource
+from gns3server.utils.hostname import is_rfc1123_hostname_valid
 
 from gns3server.compute.ubridge.ubridge_error import UbridgeError, UbridgeNamespaceError
 from ..base_node import BaseNode
@@ -89,6 +90,9 @@ class DockerVM(BaseNode):
         cpus=0,
     ):
 
+        if not is_rfc1123_hostname_valid(name):
+            raise DockerError(f"'{name}' is an invalid name to create a Docker node")
+
         super().__init__(
             name, node_id, project, manager, console=console, console_type=console_type, aux=aux, aux_type=aux_type
         )
@@ -171,6 +175,18 @@ class DockerVM(BaseNode):
                 return display
             display += 1
 
+    @BaseNode.name.setter
+    def name(self, new_name):
+        """
+        Sets the name of this Qemu VM.
+
+        :param new_name: name
+        """
+
+        if not is_rfc1123_hostname_valid(new_name):
+            raise DockerError(f"'{new_name}' is an invalid name to rename Docker container '{self._name}'")
+        super(DockerVM, DockerVM).name.__set__(self, new_name)
+
     @property
     def ethernet_adapters(self):
         return self._ethernet_adapters

gns3server/compute/dynamips/nodes/router.py

@@ -37,6 +37,7 @@ from ..dynamips_error import DynamipsError
 
 from gns3server.utils.file_watcher import FileWatcher
 from gns3server.utils.asyncio import wait_run_in_executor, monitor_process
+from gns3server.utils.hostname import is_ios_hostname_valid
 from gns3server.utils.images import md5sum
 
 
@@ -75,6 +76,9 @@ class Router(BaseNode):
         ghost_flag=False,
     ):
 
+        if not is_ios_hostname_valid(name):
+            raise DynamipsError(f"{name} is an invalid name to create a Dynamips node")
+
         super().__init__(
             name, node_id, project, manager, console=console, console_type=console_type, aux=aux, aux_type=aux_type
         )
@@ -1653,6 +1657,9 @@ class Router(BaseNode):
         :param new_name: new name string
         """
 
+        if not is_ios_hostname_valid(new_name):
+            raise DynamipsError(f"{new_name} is an invalid name to rename router '{self._name}'")
+
         await self._hypervisor.send(f'vm rename "{self._name}" "{new_name}"')
 
         # change the hostname in the startup-config

gns3server/compute/iou/iou_vm.py

@@ -42,6 +42,7 @@ from .utils.iou_export import nvram_export
 from gns3server.compute.ubridge.ubridge_error import UbridgeError
 from gns3server.utils.file_watcher import FileWatcher
 from gns3server.utils.asyncio.telnet_server import AsyncioTelnetServer
+from gns3server.utils.hostname import is_ios_hostname_valid
 from gns3server.utils.asyncio import locking
 import gns3server.utils.asyncio
 import gns3server.utils.images
@@ -70,6 +71,9 @@ class IOUVM(BaseNode):
         self, name, node_id, project, manager, application_id=None, path=None, console=None, console_type="telnet"
     ):
 
+        if not is_ios_hostname_valid(name):
+            raise IOUError(f"'{name}' is an invalid name to create an IOU node")
+
         super().__init__(name, node_id, project, manager, console=console, console_type=console_type)
 
         log.info(
@@ -334,6 +338,8 @@ class IOUVM(BaseNode):
         :param new_name: name
         """
 
+        if not is_ios_hostname_valid(new_name):
+            raise IOUError(f"'{new_name}' is an invalid name to rename IOU node '{self._name}'")
         if self.startup_config_file:
             content = self.startup_config_content
             content = re.sub(r"hostname .+$", "hostname " + new_name, content, flags=re.MULTILINE)

gns3server/compute/qemu/qemu_vm.py

@@ -22,7 +22,6 @@ order to run a QEMU VM.
 import sys
 import os
 import re
-import shlex
 import math
 import shutil
 import struct
@@ -47,6 +46,7 @@ from ..base_node import BaseNode
 from ...utils.asyncio import monitor_process
 from ...utils.images import md5sum
 from ...utils import macaddress_to_int, int_to_macaddress
+from ...utils.hostname import is_rfc1123_hostname_valid
 
 from gns3server.schemas.compute.qemu_nodes import Qemu, QemuPlatform
 
@@ -86,6 +86,9 @@ class QemuVM(BaseNode):
         platform=None,
     ):
 
+        if not is_rfc1123_hostname_valid(name):
+            raise QemuError(f"'{name}' is an invalid name to create a Qemu node")
+
         super().__init__(
             name,
             node_id,
@@ -172,6 +175,18 @@ class QemuVM(BaseNode):
 
         log.info(f'QEMU VM "{self._name}" [{self._id}] has been created')
 
+    @BaseNode.name.setter
+    def name(self, new_name):
+        """
+        Sets the name of this Qemu VM.
+
+        :param new_name: name
+        """
+
+        if not is_rfc1123_hostname_valid(new_name):
+            raise QemuError(f"'{new_name}' is an invalid name to rename Qemu node '{self._name}'")
+        super(QemuVM, QemuVM).name.__set__(self, new_name)
+
     @property
     def guest_cid(self):
         """

gns3server/controller/compute.py

@@ -23,7 +23,7 @@ import socket
 import json
 import sys
 import io
-from operator import itemgetter
+from fastapi import HTTPException
 from aiohttp import web
 
 from ..utils import parse_version
@@ -576,12 +576,13 @@ class Compute:
                 # If the 409 doesn't come from a GNS3 server
                 except ValueError:
                     raise ControllerError(msg)
-            elif response.status == 500:
-                raise aiohttp.web.HTTPInternalServerError(text=f"Internal server error {url}")
-            elif response.status == 503:
-                raise aiohttp.web.HTTPServiceUnavailable(text=f"Service unavailable {url} {body}")
             else:
-                raise NotImplementedError(f"{response.status} status code is not supported for {method} '{url}'\n{body}")
+                raise HTTPException(
+                    status_code=response.status,
+                    detail=f"HTTP error {response.status} received from compute "
+                           f"'{self.name}' for request {method} {path}: {msg}"
+                )
+
         if body and len(body):
             if raw:
                 response.body = body

gns3server/controller/node.py

@@ -427,6 +427,7 @@ class Node:
         # When updating properties used only on controller we don't need to call the compute
         update_compute = False
         old_json = self.asdict()
+        old_name = self._name
 
         compute_properties = None
         # Update node properties with additional elements
@@ -454,7 +455,13 @@ class Node:
         self._list_ports()
         if update_compute:
             data = self._node_data(properties=compute_properties)
+            try:
                 response = await self.put(None, data=data)
+            except ComputeConflictError:
+                if old_name != self.name:
+                    # special case when the new name is already updated on controller but refused by the compute
+                    self.name = old_name
+                raise
             await self.parse_node_response(response.json)
         elif old_json != self.asdict():
             # We send notif only if object has changed

gns3server/utils/hostname.py

@@ -0,0 +1,59 @@
+#!/usr/bin/env python
+#
+# Copyright (C) 2022 GNS3 Technologies Inc.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+import re
+
+
+def is_ios_hostname_valid(hostname: str) -> bool:
+    """
+    Check if an IOS hostname is valid
+
+    IOS hostname must start with a letter, end with a letter or digit, and
+    have as interior characters only letters, digits, and hyphens.
+    They must be 63 characters or fewer (ARPANET rules).
+    """
+
+    if re.search(r"""^(?!-|[0-9])[a-zA-Z0-9-]{1,63}(?<!-)$""", hostname):
+        return True
+    return False
+
+
+def is_rfc1123_hostname_valid(hostname: str) -> bool:
+    """
+    Check if a hostname is valid according to RFC 1123
+
+    Each element of the hostname must be from 1 to 63 characters long
+    and the entire hostname, including the dots, can be at most 253
+    characters long.  Valid characters for hostnames are ASCII
+    letters from a to z, the digits from 0 to 9, and the hyphen (-).
+    A hostname may not start with a hyphen.
+    """
+
+    if hostname[-1] == ".":
+        hostname = hostname[:-1]  # strip exactly one dot from the right, if present
+
+    if len(hostname) > 253:
+        return False
+
+    labels = hostname.split(".")
+
+    # the TLD must be not all-numeric
+    if re.match(r"[0-9]+$", labels[-1]):
+        return False
+
+    allowed = re.compile(r"(?!-)[a-zA-Z0-9-]{1,63}(?<!-)$")
+    return all(allowed.match(label) for label in labels)

tests/api/routes/compute/test_docker_nodes.py

@@ -33,7 +33,7 @@ def base_params() -> dict:
     """Return standard parameters"""
 
     params = {
-        "name": "PC TEST 1",
+        "name": "DOCKER-TEST-1",
         "image": "nginx",
         "start_command": "nginx-daemon",
         "adapters": 2,
@@ -71,10 +71,11 @@ async def test_docker_create(app: FastAPI, compute_client: AsyncClient, compute_
 
     with asyncio_patch("gns3server.compute.docker.Docker.list_images", return_value=[{"image": "nginx"}]):
         with asyncio_patch("gns3server.compute.docker.Docker.query", return_value={"Id": "8bd8153ea8f5"}):
-            response = await compute_client.post(app.url_path_for("compute:create_docker_node", project_id=compute_project.id),
+            response = await compute_client.post(
-                                         json=base_params)
+                app.url_path_for("compute:create_docker_node", project_id=compute_project.id), json=base_params
+            )
     assert response.status_code == status.HTTP_201_CREATED
-    assert response.json()["name"] == "PC TEST 1"
+    assert response.json()["name"] == "DOCKER-TEST-1"
     assert response.json()["project_id"] == compute_project.id
     assert response.json()["container_id"] == "8bd8153ea8f5"
     assert response.json()["image"] == "nginx:latest"
@@ -84,6 +85,40 @@ async def test_docker_create(app: FastAPI, compute_client: AsyncClient, compute_
     assert response.json()["extra_hosts"] == "test:127.0.0.1"
 
 
+@pytest.mark.parametrize(
+    "name, status_code",
+    (
+        ("valid-name.com", status.HTTP_201_CREATED),
+        ("42name", status.HTTP_201_CREATED),
+        ("424242", status.HTTP_409_CONFLICT),
+        ("name42", status.HTTP_201_CREATED),
+        ("name.424242", status.HTTP_409_CONFLICT),
+        ("-name", status.HTTP_409_CONFLICT),
+        ("name%-test", status.HTTP_409_CONFLICT),
+        ("x" * 63, status.HTTP_201_CREATED),
+        ("x" * 64, status.HTTP_409_CONFLICT),
+        (("x" * 62 + ".") * 4, status.HTTP_201_CREATED),
+        ("xx" + ("x" * 62 + ".") * 4, status.HTTP_409_CONFLICT),
+    ),
+)
+async def test_docker_create_with_invalid_name(
+        app: FastAPI,
+        compute_client: AsyncClient,
+        compute_project: Project,
+        base_params: dict,
+        name: str,
+        status_code: int
+) -> None:
+
+    base_params["name"] = name
+    with asyncio_patch("gns3server.compute.docker.Docker.list_images", return_value=[{"image": "nginx"}]):
+        with asyncio_patch("gns3server.compute.docker.Docker.query", return_value={"Id": "8bd8153ea8f5"}):
+            response = await compute_client.post(
+                app.url_path_for("compute:create_docker_node", project_id=compute_project.id), json=base_params
+            )
+    assert response.status_code == status_code
+
+
 async def test_docker_start(app: FastAPI, compute_client: AsyncClient, vm: dict) -> None:
 
     with asyncio_patch("gns3server.compute.docker.docker_vm.DockerVM.start", return_value=True) as mock:

tests/api/routes/compute/test_iou_nodes.py

@@ -46,7 +46,7 @@ def fake_iou_bin(images_dir) -> str:
 def base_params(tmpdir, fake_iou_bin) -> dict:
     """Return standard parameters"""
 
-    return {"application_id": 42, "name": "PC TEST 1", "path": "iou.bin"}
+    return {"application_id": 42, "name": "IOU-TEST-1", "path": "iou.bin"}
 
 
 @pytest.fixture
@@ -68,7 +68,7 @@ async def test_iou_create(app: FastAPI, compute_client: AsyncClient, compute_pro
 
     response = await compute_client.post(app.url_path_for("compute:create_iou_node", project_id=compute_project.id), json=base_params)
     assert response.status_code == status.HTTP_201_CREATED
-    assert response.json()["name"] == "PC TEST 1"
+    assert response.json()["name"] == "IOU-TEST-1"
     assert response.json()["project_id"] == compute_project.id
     assert response.json()["serial_adapters"] == 2
     assert response.json()["ethernet_adapters"] == 2
@@ -93,7 +93,7 @@ async def test_iou_create_with_params(app: FastAPI,
 
     response = await compute_client.post(app.url_path_for("compute:create_iou_node", project_id=compute_project.id), json=params)
     assert response.status_code == status.HTTP_201_CREATED
-    assert response.json()["name"] == "PC TEST 1"
+    assert response.json()["name"] == "IOU-TEST-1"
     assert response.json()["project_id"] == compute_project.id
     assert response.json()["serial_adapters"] == 4
     assert response.json()["ethernet_adapters"] == 0
@@ -106,6 +106,34 @@ async def test_iou_create_with_params(app: FastAPI,
         assert f.read() == "hostname test"
 
 
+@pytest.mark.parametrize(
+    "name, status_code",
+    (
+        ("valid-name", status.HTTP_201_CREATED),
+        ("42name", status.HTTP_409_CONFLICT),
+        ("name42", status.HTTP_201_CREATED),
+        ("-name", status.HTTP_409_CONFLICT),
+        ("name%-test", status.HTTP_409_CONFLICT),
+        ("x" * 63, status.HTTP_201_CREATED),
+        ("x" * 64, status.HTTP_409_CONFLICT),
+    ),
+)
+async def test_iou_create_with_invalid_name(
+        app: FastAPI,
+        compute_client: AsyncClient,
+        compute_project: Project,
+        base_params: dict,
+        name: str,
+        status_code: int
+) -> None:
+
+    base_params["name"] = name
+    response = await compute_client.post(
+        app.url_path_for("compute:create_iou_node", project_id=compute_project.id), json=base_params
+    )
+    assert response.status_code == status_code
+
+
 async def test_iou_create_startup_config_already_exist(
         app: FastAPI,
         compute_client: AsyncClient,
@@ -133,7 +161,7 @@ async def test_iou_get(app: FastAPI, compute_client: AsyncClient, compute_projec
 
     response = await compute_client.get(app.url_path_for("compute:get_iou_node", project_id=vm["project_id"], node_id=vm["node_id"]))
     assert response.status_code == status.HTTP_200_OK
-    assert response.json()["name"] == "PC TEST 1"
+    assert response.json()["name"] == "IOU-TEST-1"
     assert response.json()["project_id"] == compute_project.id
     assert response.json()["serial_adapters"] == 2
     assert response.json()["ethernet_adapters"] == 2

tests/api/routes/compute/test_qemu_nodes.py

@@ -66,7 +66,7 @@ def fake_qemu_img_binary(tmpdir):
 def base_params(tmpdir, fake_qemu_bin) -> dict:
     """Return standard parameters"""
 
-    return {"name": "PC TEST 1", "qemu_path": fake_qemu_bin}
+    return {"name": "QEMU-TEST-1", "qemu_path": fake_qemu_bin}
 
 
 @pytest.fixture
@@ -88,7 +88,7 @@ async def test_qemu_create(app: FastAPI,
 
     response = await compute_client.post(app.url_path_for("compute:create_qemu_node", project_id=compute_project.id), json=base_params)
     assert response.status_code == status.HTTP_201_CREATED
-    assert response.json()["name"] == "PC TEST 1"
+    assert response.json()["name"] == "QEMU-TEST-1"
     assert response.json()["project_id"] == compute_project.id
     assert response.json()["qemu_path"] == fake_qemu_bin
     assert response.json()["platform"] == "x86_64"
@@ -104,7 +104,7 @@ async def test_qemu_create_platform(app: FastAPI,
     base_params["platform"] = "x86_64"
     response = await compute_client.post(app.url_path_for("compute:create_qemu_node", project_id=compute_project.id), json=base_params)
     assert response.status_code == status.HTTP_201_CREATED
-    assert response.json()["name"] == "PC TEST 1"
+    assert response.json()["name"] == "QEMU-TEST-1"
     assert response.json()["project_id"] == compute_project.id
     assert response.json()["qemu_path"] == fake_qemu_bin
     assert response.json()["platform"] == "x86_64"
@@ -122,13 +122,44 @@ async def test_qemu_create_with_params(app: FastAPI,
     params["hda_disk_image"] = "linux载.img"
     response = await compute_client.post(app.url_path_for("compute:create_qemu_node", project_id=compute_project.id), json=params)
     assert response.status_code == status.HTTP_201_CREATED
-    assert response.json()["name"] == "PC TEST 1"
+    assert response.json()["name"] == "QEMU-TEST-1"
     assert response.json()["project_id"] == compute_project.id
     assert response.json()["ram"] == 1024
     assert response.json()["hda_disk_image"] == "linux载.img"
     assert response.json()["hda_disk_image_md5sum"] == "c4ca4238a0b923820dcc509a6f75849b"
 
 
+@pytest.mark.parametrize(
+    "name, status_code",
+    (
+        ("valid-name.com", status.HTTP_201_CREATED),
+        ("42name", status.HTTP_201_CREATED),
+        ("424242", status.HTTP_409_CONFLICT),
+        ("name42", status.HTTP_201_CREATED),
+        ("name.424242", status.HTTP_409_CONFLICT),
+        ("-name", status.HTTP_409_CONFLICT),
+        ("name%-test", status.HTTP_409_CONFLICT),
+        ("x" * 63, status.HTTP_201_CREATED),
+        ("x" * 64, status.HTTP_409_CONFLICT),
+        (("x" * 62 + ".") * 4, status.HTTP_201_CREATED),
+        ("xx" + ("x" * 62 + ".") * 4, status.HTTP_409_CONFLICT),
+    ),
+)
+async def test_qemu_create_with_invalid_name(
+        app: FastAPI,
+        compute_client: AsyncClient,
+        compute_project: Project,
+        base_params: dict,
+        name: str,
+        status_code: int
+) -> None:
+
+    base_params["name"] = name
+    response = await compute_client.post(
+        app.url_path_for("compute:create_qemu_node", project_id=compute_project.id), json=base_params
+    )
+    assert response.status_code == status_code
+
 # async def test_qemu_create_with_project_file(app: FastAPI,
 #                                              compute_client: AsyncClient,
 #                                              compute_project: Project,
@@ -157,7 +188,7 @@ async def test_qemu_get(app: FastAPI, compute_client: AsyncClient, compute_proje
         app.url_path_for("compute:get_qemu_node", project_id=qemu_vm["project_id"], node_id=qemu_vm["node_id"])
     )
     assert response.status_code == status.HTTP_200_OK
-    assert response.json()["name"] == "PC TEST 1"
+    assert response.json()["name"] == "QEMU-TEST-1"
     assert response.json()["project_id"] == compute_project.id
     assert response.json()["node_directory"] == os.path.join(
         compute_project.path,