NAS-136694 / 25.10 / Perform better NQN validation (#16733)

Author: bmeagherix

src/middlewared/middlewared/api/base/types/__init__.py

@@ -5,6 +5,7 @@
 from .ldap import * # noqa
 from .list import *  # noqa
 from .network import *  # noqa
+from .nvmet import *  # noqa
 from .string import *  # noqa
 from .urls import *  # noqa
 from .user import *  # noqa

src/middlewared/middlewared/api/base/types/nvmet.py

@@ -0,0 +1,49 @@
+import re
+from typing import Annotated
+from uuid import UUID
+
+from pydantic import AfterValidator, Field
+
+__all__ = [
+    "NQN"
+]
+
+MIN_NQN_LEN = 11
+MAX_NQN_LEN = 223
+
+NVMET_NQN_UUID_PREFIX = "nqn.2014-08.org.nvmexpress:uuid:"
+NVMET_NQN_UUID_PREFIX_LEN = 32
+NVMET_DISCOVERY_NQN = "nqn.2014-08.org.nvmexpress.discovery"
+UUID_STRING_LEN = 36
+NVMET_NQN_UUID_LEN = NVMET_NQN_UUID_PREFIX_LEN + UUID_STRING_LEN
+
+NQN_DATE_PATTERN = re.compile(r"^nqn.\d{4}-\d{2}\..*")
+DOMAIN_PATTERN = re.compile(r'^((?!-)[A-Za-z0-9-]{1,63}(?<!-)\.)+[A-Za-z]{2,}$')
+
+
+def _validate_nqn(nqn: str):
+    if nqn == NVMET_DISCOVERY_NQN:
+        return nqn
+    elif nqn.startswith(NVMET_NQN_UUID_PREFIX):
+        # Check for "nqn.2014-08.org.nvmexpress:uuid:11111111-2222-3333-4444-555555555555"
+        if len(nqn) != NVMET_NQN_UUID_LEN:
+            raise ValueError(f'{nqn}: uuid is incorrect length')
+        UUID(nqn[NVMET_NQN_UUID_PREFIX_LEN:])
+    elif nqn.startswith("nqn."):
+        # Check for "nqn.yyyy-mm.reverse.domain:user-string"
+        if not NQN_DATE_PATTERN.match(nqn):
+            raise ValueError(f'{nqn}: must start with "nqn.YYYY-MM.<reverse.domain>"')
+        # Now check the domain
+        if not nqn[12:] or '.' not in nqn[12:]:
+            raise ValueError(f'{nqn}: must start with "nqn.YYYY-MM.<reverse.domain>" - domain missing')
+        reverse_domain = nqn[12:].split(':', 1)[0]
+        domain_parts = reverse_domain.split('.')
+        domain_parts.reverse()
+        if not DOMAIN_PATTERN.match('.'.join(domain_parts)):
+            raise ValueError(f'{nqn}: domain does not appear to be valid')
+    else:
+        raise ValueError(f'{nqn}: must start with "nqn"')
+    return nqn
+
+
+NQN = Annotated[str, Field(min_length=MIN_NQN_LEN, max_length=MAX_NQN_LEN), AfterValidator(_validate_nqn)]

src/middlewared/middlewared/api/v25_10_0/nvmet_global.py

@@ -1,4 +1,4 @@
-from middlewared.api.base import BaseModel, Excluded, ForUpdateMetaclass, excluded_field, single_argument_args
+from middlewared.api.base import BaseModel, Excluded, ForUpdateMetaclass, NQN, excluded_field, single_argument_args
 
 __all__ = [
     "NVMetGlobalEntry",
@@ -44,6 +44,7 @@ class NVMetGlobalEntry(BaseModel):
 @single_argument_args('nvmet_update')
 class NVMetGlobalUpdateArgs(NVMetGlobalEntry, metaclass=ForUpdateMetaclass):
     id: Excluded = excluded_field()
+    basenqn: NQN
 
 
 class NVMetGlobalUpdateResult(BaseModel):

src/middlewared/middlewared/api/v25_10_0/nvmet_host.py

@@ -2,7 +2,7 @@
 
 from pydantic import Field, Secret, model_validator
 
-from middlewared.api.base import BaseModel, Excluded, ForUpdateMetaclass, NonEmptyString, excluded_field
+from middlewared.api.base import NQN, BaseModel, Excluded, ForUpdateMetaclass, NonEmptyString, excluded_field
 
 __all__ = [
     "NVMetHostEntry",
@@ -49,6 +49,7 @@ class NVMetHostEntry(BaseModel):
 
 class NVMetHostCreate(NVMetHostEntry):
     id: Excluded = excluded_field()
+    hostnqn: NQN
 
     @model_validator(mode='after')
     def validate_attrs(self):

src/middlewared/middlewared/api/v25_10_0/nvmet_subsys.py

@@ -1,8 +1,8 @@
-from typing import Annotated, Literal, Optional
+from typing import Literal, Optional
 
 from pydantic import Field
 
-from middlewared.api.base import BaseModel, Excluded, ForUpdateMetaclass, NonEmptyString, excluded_field
+from middlewared.api.base import NQN, BaseModel, Excluded, ForUpdateMetaclass, NonEmptyString, excluded_field
 
 __all__ = [
     "NVMetSubsysEntry",
@@ -14,8 +14,6 @@
     "NVMetSubsysDeleteResult",
 ]
 
-MAX_NQN_LEN = 223
-
 
 class NVMetSubsysEntry(BaseModel):
     id: int
@@ -26,7 +24,7 @@ class NVMetSubsysEntry(BaseModel):
     If `subnqn` is not provided on creation, then this name will be appended to the `basenqn` from \
     `nvmet.global.config` to generate a subnqn.
     """
-    subnqn: Annotated[NonEmptyString, Field(max_length=MAX_NQN_LEN)] | None = None
+    subnqn: NonEmptyString | None = None
     serial: str
     allow_any_host: bool = False
     """Any host can access the storage associated with this subsystem (i.e. no access control)."""
@@ -66,6 +64,7 @@ class NVMetSubsysCreate(NVMetSubsysEntry):
     hosts: Excluded = excluded_field()
     namespaces: Excluded = excluded_field()
     ports: Excluded = excluded_field()
+    subnqn: NQN | None = None
 
 
 class NVMetSubsysCreateArgs(BaseModel):

tests/api2/test_audit_nvmet.py

@@ -7,7 +7,7 @@
 REDACTED_SECRET = '********'
 MB = 1024 * 1024
 MB_100 = 100 * MB
-HOST1_NQN = 'nqn.host1'
+HOST1_NQN = 'nqn.2011-06.com.truenas:uuid-68bf9433-63ef-49f5-a921-4c0f8190fd94:host1'
 HOST1_KEY = 'DHHC-1:01:rxc6XaoJgTSN7GID7gPQidMAskFym01wNHdw5B3RA33UFFIc:'
 REDACTED_SECRET = '********'
 SUBSYS_NAME = 'subsys1'

tests/api2/test_nvmet_tcp.py

@@ -53,6 +53,8 @@
 ZVOL_RESIZE_END_MB = 150
 
 FAKE_HOSTNQN = 'nqn.2014-08.org.nvmexpress:uuid:48747223-7535-4f8e-a789-b8af8bfdea54'
+HOST1_NQN = 'nqn.2011-06.com.truenas:uuid-68bf9433-63ef-49f5-a921-4c0f8190fd94:host1'
+HOST2_NQN = 'nqn.2011-06.com.truenas:hostname2'
 DEVICE_TYPE_FILE = 'FILE'
 MB_10 = 100 * MB
 MB_100 = 100 * MB
@@ -1121,7 +1123,7 @@ def pick_from_list_by_id(_list, _id):
                 assert subsystems[0]['ports'][0] == fixture_port['id']
 
                 # Add a host -> no change
-                with nvmet_host('nqn.host1') as host1:
+                with nvmet_host(HOST1_NQN) as host1:
                     check_regular_query(1)
                     subsystems = check_verbose_query(1)
                     assert len(subsystems[0]['hosts']) == 0
@@ -1140,7 +1142,7 @@ def pick_from_list_by_id(_list, _id):
                         assert subsystems[0]['ports'][0] == fixture_port['id']
 
                         # Create/associate another host -> change
-                        with nvmet_host('nqn.host2') as host2:
+                        with nvmet_host(HOST2_NQN) as host2:
                             with nvmet_host_subsys(host2['id'], subsys1_id):
                                 check_regular_query(1)
                                 subsystems = check_verbose_query(1)