From 97f32e4ec7e433d4d43cf4a668fbd1eaacf84e17 Mon Sep 17 00:00:00 2001
From: Remco Koopmans <me@remcokoopmans.com>
Date: Sun, 11 Aug 2024 13:47:53 +0200
Subject: [PATCH] More cover

---
 CHANGELOG.md                  |  4 +++
 cert_chain_resolver/models.py |  3 ++-
 tests/test_models.py          | 51 ++++++++++++++++++++++++++++++++++-
 3 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 18d52da..dfd718e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,9 @@
 # Changelog 
 
+## 1.3.1
+
+* is_issued_by now raises MissingCertProperty if no hash algorithm found
+
 ## 1.3.0
 
 New feature and sane defaults. for the CLI the root is now by default excluded, at first it would include it if it found one, but not all certificate authorities provide a link to their root in their certs. This resulted in sometimes a root to be included and othertimes not.
diff --git a/cert_chain_resolver/models.py b/cert_chain_resolver/models.py
index ac57429..9458a1d 100644
--- a/cert_chain_resolver/models.py
+++ b/cert_chain_resolver/models.py
@@ -7,6 +7,7 @@
 from cryptography.hazmat.primitives.asymmetric.rsa import RSAPublicKey
 from cryptography.hazmat.primitives.asymmetric.ec import ECDSA, EllipticCurvePublicKey
 from cryptography.hazmat.primitives.asymmetric.padding import PKCS1v15
+from cryptography.exceptions import InvalidSignature
 
 
 import binascii
@@ -204,7 +205,7 @@ def is_issued_by(self, other):
                     ECDSA(hash_algorithm),
                 )
                 return True
-        except Exception:
+        except InvalidSignature as e:
             pass
 
         return False
diff --git a/tests/test_models.py b/tests/test_models.py
index 77e994a..923856a 100644
--- a/tests/test_models.py
+++ b/tests/test_models.py
@@ -4,6 +4,11 @@
 from cert_chain_resolver.models import Cert, CertificateChain
 from cryptography.x509.oid import ExtensionOID, AuthorityInformationAccessOID, NameOID
 import pytest
+from cryptography.hazmat.primitives.asymmetric.rsa import RSAPublicKey
+from cryptography.hazmat.primitives.asymmetric.ec import ECDSA, EllipticCurvePublicKey
+from cryptography.hazmat.primitives import hashes
+from cryptography.hazmat.primitives.asymmetric.padding import PKCS1v15
+from cryptography.exceptions import InvalidSignature
 
 
 try:
@@ -12,7 +17,7 @@
     from contextlib import contextmanager
 
     @contextmanager
-    def does_not_raise():
+    def does_not_raise():  # type: ignore
         yield
 
 
@@ -22,6 +27,18 @@ def does_not_raise():
     unicode = str
 
 
+@pytest.fixture
+def mock_x509(mocker):
+    return mocker.Mock(spec=x509.Certificate)
+
+
+@pytest.fixture
+def mock_cert(mocker, mock_x509):
+    cert = mocker.Mock(spec=Cert)
+    cert._x509 = mock_x509
+    return cert
+
+
 @pytest.mark.parametrize("cert", CERT_FIXTURES, ids=certfixture_to_id)
 def test_certcontainer_x509_helper_props(cert):
     c = Cert(cert["cert_x509"])
@@ -190,3 +207,35 @@ class CertOverride(Cert):
     c = CertOverride()
 
     assert repr(c) == '<Cert common_name="CN" subject="Subject" issuer="Issuer">'
+
+
+@pytest.mark.parametrize(
+    "key_type,expected",
+    [
+        (RSAPublicKey, True),
+        (RSAPublicKey, False),
+        (EllipticCurvePublicKey, True),
+        (EllipticCurvePublicKey, False),
+        (object, False),  # Unexpected key type FIXME: Maybe this should raise??
+    ],
+)
+def test_is_issued_by_handles_different_keys(
+    mocker, mock_x509, mock_cert, key_type, expected
+):
+    mock_public_key = mocker.Mock(spec=key_type)
+    mock_x509.public_key.return_value = mock_public_key
+    mock_x509.signature_hash_algorithm = mocker.Mock(spec=hashes.SHA256)
+
+    subject = Cert(mock_x509)
+
+    if not expected and hasattr(key_type, "verify"):
+        mock_public_key.verify.side_effect = InvalidSignature()
+
+    assert subject.is_issued_by(mock_cert) is expected
+
+
+def test_is_issued_raises_when_no_signature_hash_algo(mock_x509, mock_cert):
+    mock_x509.signature_hash_algorithm = None
+    mock_x509.public_key = lambda: None
+    with pytest.raises(MissingCertProperty):
+        Cert(mock_x509).is_issued_by(mock_cert)