Fix parsing of certificate issuer/subject with escaped special chars (#4557)

* keyinfo typing

* Fix parsing of certificate issuer/subject with escaped special characters

* tests

* rfc4514_attribute_name and multi value test

* pyca version + mypy happy dance

* aT lEaSt tTO sPAceS BeFOre iNLinE cOMment

* fix coverage

Co-authored-by: Maximilian Hils <github@maximilianhils.com>

CHANGELOG.md

@@ -63,6 +63,7 @@ If you depend on these features, please raise your voice in
 * Improve readability of SHA256 fingerprint. (@wrekone)
 * Metadata and Replay Flow Filters: Flows may be filtered based on metadata and replay status. (@rbdixon)
 * Flow control: don't read connection data faster than it can be forwarded. (@hazcod)
+* Fix parsing of certificate issuer/subject with escaped special characters (@Prinzhorn)
 * Customize markers with emoji, and filters: The `flow.mark` command may be used to mark a flow with either the default
   "red ball" marker, a single character, or an emoji like `:grapes:`. Use the `~marker` filter to filter on marker characters. (@rbdixon)
 * --- TODO: add new PRs above this line ---

mitmproxy/certs.py

@@ -145,8 +145,13 @@ class Cert(serializable.Serializable):
 
 def _name_to_keyval(name: x509.Name) -> List[Tuple[str, str]]:
     parts = []
-    for rdn in name.rdns:
-        k, v = rdn.rfc4514_string().split("=", maxsplit=1)
+    for attr in name:
+        # pyca cryptography <35.0.0 backwards compatiblity
+        if hasattr(name, "rfc4514_attribute_name"):  # pragma: no cover
+            k = attr.rfc4514_attribute_name  # type: ignore
+        else:  # pragma: no cover
+            k = attr.rfc4514_string().partition("=")[0]
+        v = attr.value
         parts.append((k, v))
     return parts
 

test/mitmproxy/net/data/text_cert_with_comma

@@ -0,0 +1,29 @@
+-----BEGIN CERTIFICATE-----
+MIIFBjCCBK2gAwIBAgIQDovzdw2S0Zbwu2H5PEFmvjAKBggqhkjOPQQDAjBnMQsw
+CQYDVQQGEwJVUzEXMBUGA1UEChMORGlnaUNlcnQsIEluYy4xPzA9BgNVBAMTNkRp
+Z2lDZXJ0IEhpZ2ggQXNzdXJhbmNlIFRMUyBIeWJyaWQgRUNDIFNIQTI1NiAyMDIw
+IENBMTAeFw0yMTAzMjUwMDAwMDBaFw0yMjAzMzAyMzU5NTlaMGYxCzAJBgNVBAYT
+AlVTMRMwEQYDVQQIEwpDYWxpZm9ybmlhMRYwFAYDVQQHEw1TYW4gRnJhbmNpc2Nv
+MRUwEwYDVQQKEwxHaXRIdWIsIEluYy4xEzARBgNVBAMTCmdpdGh1Yi5jb20wWTAT
+BgcqhkjOPQIBBggqhkjOPQMBBwNCAASt9vd1sdNJVApdEHG93CUGSyIcoiNOn6H+
+udCMvTm8DCPHz5GmkFrYRasDE77BI3q5xMidR/aW4Ll2a1A2ZvcNo4IDOjCCAzYw
+HwYDVR0jBBgwFoAUUGGmoNI1xBEqII0fD6xC8M0pz0swHQYDVR0OBBYEFCexfp+7
+JplQ2PPDU1v+MRawux5yMCUGA1UdEQQeMByCCmdpdGh1Yi5jb22CDnd3dy5naXRo
+dWIuY29tMA4GA1UdDwEB/wQEAwIHgDAdBgNVHSUEFjAUBggrBgEFBQcDAQYIKwYB
+BQUHAwIwgbEGA1UdHwSBqTCBpjBRoE+gTYZLaHR0cDovL2NybDMuZGlnaWNlcnQu
+Y29tL0RpZ2lDZXJ0SGlnaEFzc3VyYW5jZVRMU0h5YnJpZEVDQ1NIQTI1NjIwMjBD
+QTEuY3JsMFGgT6BNhktodHRwOi8vY3JsNC5kaWdpY2VydC5jb20vRGlnaUNlcnRI
+aWdoQXNzdXJhbmNlVExTSHlicmlkRUNDU0hBMjU2MjAyMENBMS5jcmwwPgYDVR0g
+BDcwNTAzBgZngQwBAgIwKTAnBggrBgEFBQcCARYbaHR0cDovL3d3dy5kaWdpY2Vy
+dC5jb20vQ1BTMIGSBggrBgEFBQcBAQSBhTCBgjAkBggrBgEFBQcwAYYYaHR0cDov
+L29jc3AuZGlnaWNlcnQuY29tMFoGCCsGAQUFBzAChk5odHRwOi8vY2FjZXJ0cy5k
+aWdpY2VydC5jb20vRGlnaUNlcnRIaWdoQXNzdXJhbmNlVExTSHlicmlkRUNDU0hB
+MjU2MjAyMENBMS5jcnQwDAYDVR0TAQH/BAIwADCCAQUGCisGAQQB1nkCBAIEgfYE
+gfMA8QB2ACl5vvCeOTkh8FZzn2Old+W+V32cYAr4+U1dJlwlXceEAAABeGq/vRoA
+AAQDAEcwRQIhAJ7miER//DRFnDJNn6uUhgau3WMt4vVfY5dGigulOdjXAiBIVCfR
+xjK1v4F31+sVaKzyyO7JAa0fzDQM7skQckSYWQB3ACJFRQdZVSRWlj+hL/H3bYbg
+IyZjrcBLf13Gg1xu4g8CAAABeGq/vTkAAAQDAEgwRgIhAJgAEkoJQRivBlwo7x67
+3oVsf1ip096WshZqmRCuL/JpAiEA3cX4rb3waLDLq4C48NSoUmcw56PwO/m2uwnQ
+prb+yh0wCgYIKoZIzj0EAwIDRwAwRAIgK+Kv7G+/KkWkNZg3PcQFp866Z7G6soxo
+a4etSZ+SRlYCIBSiXS20Wc+yjD111nPzvQUCfsP4+DKZ3K+2GKsERD6d
+-----END CERTIFICATE-----

test/mitmproxy/test_certs.py

@@ -1,5 +1,7 @@
 import os
 from pathlib import Path
+from cryptography import x509
+from cryptography.x509 import NameOID
 
 import pytest
 
@@ -231,3 +233,26 @@ class TestCert:
 
         with pytest.raises(TypeError):
             tstore.add_cert_file("encrypted-no-pass", Path(tdata.path("mitmproxy/data/mitmproxy.pem")), None)
+
+    def test_special_character(self, tdata):
+        with open(tdata.path("mitmproxy/net/data/text_cert_with_comma"), "rb") as f:
+            d = f.read()
+        c = certs.Cert.from_pem(d)
+
+        assert dict(c.issuer).get('O') == 'DigiCert, Inc.'
+        assert dict(c.subject).get('O') == 'GitHub, Inc.'
+
+    def test_multi_valued_rdns(self, tdata):
+        subject = x509.Name([
+            x509.RelativeDistinguishedName([
+                x509.NameAttribute(NameOID.TITLE, u'Test'),
+                x509.NameAttribute(NameOID.COMMON_NAME, u'Multivalue'),
+                x509.NameAttribute(NameOID.SURNAME, u'RDNs'),
+                x509.NameAttribute(NameOID.ORGANIZATION_NAME, u'TSLA'),
+            ]),
+            x509.RelativeDistinguishedName([
+                x509.NameAttribute(NameOID.ORGANIZATION_NAME, u'PyCA')
+            ]),
+        ])
+        expected = [('2.5.4.12', 'Test'), ('CN', 'Multivalue'), ('2.5.4.4', 'RDNs'), ('O', 'TSLA'), ('O', 'PyCA')]
+        assert(certs._name_to_keyval(subject)) == expected