fix: Accepting time-zone aware expiration datetime only to prevent hard-to debug issues with invalid signatures (#12704)

* Accepting time-zone aware expiration datetime only to prevent hard-to debug issues with invalid signatures.

* Applying code review suggestion

* Fixing linter issues

Author: pawelkedzior-gl

cdn/snippets.py

@@ -26,7 +26,7 @@
 # [START cloudcdn_sign_cookie]
 import argparse
 import base64
-from datetime import datetime
+from datetime import datetime, timezone
 import hashlib
 import hmac
 from urllib.parse import parse_qs, urlsplit
@@ -48,7 +48,7 @@ def sign_url(
         url: URL to sign.
         key_name: name of the signing key.
         base64_key: signing key as a base64 encoded string.
-        expiration_time: expiration time.
+        expiration_time: expiration time as time-zone aware datetime.
 
     Returns:
         Returns the Signed URL appended with the query parameters based on the
@@ -57,7 +57,7 @@ def sign_url(
     stripped_url = url.strip()
     parsed_url = urlsplit(stripped_url)
     query_params = parse_qs(parsed_url.query, keep_blank_values=True)
-    epoch = datetime.utcfromtimestamp(0)
+    epoch = datetime.fromtimestamp(0, timezone.utc)
     expiration_timestamp = int((expiration_time - epoch).total_seconds())
     decoded_key = base64.urlsafe_b64decode(base64_key)
 
@@ -87,7 +87,7 @@ def sign_url_prefix(
         url_prefix: URL prefix to sign.
         key_name: name of the signing key.
         base64_key: signing key as a base64 encoded string.
-        expiration_time: expiration time.
+        expiration_time: expiration time as time-zone aware datetime.
 
     Returns:
         Returns the Signed URL appended with the query parameters based on the
@@ -99,7 +99,7 @@ def sign_url_prefix(
     encoded_url_prefix = base64.urlsafe_b64encode(
         url_prefix.strip().encode("utf-8")
     ).decode("utf-8")
-    epoch = datetime.utcfromtimestamp(0)
+    epoch = datetime.fromtimestamp(0, timezone.utc)
     expiration_timestamp = int((expiration_time - epoch).total_seconds())
     decoded_key = base64.urlsafe_b64decode(base64_key)
 
@@ -127,15 +127,15 @@ def sign_cookie(
         url_prefix: URL prefix to sign.
         key_name: name of the signing key.
         base64_key: signing key as a base64 encoded string.
-        expiration_time: expiration time.
+        expiration_time: expiration time as time-zone aware datetime.
 
     Returns:
         Returns the Cloud-CDN-Cookie value based on the specified configuration.
     """
     encoded_url_prefix = base64.urlsafe_b64encode(
         url_prefix.strip().encode("utf-8")
     ).decode("utf-8")
-    epoch = datetime.utcfromtimestamp(0)
+    epoch = datetime.fromtimestamp(0, timezone.utc)
     expiration_timestamp = int((expiration_time - epoch).total_seconds())
     decoded_key = base64.urlsafe_b64decode(base64_key)
 
@@ -167,7 +167,7 @@ def sign_cookie(
     sign_url_parser.add_argument("base64_key", help="The base64 encoded signing key.")
     sign_url_parser.add_argument(
         "expiration_time",
-        type=lambda d: datetime.utcfromtimestamp(float(d)),
+        type=lambda d: datetime.fromtimestamp(float(d), timezone.utc),
         help="Expiration time expessed as seconds since the epoch.",
     )
 
@@ -185,7 +185,7 @@ def sign_cookie(
     )
     sign_url_prefix_parser.add_argument(
         "expiration_time",
-        type=lambda d: datetime.utcfromtimestamp(float(d)),
+        type=lambda d: datetime.fromtimestamp(float(d), timezone.utc),
         help="Expiration time expessed as seconds since the epoch.",
     )
 
@@ -200,7 +200,7 @@ def sign_cookie(
     )
     sign_cookie_parser.add_argument(
         "expiration_time",
-        type=lambda d: datetime.utcfromtimestamp(float(d)),
+        type=lambda d: datetime.fromtimestamp(float(d), timezone.utc),
         help="Expiration time expressed as seconds since the epoch.",
     )
 

cdn/snippets_test.py

@@ -18,6 +18,8 @@
 
 import datetime
 
+import pytest
+
 import snippets
 
 
@@ -27,7 +29,7 @@ def test_sign_url() -> None:
             "http://35.186.234.33/index.html",
             "my-key",
             "nZtRohdNF9m3cKM24IcK4w==",
-            datetime.datetime.utcfromtimestamp(1549751401),
+            datetime.datetime.fromtimestamp(1549751401, datetime.timezone.utc),
         )
         == "http://35.186.234.33/index.html?Expires=1549751401&KeyName=my-key&Signature=CRFqQnVfFyiUyR63OQf-HRUpIwc="
     )
@@ -37,7 +39,7 @@ def test_sign_url() -> None:
             "http://www.example.com/",
             "my-key",
             "nZtRohdNF9m3cKM24IcK4w==",
-            datetime.datetime.utcfromtimestamp(1549751401),
+            datetime.datetime.fromtimestamp(1549751401, datetime.timezone.utc),
         )
         == "http://www.example.com/?Expires=1549751401&KeyName=my-key&Signature=OqDUFfHpN5Vxga6r80bhsgxKves="
     )
@@ -46,19 +48,36 @@ def test_sign_url() -> None:
             "http://www.example.com/some/path?some=query&another=param",
             "my-key",
             "nZtRohdNF9m3cKM24IcK4w==",
-            datetime.datetime.utcfromtimestamp(1549751401),
+            datetime.datetime.fromtimestamp(1549751401, datetime.timezone.utc),
         )
         == "http://www.example.com/some/path?some=query&another=param&Expires=1549751401&KeyName=my-key&Signature=9Q9TCxSju8-W5nUkk5CuTrun2_o="
     )
 
 
+def test_sign_url_raise_exception_on_naive_expiration_datetime() -> None:
+    with pytest.raises(TypeError):
+        snippets.sign_url(
+            "http://www.example.com/some/path?some=query&another=param",
+            "my-key",
+            "nZtRohdNF9m3cKM24IcK4w==",
+            datetime.datetime.fromtimestamp(1549751401),
+        )
+    with pytest.raises(TypeError):
+        snippets.sign_url(
+            "http://www.example.com/some/path?some=query&another=param",
+            "my-key",
+            "nZtRohdNF9m3cKM24IcK4w==",
+            datetime.datetime.utcfromtimestamp(1549751401),
+        )
+
+
 def test_sign_url_prefix() -> None:
     assert snippets.sign_url_prefix(
         "http://35.186.234.33/index.html",
         "http://35.186.234.33/",
         "my-key",
         "nZtRohdNF9m3cKM24IcK4w==",
-        datetime.datetime.utcfromtimestamp(1549751401),
+        datetime.datetime.fromtimestamp(1549751401, datetime.timezone.utc),
     ) == (
         "http://35.186.234.33/index.html?URLPrefix=aHR0cDovLzM1LjE4Ni4yMzQuMzMv&"
         "Expires=1549751401&KeyName=my-key&Signature=j7HYgoQ8dIOVsW3Rw4cpkjWfRMA="
@@ -68,7 +87,7 @@ def test_sign_url_prefix() -> None:
         "http://www.example.com/",
         "my-key",
         "nZtRohdNF9m3cKM24IcK4w==",
-        datetime.datetime.utcfromtimestamp(1549751401),
+        datetime.datetime.fromtimestamp(1549751401, datetime.timezone.utc),
     ) == (
         "http://www.example.com/?URLPrefix=aHR0cDovL3d3dy5leGFtcGxlLmNvbS8=&"
         "Expires=1549751401&KeyName=my-key&Signature=UdT5nVks6Hh8QFMJI9kmXuXYBk0="
@@ -78,21 +97,40 @@ def test_sign_url_prefix() -> None:
         "http://www.example.com/some/",
         "my-key",
         "nZtRohdNF9m3cKM24IcK4w==",
-        datetime.datetime.utcfromtimestamp(1549751401),
+        datetime.datetime.fromtimestamp(1549751401, datetime.timezone.utc),
     ) == (
         "http://www.example.com/some/path?some=query&another=param&"
         "URLPrefix=aHR0cDovL3d3dy5leGFtcGxlLmNvbS9zb21lLw==&"
         "Expires=1549751401&KeyName=my-key&Signature=3th4ThmpS95I1TAKYyYSCSq3dnQ="
     )
 
 
+def test_sign_url_prefix_raise_exception_on_naive_expiration_datetime() -> None:
+    with pytest.raises(TypeError):
+        snippets.sign_url_prefix(
+            "http://www.example.com/some/path?some=query&another=param",
+            "http://www.example.com/some/",
+            "my-key",
+            "nZtRohdNF9m3cKM24IcK4w==",
+            datetime.datetime.fromtimestamp(1549751401),
+        )
+    with pytest.raises(TypeError):
+        snippets.sign_url_prefix(
+            "http://www.example.com/some/path?some=query&another=param",
+            "http://www.example.com/some/",
+            "my-key",
+            "nZtRohdNF9m3cKM24IcK4w==",
+            datetime.datetime.utcfromtimestamp(1549751401),
+        )
+
+
 def test_sign_cookie() -> None:
     assert (
         snippets.sign_cookie(
             "http://35.186.234.33/index.html",
             "my-key",
             "nZtRohdNF9m3cKM24IcK4w==",
-            datetime.datetime.utcfromtimestamp(1549751401),
+            datetime.datetime.fromtimestamp(1549751401, datetime.timezone.utc),
         )
         == "Cloud-CDN-Cookie=URLPrefix=aHR0cDovLzM1LjE4Ni4yMzQuMzMvaW5kZXguaHRtbA==:Expires=1549751401:KeyName=my-key:Signature=uImwlOBCPs91mlCyG9vyyZRrNWU="
     )
@@ -102,7 +140,24 @@ def test_sign_cookie() -> None:
             "http://www.example.com/foo/",
             "my-key",
             "nZtRohdNF9m3cKM24IcK4w==",
-            datetime.datetime.utcfromtimestamp(1549751401),
+            datetime.datetime.fromtimestamp(1549751401, datetime.timezone.utc),
         )
         == "Cloud-CDN-Cookie=URLPrefix=aHR0cDovL3d3dy5leGFtcGxlLmNvbS9mb28v:Expires=1549751401:KeyName=my-key:Signature=Z9uYAu73YHioRScZDxnP-TnS274="
     )
+
+
+def test_sign_cookie_raise_exception_on_naive_expiration_datetime() -> None:
+    with pytest.raises(TypeError):
+        snippets.sign_cookie(
+            "http://www.example.com/foo/",
+            "my-key",
+            "nZtRohdNF9m3cKM24IcK4w==",
+            datetime.datetime.fromtimestamp(1549751401),
+        )
+    with pytest.raises(TypeError):
+        snippets.sign_cookie(
+            "http://www.example.com/foo/",
+            "my-key",
+            "nZtRohdNF9m3cKM24IcK4w==",
+            datetime.datetime.utcfromtimestamp(1549751401),
+        )