From 5dadbaf3d0997d27d91e480459cda2ee42262f53 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Wed, 22 Apr 2020 15:42:42 -0400
Subject: [PATCH 01/11] Report the certificate expiration as a prometheus
 metric.

---
 .gitignore            |  1 +
 setup.py              |  1 +
 sygnal/apnspushkin.py | 39 ++++++++++++++++++++++++++++++++-------
 tests/test_apns.py    |  2 ++
 4 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/.gitignore b/.gitignore
index 2b31fa4c..756dc9a0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -7,5 +7,6 @@ _trial_temp*
 
 /.idea
 /.eggs
+/*.egg-info
 /build
 /dist
diff --git a/setup.py b/setup.py
index 14d1790b..4b7fe8f6 100755
--- a/setup.py
+++ b/setup.py
@@ -39,6 +39,7 @@ def read(fname):
         "Twisted>=19.2.1",
         "prometheus_client>=0.7.0,<0.8",
         "aioapns>=1.7",
+        "pyOpenSSL>=17.5.0",
         "pyyaml>=5.1.1",
         "service_identity>=18.1.0",
         "jaeger-client>=4.0.0",
diff --git a/sygnal/apnspushkin.py b/sygnal/apnspushkin.py
index 3d875de5..a64a5b9d 100644
--- a/sygnal/apnspushkin.py
+++ b/sygnal/apnspushkin.py
@@ -16,6 +16,7 @@
 # limitations under the License.
 import asyncio
 import base64
+from datetime import datetime
 import logging
 import os
 from uuid import uuid4
@@ -23,6 +24,7 @@
 import aioapns
 from aioapns import APNs, NotificationRequest
 from opentracing import logs, tags
+import OpenSSL
 from prometheus_client import Histogram, Counter, Gauge
 from twisted.internet.defer import Deferred
 
@@ -51,6 +53,12 @@
     labelnames=["pushkin", "code"],
 )
 
+CERTIFICATE_EXPIRATION_GAUGE = Gauge(
+    "sygnal_time_to_certificate_expiration",
+    "The number of seconds until the client certificate expires",
+    labelnames=["pushkin"],
+)
+
 
 class ApnsPushkin(Pushkin):
     """
@@ -87,17 +95,17 @@ def __init__(self, name, sygnal, config):
         else:
             raise PushkinSetupException(f"Invalid platform: {platform}")
 
-        certfile = self.get_config("certfile")
+        self._certfile = self.get_config("certfile")
         keyfile = self.get_config("keyfile")
-        if not certfile and not keyfile:
+        if not self._certfile and not keyfile:
             raise PushkinSetupException(
                 "You must provide a path to an APNs certificate, or an APNs token."
             )
 
-        if certfile:
-            if not os.path.exists(certfile):
+        if self._certfile:
+            if not os.path.exists(self._certfile):
                 raise PushkinSetupException(
-                    f"The APNs certificate '{certfile}' does not exist."
+                    f"The APNs certificate '{self._certfile}' does not exist."
                 )
         else:
             # keyfile
@@ -112,10 +120,13 @@ def __init__(self, name, sygnal, config):
             if not self.get_config("topic"):
                 raise PushkinSetupException("You must supply topic.")
 
-        if self.get_config("certfile") is not None:
+        if self._certfile is not None:
             self.apns_client = APNs(
-                client_cert=self.get_config("certfile"), use_sandbox=self.use_sandbox
+                client_cert=self._certfile, use_sandbox=self.use_sandbox
             )
+
+            # TODO Should this continually report.
+            self._report_certificate_expiration()
         else:
             self.apns_client = APNs(
                 key=self.get_config("keyfile"),
@@ -128,6 +139,20 @@ def __init__(self, name, sygnal, config):
         # without this, aioapns will retry every second forever.
         self.apns_client.pool.max_connection_attempts = 3
 
+    def _report_certificate_expiration(self):
+        """Export the time until the certificate expires as a metric."""
+        if self._certfile is not None:
+            with open(self._certfile, "rb") as f:
+                cert = f.read()
+
+            x509 = OpenSSL.crypto.load_certificate(OpenSSL.crypto.FILETYPE_PEM, cert)
+            # Convert from a string to a datetime object.
+            expiration_date = datetime.strptime(x509.get_notAfter(), "%Y%m%d%H%M%SZ")
+
+            seconds_left = int((expiration_date - datetime.utcnow()).total_seconds())
+
+            CERTIFICATE_EXPIRATION_GAUGE.set(seconds_left)
+
     async def _dispatch_request(self, log, span, device, shaved_payload, prio):
         """
         Actually attempts to dispatch the notification once.
diff --git a/tests/test_apns.py b/tests/test_apns.py
index bd7c3334..5251c684 100644
--- a/tests/test_apns.py
+++ b/tests/test_apns.py
@@ -34,6 +34,8 @@ def setUp(self):
 
         # pretend our certificate exists
         patch("os.path.exists", lambda x: x == TEST_CERTFILE_PATH).start()
+        # Since no certificate exists, don't try to read it.
+        patch("sygnal.apnspushkin.ApnsPushkin._report_certificate_expiration").start()
         self.addCleanup(patch.stopall)
 
         super(ApnsTestCase, self).setUp()

From f4b95a7dbf59f7c09494b4bcb7816b960720ac6a Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Wed, 22 Apr 2020 15:52:19 -0400
Subject: [PATCH 02/11] Repeatedly report the expiration time.

---
 sygnal/apnspushkin.py | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/sygnal/apnspushkin.py b/sygnal/apnspushkin.py
index a64a5b9d..fff01d11 100644
--- a/sygnal/apnspushkin.py
+++ b/sygnal/apnspushkin.py
@@ -27,6 +27,7 @@
 import OpenSSL
 from prometheus_client import Histogram, Counter, Gauge
 from twisted.internet.defer import Deferred
+from twisted.internet.task import LoopingCall
 
 from sygnal import apnstruncate
 from sygnal.exceptions import (
@@ -125,7 +126,6 @@ def __init__(self, name, sygnal, config):
                 client_cert=self._certfile, use_sandbox=self.use_sandbox
             )
 
-            # TODO Should this continually report.
             self._report_certificate_expiration()
         else:
             self.apns_client = APNs(
@@ -149,9 +149,13 @@ def _report_certificate_expiration(self):
             # Convert from a string to a datetime object.
             expiration_date = datetime.strptime(x509.get_notAfter(), "%Y%m%d%H%M%SZ")
 
-            seconds_left = int((expiration_date - datetime.utcnow()).total_seconds())
+            def report_expiry():
+                seconds_left = int((expiration_date - datetime.utcnow()).total_seconds())
+                CERTIFICATE_EXPIRATION_GAUGE.set(seconds_left)
 
-            CERTIFICATE_EXPIRATION_GAUGE.set(seconds_left)
+            # Report the metric every 60 seconds.
+            looper = LoopingCall(report_expiry)
+            looper.start(60, now=True)
 
     async def _dispatch_request(self, log, span, device, shaved_payload, prio):
         """

From 90585856077a8dfbac8c40a916cc9cdd0b4cc699 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Wed, 22 Apr 2020 15:53:12 -0400
Subject: [PATCH 03/11] Remove a useless if-statement.

---
 sygnal/apnspushkin.py | 43 ++++++++++++++++++++-----------------------
 1 file changed, 20 insertions(+), 23 deletions(-)

diff --git a/sygnal/apnspushkin.py b/sygnal/apnspushkin.py
index fff01d11..f76705ff 100644
--- a/sygnal/apnspushkin.py
+++ b/sygnal/apnspushkin.py
@@ -96,17 +96,17 @@ def __init__(self, name, sygnal, config):
         else:
             raise PushkinSetupException(f"Invalid platform: {platform}")
 
-        self._certfile = self.get_config("certfile")
+        certfile = self.get_config("certfile")
         keyfile = self.get_config("keyfile")
-        if not self._certfile and not keyfile:
+        if not certfile and not keyfile:
             raise PushkinSetupException(
                 "You must provide a path to an APNs certificate, or an APNs token."
             )
 
-        if self._certfile:
-            if not os.path.exists(self._certfile):
+        if certfile:
+            if not os.path.exists(certfile):
                 raise PushkinSetupException(
-                    f"The APNs certificate '{self._certfile}' does not exist."
+                    f"The APNs certificate '{certfile}' does not exist."
                 )
         else:
             # keyfile
@@ -121,12 +121,10 @@ def __init__(self, name, sygnal, config):
             if not self.get_config("topic"):
                 raise PushkinSetupException("You must supply topic.")
 
-        if self._certfile is not None:
-            self.apns_client = APNs(
-                client_cert=self._certfile, use_sandbox=self.use_sandbox
-            )
+        if certfile is not None:
+            self.apns_client = APNs(client_cert=certfile, use_sandbox=self.use_sandbox)
 
-            self._report_certificate_expiration()
+            self._report_certificate_expiration(certfile)
         else:
             self.apns_client = APNs(
                 key=self.get_config("keyfile"),
@@ -139,23 +137,22 @@ def __init__(self, name, sygnal, config):
         # without this, aioapns will retry every second forever.
         self.apns_client.pool.max_connection_attempts = 3
 
-    def _report_certificate_expiration(self):
+    def _report_certificate_expiration(self, certfile):
         """Export the time until the certificate expires as a metric."""
-        if self._certfile is not None:
-            with open(self._certfile, "rb") as f:
-                cert = f.read()
+        with open(certfile, "rb") as f:
+            cert = f.read()
 
-            x509 = OpenSSL.crypto.load_certificate(OpenSSL.crypto.FILETYPE_PEM, cert)
-            # Convert from a string to a datetime object.
-            expiration_date = datetime.strptime(x509.get_notAfter(), "%Y%m%d%H%M%SZ")
+        x509 = OpenSSL.crypto.load_certificate(OpenSSL.crypto.FILETYPE_PEM, cert)
+        # Convert from a string to a datetime object.
+        expiration_date = datetime.strptime(x509.get_notAfter(), "%Y%m%d%H%M%SZ")
 
-            def report_expiry():
-                seconds_left = int((expiration_date - datetime.utcnow()).total_seconds())
-                CERTIFICATE_EXPIRATION_GAUGE.set(seconds_left)
+        def report_expiry():
+            seconds_left = int((expiration_date - datetime.utcnow()).total_seconds())
+            CERTIFICATE_EXPIRATION_GAUGE.set(seconds_left)
 
-            # Report the metric every 60 seconds.
-            looper = LoopingCall(report_expiry)
-            looper.start(60, now=True)
+        # Report the metric every 60 seconds.
+        looper = LoopingCall(report_expiry)
+        looper.start(60, now=True)
 
     async def _dispatch_request(self, log, span, device, shaved_payload, prio):
         """

From f70637daa71cf12bb9ea972bcec41d81e3f03ada Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Wed, 22 Apr 2020 15:55:45 -0400
Subject: [PATCH 04/11] Set the gauge's label.

---
 sygnal/apnspushkin.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sygnal/apnspushkin.py b/sygnal/apnspushkin.py
index f76705ff..a7c46def 100644
--- a/sygnal/apnspushkin.py
+++ b/sygnal/apnspushkin.py
@@ -148,7 +148,7 @@ def _report_certificate_expiration(self, certfile):
 
         def report_expiry():
             seconds_left = int((expiration_date - datetime.utcnow()).total_seconds())
-            CERTIFICATE_EXPIRATION_GAUGE.set(seconds_left)
+            CERTIFICATE_EXPIRATION_GAUGE.labels(pushkin=self.name).set(seconds_left)
 
         # Report the metric every 60 seconds.
         looper = LoopingCall(report_expiry)

From 12f195a0cfd221636712c393a60d8a71412aedf7 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Thu, 23 Apr 2020 13:29:23 -0400
Subject: [PATCH 05/11] Convert from bytes to string.

---
 sygnal/apnspushkin.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/sygnal/apnspushkin.py b/sygnal/apnspushkin.py
index a7c46def..e3602718 100644
--- a/sygnal/apnspushkin.py
+++ b/sygnal/apnspushkin.py
@@ -144,7 +144,9 @@ def _report_certificate_expiration(self, certfile):
 
         x509 = OpenSSL.crypto.load_certificate(OpenSSL.crypto.FILETYPE_PEM, cert)
         # Convert from a string to a datetime object.
-        expiration_date = datetime.strptime(x509.get_notAfter(), "%Y%m%d%H%M%SZ")
+        expiration_date = datetime.strptime(
+            x509.get_notAfter().decode(), "%Y%m%d%H%M%SZ"
+        )
 
         def report_expiry():
             seconds_left = int((expiration_date - datetime.utcnow()).total_seconds())

From 2b07462e654104e52687cfd71e02b0c85bc308c0 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Thu, 23 Apr 2020 13:30:54 -0400
Subject: [PATCH 06/11] Report the expiration time as a unix epoch time instead
 of seconds to expiration.

---
 sygnal/apnspushkin.py | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/sygnal/apnspushkin.py b/sygnal/apnspushkin.py
index e3602718..c39daeba 100644
--- a/sygnal/apnspushkin.py
+++ b/sygnal/apnspushkin.py
@@ -27,7 +27,6 @@
 import OpenSSL
 from prometheus_client import Histogram, Counter, Gauge
 from twisted.internet.defer import Deferred
-from twisted.internet.task import LoopingCall
 
 from sygnal import apnstruncate
 from sygnal.exceptions import (
@@ -56,7 +55,7 @@
 
 CERTIFICATE_EXPIRATION_GAUGE = Gauge(
     "sygnal_time_to_certificate_expiration",
-    "The number of seconds until the client certificate expires",
+    "The expiry date of the client certificate in seconds since the epoch",
     labelnames=["pushkin"],
 )
 
@@ -138,7 +137,7 @@ def __init__(self, name, sygnal, config):
         self.apns_client.pool.max_connection_attempts = 3
 
     def _report_certificate_expiration(self, certfile):
-        """Export the time until the certificate expires as a metric."""
+        """Export the epoch time that the certificate expires as a metric."""
         with open(certfile, "rb") as f:
             cert = f.read()
 
@@ -147,14 +146,9 @@ def _report_certificate_expiration(self, certfile):
         expiration_date = datetime.strptime(
             x509.get_notAfter().decode(), "%Y%m%d%H%M%SZ"
         )
-
-        def report_expiry():
-            seconds_left = int((expiration_date - datetime.utcnow()).total_seconds())
-            CERTIFICATE_EXPIRATION_GAUGE.labels(pushkin=self.name).set(seconds_left)
-
-        # Report the metric every 60 seconds.
-        looper = LoopingCall(report_expiry)
-        looper.start(60, now=True)
+        # Convert the expiration time to seconds since the epoch.
+        epoch_time = int((expiration_date - datetime(1970, 1, 1)).total_seconds())
+        CERTIFICATE_EXPIRATION_GAUGE.labels(pushkin=self.name).set(epoch_time)
 
     async def _dispatch_request(self, log, span, device, shaved_payload, prio):
         """

From 67c2fdee4922f5dde5f7638bcc99cbf8dd30d158 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Thu, 23 Apr 2020 13:36:14 -0400
Subject: [PATCH 07/11] Clarify metric name.

---
 sygnal/apnspushkin.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sygnal/apnspushkin.py b/sygnal/apnspushkin.py
index c39daeba..64439cca 100644
--- a/sygnal/apnspushkin.py
+++ b/sygnal/apnspushkin.py
@@ -54,7 +54,7 @@
 )
 
 CERTIFICATE_EXPIRATION_GAUGE = Gauge(
-    "sygnal_time_to_certificate_expiration",
+    "sygnal_cert_expiry",
     "The expiry date of the client certificate in seconds since the epoch",
     labelnames=["pushkin"],
 )

From 9d6b7944e35a5654639334604f4c4a8713c88a24 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Mon, 27 Apr 2020 09:28:41 -0400
Subject: [PATCH 08/11] Note that it is the client certificate in the metric
 name.

---
 sygnal/apnspushkin.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sygnal/apnspushkin.py b/sygnal/apnspushkin.py
index 64439cca..59a9505d 100644
--- a/sygnal/apnspushkin.py
+++ b/sygnal/apnspushkin.py
@@ -54,7 +54,7 @@
 )
 
 CERTIFICATE_EXPIRATION_GAUGE = Gauge(
-    "sygnal_cert_expiry",
+    "sygnal_client_cert_expiry",
     "The expiry date of the client certificate in seconds since the epoch",
     labelnames=["pushkin"],
 )

From 254e95c104e0ad11fa3c23501704919c423460bc Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Tue, 28 Apr 2020 14:42:32 -0400
Subject: [PATCH 09/11] Switch to using timestamp().

---
 sygnal/apnspushkin.py | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/sygnal/apnspushkin.py b/sygnal/apnspushkin.py
index 59a9505d..2f5e3150 100644
--- a/sygnal/apnspushkin.py
+++ b/sygnal/apnspushkin.py
@@ -146,9 +146,10 @@ def _report_certificate_expiration(self, certfile):
         expiration_date = datetime.strptime(
             x509.get_notAfter().decode(), "%Y%m%d%H%M%SZ"
         )
-        # Convert the expiration time to seconds since the epoch.
-        epoch_time = int((expiration_date - datetime(1970, 1, 1)).total_seconds())
-        CERTIFICATE_EXPIRATION_GAUGE.labels(pushkin=self.name).set(epoch_time)
+        # Report the expiration time as seconds since the epoch.
+        CERTIFICATE_EXPIRATION_GAUGE.labels(pushkin=self.name).set(
+            expiration_date.timestamp()
+        )
 
     async def _dispatch_request(self, log, span, device, shaved_payload, prio):
         """

From fb206da38e180f3b748d1572802641480957dc18 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Tue, 28 Apr 2020 14:46:43 -0400
Subject: [PATCH 10/11] Use cryptography, not OpenSSL directly.

---
 setup.py              |  2 +-
 sygnal/apnspushkin.py | 17 +++++++----------
 2 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/setup.py b/setup.py
index 4b7fe8f6..2c6b5e83 100755
--- a/setup.py
+++ b/setup.py
@@ -39,7 +39,7 @@ def read(fname):
         "Twisted>=19.2.1",
         "prometheus_client>=0.7.0,<0.8",
         "aioapns>=1.7",
-        "pyOpenSSL>=17.5.0",
+        "cryptography>=2.1.4",
         "pyyaml>=5.1.1",
         "service_identity>=18.1.0",
         "jaeger-client>=4.0.0",
diff --git a/sygnal/apnspushkin.py b/sygnal/apnspushkin.py
index 2f5e3150..2081b182 100644
--- a/sygnal/apnspushkin.py
+++ b/sygnal/apnspushkin.py
@@ -16,15 +16,16 @@
 # limitations under the License.
 import asyncio
 import base64
-from datetime import datetime
+from datetime import timezone
 import logging
 import os
 from uuid import uuid4
 
 import aioapns
 from aioapns import APNs, NotificationRequest
+from cryptography.hazmat.backends import default_backend
+from cryptography.x509 import load_pem_x509_certificate
 from opentracing import logs, tags
-import OpenSSL
 from prometheus_client import Histogram, Counter, Gauge
 from twisted.internet.defer import Deferred
 
@@ -139,16 +140,12 @@ def __init__(self, name, sygnal, config):
     def _report_certificate_expiration(self, certfile):
         """Export the epoch time that the certificate expires as a metric."""
         with open(certfile, "rb") as f:
-            cert = f.read()
+            cert_bytes = f.read()
 
-        x509 = OpenSSL.crypto.load_certificate(OpenSSL.crypto.FILETYPE_PEM, cert)
-        # Convert from a string to a datetime object.
-        expiration_date = datetime.strptime(
-            x509.get_notAfter().decode(), "%Y%m%d%H%M%SZ"
-        )
-        # Report the expiration time as seconds since the epoch.
+        cert = load_pem_x509_certificate(cert_bytes, default_backend())
+        # Report the expiration time as seconds since the epoch (in UTC time).
         CERTIFICATE_EXPIRATION_GAUGE.labels(pushkin=self.name).set(
-            expiration_date.timestamp()
+            cert.not_valid_after.replace(tzinfo=timezone.utc).timestamp()
         )
 
     async def _dispatch_request(self, log, span, device, shaved_payload, prio):

From 46a8925daea677451a91ed261047c9a04e4ed962 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Wed, 29 Apr 2020 06:39:42 -0400
Subject: [PATCH 11/11] Add a newsfragement.

---
 changelog.d/106.feature | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 changelog.d/106.feature

diff --git a/changelog.d/106.feature b/changelog.d/106.feature
new file mode 100644
index 00000000..c138ba64
--- /dev/null
+++ b/changelog.d/106.feature
@@ -0,0 +1 @@
+Report the APNs certificate expiry as a prometheus metric.