Fix #1434: Skip hash check of root certificate if not specified in co…

…nfig (#1443)

* Upgrade to python-autograph-utils 0.3.0

* Make root_hash optional in signature checks parameters

* Fix #1434: Skip hash check of root certificate if not specified in config

checks/normandy/recipe_signatures.py

@@ -7,6 +7,7 @@
 import json
 import logging
 import random
+from typing import Optional
 
 from autograph_utils import MemoryCache, SignatureVerifier, decode_mozilla_hash
 
@@ -31,14 +32,19 @@ async def validate_signature(verifier, recipe):
     return await verifier.verify(data, signature, x5u)
 
 
-async def run(server: str, collection: str, root_hash: str) -> CheckResult:
+async def run(
+    server: str, collection: str, root_hash: Optional[str] = None
+) -> CheckResult:
     """Fetch recipes from Remote Settings and verify that each attached signature
     is verified with the related recipe attributes.
 
     :param server: URL of Remote Settings server.
     :param collection: Collection id to obtain recipes from (eg. ``"normandy-recipes"``.
     :param root_hash: The expected hash for the first certificate in a chain.
     """
+    root_hash_bytes: Optional[bytes] = (
+        decode_mozilla_hash(root_hash) if root_hash else None
+    )
     expected = random.randint(999999000000, 999999999999)
     resp = await fetch_json(
         RECIPES_URL.format(server=server, collection=collection, expected=expected)
@@ -50,7 +56,7 @@ async def run(server: str, collection: str, root_hash: str) -> CheckResult:
     errors = {}
 
     async with ClientSession() as session:
-        verifier = SignatureVerifier(session, cache, decode_mozilla_hash(root_hash))
+        verifier = SignatureVerifier(session, cache, root_hash_bytes)
 
         for recipe in recipes:
             try:

checks/remotesettings/validate_signatures.py

@@ -7,7 +7,7 @@
 import logging
 import operator
 import time
-from typing import List
+from typing import List, Optional
 
 import canonicaljson
 from autograph_utils import (
@@ -44,7 +44,12 @@ async def validate_signature(verifier, metadata, records, timestamp):
     return await verifier.verify(data, signature, x5u)
 
 
-async def run(server: str, buckets: List[str], root_hash: str) -> CheckResult:
+async def run(
+    server: str, buckets: List[str], root_hash: Optional[str] = None
+) -> CheckResult:
+    root_hash_bytes: Optional[bytes] = (
+        decode_mozilla_hash(root_hash) if root_hash else None
+    )
     client = KintoClient(server_url=server)
     entries = [
         entry
@@ -67,7 +72,7 @@ async def run(server: str, buckets: List[str], root_hash: str) -> CheckResult:
     cache = MemoryCache()
 
     async with ClientSession() as session:
-        verifier = SignatureVerifier(session, cache, decode_mozilla_hash(root_hash))
+        verifier = SignatureVerifier(session, cache, root_hash=root_hash_bytes)
 
         # Validate signatures sequentially.
         errors = {}

pyproject.toml

@@ -44,7 +44,7 @@ cryptography = "^42.0.7"
 websockets = "^12.0"
 requests = "^2.31.0"
 beautifulsoup4 = "^4.12.3"
-autograph-utils = "^0.2.0"
+autograph-utils = "^0.3.0"
 canonicaljson-rs = "^0.6.0"
 
 [tool.poetry.group.taskcluster.dependencies]

tests/checks/normandy/test_recipe_signatures.py

@@ -58,7 +58,7 @@ async def test_positive(mock_aioresponses, mock_randint):
     )
 
     with patch_async(f"{MODULE}.validate_signature", return_value=True):
-        status, data = await run(server_url, "normandy-recipes", root_hash="AA")
+        status, data = await run(server_url, "normandy-recipes")
 
     assert status is True
     assert data == {}
@@ -83,7 +83,7 @@ async def test_negative(mock_aioresponses, mock_randint):
     )
 
     with patch_async(f"{MODULE}.validate_signature", side_effect=ValueError("boom")):
-        status, data = await run(server_url, "normandy-recipes", root_hash="AA")
+        status, data = await run(server_url, "normandy-recipes")
 
     assert status is False
     assert data == {"12": "ValueError('boom')"}
@@ -94,7 +94,7 @@ async def test_invalid_x5u(mock_aioresponses):
     mock_aioresponses.get(x5u, body=CERT)
     cache = autograph_utils.MemoryCache()
     async with ClientSession() as session:
-        verifier = autograph_utils.SignatureVerifier(session, cache, "fake-hash")
+        verifier = autograph_utils.SignatureVerifier(session, cache, root_hash=None)
 
         recipe = {
             "signature": {"signature": "abc", "x5u": x5u},

tests/checks/remotesettings/test_validate_signatures.py

@@ -1,3 +1,5 @@
+from unittest import mock
+
 import pytest
 from aiohttp import ClientResponseError
 
@@ -49,7 +51,7 @@ async def test_positive(mock_responses):
     )
 
     with patch_async(f"{MODULE}.validate_signature"):
-        status, data = await run(server_url, ["bid"], root_hash="AA")
+        status, data = await run(server_url, ["bid"])
 
     assert status is True
     assert data == {}
@@ -77,14 +79,42 @@ async def test_negative(mock_responses, mock_aioresponses):
         },
     )
 
-    status, data = await run(server_url, ["bid"], root_hash="AA")
+    status, data = await run(server_url, ["bid"])
 
     assert status is False
     assert data == {
         "bid/cid": "CertificateExpired(datetime.datetime(2019, 11, 11, 22, 44, 31))"
     }
 
 
+async def test_root_hash_is_decoded_if_specified(mock_responses, mock_aioresponses):
+    server_url = "http://fake.local/v1"
+    changes_url = server_url + RECORDS_URL.format("monitor", "changes")
+    mock_responses.get(
+        changes_url,
+        payload={
+            "data": [
+                {"id": "abc", "bucket": "bid", "collection": "cid", "last_modified": 42}
+            ]
+        },
+    )
+    mock_responses.get(
+        server_url + CHANGESET_URL.format("bid", "cid"),
+        payload={
+            "metadata": {"signature": {"x5u": "http://fake-x5u-url/", "signature": ""}},
+            "changes": [],
+            "timestamp": 42,
+        },
+    )
+
+    with mock.patch(f"{MODULE}.SignatureVerifier") as mocked:
+        with mock.patch(f"{MODULE}.validate_signature"):
+            await run(server_url, ["bid"], root_hash="00:FF")
+
+    [[_, kwargs]] = mocked.call_args_list
+    assert kwargs["root_hash"] == b"\x00\xff"
+
+
 async def test_missing_signature():
     with pytest.raises(AssertionError) as exc_info:
         await validate_signature(verifier=None, metadata={}, records=[], timestamp=42)
@@ -112,7 +142,7 @@ async def test_retry_fetch_records(mock_responses):
     )
 
     with patch_async(f"{MODULE}.validate_signature"):
-        status, data = await run(server_url, ["bid"], root_hash="AA")
+        status, data = await run(server_url, ["bid"])
 
     assert status is True
 
@@ -142,7 +172,7 @@ async def test_retry_fetch_x5u(mock_responses, mock_aioresponses):
         },
     )
 
-    status, data = await run(server_url, ["bid"], root_hash="AA")
+    status, data = await run(server_url, ["bid"])
 
     assert status is False
     # Here we can see that it fails for other reasons than x5u.
@@ -177,4 +207,4 @@ async def test_unexpected_error_raises(mock_responses, mock_aioresponses):
     )
 
     with pytest.raises(ClientResponseError):
-        await run(server_url, ["bid"], root_hash="AA")
+        await run(server_url, ["bid"])