Remove utcnow() deprecation warning. #437
Conversation
Python 3.12 deprecates utcnow(), but we cannot do the recommended thing and replace
utcnow() with datetime.now(timezone.utc) as the datetime objects that the cryptography
x509 library puts in certificates do not have any timezone info. If we try to
compare them with something that has timezone info we will get an exception:
TypeError: can't compare offset-naive and offset-aware datetimes
We must continue to make an explictly offset-naive timestamp.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #437 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 25 25
Lines 4868 4868
=========================================
Hits 4868 4868 ☔ View full report in Codecov by Sentry. |
| # TypeError: can't compare offset-naive and offset-aware datetimes | ||
| # | ||
| # We must continue to make an explicitly offset-naive timestamp. | ||
| utcnow = partial(datetime.datetime.now, None) |
There was a problem hiding this comment.
Hm, this doesn't look equivalent, I would have expected something like:
datetime.datetime.now(datetime.timezone.utc).replace(tzinfo=None)
There was a problem hiding this comment.
Looking into this a bit deeper it looks like we are going the wrong way here. We should be moving towards timezone-aware datetimes, as in:
- Making
utcnow = partial(datetime.datetime.now, datetime.timezone.utc) - Using
certificate.not_valid_before_utcandcertificate.not_valid_after_utc: https://cryptography.io/en/latest/x509/reference/#cryptography.x509.Certificate.not_valid_before_utc - Requiring
cryptography >= 42.0.0
| .not_valid_before(datetime.datetime.now(None)) | ||
| .not_valid_after(datetime.datetime.now(None) + datetime.timedelta(days=10)) |
There was a problem hiding this comment.
I think here we should use tls.utcnow() for consistency
|
Unless I'm mistaken, the "right" way to fix this would be to wait for |
|
If they are going to fix it, then yes, that is the right way! |
We could also have a two-step approach. Right now
def utcnow() -> datetime.datetime:
return datetime.datetime.now(datetime.timezone.utc).replace(tzinfo=None)When cryptography 42 comes outUse timezone-aware datetimes everywhere! It would be good to check that |
|
I checked CertificateBuilder in the dev version of 42, and it accepts either naive or non-naive date times, normalizing to naive before it calls into the rust code, so I think we're ok there. I'm guessing cryptography 42 will probably release relatively soon, possibly sooner than we will, so it's probably not worth merging this branch. I'll redo things when 42 comes out. |
|
cryptography 42 is out :) |
|
I'm closing this PR in favor of a new one for cryptography 42 on a different branch. |
Python 3.12 deprecates utcnow(), but we cannot do the recommended thing and replace utcnow() with datetime.now(timezone.utc) as the datetime objects that the cryptography x509 library puts in certificates do not have any timezone info. If we try to compare them with something that has timezone info we will get an exception:
We must continue to make an explictly offset-naive timestamp.