dependency cryptography now raises Exceptions on initial parse, breaking on TLS test on malformed certs

[maertsen]

Just caught this unhandled exception in my logs. Not sure about impact, probably minor.

[2022-02-08 13:26:40,572: ERROR/MainProcess] Chord 'd9709f8d-d6f1-48e6-969f-1318d73e6e85' raised: ChordError("Dependency a1f78bd6-e955-4c83-8b07-fb1b939ae9ac raised InvalidVersion('3 is not a valid X509 version')")
Traceback (most recent call last):
  File "/usr/local/lib/python3.7/dist-packages/celery/app/trace.py", line 450, in trace_task
    R = retval = fun(*args, **kwargs)
  File "/usr/local/lib/python3.7/dist-packages/celery/app/trace.py", line 731, in __protected_call__
    return self.run(*args, **kwargs)
  File "/app/checks/tasks/tls.py", line 506, in batch_mail_smtp_starttls
    return do_mail_smtp_starttls(mailservers, url, self, *args, **kwargs)
  File "/app/checks/tasks/tls.py", line 1692, in do_mail_smtp_starttls
    server, dane_cb_data, task)
  File "/app/checks/tasks/tls.py", line 1750, in check_mail_tls
    conn.get_peer_certificate_chain())
  File "/app/checks/tasks/tls.py", line 1375, in __init__
    super(DebugCertChainMail, self).__init__(chain)
  File "/app/checks/tasks/tls.py", line 1228, in __init__
    for cert in chain
  File "/app/checks/tasks/tls.py", line 1228, in <listcomp>
    for cert in chain
  File "/usr/local/lib/python3.7/dist-packages/cryptography/x509/base.py", line 531, in load_pem_x509_certificate
    return rust_x509.load_pem_x509_certificate(data)
cryptography.x509.base.InvalidVersion: 3 is not a valid X509 version

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/lib/python3.7/dist-packages/celery/backends/redis.py", line 519, in on_chord_part_return
    resl = [unpack(tup, decode) for tup in resl]
  File "/usr/local/lib/python3.7/dist-packages/celery/backends/redis.py", line 519, in <listcomp>
    resl = [unpack(tup, decode) for tup in resl]
  File "/usr/local/lib/python3.7/dist-packages/celery/backends/redis.py", line 425, in _unpack_chord_result
    raise ChordError(f'Dependency {tid} raised {retval!r}')
celery.exceptions.ChordError: Dependency a1f78bd6-e955-4c83-8b07-fb1b939ae9ac raised InvalidVersion('3 is not a valid X509 version')
[2022-02-08 13:26:40,573: ERROR/MainProcess] checks.batch.scheduler.error_callback[None]: Task '1bf65f3b-7e6f-4757-99e1-f5b92ba7623d' raised error: ChordError("Dependency a1f78bd6-e955-4c83-8b07-fb1b939ae9ac raised InvalidVersion('3 is not a valid X509 version')")
[2022-02-08 13:26:40,611: ERROR/MainProcess] Task checks.tasks.tls.batch_mail_smtp_starttls[a1f78bd6-e955-4c83-8b07-fb1b939ae9ac] raised unexpected: InvalidVersion('3 is not a valid X509 version')
Traceback (most recent call last):
  File "/usr/local/lib/python3.7/dist-packages/celery/app/trace.py", line 450, in trace_task
    R = retval = fun(*args, **kwargs)
  File "/usr/local/lib/python3.7/dist-packages/celery/app/trace.py", line 731, in __protected_call__
    return self.run(*args, **kwargs)
  File "/app/checks/tasks/tls.py", line 506, in batch_mail_smtp_starttls
    return do_mail_smtp_starttls(mailservers, url, self, *args, **kwargs)
  File "/app/checks/tasks/tls.py", line 1692, in do_mail_smtp_starttls
    server, dane_cb_data, task)
  File "/app/checks/tasks/tls.py", line 1750, in check_mail_tls
    conn.get_peer_certificate_chain())
  File "/app/checks/tasks/tls.py", line 1375, in __init__
    super(DebugCertChainMail, self).__init__(chain)
  File "/app/checks/tasks/tls.py", line 1228, in __init__
    for cert in chain
  File "/app/checks/tasks/tls.py", line 1228, in <listcomp>
    for cert in chain
  File "/usr/local/lib/python3.7/dist-packages/cryptography/x509/base.py", line 531, in load_pem_x509_certificate
    return rust_x509.load_pem_x509_certificate(data)
cryptography.x509.base.InvalidVersion: 3 is not a valid X509 version

domain that triggered this is mookenmiddelaar.nl. It passes all the TLS tests except for DANE. I ran across this while investigating errors during a batch test run of #30.

[maertsen]

cert

user@dev-internetnl:~$ cat /tmp/ciscocloud.irvn.nl.pem 
-----BEGIN CERTIFICATE-----
MIID8jCCAtqgAwIBAwIJAIfPo4qHYr1IMA0GCSqGSIb3DQEBCwUAMIGPMQswCQYD
VQQGEwJOTDEbMBkGA1UEAwwSY2lzY29jbG91ZC5pcnZuLm5sMREwDwYDVQQHDAhO
aWptZWdlbjEeMBwGA1UECgwVTUdSIFJpamsgdmFuIE5pam1lZ2VuMRMwEQYDVQQI
DApHZWxkZXJsYW5kMRswGQYDVQQLDBJGdW5jdGlvbmVlbCBCZWhlZXIwHhcNMjIw
MTE4MTQzMzQ1WhcNMjcwMTE3MTQzMzQ1WjCBjzELMAkGA1UEBhMCTkwxGzAZBgNV
BAMMEmNpc2NvY2xvdWQuaXJ2bi5ubDERMA8GA1UEBwwITmlqbWVnZW4xHjAcBgNV
BAoMFU1HUiBSaWprIHZhbiBOaWptZWdlbjETMBEGA1UECAwKR2VsZGVybGFuZDEb
MBkGA1UECwwSRnVuY3Rpb25lZWwgQmVoZWVyMIIBIjANBgkqhkiG9w0BAQEFAAOC
AQ8AMIIBCgKCAQEAwGQqOBz8n/216oke1BQbhJED1upUUchsYpMCiagl/WgdNEBo
ajkLiSiLioSj3xATNGPep1zlS2i0uy9qbxndMEdQeUffkhkbU2vhqMwWFwUTeBHk
4CpIAJYgt9Y4TylUQgznUpXhiIMcnFuv2MBu11cz8XyIjAoXTReBpqAxIFyKJ+aZ
kPdYWDYCVTnv3b9KLAg6PKqQFcbflbZ6NC9uHTeh2bsqdfqp/3alnCugt5alxFti
l9hTIM/LmcVkXSP/5ywpTRRBhQDc7DQypUz12Jja2x/as4LSP0oiFo9S25n8VgR5
MmYbtdhbVTrS0Gvdq4ZaGWHnDZdhdu1hJvVBPwIDAQABo08wTTAjBgNVHREEHDAa
ggtteDEuaXJ2bi5ubIILbXgyLmlydm4ubmwwDgYDVR0PAQH/BAQDAgXgMBYGA1Ud
JQEB/wQMMAoGCCsGAQUFBwMEMA0GCSqGSIb3DQEBCwUAA4IBAQCiRs7wQtRO1Ioi
JIUqV47GMOA//UEMX4RlZT5tWBLkDg/sLFDLIyUBFEzuAZH7Hc5FTeGWdmBxbcUk
4hM2Im5IMYqAmyUbOJpnKDjm4PC8VsBmgQXxIIWS8bc8x+kbXcgbYPczs6yPNxDl
oPuiJgmf38PQE7A8+EhdIyyzVVbosThsOck1SEkr2DJ3L73WBNVdubLEtHEuQ4ML
rd8/G926rJad5cmIAPbkoNYXiZrJ7iyK95Kc5tlZ+nc1rQeQBEO2gOQuTS8eCS70
8lLhKu5nbW8CJF0gB7xYMxec+LN9NNosjkLLDjEt+idGacKUT1vGUZ/tXbtU60Yt
DhLsqIuH
-----END CERTIFICATE-----

openssl output

user@dev-internetnl:~$ openssl x509 -in /tmp/ciscocloud.irvn.nl.pem -noout -text
Certificate:
    Data:
        Version: Unknown (3)
        Serial Number:
            87:cf:a3:8a:87:62:bd:48
        Signature Algorithm: sha256WithRSAEncryption
        Issuer: C = NL, CN = ciscocloud.irvn.nl, L = Nijmegen, O = MGR Rijk van Nijmegen, ST = Gelderland, OU = Functioneel Beheer
        Validity
            Not Before: Jan 18 14:33:45 2022 GMT
            Not After : Jan 17 14:33:45 2027 GMT
        Subject: C = NL, CN = ciscocloud.irvn.nl, L = Nijmegen, O = MGR Rijk van Nijmegen, ST = Gelderland, OU = Functioneel Beheer
        Subject Public Key Info:
            Public Key Algorithm: rsaEncryption
                RSA Public-Key: (2048 bit)
                Modulus:
                    00:c0:64:2a:38:1c:fc:9f:fd:b5:ea:89:1e:d4:14:
                    1b:84:91:03:d6:ea:54:51:c8:6c:62:93:02:89:a8:
                    25:fd:68:1d:34:40:68:6a:39:0b:89:28:8b:8a:84:
                    a3:df:10:13:34:63:de:a7:5c:e5:4b:68:b4:bb:2f:
                    6a:6f:19:dd:30:47:50:79:47:df:92:19:1b:53:6b:
                    e1:a8:cc:16:17:05:13:78:11:e4:e0:2a:48:00:96:
                    20:b7:d6:38:4f:29:54:42:0c:e7:52:95:e1:88:83:
                    1c:9c:5b:af:d8:c0:6e:d7:57:33:f1:7c:88:8c:0a:
                    17:4d:17:81:a6:a0:31:20:5c:8a:27:e6:99:90:f7:
                    58:58:36:02:55:39:ef:dd:bf:4a:2c:08:3a:3c:aa:
                    90:15:c6:df:95:b6:7a:34:2f:6e:1d:37:a1:d9:bb:
                    2a:75:fa:a9:ff:76:a5:9c:2b:a0:b7:96:a5:c4:5b:
                    62:97:d8:53:20:cf:cb:99:c5:64:5d:23:ff:e7:2c:
                    29:4d:14:41:85:00:dc:ec:34:32:a5:4c:f5:d8:98:
                    da:db:1f:da:b3:82:d2:3f:4a:22:16:8f:52:db:99:
                    fc:56:04:79:32:66:1b:b5:d8:5b:55:3a:d2:d0:6b:
                    dd:ab:86:5a:19:61:e7:0d:97:61:76:ed:61:26:f5:
                    41:3f
                Exponent: 65537 (0x10001)
        X509v3 extensions:
            X509v3 Subject Alternative Name: 
                DNS:mx1.irvn.nl, DNS:mx2.irvn.nl
            X509v3 Key Usage: critical
                Digital Signature, Non Repudiation, Key Encipherment
            X509v3 Extended Key Usage: critical
                E-mail Protection
    Signature Algorithm: sha256WithRSAEncryption
         a2:46:ce:f0:42:d4:4e:d4:8a:22:24:85:2a:57:8e:c6:30:e0:
         3f:fd:41:0c:5f:84:65:65:3e:6d:58:12:e4:0e:0f:ec:2c:50:
         cb:23:25:01:14:4c:ee:01:91:fb:1d:ce:45:4d:e1:96:76:60:
         71:6d:c5:24:e2:13:36:22:6e:48:31:8a:80:9b:25:1b:38:9a:
         67:28:38:e6:e0:f0:bc:56:c0:66:81:05:f1:20:85:92:f1:b7:
         3c:c7:e9:1b:5d:c8:1b:60:f7:33:b3:ac:8f:37:10:e5:a0:fb:
         a2:26:09:9f:df:c3:d0:13:b0:3c:f8:48:5d:23:2c:b3:55:56:
         e8:b1:38:6c:39:c9:35:48:49:2b:d8:32:77:2f:bd:d6:04:d5:
         5d:b9:b2:c4:b4:71:2e:43:83:0b:ad:df:3f:1b:dd:ba:ac:96:
         9d:e5:c9:88:00:f6:e4:a0:d6:17:89:9a:c9:ee:2c:8a:f7:92:
         9c:e6:d9:59:fa:77:35:ad:07:90:04:43:b6:80:e4:2e:4d:2f:
         1e:09:2e:f4:f2:52:e1:2a:ee:67:6d:6f:02:24:5d:20:07:bc:
         58:33:17:9c:f8:b3:7d:34:da:2c:8e:42:cb:0e:31:2d:fa:27:
         46:69:c2:94:4f:5b:c6:51:9f:ed:5d:bb:54:eb:46:2d:0e:12:
         ec:a8:8b:87

stack trace

user@dev-internetnl:~$ python3
Python 3.9.2 (default, Feb 28 2021, 17:03:44) 
[GCC 10.2.1 20210110] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from cryptography import x509
>>> x509.load_pem_x509_certificate(open('/tmp/ciscocloud.irvn.nl.pem', 'rb').read())
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3/dist-packages/cryptography/x509/base.py", line 72, in load_pem_x509_certificate
    return backend.load_pem_x509_certificate(data)
  File "/usr/lib/python3/dist-packages/cryptography/hazmat/backends/openssl/backend.py", line 1378, in load_pem_x509_certificate
    return _Certificate(self, x509)
  File "/usr/lib/python3/dist-packages/cryptography/hazmat/backends/openssl/x509.py", line 40, in __init__
    raise x509.InvalidVersion(
cryptography.x509.base.InvalidVersion: 3 is not a valid X509 version

[maertsen]

Looking further into this, because it blocks my own progress:

• Current prod completes a test without apparent errors, though it flags the self-signed certificate https://internet.nl/mail/mookenmiddelaar.nl/667958/#.
• Current master throws exceptions, which lead me to assume that there has been some change in behaviour in either cryptography or underlying dependencies (e.g. rust_x509.

And indeed, this seems to be the case, quoting the cryptography changelog for version v35.0.0, released september last year:

BACKWARDS INCOMPATIBLE: Additional forms of invalid ASN.1 found during X.509 parsing will raise an error on initial parse rather than when the malformed field is accessed.

[maertsen]

@stitch did you run into this same issue in v1.4.3?

[maertsen]

diff --git a/checks/tasks/tls.py b/checks/tasks/tls.py
index 158d07c..eb0a9af 100644
--- a/checks/tasks/tls.py
+++ b/checks/tasks/tls.py
@@ -13,7 +13,7 @@ from timeit import default_timer as timer
 from celery import shared_task
 from celery.exceptions import SoftTimeLimitExceeded
 from cryptography.x509 import load_pem_x509_certificate, NameOID, ExtensionOID
-from cryptography.x509 import ExtensionNotFound, SignatureAlgorithmOID, DNSName
+from cryptography.x509 import ExtensionNotFound, InvalidVersion, SignatureAlgorithmOID, DNSName
 from cryptography.hazmat.backends import default_backend
 from cryptography.hazmat.backends.openssl.ec import _EllipticCurvePublicKey
 from cryptography.hazmat.backends.openssl.dh import _DHPublicKey
@@ -1599,7 +1599,7 @@ def cert_checks(
             debug_chain = starttls_details.debug_chain
             conn_port = starttls_details.conn_port
     except (socket.error, http.client.BadStatusLine, NoIpError,
-            ConnectionHandshakeException, ConnectionSocketException):
+            ConnectionHandshakeException, ConnectionSocketException, InvalidVersion):
         return dict(tls_cert=False)
 
     if debug_chain is None:

is what I'm using now as a workaround. It's not great, because it effectively nukes all cert tests and returns "Test error. Please try again later.", which is definitely not going to help any real user.

Either we pin cryptography to an old version (yuk), or fix this properly, which perhaps involves adding an extra result_ to categories.py? This is as far as I will take this, given my focus on #30.

[mxsasha]

is what I'm using now as a workaround. It's not great, because it effectively nukes all cert tests and returns "Test error. Please try again later.", which is definitely not going to help any real user.

While I agree, I think before we invest time in this, we should work out a way forward for #714, as that may affect whether this code will stay. This scenario seems rare enough that we can afford to leave it as is for a while longer.