Skip to content

Commit

Permalink
replace expires with not_after in Certificate.sign()
Browse files Browse the repository at this point in the history
  • Loading branch information
mathiasertl committed Oct 5, 2024
1 parent 0f263af commit 5f23392
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 32 deletions.
13 changes: 6 additions & 7 deletions ca/django_ca/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -584,7 +584,7 @@ class CertificateAdmin(DjangoObjectActions, CertificateMixin[Certificate], Certi
change_actions = ("revoke_change", "resign")
add_form_template = "admin/django_ca/certificate/add_form.html"
change_form_template = "admin/django_ca/certificate/change_form.html"
list_display = ("primary_name", "profile", "serial_field", "status", "expires_date")
list_display = ("primary_name", "profile", "serial_field", "status", "not_after_date")
list_filter = ("profile", AutoGeneratedFilter, StatusListFilter, "ca")
readonly_fields = (
"not_after",
Expand Down Expand Up @@ -1017,12 +1017,12 @@ def status(self, obj: Certificate) -> "StrPromise":

status.short_description = _("Status") # type: ignore[attr-defined] # django standard

def expires_date(self, obj: Certificate) -> date:
def not_after_date(self, obj: Certificate) -> date:
"""Get the date (without time) when a cert expires."""
return obj.not_after.date()

expires_date.short_description = _("Expires") # type: ignore[attr-defined] # django standard
expires_date.admin_order_field = "not_after" # type: ignore[attr-defined] # django standard
not_after_date.short_description = _("Expires") # type: ignore[attr-defined] # django standard
not_after_date.admin_order_field = "not_after" # type: ignore[attr-defined] # django standard

# TYPE NOTE: django-stubs typehints obj as Model, but we can be more specific here
def save_model( # type: ignore[override]
Expand All @@ -1048,7 +1048,7 @@ def save_model( # type: ignore[override]
# NOTE: Use replace() and not astimzeone(), as we want to expire at midnight in UTC time.
# astimezone() will assume that the naive datetime object is in the system local time and
# convert it to what the system would consider midnight in its local time.
expires = datetime.combine(data["not_after"], datetime.min.time()).replace(tzinfo=tz.utc)
not_after = datetime.combine(data["not_after"], datetime.min.time()).replace(tzinfo=tz.utc)

# Set Subject Alternative Name from form
extensions: ConfigurableExtensionDict = {}
Expand Down Expand Up @@ -1098,7 +1098,7 @@ def save_model( # type: ignore[override]
csr,
subject=data["subject"],
algorithm=data["algorithm"],
expires=expires,
not_after=not_after,
extensions=list(extensions.values()),
)

Expand Down Expand Up @@ -1141,7 +1141,6 @@ def lookups( # type: ignore # we are more specific here

def queryset(self, request: HttpRequest, queryset: QuerySetTypeVar) -> QuerySetTypeVar:
now = timezone.now()
print("###", now)

if self.value() == "0":
return queryset.filter(expires__gt=now)
Expand Down
20 changes: 14 additions & 6 deletions ca/django_ca/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
from django_ca.acme.constants import BASE64_URL_ALPHABET, IdentifierType, Status
from django_ca.conf import CertificateRevocationListProfile, model_settings
from django_ca.constants import REVOCATION_REASONS, ReasonFlags
from django_ca.deprecation import RemovedInDjangoCA230Warning, deprecate_argument
from django_ca.extensions import get_extension_name
from django_ca.key_backends import KeyBackend, key_backends
from django_ca.managers import (
Expand Down Expand Up @@ -707,12 +708,14 @@ def extensions_for_certificate(self) -> ConfigurableExtensionDict:

return extensions

@deprecate_argument("expires", RemovedInDjangoCA230Warning, replacement="not_after")
def sign(
self,
key_backend_options: BaseModel,
csr: x509.CertificateSigningRequest,
subject: x509.Name,
algorithm: Optional[AllowedHashTypes] = None,
not_after: Optional[datetime] = None,
expires: Optional[datetime] = None,
extensions: Optional[list[ConfigurableExtension]] = None,
) -> x509.Certificate:
Expand All @@ -734,17 +737,22 @@ def sign(
Subject for the certificate
algorithm : :class:`~cg:cryptography.hazmat.primitives.hashes.HashAlgorithm`, optional
Hash algorithm used for signing the certificate, defaults to the algorithm used in the CA.
expires : datetime, optional
not_after : datetime, optional
When the certificate expires. If not provided, the ``CA_DEFAULT_EXPIRES`` setting will be used.
extensions : list of :py:class:`~cg:cryptography.x509.Extension`, optional
List of extensions to add to the certificates. The function will add some extensions unless
provided here, see above for details.
"""
if algorithm is None:
algorithm = self.algorithm
if expires is None:
expires = timezone.now() + model_settings.CA_DEFAULT_EXPIRES
expires = expires.replace(second=0, microsecond=0)
if not_after is not None and expires is not None:
raise ValueError("`not_before` and `expires` cannot both be set.")
if expires is not None:
not_after = expires

if not_after is None:
not_after = timezone.now() + model_settings.CA_DEFAULT_EXPIRES
not_after = not_after.replace(second=0, microsecond=0)
if extensions is None:
extensions = []

Expand All @@ -770,7 +778,7 @@ def sign(
sender=self.__class__,
ca=self,
csr=csr,
expires=expires,
not_after=not_after,
algorithm=algorithm,
subject=subject,
extensions=certificate_extensions,
Expand All @@ -785,7 +793,7 @@ def sign(
algorithm=algorithm,
issuer=self.subject,
subject=subject,
expires=expires,
expires=not_after,
extensions=certificate_extensions,
)

Expand Down
14 changes: 7 additions & 7 deletions ca/django_ca/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ def api_sign_certificate(
message.get_csr(),
subject=message.subject.cryptography, # pylint: disable=no-member # false positive
algorithm=message.get_algorithm(),
expires=message.not_after,
not_after=message.not_after,
extensions=parsed_extensions,
)

Expand Down Expand Up @@ -416,13 +416,13 @@ def acme_issue_certificate(acme_certificate_pk: int) -> None:

# Honor not_after from the order if set
if acme_cert.order.not_after:
expires = acme_cert.order.not_after
not_after = acme_cert.order.not_after

# Make sure expires_datetime is tz-aware, even if USE_TZ=False.
if timezone.is_naive(expires):
expires = timezone.make_aware(expires)
# Make sure not_after is tz-aware, even if USE_TZ=False.
if timezone.is_naive(not_after):
not_after = timezone.make_aware(not_after)
else:
expires = datetime.now(tz=tz.utc) + model_settings.CA_ACME_DEFAULT_CERT_VALIDITY
not_after = datetime.now(tz=tz.utc) + model_settings.CA_ACME_DEFAULT_CERT_VALIDITY

csr = acme_cert.parse_csr()

Expand All @@ -431,7 +431,7 @@ def acme_issue_certificate(acme_certificate_pk: int) -> None:

# Finally, actually create a certificate
cert = Certificate.objects.create_cert(
ca, key_backend_options, csr=csr, profile=profile, not_after=expires, extensions=extensions
ca, key_backend_options, csr=csr, profile=profile, not_after=not_after, extensions=extensions
)

acme_cert.cert = cert
Expand Down
37 changes: 34 additions & 3 deletions ca/django_ca/tests/models/test_certificate_authority.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
from pytest_django.fixtures import SettingsWrapper

from django_ca.conf import model_settings
from django_ca.deprecation import RemovedInDjangoCA230Warning
from django_ca.key_backends.storages import StoragesUsePrivateKeyOptions
from django_ca.models import Certificate, CertificateAuthority
from django_ca.pydantic import CertificatePoliciesModel
Expand Down Expand Up @@ -815,14 +816,44 @@ def test_sign_with_non_default_values(subject: x509.Name, usable_root: Certifica
"""Pass non-default parameters."""
csr = CERT_DATA["child-cert"]["csr"]["parsed"]
algorithm = hashes.SHA256()
expires = datetime.now(tz=timezone.utc) + model_settings.CA_DEFAULT_EXPIRES + timedelta(days=3)
not_after = datetime.now(tz=timezone.utc) + model_settings.CA_DEFAULT_EXPIRES + timedelta(days=3)
with assert_sign_cert_signals():
cert = usable_root.sign(
key_backend_options, csr, subject=subject, algorithm=algorithm, expires=expires
key_backend_options, csr, subject=subject, algorithm=algorithm, not_after=not_after
)

assert_certificate(cert, subject, hashes.SHA256, signer=usable_root)
assert cert.not_valid_after_utc == expires
assert cert.not_valid_after_utc == not_after


@pytest.mark.freeze_time(TIMESTAMPS["everything_valid"])
def test_sign_with_deprecated_expires(subject: x509.Name, usable_root: CertificateAuthority) -> None:
"""Pass non-default parameters."""
csr = CERT_DATA["child-cert"]["csr"]["parsed"]
algorithm = hashes.SHA256()
not_after = datetime.now(tz=timezone.utc) + model_settings.CA_DEFAULT_EXPIRES + timedelta(days=3)
warning = (
r"^Argument `expires` is deprecated and will be removed in django-ca 2.3, use `not_after` instead\.$"
)
with pytest.warns(RemovedInDjangoCA230Warning, match=warning):
cert = usable_root.sign(
key_backend_options, csr, subject=subject, algorithm=algorithm, expires=not_after
)
assert cert.not_valid_after_utc == not_after


def test_sign_with_not_after_and_expires(root: CertificateAuthority, subject: x509.Name) -> None:
"""Test error when passing extensions that may not be passed to this function."""
not_after = datetime.now(tz=timezone.utc) + model_settings.CA_DEFAULT_EXPIRES + timedelta(days=3)
csr = CERT_DATA["child-cert"]["csr"]["parsed"]
warning = (
r"^Argument `expires` is deprecated and will be removed in django-ca 2.3, use `not_after` instead\.$"
)
with (
pytest.warns(RemovedInDjangoCA230Warning, match=warning),
pytest.raises(ValueError, match=r"^`not_before` and `expires` cannot both be set\.$"),
):
root.sign(key_backend_options, csr, subject=subject, not_after=not_after, expires=not_after)


@pytest.mark.parametrize(
Expand Down
12 changes: 8 additions & 4 deletions docs/source/changelog/TBR_2.1.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,14 @@ Dependencies
Python API
**********

* :py:func:`CertificateAuthorityManager.init() <django_ca.managers.CertificateAuthorityManager.init>`,
:py:func:`CertificateManager.create_cert() <django_ca.managers.CertificateManager.create_cert>` and
:func:`Profile.create_cert() <django_ca.profiles.Profile.create_cert>` now takes a ``not_after`` parameter,
replacing ``expires``. The latter is deprecated and will be removed in django-ca 2.3.0.
* Functions that create a certificate now take a ``not_after`` parameter, replacing ``expires``. The
``expires`` parameter is deprecated and will be removed in django-ca 2.3.0. The following functions are
affected:

* :func:`django_ca.models.CertificateAuthority.sign`
* :func:`django_ca.managers.CertificateAuthorityManager.init`
* :func:`django_ca.managers.CertificateManager.create_cert`
* :func:`django_ca.profiles.Profile.create_cert`

***************
Database models
Expand Down
12 changes: 7 additions & 5 deletions docs/source/deprecation.rst
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@ Deprecation timeline
Python API
==========

* The ``expires`` parameter to :py:func:`CertificateAuthorityManager.init()
<django_ca.managers.CertificateAuthorityManager.init>`, :py:func:`CertificateManager.create_cert()
<django_ca.managers.CertificateManager.create_cert>` and :func:`Profile.create_cert()
<django_ca.profiles.Profile.create_cert>` will be removed. Use ``not_after`` instead (deprecated since
2.1.0).
* The ``expires`` parameter to functions that create a certificate will be removed. Use ``not_after`` instead
(deprecated since 2.1.0). The following functions are affected:

* :func:`django_ca.models.CertificateAuthority.sign`
* :func:`django_ca.managers.CertificateAuthorityManager.init`
* :func:`django_ca.managers.CertificateManager.create_cert`
* :func:`django_ca.profiles.Profile.create_cert`

***********
2.2.0 (TBR)
Expand Down

0 comments on commit 5f23392

Please sign in to comment.