Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[python] Check version of cryptography module #14512

Closed
wants to merge 1 commit into from

Conversation

yol
Copy link
Member

@yol yol commented Oct 3, 2018

We have had multiple issues with Ubuntu 16.04 using an outdated version
of the Python cryptography module that causes strange SSL behavior.
Example for this is not being able to connect to SSL sites.

This commit introduces a check on initialization of the interpreter
to make sure that we use a compatible version.

See also:
pyca/pyopenssl#542 (comment)
https://forum.kodi.tv/showthread.php?tid=335786

I just put the check inside the interpreter initialization - not sure if it's the most appropriate place? Also I don't have much experience with Python so please check.

How Has This Been Tested?

Build/Run on Linux x64

Screenshots (if appropriate):

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • None of the above (please explain below)

@yol yol added Type: Fix non-breaking change which fixes an issue Component: Python v18 Leia labels Oct 3, 2018
@yol yol self-assigned this Oct 3, 2018
@yol yol requested review from wsnipex and notspiff October 3, 2018 14:09
We have had multiple issues with Ubuntu 16.04 using an outdated version
of the Python cryptography module that causes strange SSL behavior.
Example for this is not being able to connect to SSL sites.

This commit introduces a check on initialization of the interpreter
to make sure that we use a compatible version.

See also:
pyca/pyopenssl#542 (comment)
https://forum.kodi.tv/showthread.php?tid=335786
@yol yol force-pushed the python-check-cryptography branch from 52e5158 to 557faa5 Compare October 3, 2018 14:14
@wsnipex
Copy link
Member

wsnipex commented Oct 3, 2018

I don't think that failing to initialize python because of this is a good idea.
Just checked what pushing a fixed version to our PPA encompasses and - as expected - python-cryptography has several dependencies that need to either be bumped or don't exist at all in ubuntu 16.04

@yol
Copy link
Member Author

yol commented Oct 3, 2018

What's the alternative really? It reproducibly breaks SSL (for some users at least?), meaning that even core functionality (such as installing add-ons) will not be available.

@wsnipex
Copy link
Member

wsnipex commented Oct 4, 2018

can we just log and throw up a gui error?

@yol
Copy link
Member Author

yol commented Oct 5, 2018

Yes, but then we probably have to go about it differently - we'd need to do the check once at startup, or the user will get spammed all the time. I put it where it is now because there the interpreter is already initialized for sure. Any suggestions where we could place it otherwise? Or save a flag to only warn once?

Is it OK to warn the user once at startup? And do you mean like a toast-style notification or a full message box?

@yol
Copy link
Member Author

yol commented Oct 14, 2018

@notspiff Do you have a suggestion how we could easily show a message box at startup? To use PythonInvoker, it seems I'd have to put the script into a file. Would it be possible/reasonable to make a small add-on that just checks the version once at startup?

@wsnipex
Copy link
Member

wsnipex commented Oct 16, 2018

maybe in the version check addon? https://github.com/xbmc/xbmc/tree/master/addons/service.xbmc.versioncheck

although it can be disabled, at least on a fresh install, it should be enabled.

@yol
Copy link
Member Author

yol commented Oct 18, 2018

Although it's called versioncheck, it's doing something completely different (check Kodi version). I'm not sure whether putting that check in there alongside would be a good idea.

@MartijnKaijser ?

@MartijnKaijser
Copy link
Member

I'm fine with adding it

@MartijnKaijser MartijnKaijser added On hold PR that is not currently being worked on (e.g. waiting for a 3rd-party release or feedback) and removed v18 Leia labels Oct 28, 2018
@yol
Copy link
Member Author

yol commented Oct 30, 2018

Superseded by XBMC-Addons/service.xbmc.versioncheck#18

@yol yol closed this Oct 30, 2018
@yol yol deleted the python-check-cryptography branch October 30, 2018 16:53
@yol yol removed the On hold PR that is not currently being worked on (e.g. waiting for a 3rd-party release or feedback) label Oct 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Python Type: Fix non-breaking change which fixes an issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants