sethmlarson / truststore

Verify certificates using OS trust stores
https://truststore.readthedocs.io
MIT License
151 stars 18 forks source link

_windows.py - _verify_peercerts_impl: "IndexError: list index out of range" #128

Closed jpineaufr closed 4 months ago

jpineaufr commented 7 months ago

Hello,

Maybe I'm not using this library correctly with MQTTLibrary. But I prefer to inform you of my problem.

In the _api.py file, the _verify_peercerts function can initialize the cert_bytes variable as an empty array. This variable is assigned to the cert_chain parameter of the _verify_peercerts_impl function.

In a Windows env (_windows.py), the _verify_peercerts_impl uses the cert_chain as a non-empty array without check. If the cert_chain is empty, the function throws an exception: "IndexError: list index out of range"

For example to resolve this in my specific case, I use this in _windows.py (l. 322):

def _verify_peercerts_impl(
    ssl_context: ssl.SSLContext,
    cert_chain: list[bytes],
    server_hostname: str | None = None,
) -> None:
    """ If cert_chain is empty, no verification is required."""
    if not cert_chain:
        return
    """Verify the cert_chain from the server using Windows APIs."""
    pCertContext = None
   [...]

Regards

sethmlarson commented 7 months ago

Interesting! In this case it seems that the peer is returning no certificates, is that correct and expected?

jpineaufr commented 7 months ago

If I understand how the paho MQTT client works, this seems expected. I am facing this case only on the TLS connection to the MQTT broker.

This problem is not critical. This is just for information if anyone has the same situation.

twangboy commented 7 months ago

I am also seeing this. I'm trying to implement truststore into Salt and I'm seeing this error when Salt tries to download files.

sethmlarson commented 7 months ago

@twangboy Thanks for reporting this, are there any special circumstances you're using Truststore under that might return zero certificates in the handshake? I'll get a fix for this incorrect exception out.

twangboy commented 7 months ago

Yeah, I'm not that familiar with the code. I'm trying to figure out what we're doing...