Skip to content

Commit

Permalink
add deprecation path for old openssl-style subject strings
Browse files Browse the repository at this point in the history
  • Loading branch information
mathiasertl committed Nov 18, 2023
1 parent 93818b6 commit 12a3a33
Show file tree
Hide file tree
Showing 15 changed files with 298 additions and 174 deletions.
15 changes: 8 additions & 7 deletions ca/django_ca/management/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
from django_ca.constants import EXTENSION_DEFAULT_CRITICAL, KEY_USAGE_NAMES, ReasonFlags
from django_ca.models import Certificate, CertificateAuthority
from django_ca.typehints import AllowedHashTypes, AlternativeNameExtensionType
from django_ca.utils import is_power2, parse_encoding, parse_general_name, x509_name
from django_ca.utils import is_power2, parse_encoding, parse_general_name

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


class NameAction(SingleValueAction[str, x509.Name]):
class NameAction(SingleValueAction[str, str]):
"""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.
>>> parser = argparse.ArgumentParser()
>>> parser.add_argument('--name', action=NameAction) # doctest: +ELLIPSIS
NameAction(...)
>>> parser.parse_args(["--name", "/CN=example.com"])
Namespace(name=<Name(CN=example.com)>)
>>> parser.parse_args(["--name", "CN=example.com"])
Namespace(name=CN=example.com)
"""

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


Expand Down
38 changes: 36 additions & 2 deletions ca/django_ca/management/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,14 @@
from django_ca.management import actions, mixins
from django_ca.models import CertificateAuthority, X509CertMixin
from django_ca.profiles import Profile
from django_ca.typehints import ActionsContainer, AllowedHashTypes, ArgumentGroup, ExtensionMapping
from django_ca.utils import add_colons, name_for_display
from django_ca.typehints import (
ActionsContainer,
AllowedHashTypes,
ArgumentGroup,
ExtensionMapping,
SubjectFormats,
)
from django_ca.utils import add_colons, name_for_display, parse_name_rfc4514, x509_name


class BinaryOutputWrapper(OutputWrapper):
Expand Down Expand Up @@ -247,6 +253,31 @@ def _add_extension(self, extensions: ExtensionMapping, value: x509.ExtensionType
"""Add an extension to the passed extension dictionary."""
extensions[value.oid] = x509.Extension(oid=value.oid, critical=critical, value=value)

def add_subject_format_option(self, parser: ActionsContainer) -> None:
"""Add the --subject-format option."""
parser.add_argument(
"--subject-format",
choices=("openssl", "rfc4514"),
default="openssl",
help='Format for parsing the subject. Use "openssl" (the default before django-ca 2.0) to pass '
'slash-separated subjects (e.g. "/C=AT/O=Org/CN=example.com") and "rfc4514" to pass RFC 4514 '
'conforming strings (e.g. "C=AT,O=Org,CN=example.com"). The default is %(default)s, but will '
"switch to rfc4514 in django-ca 2.0. Support for openssl-style strings will be removed in "
"django-ca 2.2.",
)

def parse_x509_name(self, value: str, name_format: SubjectFormats) -> x509.Name:
"""Parse a `name` in the given `format`."""
if name_format == "openssl":
return x509_name(value)
if name_format == "rfc4514":
try:
return parse_name_rfc4514(value)
except ValueError as ex:
raise CommandError(str(ex)) from ex
# COVERAGE NOTE: Already covered by argparse
raise ValueError(f"{name_format}: Unknown subject format.") # pragma: no cover

def add_authority_information_access_group(
self,
parser: CommandParser,
Expand Down Expand Up @@ -463,6 +494,9 @@ def add_subject_group(self, parser: CommandParser) -> None:
"""Add argument for a subject."""
group = parser.add_argument_group("Certificate subject", self.subject_help)

# Add the --subject-format option
self.add_subject_format_option(group)

# NOTE: Don't set the default value here because it would mask the user not setting anything at all.
self.add_subject(
group,
Expand Down
24 changes: 18 additions & 6 deletions ca/django_ca/management/commands/init_ca.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,13 @@
from django_ca.management.mixins import CertificateAuthorityDetailMixin
from django_ca.models import CertificateAuthority
from django_ca.tasks import cache_crl, generate_ocsp_key, run_task
from django_ca.typehints import AllowedHashTypes, ArgumentGroup, ExtensionMapping, ParsableKeyType
from django_ca.typehints import (
AllowedHashTypes,
ArgumentGroup,
ExtensionMapping,
ParsableKeyType,
SubjectFormats,
)
from django_ca.utils import (
format_general_name,
parse_general_name,
Expand Down Expand Up @@ -148,6 +154,7 @@ def add_arguments(self, parser: CommandParser) -> None:
default=timedelta(365 * 10),
help="CA certificate expires in DAYS days (default: %(default)s).",
)
self.add_subject_format_option(general_group)
self.add_algorithm(
general_group, default_text=f"{default} for RSA/EC keys, {dsa_default} for DSA keys"
)
Expand Down Expand Up @@ -247,7 +254,7 @@ def add_arguments(self, parser: CommandParser) -> None:
def handle( # pylint: disable=too-many-arguments,too-many-locals,too-many-branches,too-many-statements
self,
name: str,
subject: x509.Name,
subject: str,
parent: Optional[CertificateAuthority],
expires: timedelta,
key_size: Optional[int],
Expand Down Expand Up @@ -299,6 +306,8 @@ def handle( # pylint: disable=too-many-arguments,too-many-locals,too-many-branc
# OCSP responder configuration
ocsp_responder_key_validity: Optional[int],
ocsp_response_validity: Optional[int],
# subject_format will be removed in django-ca 2.2
subject_format: SubjectFormats,
**options: Any,
) -> None:
if not os.path.exists(ca_settings.CA_DIR): # pragma: no cover
Expand Down Expand Up @@ -356,16 +365,19 @@ def handle( # pylint: disable=too-many-arguments,too-many-locals,too-many-branc
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, subject_format)

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

# See if we can work with the private key
if parent:
self.test_private_key(parent, parent_password)

subject = sort_name(subject)
parsed_subject = sort_name(parsed_subject)
extensions: ExtensionMapping = {
ExtensionOID.KEY_USAGE: x509.Extension(
oid=ExtensionOID.KEY_USAGE, critical=key_usage_critical, value=key_usage
Expand Down Expand Up @@ -464,7 +476,7 @@ def handle( # pylint: disable=too-many-arguments,too-many-locals,too-many-branc
try:
ca = CertificateAuthority.objects.init(
name=name,
subject=subject,
subject=parsed_subject,
expires=expires_datetime,
algorithm=algorithm,
parent=parent,
Expand Down
16 changes: 10 additions & 6 deletions ca/django_ca/management/commands/resign_cert.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
from django_ca.management.base import BaseSignCertCommand
from django_ca.models import Certificate, CertificateAuthority, Watcher
from django_ca.profiles import Profile, profiles
from django_ca.typehints import AllowedHashTypes, ExtensionMapping
from django_ca.typehints import AllowedHashTypes, ExtensionMapping, SubjectFormats


class Command(BaseSignCertCommand): # pylint: disable=missing-class-docstring
Expand Down Expand Up @@ -61,11 +61,11 @@ def get_profile(self, profile: Optional[str], cert: Certificate) -> Profile:
f'Profile "{cert.profile}" for original certificate is no longer defined, please set one via the command line.' # NOQA: E501
)

def handle( # pylint: disable=too-many-arguments,too-many-locals
def handle( # pylint: disable=too-many-arguments,too-many-locals,too-many-branches
self,
cert: Certificate,
ca: Optional[CertificateAuthority],
subject: Optional[x509.Name],
subject: Optional[str],
expires: Optional[timedelta],
watch: List[str],
password: Optional[bytes],
Expand Down Expand Up @@ -96,6 +96,8 @@ def handle( # pylint: disable=too-many-arguments,too-many-locals
# TLSFeature extension
tls_feature: Optional[x509.TLSFeature],
tls_feature_critical: bool,
# subject_format will be removed in django-ca 2.2
subject_format: SubjectFormats,
**options: Any,
) -> None:
if ca is None:
Expand All @@ -114,7 +116,9 @@ def handle( # pylint: disable=too-many-arguments,too-many-locals
watchers = list(cert.watchers.all())

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

# Process any extensions given via the command-line
extensions: ExtensionMapping = {}
Expand Down Expand Up @@ -169,7 +173,7 @@ def handle( # pylint: disable=too-many-arguments,too-many-locals
# 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 = subject.get_attributes_for_oid(NameOID.COMMON_NAME)
common_names = parsed_subject.get_attributes_for_oid(NameOID.COMMON_NAME)
if not common_names and ExtensionOID.SUBJECT_ALTERNATIVE_NAME not in extensions:
raise CommandError(
"Must give at least a Common Name in --subject or one or more "
Expand All @@ -182,7 +186,7 @@ def handle( # pylint: disable=too-many-arguments,too-many-locals
csr=cert.csr.loaded,
profile=profile_obj,
expires=expires,
subject=subject,
subject=parsed_subject,
algorithm=algorithm,
extensions=extensions.values(),
password=password,
Expand Down
17 changes: 12 additions & 5 deletions ca/django_ca/management/commands/sign_cert.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
from django_ca.management.base import BaseSignCertCommand
from django_ca.models import Certificate, CertificateAuthority, Watcher
from django_ca.profiles import profiles
from django_ca.typehints import AllowedHashTypes, ExtensionMapping
from django_ca.typehints import AllowedHashTypes, ExtensionMapping, SubjectFormats


class Command(BaseSignCertCommand): # pylint: disable=missing-class-docstring
Expand Down Expand Up @@ -100,7 +100,7 @@ def add_arguments(self, parser: CommandParser) -> None:
def handle( # pylint: disable=too-many-arguments,too-many-locals,too-many-branches,too-many-statements
self,
ca: CertificateAuthority,
subject: Optional[x509.Name],
subject: Optional[str],
expires: Optional[timedelta],
watch: List[str],
password: Optional[bytes],
Expand Down Expand Up @@ -135,6 +135,8 @@ def handle( # pylint: disable=too-many-arguments,too-many-locals,too-many-branc
# TLSFeature extension
tls_feature: Optional[x509.TLSFeature],
tls_feature_critical: bool,
# subject_format will be removed in django-ca 2.2
subject_format: SubjectFormats,
**options: Any,
) -> None:
# Validate parameters early so that we can return better feedback to the user.
Expand Down Expand Up @@ -187,9 +189,14 @@ def handle( # pylint: disable=too-many-arguments,too-many-locals,too-many-branc
if tls_feature is not None:
self._add_extension(extensions, tls_feature, tls_feature_critical)

cname = None
# Parse the subject
parsed_subject = None
if subject is not None:
cname = subject.get_attributes_for_oid(NameOID.COMMON_NAME)
parsed_subject = self.parse_x509_name(subject, subject_format)

cname = None
if parsed_subject is not None:
cname = parsed_subject.get_attributes_for_oid(NameOID.COMMON_NAME)
if not cname and ExtensionOID.SUBJECT_ALTERNATIVE_NAME not in extensions:
raise CommandError(
"Must give at least a Common Name in --subject or one or more "
Expand Down Expand Up @@ -223,7 +230,7 @@ def handle( # pylint: disable=too-many-arguments,too-many-locals,too-many-branc
expires=expires,
extensions=extensions.values(),
password=password,
subject=subject,
subject=parsed_subject,
algorithm=algorithm,
)
except Exception as ex:
Expand Down
16 changes: 11 additions & 5 deletions ca/django_ca/profiles.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import warnings
from datetime import datetime, timedelta
from threading import local
from typing import Any, Dict, Iterable, Iterator, List, Optional, Tuple, Union
from typing import Any, Dict, Iterable, Iterator, List, Optional, Union

from cryptography import x509
from cryptography.x509.oid import AuthorityInformationAccessOID, ExtensionOID, NameOID
Expand Down Expand Up @@ -101,12 +101,18 @@ def __init__(
extensions = {}

if subject is None:
self.subject: Optional[Union[typing.Literal[False], x509.Name]] = ca_settings.CA_DEFAULT_SUBJECT
self.subject: Optional[x509.Name] = ca_settings.CA_DEFAULT_SUBJECT
elif subject is False:
self.subject = False
self.subject = None
elif isinstance(subject, x509.Name):
self.subject = subject
else:
warnings.warn(
f"{subject}: Support for passing a value of type {subject.__class__} is deprecated and will "
"be removed in django-ca 1.28.0.",
RemovedInDjangoCA128Warning,
stacklevel=2,
)
self.subject = x509_name(subject)

if algorithm is not None:
Expand Down Expand Up @@ -291,7 +297,7 @@ def create_cert( # pylint: disable=too-many-arguments
add_issuer_alternative_name=add_issuer_alternative_name,
)

if self.subject is not False and self.subject is not None:
if self.subject is not None:
if subject is not None:
subject = merge_x509_names(self.subject, subject)
else:
Expand Down Expand Up @@ -385,7 +391,7 @@ def serialize(self) -> SerializedProfile:
extensions[EXTENSION_KEYS[key]] = serialize_extension(extension)

serialized_name = None
if self.subject is not None and self.subject is not False:
if self.subject is not None:
serialized_name = serialize_name(self.subject)

data: SerializedProfile = {
Expand Down
10 changes: 5 additions & 5 deletions ca/django_ca/tests/commands/test_init_ca.py
Original file line number Diff line number Diff line change
Expand Up @@ -772,15 +772,15 @@ def test_empty_subject_fields(self) -> None:
def test_no_cn(self) -> None:
"""Test creating a CA with no CommonName."""
name = "test_no_cn"
subject = "/ST=/L=/O=/OU=smth"
error = r"^Subject must contain a common name \(/CN=...\)\.$"
subject = "C=AT,ST=Vienna,L=Vienna,O=Org,OU=OrgUnit"
error = r"^Subject must contain a common name \(CN=\.\.\.\)\.$"
with self.assertCreateCASignals(False, False), self.assertCommandError(error):
self.cmd("init_ca", name, subject)
self.cmd("init_ca", name, subject, subject_format="rfc4514")

error = r"CommonName must not be an empty value"
subject = "/ST=/L=/O=/OU=smth/CN="
subject = "C=AT,ST=Vienna,L=Vienna,O=Org,OU=OrgUnit,CN="
with self.assertCreateCASignals(False, False), self.assertCommandError(error):
self.cmd("init_ca", name, subject)
self.cmd("init_ca", name, subject, subject_format="rfc4514")

@override_tmpcadir(CA_MIN_KEY_SIZE=1024)
def test_parent(self) -> None:
Expand Down
5 changes: 3 additions & 2 deletions ca/django_ca/tests/commands/test_resign_cert.py
Original file line number Diff line number Diff line change
Expand Up @@ -540,8 +540,9 @@ def test_overwrite(self) -> None:
"--tls-feature",
"status_request_v2",
"--tls-feature-critical",
"--subject-format=rfc4514",
"--subject",
f"/CN={cname}",
f"CN={cname}",
"--watch",
watcher,
"--subject-alternative-name",
Expand Down Expand Up @@ -640,7 +641,7 @@ def test_no_cn(self) -> None:
r"--subject-alternative-name/--name arguments\.$"
)
with self.assertCreateCertSignals(False, False), self.assertCommandError(msg):
self.cmd("resign_cert", cert, subject=subject)
self.cmd("resign_cert", cert, subject=subject.rfc4514_string())

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

0 comments on commit 12a3a33

Please sign in to comment.