xbmc / metadata.themoviedb.org.python

Other
44 stars 41 forks source link

SSL Certificate Verication Failed Error on Xbox with v19 nightlie (v19 not released yet on Xbox) #84

Closed thexai closed 3 years ago

thexai commented 3 years ago

Currently this addon does not work on Xbox due SSL certificates verification issue:

SSL_CERTIFICATE_VERIFICATION_ERROR

On Xbox and probably other systems that do not have access to the "trusted root store" from OS it is necessary to specify a valid path to the CA cert bundle. It is the same issue that has been solved recently for CurlFile.cpp (https://github.com/xbmc/xbmc/issues/19665 | https://github.com/xbmc/xbmc/pull/19861) that now also needs to be addressed for addons (Python 3). Supposedly Python 2 did not check the validity of the certificates by default and therefore this problem did not exist before.

Fortunately it seems pretty easy to fix:

import ssl and create default context with:

CONTEXT = ssl.create_default_context()

use load_verify_locations() and pass relative path to kodi build-in CA certs bundle

CONTEXT.load_verify_locations(cafile='system/certs/cacert.pem')

Use SSL context in ulropen

response = urlopen(req, context=CONTEXT)

I simulated Xbox behavior with MSYS2 on Windows 10 and this code that emulates api_utils.py from this addon:

 try:
    import ssl
    from urllib2 import Request, urlopen
    from urllib2 import URLError
except ImportError:
    import ssl
    from urllib.request import Request, urlopen
    from urllib.error import URLError

HEADERS = {}
theerror = ''

CONTEXT = ssl.create_default_context()
CONTEXT.load_verify_locations(cafile='certs/cacert.pem')

req = Request('https://www.google.com', headers=HEADERS)

try:
    response = urlopen(req, context=CONTEXT)
except URLError as e:
    if hasattr(e, 'reason'):
        theerror = {'error': 'failed to reach the remote site\nReason: {}'.format(e.reason)}
    elif hasattr(e, 'code'):
        theerror = {'error': 'remote site unable to fulfill the request\nError code: {}'.format(e.code)}
    print(theerror)

resp = response.read()

print (resp)

urlopen without SSL context fails with same error that addon on Xbox (because MSYS2 also does not have access by default to trusted CA store).

Once added SSL context lines is fixed and prints correct URL response.

At the moment I have not wanted to push a PR because I think the people who habitually maintain Kodi addons will do it better (my knowledge of Python is pretty basic).

Maybe some more checking would also be missing here: for example running the code as it is currently and add SSL context only if the request fails with an SSL related error. The reason is that in Windows 10 as it is now it is already fine because the SSL certificates of the OS are used which is probably better because they will always be more up-to-date.

fuzzard commented 3 years ago

I wonder if the SSL setup could/should be done in core, as I assume this would effect every add-on.

Maybe in the preamble https://github.com/xbmc/xbmc/blob/43dc535375ec3fcbd6ac4d66452c876e7c7d4dd1/xbmc/interfaces/python/AddonPythonInvoker.cpp#L19

That gets done for every python script, so not sure if it would cause problems with multiple invocation though.

thexai commented 3 years ago

I assume this would effect every add-on.

Not all addons are affected: versioncheck seems works fine and The Movie DB scraper (no Python) too. Probably only affected these that use urllib2 ?

This code can be used to load custom certs only if fail to load default system's certs:

CONTEXT = ssl.create_default_context()
if len(CONTEXT.get_ca_certs()) == 0:
  CONTEXT.load_verify_locations(cafile='system/certs/cacert.pem')

On Windows 10 (desktop) ssl.create_default_context() uses system's default CA certs trust store and load_verify_locations() only is executed if no others certs are loaded. https://docs.python.org/3/library/ssl.html#ssl.create_default_context

This code works fine on both Windows 10 and MSYS2.

Then only is need add context=CONTEXT on every urlopen call as SSL context is reusable.

rmrector commented 3 years ago

This is a system level problem, it is not appropriate to work around this in every affected Python add-on.

This article from RedHat, Certificate verification in Python standard library HTTP clients, suggests an environment variable SSL_CERT_FILE that can be set before invoking Python. This should be set before Kodi core invokes Python and the add-on.

FYI, the other unaffected add-ons are likely using the "requests" Python module, which by default avoids the system CA cert store anyway, bundling or depending on a more consistent set.

thexai commented 3 years ago

SSL_CERT_FILE env var has worked with my MSYS2 test install. Even can set it in Windows and propagates it inside MSYS2.

Unfortunately, it doesn't seem to work on Xbox. I have to do more tests... maybe a test add-on to see the effective environment variables that exist from Python side.