tomaae / homeassistant-mikrotik_router

Mikrotik router integration for Home Assistant
Apache License 2.0
301 stars 50 forks source link

[Feature] Allow integration to run with limited access rights #235

Closed ondras12345 closed 2 years ago

ondras12345 commented 2 years ago

Describe the issue

I just updated from v2.0 to v2.1 and I started getting hundreds of these messages in the log:

2022-08-16 13:50:36.978 ERROR (SyncWorker_4) [custom_components.mikrotik_router.mikrotikapi] Mikrotik 192.168.1.1 error while execute : not enough permissions (9)
2022-08-16 13:50:37.116 WARNING (SyncWorker_4) [custom_components.mikrotik_router.mikrotikapi] Mikrotik Reconnected to 192.168.1.1

I believe this is because the user on my mikrotik device only has policy=read,api. However, I have done this on purpose to prevent full control of the router if an attacker gets hold of my Home Assistant instance. Since the previous version worked just fine with this setup, I believe it should be possible to fix this.

How to reproduce the issue

1.

  /user group
  add name=api-read policy=\
    read,api,!local,!telnet,!ssh,!ftp,!reboot,!write,!policy,!test,!winbox,!password,!web,!sniff,!sensitive,!romon,!dude,!tikapp
  /user
  add group=api-read name=homeassistant
  1. Set up this integration. I only have "Port traffic sensors" enabled in configuration.
  2. Observe error messages in Home Assistant log, as well as this
    [user@MikroTik] > /log pr
    13:55:57 system,info,account user homeassistant logged out from <IP> via api 
    13:56:06 system,info,account user homeassistant logged in from <IP> via api 
    13:56:07 system,info,account user homeassistant logged in from <IP> via api

Expected behavior

Only one error message, functions that require write policy don't work.

Software versions

I have another RouterBoard laying around. If you want me to do some experimentation (git bisect and similar), please let me know.

After I enabled debug, I see that this has something to do with the new update functionality.

2022-08-16 14:06:58.153 DEBUG (SyncWorker_1) [custom_components.mikrotik_router.mikrotikapi] API query: /system/package/update
2022-08-16 14:06:58.163 ERROR (SyncWorker_1) [custom_components.mikrotik_router.mikrotikapi] Mikrotik 192.168.1.1 error while execute : not enough permissions (9)
tomaae commented 2 years ago

Not surprised there, many features wont work with just those access rights. It could be possible to work around this, but only if users and groups are available with those limited rights. Still would be quite an undertaking.

To continue using the integration, you can hash line in mikrotik_controller.py: 1460: self.execute("/system/package/update", "check-for-updates", None, None)

ondras12345 commented 2 years ago

Thanks for the patch, I'll try it out.

It could be possible to work around this, but only if users and groups are available with those limited rights.

I'd be tempted to just do it the pythonic way (ask for forgiveness, not for permission) and try the API call to see if it fails with "not enough permission". I don't think the router will disconnect the user upon failure. I believe your integration disconnects itself when it encounters the error.

tomaae commented 2 years ago

yes, integration will stops processing when it encounters the error. that is necessery due to data dependencies. Asking for forgiveness would be actually be much more complicated and time consuming. It would need exact list of dependencies for every type of entity and check on them all to see if any is failing, so data presented to HA are not corrupted.

daniele-athome commented 2 years ago

To continue using the integration, you can hash line in mikrotik_controller.py: 1460: self.execute("/system/package/update", "check-for-updates", None, None)

Patch seems to be working for me: I don't have the log message any more (I too have same policies active as @ondras12345). Thanks!

tomaae commented 2 years ago

interesting is that /system/health print does not work without write and policy permissions. that means no health sensors.

tomaae commented 2 years ago

There will be one time warning about limited functionality durring integration startup. Everything is selectively disabled based on access rights. Switches will still exist, but since HA does not support disabling switch interactions while still showing status, they wont do anything and just flip back after 2 seconds. Leaving scripts alone, since they can be configured to run regardless of permissions.