[fix] Address extension validation follow-ups

Related to #98

Author: czarflix

django_x509/base/models.py

@@ -601,6 +601,11 @@ def _get_extension_values(self, value):
             return value.split(",")
         return []
 
+    def _get_der_length_bytes(self, length):
+        if length < 128:
+            return bytes([length])
+        return b"\x81" + bytes([length])
+
     def _validate_supported_extensions(self):
         for index, ext in enumerate(self.extensions or []):
             if not isinstance(ext, dict):
@@ -623,7 +628,8 @@ def _validate_supported_extensions(self):
                     self._raise_extensions_validation_error(
                         f"{path}.value", _("nsComment extension requires a value.")
                     )
-                if len(value) > 255:
+                value_bytes = value.encode("utf-8")
+                if len(value_bytes) > 255:
                     self._raise_extensions_validation_error(
                         f"{path}.value",
                         _("nsComment value exceeds maximum length of 255 bytes"),
@@ -760,9 +766,9 @@ def _add_extensions(self, builder, public_key):
                         "emailprotection": ExtendedKeyUsageOID.EMAIL_PROTECTION,
                     }
                     oids = []
-                    values = val if isinstance(val, list) else val.split(",")
+                    values = self._get_extension_values(val)
                     for v in values:
-                        v_clean = v.strip().lower()
+                        v_clean = str(v).strip().lower()
                         if v_clean not in eku_map:
                             raise ValidationError(
                                 _("Unsupported extendedKeyUsage value: %s") % v_clean
@@ -778,11 +784,14 @@ def _add_extensions(self, builder, public_key):
                         raise ValidationError(
                             _("nsComment extension requires a value.")
                         )
-                    if len(val) > 255:
+                    val_bytes = val.encode("utf-8")
+                    if len(val_bytes) > 255:
                         raise ValidationError(
                             _("nsComment value exceeds maximum length of 255 bytes")
                         )
-                    raw_val = b"\x16" + bytes([len(val)]) + val.encode("utf-8")
+                    raw_val = (
+                        b"\x16" + self._get_der_length_bytes(len(val_bytes)) + val_bytes
+                    )
                     builder = builder.add_extension(
                         x509.UnrecognizedExtension(
                             x509.ObjectIdentifier("2.16.840.1.113730.1.13"), raw_val
@@ -801,9 +810,9 @@ def _add_extensions(self, builder, public_key):
                         "objca": 0x01,
                     }
                     bits = 0
-                    values = val if isinstance(val, list) else val.split(",")
+                    values = self._get_extension_values(val)
                     for v in values:
-                        v_clean = v.strip().lower()
+                        v_clean = str(v).strip().lower()
                         if v_clean not in ns_cert_type_map:
                             raise ValidationError(
                                 _("Unsupported nsCertType value: %s") % v_clean

django_x509/schemas.py

@@ -16,7 +16,7 @@ def _build_extension_schema(name, title, description, value_schema):
             "critical": {"type": "boolean", "default": False},
             "value": value_schema,
         },
-        "required": ["name", "critical", "value"],
+        "required": ["name", "value"],
     }
 
 

django_x509/tests/test_admin.py

@@ -11,6 +11,8 @@
 from openwisp_utils.tests import AdminActionPermTestMixin
 from swapper import load_model
 
+from django_x509.base.admin import BaseAdmin
+
 from . import MessagingRequest
 
 User = get_user_model()
@@ -252,6 +254,19 @@ def test_revoke_action(self):
         self.assertEqual(len(m), 1)
         self.assertEqual(str(m[0]), "1 certificate was revoked.")
 
+    def test_revoke_action_multiple(self):
+        req = MessagingRequest()
+        req.user = MockSuperUser()
+        cert = Cert.objects.create(name="test_cert_2", ca=self.ca)
+        ma = self.cert_admin(Cert, self.site)
+        ma.revoke_action(req, [self.cert, cert])
+        message = req.get_message_strings()
+        self.assertEqual(message, ["2 certificates were revoked."])
+
+    def test_base_admin_default_extensions_schema(self):
+        ma = BaseAdmin(Ca, self.site)
+        self.assertEqual(ma.get_extensions_schema(), [])
+
     def test_revoke_action_perms(self):
         user = User.objects.create(
             username="tester", email="tester@openwisp.org", is_staff=True

django_x509/tests/test_extensions.py

@@ -1,18 +1,24 @@
 from copy import deepcopy
 
 from cryptography import x509
-from django.core.exceptions import ValidationError
+from django.core.exceptions import ImproperlyConfigured, ValidationError
 from django.test import TestCase
+from swapper import load_model
 
+from django_x509 import settings as app_settings
 from django_x509.schemas import (
     DEFAULT_CA_EXTENSIONS_SCHEMA,
     DEFAULT_CERT_EXTENSIONS_SCHEMA,
 )
 
 from . import TestX509Mixin
 
+Cert = load_model("django_x509", "Cert")
+
 
 class ExtensionsSchemaTests(TestX509Mixin, TestCase):
+    ns_comment_oid = x509.ObjectIdentifier("2.16.840.1.113730.1.13")
+
     def test_default_ca_schema_rejects_cert_only_extension(self):
         with self.assertRaises(ValidationError) as context:
             self._create_ca(
@@ -73,6 +79,29 @@ def test_legacy_string_values_are_normalized(self):
         self.assertEqual(cert.extensions[0]["value"], ["client", "server"])
         self.assertEqual(cert.extensions[1]["value"], ["clientAuth", "serverAuth"])
 
+    def test_missing_critical_defaults_to_false(self):
+        cert = self._create_cert(
+            extensions=[{"name": "nsComment", "value": "comment without critical"}]
+        )
+        ns_comment = cert.x509.extensions.get_extension_for_oid(self.ns_comment_oid)
+        self.assertFalse(cert.extensions[0].get("critical", False))
+        self.assertFalse(ns_comment.critical)
+
+    def test_nscomment_rejects_values_over_255_utf8_bytes(self):
+        with self.assertRaises(ValidationError) as context:
+            self._create_cert(extensions=[{"name": "nsComment", "value": "é" * 128}])
+        self.assertIn("extensions", context.exception.message_dict)
+        self.assertIn(
+            "nsComment value exceeds maximum length of 255 bytes",
+            str(context.exception.message_dict["extensions"][0]),
+        )
+
+    def test_nscomment_uses_long_form_der_length_for_128_bytes(self):
+        cert = self._create_cert(extensions=[{"name": "nsComment", "value": "a" * 128}])
+        ns_comment = cert.x509.extensions.get_extension_for_oid(self.ns_comment_oid)
+        self.assertEqual(ns_comment.value.value[:3], b"\x16\x81\x80")
+        self.assertEqual(ns_comment.value.value[3:], b"a" * 128)
+
     def test_ca_schema_setting_override(self):
         schema = deepcopy(DEFAULT_CA_EXTENSIONS_SCHEMA)
         schema["items"]["oneOf"][0]["properties"]["value"]["minLength"] = 20
@@ -140,3 +169,75 @@ def test_schema_override_still_rejects_incompatible_nscomment_shape(self):
             "nsComment extension requires a string value.",
             str(context.exception.message_dict["extensions"][0]),
         )
+
+    def test_invalid_schema_override_raises_improperly_configured(self):
+        with self.settings(DJANGO_X509_CA_EXTENSIONS_SCHEMA={"type": "madeup"}):
+            with self.assertRaises(ImproperlyConfigured):
+                app_settings.get_ca_extensions_schema()
+
+    def test_normalize_extensions_none_becomes_empty_list(self):
+        cert = Cert()
+        cert.extensions = None
+        cert._normalize_extensions(DEFAULT_CERT_EXTENSIONS_SCHEMA)
+        self.assertEqual(cert.extensions, [])
+
+    def test_normalize_extensions_keeps_non_dict_entries(self):
+        cert = Cert()
+        cert.extensions = ["invalid-entry"]
+        cert._normalize_extensions(DEFAULT_CERT_EXTENSIONS_SCHEMA)
+        self.assertEqual(cert.extensions, ["invalid-entry"])
+
+    def test_get_extension_values_returns_empty_list_for_unexpected_types(self):
+        self.assertEqual(Cert()._get_extension_values({"unexpected": "mapping"}), [])
+
+    def test_raise_extensions_validation_error_without_path(self):
+        with self.assertRaises(ValidationError) as context:
+            Cert()._raise_extensions_validation_error("", "bad data")
+        self.assertEqual(
+            str(context.exception.message_dict["extensions"][0]),
+            "Extensions data: bad data",
+        )
+
+    def test_validate_supported_extensions_rejects_non_boolean_critical(self):
+        cert = Cert()
+        cert.extensions = [{"name": "nsComment", "critical": "yes", "value": "ok"}]
+        with self.assertRaises(ValidationError) as context:
+            cert._validate_supported_extensions()
+        self.assertIn("extensions", context.exception.message_dict)
+        self.assertIn(
+            "Critical flag must be a boolean value.",
+            str(context.exception.message_dict["extensions"][0]),
+        )
+
+    def test_validate_supported_extensions_rejects_empty_nscomment_value(self):
+        cert = Cert()
+        cert.extensions = [{"name": "nsComment", "critical": False, "value": ""}]
+        with self.assertRaises(ValidationError) as context:
+            cert._validate_supported_extensions()
+        self.assertIn("extensions", context.exception.message_dict)
+        self.assertIn(
+            "nsComment extension requires a value.",
+            str(context.exception.message_dict["extensions"][0]),
+        )
+
+    def test_validate_supported_extensions_rejects_empty_extended_key_usage(self):
+        cert = Cert()
+        cert.extensions = [{"name": "extendedKeyUsage", "critical": False, "value": []}]
+        with self.assertRaises(ValidationError) as context:
+            cert._validate_supported_extensions()
+        self.assertIn("extensions", context.exception.message_dict)
+        self.assertIn(
+            "extendedKeyUsage extension requires at least one valid value.",
+            str(context.exception.message_dict["extensions"][0]),
+        )
+
+    def test_validate_supported_extensions_rejects_empty_ns_cert_type(self):
+        cert = Cert()
+        cert.extensions = [{"name": "nsCertType", "critical": False, "value": []}]
+        with self.assertRaises(ValidationError) as context:
+            cert._validate_supported_extensions()
+        self.assertIn("extensions", context.exception.message_dict)
+        self.assertIn(
+            "nsCertType extension requires at least one valid type.",
+            str(context.exception.message_dict["extensions"][0]),
+        )