Skip to content

Commit

Permalink
parse subject directly and handle in argparse
Browse files Browse the repository at this point in the history
  • Loading branch information
mathiasertl committed Dec 29, 2024
1 parent 01c6c48 commit 83079ed
Show file tree
Hide file tree
Showing 10 changed files with 75 additions and 95 deletions.
3 changes: 2 additions & 1 deletion ca/django_ca/api/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ async def authenticate( # pylint: disable=invalid-overridden-method

# NOTE: ahas_perm() is introduced in Django 5.2.
if hasattr(user, "ahasperm"): # pragma: only django>5.1
has_perm = await user.ahas_perm(self.permission)
# TYPEHINT NOTE: mypy will complain on currently released django==5.1.
has_perm = await user.ahas_perm(self.permission) # type: ignore[attr-defined]
else: # pragma: only django<5.2
has_perm = await sync_to_async(user.has_perm)(self.permission)

Expand Down
11 changes: 5 additions & 6 deletions ca/django_ca/management/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
from django_ca.models import Certificate, CertificateAuthority
from django_ca.pydantic.validators import is_power_two_validator
from django_ca.typehints import AllowedHashTypes, AlternativeNameExtensionType, EllipticCurves
from django_ca.utils import parse_encoding, parse_general_name
from django_ca.utils import parse_encoding, parse_general_name, parse_name_rfc4514

ActionType = typing.TypeVar("ActionType") # pylint: disable=invalid-name
ParseType = typing.TypeVar("ParseType") # pylint: disable=invalid-name
Expand Down Expand Up @@ -479,7 +479,7 @@ def parse_value(self, value: str) -> ReasonFlags:
return ReasonFlags[value]


class NameAction(SingleValueAction[str, str]):
class NameAction(SingleValueAction[str, x509.Name]):
"""Action to parse a string into a :py:class:`cg:~cryptography.x509.Name`.
Note that this action does *not* take care of sorting the subject in any way.
Expand All @@ -491,11 +491,10 @@ class NameAction(SingleValueAction[str, str]):
Namespace(name=CN=example.com)
"""

def parse_value(self, value: str) -> str:
# TODO: In django-ca 2.0, parse subject here directly using parse_name_rfc4514().
def parse_value(self, value: str) -> x509.Name:
try:
return value
except ValueError as ex: # pragma: no cover # pragma: only django-ca<2.0
return parse_name_rfc4514(value)
except ValueError as ex:
raise argparse.ArgumentError(self, str(ex)) from ex


Expand Down
9 changes: 1 addition & 8 deletions ca/django_ca/management/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
ConfigurableExtension,
ConfigurableExtensionType,
)
from django_ca.utils import add_colons, name_for_display, parse_name_rfc4514
from django_ca.utils import add_colons, name_for_display

if typing.TYPE_CHECKING:
from django_stubs_ext import StrOrPromise
Expand Down Expand Up @@ -412,13 +412,6 @@ def add_extension(
x509.Extension(oid=value.oid, critical=critical, value=value) # type: ignore[arg-type]
)

def parse_x509_name(self, value: str) -> x509.Name:
"""Parse a `name` in the given `format`."""
try:
return parse_name_rfc4514(value)
except ValueError as ex:
raise CommandError(ex) from ex

def add_extended_key_usage_group(self, parser: CommandParser) -> None:
"""Add argument group for the Extended Key Usage extension."""
ext_name = constants.EXTENSION_NAMES[ExtensionOID.EXTENDED_KEY_USAGE]
Expand Down
9 changes: 3 additions & 6 deletions ca/django_ca/management/commands/init_ca.py
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ def add_extension(
def handle( # pylint: disable=too-many-locals # noqa: PLR0912,PLR0913,PLR0915
self,
name: str,
subject: str,
subject: x509.Name,
parent: Optional[CertificateAuthority],
expires: timedelta,
# private key storage options
Expand Down Expand Up @@ -418,11 +418,8 @@ def handle( # pylint: disable=too-many-locals # noqa: PLR0912,PLR0913,PLR0915
responder_value = format_general_name(ca_issuer.access_location)
raise CommandError(f"{responder_value}: CA issuer cannot be added to root CAs.")

# Parse the subject
parsed_subject = self.parse_x509_name(subject)

# We require a valid common name
common_name = next((attr.value for attr in parsed_subject if attr.oid == NameOID.COMMON_NAME), False)
common_name = next((attr.value for attr in subject if attr.oid == NameOID.COMMON_NAME), False)
if not common_name:
raise CommandError("Subject must contain a common name (CN=...).")

Expand Down Expand Up @@ -545,7 +542,7 @@ def handle( # pylint: disable=too-many-locals # noqa: PLR0912,PLR0913,PLR0915
name=name,
key_backend=key_backend,
key_backend_options=key_backend_options,
subject=parsed_subject,
subject=subject,
not_after=not_after_datetime,
algorithm=algorithm,
parent=parent,
Expand Down
10 changes: 4 additions & 6 deletions ca/django_ca/management/commands/resign_cert.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def handle( # pylint: disable=too-many-locals # noqa: PLR0912, PLR0913
self,
cert: Certificate,
ca: Optional[CertificateAuthority],
subject: Optional[str],
subject: Optional[x509.Name],
expires: Optional[timedelta],
watch: list[str],
profile: Optional[str],
Expand Down Expand Up @@ -117,9 +117,7 @@ def handle( # pylint: disable=too-many-locals # noqa: PLR0912, PLR0913
watchers = list(cert.watchers.all())

if subject is None:
parsed_subject = cert.subject
else:
parsed_subject = self.parse_x509_name(subject)
subject = cert.subject

# Process any extensions given via the command-line
extensions: list[ConfigurableExtension] = []
Expand Down Expand Up @@ -177,7 +175,7 @@ def handle( # pylint: disable=too-many-locals # noqa: PLR0912, PLR0913
# NOTE: This can only happen here in two edge cases:
# * Pass a subject without common name AND a certificate does *not* have a subject alternative name.
# * An imported certificate that has neither Common Name nor subject alternative name.
common_names = parsed_subject.get_attributes_for_oid(NameOID.COMMON_NAME)
common_names = subject.get_attributes_for_oid(NameOID.COMMON_NAME)
has_subject_alternative_name = next(
(ext for ext in extensions if ext.oid == ExtensionOID.SUBJECT_ALTERNATIVE_NAME), None
)
Expand All @@ -194,7 +192,7 @@ def handle( # pylint: disable=too-many-locals # noqa: PLR0912, PLR0913
csr=cert.csr.loaded,
profile=profile_obj,
not_after=expires,
subject=parsed_subject,
subject=subject,
algorithm=algorithm,
extensions=extensions,
)
Expand Down
13 changes: 4 additions & 9 deletions ca/django_ca/management/commands/sign_cert.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def add_arguments(self, parser: CommandParser) -> None:
def handle( # pylint: disable=too-many-locals # noqa: PLR0912, PLR0913
self,
ca: CertificateAuthority,
subject: Optional[str],
subject: Optional[x509.Name],
expires: Optional[timedelta],
watch: list[str],
csr_path: str,
Expand Down Expand Up @@ -150,14 +150,9 @@ def handle( # pylint: disable=too-many-locals # noqa: PLR0912, PLR0913
if tls_feature is not None:
self.add_extension(extensions, tls_feature, tls_feature_critical)

# Parse the subject
parsed_subject = None
if subject is not None:
parsed_subject = self.parse_x509_name(subject)

cname = None
if parsed_subject is not None:
cname = parsed_subject.get_attributes_for_oid(NameOID.COMMON_NAME)
if subject is not None:
cname = subject.get_attributes_for_oid(NameOID.COMMON_NAME)
if not cname and subject_alternative_name is None:
raise CommandError(
"Must give at least a Common Name in --subject or one or more "
Expand Down Expand Up @@ -190,7 +185,7 @@ def handle( # pylint: disable=too-many-locals # noqa: PLR0912, PLR0913
profile=profile_obj,
not_after=expires,
extensions=extensions,
subject=parsed_subject,
subject=subject,
algorithm=algorithm,
)
except Exception as ex:
Expand Down
2 changes: 1 addition & 1 deletion ca/django_ca/tests/commands/test_init_ca.py
Original file line number Diff line number Diff line change
Expand Up @@ -1534,7 +1534,7 @@ def test_invalid_common_name(value: str, msg: str) -> None:

def test_unparsable_subject(ca_name: str) -> None:
"""Test error when you pass an unparsable subject."""
with assert_command_error(r"^/CN=example\.com: Could not parse name as RFC 4514 string\.$"):
with assert_command_error(r"/CN=example\.com: Could not parse name as RFC 4514 string\.$"):
cmd("init_ca", ca_name, "/CN=example.com")


Expand Down
2 changes: 1 addition & 1 deletion ca/django_ca/tests/commands/test_resign_cert.py
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,7 @@ def test_no_cn(self) -> None:
r"--subject-alternative-name/--name arguments\.$"
)
with assert_create_cert_signals(False, False), assert_command_error(msg):
cmd("resign_cert", cert, subject=subject.rfc4514_string())
cmd("resign_cert", cert, subject=subject)

@override_tmpcadir()
def test_error(self) -> None:
Expand Down
Loading

0 comments on commit 83079ed

Please sign in to comment.