From d9cac6fbcd83fa47ad8924f778713f846729e2d8 Mon Sep 17 00:00:00 2001
From: Matthew Hughes <34972397+matthewhughes934@users.noreply.github.com>
Date: Thu, 16 Sep 2021 09:53:36 +0100
Subject: [PATCH] Make Cert.not{before,after} timezone aware (#4805)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

I noticed when running tests the output of
`web/src/js/__tests__/ducks/_tflow.ts` would change depending on how I
set my timezone, e.g.

    $ TZ=America/Los_Angeles pytest --quiet \
        test/mitmproxy/tools/web/test_app.py >/dev/null \
        && grep --extended-regexp 'not(after|before)' web/src/js/__tests__/ducks/_tflow.ts
                "notafter": 2235132207,
                "notbefore": 1604415807,
    $ TZ=Asia/Tokyo pytest --quiet \
        test/mitmproxy/tools/web/test_app.py >/dev/null \
        && grep --extended-regexp 'not(after|before)'  web/src/js/__tests__/ducks/_tflow.ts
                "notafter": 2235074607,
                "notbefore": 1604354607

It looks like this is because the `cert_to_json` function simply calls
`timestamp` the `datetime` object from
`x509.Certificate.not_valid_before`, however, this `datetime` object is
not timestamp aware, from the docs [1]:

> A naïve datetime representing the beginning of the validity period for
the certificate in UTC

So when serializing to JSON, first convert the `datetime` to UTC then
call `timestamp`.

A test was added by inspecting one of the test certs with:

    $ openssl x509 -in test/mitmproxy/net/data/text_cert_2 -text

Extracting the date and asserting on that.

The corresponding test has also been re-run so that `_tflow.ts` was
regenerated with it's correct value. Snapshots were also updated via:

    $(npm bin)/jest --updateSnapshot

[1] https://cryptography.io/en/latest/x509/reference/#cryptography.x509.Certificate.not_valid_after
---
 mitmproxy/certs.py                            |  6 ++++--
 test/mitmproxy/test_certs.py                  | 21 +++++++++++++++++--
 .../__snapshots__/FlowViewSpec.tsx.snap       |  4 ++--
 web/src/js/__tests__/ducks/_tflow.ts          |  4 ++--
 4 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/mitmproxy/certs.py b/mitmproxy/certs.py
index 7266e2d67..4bb2f524f 100644
--- a/mitmproxy/certs.py
+++ b/mitmproxy/certs.py
@@ -89,11 +89,13 @@ class Cert(serializable.Serializable):
 
     @property
     def notbefore(self) -> datetime.datetime:
-        return self._cert.not_valid_before
+        # x509.Certificate.not_valid_before is a naive datetime in UTC
+        return self._cert.not_valid_before.replace(tzinfo=datetime.timezone.utc)
 
     @property
     def notafter(self) -> datetime.datetime:
-        return self._cert.not_valid_after
+        # x509.Certificate.not_valid_after is a naive datetime in UTC
+        return self._cert.not_valid_after.replace(tzinfo=datetime.timezone.utc)
 
     def has_expired(self) -> bool:
         return datetime.datetime.utcnow() > self._cert.not_valid_after
diff --git a/test/mitmproxy/test_certs.py b/test/mitmproxy/test_certs.py
index 3ac294ce2..1084b0a41 100644
--- a/test/mitmproxy/test_certs.py
+++ b/test/mitmproxy/test_certs.py
@@ -1,4 +1,5 @@
 import os
+from datetime import datetime, timezone
 from pathlib import Path
 from cryptography import x509
 from cryptography.x509 import NameOID
@@ -173,8 +174,24 @@ class TestCert:
         assert c2.cn == "www.inode.co.nz"
         assert len(c2.altnames) == 2
         assert c2.fingerprint()
-        assert c2.notbefore
-        assert c2.notafter
+        assert c2.notbefore == datetime(
+            year=2010,
+            month=1,
+            day=11,
+            hour=19,
+            minute=27,
+            second=36,
+            tzinfo=timezone.utc,
+        )
+        assert c2.notafter == datetime(
+            year=2011,
+            month=1,
+            day=12,
+            hour=9,
+            minute=14,
+            second=55,
+            tzinfo=timezone.utc,
+        )
         assert c2.subject
         assert c2.keyinfo == ("RSA", 2048)
         assert c2.serial
diff --git a/web/src/js/__tests__/components/__snapshots__/FlowViewSpec.tsx.snap b/web/src/js/__tests__/components/__snapshots__/FlowViewSpec.tsx.snap
index 5fed82aa8..6332d2543 100644
--- a/web/src/js/__tests__/components/__snapshots__/FlowViewSpec.tsx.snap
+++ b/web/src/js/__tests__/components/__snapshots__/FlowViewSpec.tsx.snap
@@ -639,7 +639,7 @@ exports[`FlowView 4`] = `
               Valid from
             </td>
             <td>
-              2020-11-03 06:03:27
+              2020-11-03 07:03:27
             </td>
           </tr>
           <tr>
@@ -647,7 +647,7 @@ exports[`FlowView 4`] = `
               Valid to
             </td>
             <td>
-              2040-10-29 06:03:27
+              2040-10-29 07:03:27
             </td>
           </tr>
           <tr>
diff --git a/web/src/js/__tests__/ducks/_tflow.ts b/web/src/js/__tests__/ducks/_tflow.ts
index 4b5288e20..f2685e1cc 100644
--- a/web/src/js/__tests__/ducks/_tflow.ts
+++ b/web/src/js/__tests__/ducks/_tflow.ts
@@ -107,8 +107,8 @@ export function THTTPFlow(): Required<HTTPFlow> {
                     "RSA",
                     2048
                 ],
-                "notafter": 2235103407,
-                "notbefore": 1604383407,
+                "notafter": 2235107007,
+                "notbefore": 1604387007,
                 "serial": "247170098335718583458667965517443538258472437317",
                 "sha256": "e5f62a1175031b6feb959bc8e6dd0f8e2546dbbf7c32da39534309d8aa92967c",
                 "subject": [