pyca / cryptography

cryptography is a package designed to expose cryptographic primitives and recipes to Python developers.
https://cryptography.io
Other
6.59k stars 1.51k forks source link

ocsp response contains offset-naive datetime objects #11170

Closed francois4224 closed 3 months ago

francois4224 commented 3 months ago
import datetime as dt

from cryptography.hazmat.primitives import hashes
from cryptography.hazmat.primitives.asymmetric.rsa import generate_private_key
from cryptography.x509 import CertificateBuilder, Name, ReasonFlags, ocsp

builder = ocsp.OCSPResponseBuilder()
key = generate_private_key(public_exponent=65537, key_size=512)

cert = CertificateBuilder(
    issuer_name=Name([]),
    subject_name=Name([]),
    public_key=key.public_key(),
    not_valid_before=dt.datetime.now(dt.UTC),
    not_valid_after=dt.datetime.now(dt.UTC),
    serial_number=123,
).sign(private_key=key, algorithm=hashes.SHA256())

res = (
    builder.add_response(
        cert=cert,
        issuer=cert,
        algorithm=hashes.SHA256(),
        cert_status=ocsp.OCSPCertStatus.REVOKED,
        this_update=dt.datetime.now(dt.UTC),
        next_update=dt.datetime.now(dt.UTC),
        revocation_time=dt.datetime.now(dt.UTC),
        revocation_reason=ReasonFlags("unspecified"),
    )
    .responder_id(ocsp.OCSPResponderEncoding.HASH, cert)
    .sign(private_key=key, algorithm=hashes.SHA256())
)

# TypeError: can't subtract offset-naive and offset-aware datetimes
dt.datetime.now(dt.UTC) - res.revocation_time
francois4224 commented 3 months ago

Hi, it seems that timezone is not handled correctly in ocsp response object.

From spec https://www.rfc-editor.org/rfc/rfc6960#section-4.2.2.1, it should be always in Greenwich Mean Time, but datetime from Cryptography is not timezone aware.

alex commented 3 months ago

Where in the RFC do you see Greenwich Mean Time?

reaperhulk commented 3 months ago

revocation_time returns a naïve object which we define in the docs to be UTC. We should probably add a revocation_time_utc which returns a tz-aware object, as we've done with others like this as Python slowly deprecates näive objects.

For now, if you need to do a comparsion you can either strip tzinfo off your other dt object or use object.revocation_time.replace(tzinfo=datetime.utc)

alex commented 3 months ago

Good catch Paul. Francois, would you be interested in submitting a PR to add revocation_time_utc? https://github.com/pyca/cryptography/commit/ce94de03e8feca67388529671cd366d23c966f58 is a model for what this looks like

On Thu, Jun 27, 2024 at 8:13 AM Paul Kehrer @.***> wrote:

revocation_time returns a naïve object which we define in the docs to be UTC. We should probably add a revocation_time_utc which returns a tz-aware object, as we've done with others like this as Python slowly deprecates näive objects.

— Reply to this email directly, view it on GitHub https://github.com/pyca/cryptography/issues/11170#issuecomment-2194526333, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAAGBET6TPXQ2FQGHLG6RDZJP6WDAVCNFSM6AAAAABJ7SHELSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOJUGUZDMMZTGM . You are receiving this because you commented.Message ID: @.***>

-- All that is necessary for evil to succeed is for good people to do nothing.

magnuswatn commented 3 months ago

I just stumbled upon this myself, so I can take a look, if @francois4224 hasn't started on it.

reaperhulk commented 3 months ago

Feel free! It's possible we've missed other _utc variants that are needed as well, but if you're going to submit a PR you can look at how we've done the deprecation and implementation previously https://github.com/pyca/cryptography/blob/85fba50add6b7129898f69d69a2338475de2aae5/src/rust/src/x509/certificate.rs#L198-L233