Fix key info matching behavior (#287)

Co-authored-by: Andrey Kislyuk <kislyuk@openai.com>

Author: kislyuk

signxml/verifier.py

@@ -82,9 +82,9 @@ class SignatureConfiguration:
     Ignore the presence of a KeyValue element when X509Data is present in the signature and used for verifying.
     The presence of both elements is an ambiguity and a security hazard. The public key used to sign the
     document is already encoded in the certificate (which is in X509Data), so the verifier must either ignore
-    KeyValue or make sure it matches what's in the certificate. SignXML does not implement the functionality
-    necessary to match the keys, and throws an InvalidInput error instead. Set this to True to bypass the error
-    and validate the signature using X509Data only.
+    KeyValue or make sure it matches what's in the certificate. When set to ``False``, SignXML compares KeyValue
+    (and DEREncodedKeyValue) against the X.509 certificate and raises InvalidInput on mismatch. Set this to
+    ``True`` to bypass the check and validate the signature using X509Data only.
     """
 
     default_reference_c14n_method: CanonicalizationMethod = CanonicalizationMethod.CANONICAL_XML_1_1
@@ -261,7 +261,7 @@ def get_cert_chain_verifier(self, ca_pem_file):
         return X509CertChainVerifier(ca_pem_file=ca_pem_file)
 
     def _match_key_values(self, key_value, der_encoded_key_value, signing_cert, signature_alg):
-        if self.config.ignore_ambiguous_key_info is False:
+        if self.config.ignore_ambiguous_key_info is True:
             return
         cert_pub_key = signing_cert.public_key()
         # If both X509Data and KeyValue are present, match one against the other and raise an error on mismatch

test/test.py

@@ -648,6 +648,26 @@ def _build_transforms_for_reference(transforms_node, reference, exclude_c14n_tra
     def test_excision_of_untrusted_comments(self):
         pass  # TODO: test comments excision
 
+    def test_mismatched_key_value_with_x509_data(self):
+        crt, key = self.load_example_keys()
+        data = etree.parse(os.path.join(os.path.dirname(__file__), "example.xml")).getroot()
+        signer = XMLSigner()
+        signed = signer.sign(data, key=key, cert=crt, always_add_key_value=True)
+        key_info = signed.find(".//ds:KeyInfo", namespaces=namespaces)
+        key_value = key_info.find("ds:KeyValue", namespaces=namespaces)
+        key_info.remove(key_value)
+        mismatched_key = rsa.generate_private_key(public_exponent=65537, key_size=2048)
+        signer._serialize_key_value(mismatched_key, key_info)
+        signed_xml = etree.tostring(signed)
+
+        with self.assertRaisesRegex(
+            InvalidInput,
+            "Both X509Data and KeyValue found and they represent different public keys",
+        ):
+            XMLVerifier().verify(signed_xml, x509_cert=crt)
+
+        XMLVerifier().verify(signed_xml, x509_cert=crt, ignore_ambiguous_key_info=True)
+
     def test_ws_security(self):
         wsse_dir = os.path.join(interop_dir, "ws-security", "ws.js")
         with open(os.path.join(wsse_dir, "examples", "server_public.pem"), "rb") as fh: