10000 Suppress InsecureRequestWarning when verify_ssl is False for camera.synology by snjoetw · Pull Request #9777 · home-assistant/core · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Suppress InsecureRequestWarning when verify_ssl is False for camera.synology #9777

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

Closed
wants to merge 3 commits into from

Conversation

snjoetw
Copy link
Contributor
@snjoetw snjoetw commented Oct 9, 2017

Description:

camera.synology is spewing logs like this:

/usr/local/lib/python3.6/site-packages/requests/packages/urllib3/connectionpool.py:852: InsecureRequestWarning: Unverified HTTPS request is being made. Adding certificate verification is strongly advised. See: https://urllib3.readthedocs.io/en/latest/advanced-usage.html#ssl-warnings
  InsecureRequestWarning)

I'm guessing most users are running synology surveillance camera without valid SSL certificate, so this is annoying. The change made is that if user specifically turned off ssl_verify, then we should suppress InsecureRequestWarning as well

Checklist:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass

@@ -44,6 +44,11 @@ def async_setup_platform(hass, config, async_add_devices, discovery_info=None):
verify_ssl = config.get(CONF_VERIFY_SSL)
timeout = config.get(CONF_TIMEOUT)

if not verify_ssl:
from requests.packages import urllib3
_LOGGER.warning('InsecureRequestWarning is disabled for camera.synology platform')

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (90 > 79 characters)

@snjoetw
Copy link
Contributor Author
snjoetw commented Oct 9, 2017

Also updated CODEOWNERS file as suggested by @andrey-git in #9754

Copy link
Contributor
@sdague sdague left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is dangerous and something that you really want to avoid as it has global implications.

8000
if not verify_ssl:
from requests.packages import urllib3
_LOGGER.warning('InsecureRequestWarning is disabled for camera.synology platform')
urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem with this is you are disabling ALL warnings for urllib3 across the system. That means things like connections to other web services apis also get disabled. This is really not the right way to do that.

@pvizeli
Copy link
Member
pvizeli commented Oct 9, 2017

I close this while we want do that. Please change your synology settup to work with correct certificate or life with that warnings.

@pvizeli pvizeli closed this Oct 9, 2017
@home-assistant home-assistant locked and limited conversation to collaborators Mar 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants
0