home-assistant / core

:house_with_garden: Open source home automation that puts local control and privacy first.
https://www.home-assistant.io
Apache License 2.0
69.88k stars 28.98k forks source link

Amcrest integration - Login error if HTTPS is enabled #67526

Open dffffffff opened 2 years ago

dffffffff commented 2 years ago

The problem

Integrations fails to connect to camera if HTTPS is enabled, if HTTPS is disabled the integration can login and functions as expected.

Camera Software Version V2.800.00AC001.0.R, Build Date: 2020-09-04 WEB Version V3.2.1.880675 ONVIF Version 19.06(V2.6.1.845551)

What version of Home Assistant Core has the issue?

Home Assistant 2022.3.0

What was the last working version of Home Assistant Core?

Home Assistant 2022.1

What type of installation are you running?

Home Assistant Container

Integration causing the issue

Amcrest

Link to integration documentation on our website

https://www.home-assistant.io/integrations/amcrest/

Diagnostics information

No response

Example YAML snippet

amcrest:
   host: hostname.removed.net
   name: driveway
   username: username
   password: password
   stream_source: rtsp
   binary_sensors:
    - motion_detected
    - online

Anything in the logs that might be useful for us?

2022-03-03 07:30:29 ERROR (MainThread) [homeassistant.components.amcrest] driveway camera offline: Login error:
2022-03-03 07:30:29 WARNING (MainThread) [homeassistant.components.amcrest] Error while processing events from driveway camera: LoginError()
2022-03-03 07:30:29 ERROR (MainThread) [homeassistant.components.amcrest.binary_sensor] Could not update driveway Motion Detected binary sensor due to error: LoginError
2022-03-03 07:30:30 ERROR (MainThread) [homeassistant.components.camera] Error while setting up amcrest platform for camera
Traceback (most recent call last):
  File "/usr/local/lib/python3.9/site-packages/amcrest/http.py", line 156, in _async_generate_token
    resp = (await self._async_command(cmd)).content.decode()
  File "/usr/local/lib/python3.9/site-packages/amcrest/http.py", line 351, in _async_command
    raise LoginError()
amcrest.exceptions.LoginError

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/helpers/entity_platform.py", line 249, in _async_setup_platform
    await asyncio.shield(task)
  File "/usr/src/homeassistant/homeassistant/components/amcrest/camera.py", line 143, in async_setup_platform
    serial_number = await device.api.async_serial_number
  File "/usr/local/lib/python3.9/site-packages/amcrest/system.py", line 134, in async_serial_number
    return pretty(await self._async_magic_box("getSerialNo"))
  File "/usr/local/lib/python3.9/site-packages/amcrest/http.py", line 471, in _async_magic_box
    ret = await self.async_command(f"magicBox.cgi?action={action}")
  File "/usr/src/homeassistant/homeassistant/components/amcrest/__init__.py", line 184, in async_command
    ret = await super().async_command(*args, **kwargs)
  File "/usr/local/lib/python3.9/site-packages/amcrest/http.py", line 229, in async_command
    await self._async_generate_token()
  File "/usr/local/lib/python3.9/site-packages/amcrest/http.py", line 162, in _async_generate_token
    resp = (await self._async_command(cmd)).content.decode()
  File "/usr/local/lib/python3.9/site-packages/amcrest/http.py", line 351, in _async_command
    raise LoginError()
amcrest.exceptions.LoginError

Additional information

No response

probot-home-assistant[bot] commented 2 years ago

amcrest documentation amcrest source (message by IssueLinks)

probot-home-assistant[bot] commented 2 years ago

Hey there @flacjacket, mind taking a look at this issue as it has been labeled with an integration (amcrest) you are listed as a code owner for? Thanks! (message by CodeOwnersMention)

dffffffff commented 2 years ago

hi @flacjacket, is this getting looked at? Since the switch from aiohttp to httpx ive found 2 critical bugs that broke the intergration for anyone using HTTPS over HTTP

flacjacket commented 2 years ago

Hi @dffffffff, I can try to see if I can reproduce this, but I may run into some issues with getting a self-signed certificate, but I think I can work around them. If you have an idea as to why this might be failing, that would be helpful. The login error is unfortunately not too helpful for us in this case.

dffffffff commented 2 years ago

Hi @flacjacket, im using a wildcard cert from letsencrypt. I reproduced the error with logging set to debug but there was no additional information im afraid.

wjbuys commented 2 years ago

I'm having the exact same issue; I do get a little more information in the logs (replaced my actual camera hostname with my-camera):

2022-03-10 12:10:14 DEBUG (MainThread) [amcrest.http] <Unconnected @ my-camera> Running query 26 attempt 1
2022-03-10 12:10:14 DEBUG (MainThread) [amcrest.http] <Unconnected @ my-camera> Query 26: Unauthorized (401)
2022-03-10 12:11:14 DEBUG (MainThread) [homeassistant.components.amcrest] Testing if Front Door back online
2022-03-10 12:11:14 DEBUG (MainThread) [amcrest.http] <Unconnected @ my-camera> Trying async Basic Authentication
2022-03-10 12:11:14 DEBUG (MainThread) [amcrest.http] <Unconnected @ my-camera> Async HTTP query 27: http://my-camera:80/cgi-bin/magicBox.cgi?action=getMachineName

That URL will issue a 302 redirect to HTTPS if I try to get it using curl; is it possible the httpx library is not providing the Auth headers across redirects?

wjbuys commented 2 years ago

FWIW, I've tried connecting to the camera using the latest version of python-amcrest's amcrest-cli directly, and this seems to work without issue; it does seem like this is specific to the HA integration:

   package amcrest 1.9.7, Python 3.8.6
    - amcrest-cli
    - amcrest-tui
$ amcrest-cli --software-information
('2.800.00AC001.0.R', '2020-09-04\r\n')
github-actions[bot] commented 2 years ago

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment πŸ‘ This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

wjbuys commented 2 years ago

Pretty sure this is still an issue, but will confirm and update logs.

pjatx commented 1 year ago

Any updates on this? Think I'm running into the same issue when setting up my camera.

Scags104 commented 1 year ago

im having the same issue trying to get this set up. just bought this camera to use with this integration.

rwjack commented 1 year ago

I'm guessing this is the next step for me, but how the hell did you guys manage to enable SSL with self-signed or Letsencrypt certs?

I create self signed PEM certs using mkcert, and when I press upload, I just get a green checkmark, but the certificate doesn't actually upload.

rwjack commented 1 year ago

Confirmed by amcrest support, self signed certs (apart from the one you generate on the camera) are not possible as of now.

But still, even with the one generated on the camera, I'm also having this issue. Camera doesn't work with https being enabled, and the port set to 443. The integration probably still tries to speak plain http to the https port.

issue-triage-workflows[bot] commented 1 year ago

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment πŸ‘ This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

rwjack commented 1 year ago

Not stale, this is still an active issue

issue-triage-workflows[bot] commented 10 months ago

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment πŸ‘ This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

rwjack commented 9 months ago

Not stale, this is still an active issue

issue-triage-workflows[bot] commented 6 months ago

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment πŸ‘ This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

rwjack commented 6 months ago

Still active

issue-triage-workflows[bot] commented 3 months ago

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment πŸ‘ This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

rwjack commented 3 months ago

Still an issue.On 14. 3. 2024., at 12:11, issue-triage-workflows[bot] @.***> wrote:ο»Ώ There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment πŸ‘ This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

β€”Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you commented.Message ID: @.***>

issue-triage-workflows[bot] commented 4 weeks ago

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment πŸ‘ This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

rwjack commented 4 weeks ago

active