Closed johnlage closed 4 years ago
Did a little digging, the first exception occurs because the TLS implementation in pydle is incorrect and needs to be redone. Not sure what is going on with the second exception but the two are likely related.
the verify_tls
method invokes socket.getpeercert()
which explicitly cannot be called yet because the verify callback occurs before negotiation is complete as per the documentation.
Due to the early negotiation phase of the TLS connection, only limited methods and attributes are usable like SSLSocket.selected_alpn_protocol() and SSLSocket.context. SSLSocket.getpeercert(), SSLSocket.getpeercert(), SSLSocket.cipher() and SSLSocket.compress() methods require that the TLS connection has progressed beyond the TLS Client Hello and therefore will not contain return meaningful values nor can they be called safely.
Looks like #84 and #70 solve the problem by removing the validation step entirely. I am not comfortable with doing this, especially since one can pass verify_tls = False
to the Client.Connect()
method to disable validation.
As a workaround while I (or someone beating me to it) hopefully fixes the TLS implementation you can, for the moment, ask Pydle not to validate the TLS cert by passing tls_verify = False
to your client.connect(...)
call. Granted this isn't a secure route, but it will at least not cause Pydle to crash horribly.
@theunkn0wn1 Are you sure #84 doesn't fix this? It sets context.check_hostname = True
and additionally sets context.verify_mode = ssl.CERT_REQUIRED
, and according to the docs this is sufficient for verification:
Whether to match the peer cert’s hostname with match_hostname() in SSLSocket.do_handshake(). The context’s verify_mode must be set to CERT_OPTIONAL or CERT_REQUIRED, and you must pass server_hostname to wrap_socket() in order to match the hostname. Enabling hostname checking automatically sets verify_mode from CERT_NONE to CERT_REQUIRED. It cannot be set back to CERT_NONE as long as hostname checking is enabled. The PROTOCOL_TLS_CLIENT protocol enables hostname checking by default. With other protocols, hostname checking must be enabled explicitly.
Got curious and tested this myself: https://0x0.st/izWV.py results: https://0x0.st/izWW.txt
I think #84 is the right fix.
Interesting, that does look to do correctly exactly what the current implementation fails to do.
Sorry for the delay on this, Classes have been taking more time than expected :man_facepalming: .
I will verify #84 tonight (~12 hours) and prepare a release.
version 0.9.4rc1 is now released.
It errors out (on my system, Windows 10 x64) with the following tracebacks:
and
I notice 2 open PRs (#84 and #70) trying to fix it, but don't know for sure if they work. The ability to confirm the server's certs/TLS feels like it should either not fail horribly, or it shouldn't be there at all.