Skip to content

Commit

Permalink
remove the cn_in_san options
Browse files Browse the repository at this point in the history
  • Loading branch information
mathiasertl committed Jan 5, 2024
1 parent 55edf10 commit 01f7110
Show file tree
Hide file tree
Showing 24 changed files with 99 additions and 761 deletions.
38 changes: 5 additions & 33 deletions ca/django_ca/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
from django_object_actions import DjangoObjectActions

from django_ca import ca_settings, constants
from django_ca.constants import EXTENSION_DEFAULT_CRITICAL, EXTENSION_KEY_OIDS, EXTENSION_KEYS, ReasonFlags
from django_ca.constants import EXTENSION_KEY_OIDS, EXTENSION_KEYS, ReasonFlags
from django_ca.extensions import CERTIFICATE_EXTENSIONS, get_extension_name
from django_ca.extensions.utils import certificate_policies_is_simple, extension_as_admin_html
from django_ca.forms import (
Expand Down Expand Up @@ -123,6 +123,8 @@
]
QuerySetTypeVar = typing.TypeVar("QuerySetTypeVar", bound=QuerySet)

EXTENSION_FIELDS = tuple((key for key in CERTIFICATE_EXTENSIONS if key != "subject_alternative_name"))


@admin.register(Watcher)
class WatcherAdmin(WatcherAdminBase):
Expand Down Expand Up @@ -626,16 +628,7 @@ class CertificateAdmin(DjangoObjectActions, CertificateMixin[Certificate], Certi
),
},
),
(
_("X.509 Extensions"),
{
"fields": CERTIFICATE_EXTENSIONS,
"classes": (
"collapse",
"x509-extensions",
),
},
),
(_("X.509 Extensions"), {"fields": EXTENSION_FIELDS, "classes": ("collapse", "x509-extensions")}),
)

# same as add_fieldsets but without the csr
Expand All @@ -654,10 +647,7 @@ class CertificateAdmin(DjangoObjectActions, CertificateMixin[Certificate], Certi
],
},
),
(
_("X.509 Extensions"),
{"fields": CERTIFICATE_EXTENSIONS},
),
(_("X.509 Extensions"), {"fields": EXTENSION_FIELDS}),
)
x509_fieldset_index = 1

Expand Down Expand Up @@ -726,17 +716,6 @@ def get_changeform_initial_data(self, request: HttpRequest) -> Dict[str, Any]:
# resign the cert, so we add initial data from the original cert

resign_obj = request._resign_obj # pylint: disable=protected-access
san = resign_obj.x509_extensions.get(ExtensionOID.SUBJECT_ALTERNATIVE_NAME)
if san is None:
san_value = []
san_critical = EXTENSION_DEFAULT_CRITICAL[ExtensionOID.SUBJECT_ALTERNATIVE_NAME]
else:
san_value = list(san.value)
san_critical = san.critical

# Since Django 4.1, tuples are no longer passed to a MultiWidgets decompress() method. We must
# thus pass a three-tuple as initial value, each corresponding to the value of one of the widgets.
subject_alternative_name = (san_value, False, san_critical)

if resign_obj.algorithm is not None:
hash_algorithm_name = constants.HASH_ALGORITHM_NAMES[type(resign_obj.algorithm)]
Expand All @@ -750,7 +729,6 @@ def get_changeform_initial_data(self, request: HttpRequest) -> Dict[str, Any]:
"ca": resign_obj.ca,
"profile": profile,
"subject": resign_obj.subject,
"subject_alternative_name": subject_alternative_name,
"watchers": resign_obj.watchers.all(),
}

Expand Down Expand Up @@ -1035,9 +1013,6 @@ def save_model( # type: ignore[override]

# Set Subject Alternative Name from form
extensions: Dict[x509.ObjectIdentifier, x509.Extension[x509.ExtensionType]] = {}
subject_alternative_name, cn_in_san = data["subject_alternative_name"]
if subject_alternative_name:
extensions[ExtensionOID.SUBJECT_ALTERNATIVE_NAME] = subject_alternative_name

# Update extensions handled through the form
for key in CERTIFICATE_EXTENSIONS:
Expand Down Expand Up @@ -1067,8 +1042,6 @@ def save_model( # type: ignore[override]

if EXTENSION_KEYS[oid] in CERTIFICATE_EXTENSIONS: # already handled in form
continue
if oid == ExtensionOID.SUBJECT_ALTERNATIVE_NAME: # already handled above
continue

# Add any extension from the profile currently not changeable in the web interface
extensions[oid] = ext # pragma: no cover # all extensions should be handled above!
Expand All @@ -1083,7 +1056,6 @@ def save_model( # type: ignore[override]
algorithm=data["algorithm"],
expires=expires,
extensions=extensions.values(),
cn_in_san=cn_in_san,
password=data["password"],
)
)
Expand Down
3 changes: 0 additions & 3 deletions ca/django_ca/ca_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,6 @@ def _get_hash_algorithm(setting: str, default: "HashAlgorithms") -> "AllowedHash
"description": _(
"A certificate for an enduser, allows client authentication, code and email signing."
),
"cn_in_san": False,
"extensions": {
"key_usage": {
"critical": True,
Expand All @@ -206,7 +205,6 @@ def _get_hash_algorithm(setting: str, default: "HashAlgorithms") -> "AllowedHash
},
"ocsp": {
"description": _("A certificate for an OCSP responder."),
"cn_in_san": False, # CAs frequently use human-readable name as CN
"add_ocsp_url": False,
"autogenerated": True,
"subject": False,
Expand Down Expand Up @@ -290,7 +288,6 @@ def _get_hash_algorithm(setting: str, default: "HashAlgorithms") -> "AllowedHash

for profile_name, profile in CA_PROFILES.items():
profile.setdefault("subject", CA_DEFAULT_SUBJECT)
profile.setdefault("cn_in_san", True)

if subject := profile.get("subject"):
profile["subject"] = _normalize_x509_name(subject, f"subject in {profile_name} profile.")
Expand Down
2 changes: 1 addition & 1 deletion ca/django_ca/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ class ExtendedKeyUsageOID(_ExtendedKeyUsageOID):
ExtensionOID.SIGNED_CERTIFICATE_TIMESTAMPS: _( # defined in RFC 6962
"may or may not be critical (recommended: non-critical)"
),
ExtensionOID.SUBJECT_ALTERNATIVE_NAME: _("SHOULD mark this extension as non-critical"),
ExtensionOID.SUBJECT_ALTERNATIVE_NAME: _("SHOULD be non-critical"),
ExtensionOID.SUBJECT_INFORMATION_ACCESS: _("MUST be non-critical"),
ExtensionOID.SUBJECT_KEY_IDENTIFIER: _("MUST be non-critical"),
ExtensionOID.TLS_FEATURE: _("SHOULD NOT be critical"), # defined in RFC 7633
Expand Down
1 change: 1 addition & 0 deletions ca/django_ca/extensions/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
"issuer_alternative_name",
"key_usage",
"ocsp_no_check",
"subject_alternative_name",
"tls_feature",
]
)
Expand Down
52 changes: 16 additions & 36 deletions ca/django_ca/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,15 @@
from django.utils.safestring import mark_safe
from django.utils.translation import gettext_lazy as _

from django_ca import ca_settings, constants, utils, widgets
from django_ca import constants, utils, widgets
from django_ca.constants import (
EXTENDED_KEY_USAGE_HUMAN_READABLE_NAMES,
EXTENDED_KEY_USAGE_NAMES,
KEY_USAGE_NAMES,
REVOCATION_REASONS,
)
from django_ca.extensions import get_extension_name
from django_ca.typehints import CRLExtensionTypeTypeVar, ExtensionTypeTypeVar
from django_ca.typehints import AlternativeNameTypeVar, CRLExtensionTypeTypeVar, ExtensionTypeTypeVar
from django_ca.utils import parse_general_name
from django_ca.widgets import KeyValueWidget, SubjectWidget

Expand Down Expand Up @@ -290,6 +290,18 @@ def get_value(self, *value: Any) -> Optional[ExtensionTypeTypeVar]:
"""


class AlternativeNameField(ExtensionField[AlternativeNameTypeVar]):
"""Form field for a :py:class:`~cg:cryptography.x509.IssuerAlternativeName` extension."""

extension_type: Type[AlternativeNameTypeVar]
fields = (GeneralNamesField(required=False),)

def get_value(self, value: List[x509.GeneralName]) -> Optional[AlternativeNameTypeVar]:
if not value:
return None
return self.extension_type(general_names=value)


class MultipleChoiceExtensionField(ExtensionField[ExtensionTypeTypeVar]):
"""Base class for extensions that are basically a multiple choice field (plus critical)."""

Expand Down Expand Up @@ -447,18 +459,12 @@ class FreshestCRLField(DistributionPointField[x509.FreshestCRL]):
widget = widgets.FreshestCRLWidget


class IssuerAlternativeNameField(ExtensionField[x509.IssuerAlternativeName]):
class IssuerAlternativeNameField(AlternativeNameField[x509.IssuerAlternativeName]):
"""Form field for a :py:class:`~cg:cryptography.x509.IssuerAlternativeName` extension."""

extension_type = x509.IssuerAlternativeName
fields = (GeneralNamesField(required=False),)
widget = widgets.IssuerAlternativeNameWidget

def get_value(self, value: List[x509.GeneralName]) -> Optional[x509.IssuerAlternativeName]:
if not value:
return None
return x509.IssuerAlternativeName(general_names=value)


class KeyUsageField(MultipleChoiceExtensionField[x509.KeyUsage]):
"""Form field for a :py:class:`~cg:cryptography.x509.KeyUsage` extension."""
Expand Down Expand Up @@ -486,38 +492,12 @@ def get_value(self, value: bool) -> Optional[x509.OCSPNoCheck]:
return None


class SubjectAlternativeNameField(ExtensionField[x509.SubjectAlternativeName]):
class SubjectAlternativeNameField(AlternativeNameField[x509.SubjectAlternativeName]):
"""Form field for a :py:class:`~cg:cryptography.x509.SubjectAlternativeName` extension."""

extension_type = x509.SubjectAlternativeName
fields = (
GeneralNamesField(required=False),
forms.BooleanField(required=False),
)
widget = widgets.SubjectAlternativeNameWidget

def compress( # type: ignore[override] # this is a special case
self, data_list: List[Any]
) -> Tuple[Optional[x509.Extension[ExtensionTypeTypeVar]], bool]:
default_cn_in_san = ca_settings.CA_PROFILES[ca_settings.CA_DEFAULT_PROFILE]["cn_in_san"]
if not data_list: # pragma: no cover
return None, default_cn_in_san

*value, critical = data_list
ext_value, cn_in_san = self.get_value(*value)
if ext_value is None:
return None, cn_in_san
ext = x509.Extension(critical=critical, oid=self.extension_type.oid, value=ext_value)
# TYPE NOTE: mypy complains about the non-generic type being returned
return ext, cn_in_san # type: ignore[return-value]

def get_value( # type: ignore[override]
self, names: List[x509.GeneralName], cn_in_san: bool
) -> Tuple[Optional[x509.SubjectAlternativeName], bool]:
if not names:
return None, cn_in_san
return x509.SubjectAlternativeName(general_names=names), cn_in_san


class TLSFeatureField(MultipleChoiceExtensionField[x509.TLSFeature]):
"""Form field for a :py:class:`~cg:cryptography.x509.TLSFeature` extension."""
Expand Down
20 changes: 2 additions & 18 deletions ca/django_ca/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
from django_ca import ca_settings, constants, fields
from django_ca.models import Certificate, CertificateAuthority, X509CertMixin
from django_ca.querysets import CertificateAuthorityQuerySet
from django_ca.utils import parse_general_name
from django_ca.widgets import ProfileWidget

if typing.TYPE_CHECKING:
Expand Down Expand Up @@ -119,7 +118,7 @@ class CreateCertificateBaseForm(CertificateModelForm):
subject = fields.SubjectField(label=_("Subject"), required=False)
subject_alternative_name = fields.SubjectAlternativeNameField(
required=False,
help_text=_("""Coma-separated list of alternative names for the certificate."""),
help_text=_("""Alternative names for the certificate (one per line)."""),
)
profile = forms.ChoiceField(
required=False,
Expand Down Expand Up @@ -214,7 +213,7 @@ def clean(self) -> Optional[Dict[str, Any]]:
password = typing.cast(Optional[str], data.get("password"))
subject = typing.cast(Optional[x509.Name], data.get("subject"))
algorithm = typing.cast(Optional[hashes.HashAlgorithm], data.get("algorithm"))
subject_alternative_name, include_common_name = data.get("subject_alternative_name", (None, False))
subject_alternative_name = data.get("subject_alternative_name", (None, False))

subject_alternative_name = typing.cast(
Optional[x509.Extension[x509.SubjectAlternativeName]], subject_alternative_name
Expand Down Expand Up @@ -248,21 +247,6 @@ def clean(self) -> Optional[Dict[str, Any]]:
if subject is not None:
common_names = subject.get_attributes_for_oid(NameOID.COMMON_NAME)

# If the user decided to include any Common Names in the Subject Alternative Name extension, we check
# that they can be parsed as general name.
if include_common_name:
for common_name in common_names:
try:
parse_general_name(common_name.value) # type: ignore[arg-type]
except ValueError:
self.add_error(
"subject_alternative_name",
_(
"The CommonName cannot be parsed as general name. Either change the "
"CommonName or do not include it."
),
)

# Make sure that we have at least a Common Name *or* a Subject Alternative Name extension.
if subject is not None and not common_names and not subject_alternative_name:
self.add_error(
Expand Down
1 change: 0 additions & 1 deletion ca/django_ca/management/commands/resign_cert.py
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,6 @@ def handle( # pylint: disable=too-many-locals # noqa: PLR0912,PLR0913
algorithm=algorithm,
extensions=extensions.values(),
password=password,
cn_in_san=False, # we already copy the SAN/CN from the original cert
)
except Exception as ex:
raise CommandError(ex) from ex
Expand Down
34 changes: 0 additions & 34 deletions ca/django_ca/management/commands/sign_cert.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,40 +47,8 @@ class Command(BaseSignCertCommand):
--profile. The --subject option allows you to name a CommonName (which is not usually
in the defaults) and override any default values."""

def add_cn_in_san(self, parser: CommandParser) -> None:
"""Add argument group for the CommonName-in-SubjectAlternativeName options."""
if ca_settings.CA_PROFILES[ca_settings.CA_DEFAULT_PROFILE]["cn_in_san"]:
cn_in_san_default = " (default)"
cn_not_in_san_default = ""
else:
cn_in_san_default = ""
cn_not_in_san_default = " (default)"

group = parser.add_argument_group(
"CommonName in subjectAltName",
"""Whether or not to automatically include the CommonName (given in --subject) in the
list of subjectAltNames (given by --alt).""",
)
group = group.add_mutually_exclusive_group()

group.add_argument(
"--cn-not-in-san",
default=None,
action="store_false",
dest="cn_in_san",
help=f"Do not add the CommonName as subjectAlternativeName{cn_not_in_san_default}.",
)
group.add_argument(
"--cn-in-san",
default=None,
action="store_true",
dest="cn_in_san",
help=f"Add the CommonName as subjectAlternativeName{cn_in_san_default}.",
)

def add_arguments(self, parser: CommandParser) -> None:
general_group = self.add_base_args(parser)
self.add_cn_in_san(parser)

general_group.add_argument(
"--csr",
Expand All @@ -106,7 +74,6 @@ def handle( # pylint: disable=too-many-locals # noqa: PLR0912,PLR0913,PLR0915
expires: Optional[timedelta],
watch: List[str],
password: Optional[bytes],
cn_in_san: bool,
csr_path: str,
bundle: bool,
profile: Optional[str],
Expand Down Expand Up @@ -228,7 +195,6 @@ def handle( # pylint: disable=too-many-locals # noqa: PLR0912,PLR0913,PLR0915
ca,
csr,
profile=profile_obj,
cn_in_san=cn_in_san,
expires=expires,
extensions=extensions.values(),
password=password,
Expand Down
6 changes: 0 additions & 6 deletions ca/django_ca/managers.py
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,6 @@ def create_cert( # noqa: PLR0913
expires: Expires = None,
algorithm: Optional[AllowedHashTypes] = None,
extensions: Optional[Iterable[x509.Extension[x509.ExtensionType]]] = None,
cn_in_san: Optional[bool] = None,
add_crl_url: Optional[bool] = None,
add_ocsp_url: Optional[bool] = None,
add_issuer_url: Optional[bool] = None,
Expand Down Expand Up @@ -606,10 +605,6 @@ def create_cert( # noqa: PLR0913
Passed to :py:func:`Profiles.create_cert() <django_ca.profiles.Profile.create_cert>`.
extensions : list or of :py:class:`~cg:cryptography.x509.Extension`
Passed to :py:func:`Profiles.create_cert() <django_ca.profiles.Profile.create_cert>`.
cn_in_san : bool, optional
Passed to :py:func:`Profiles.create_cert() <django_ca.profiles.Profile.create_cert>`.
cn_in_san : bool, optional
Passed to :py:func:`Profiles.create_cert() <django_ca.profiles.Profile.create_cert>`.
add_crl_url : bool, optional
Passed to :py:func:`Profiles.create_cert() <django_ca.profiles.Profile.create_cert>`.
add_ocsp_url : bool, optional
Expand All @@ -634,7 +629,6 @@ def create_cert( # noqa: PLR0913
expires=expires,
algorithm=algorithm,
extensions=extensions,
cn_in_san=cn_in_san,
add_crl_url=add_crl_url,
add_ocsp_url=add_ocsp_url,
add_issuer_url=add_issuer_url,
Expand Down
Loading

0 comments on commit 01f7110

Please sign in to comment.