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

Network plugin in tags/v1.6 seems to ignore list in acl #331

Closed
gitmirko opened this issue Aug 12, 2019 · 14 comments
Closed

Network plugin in tags/v1.6 seems to ignore list in acl #331

gitmirko opened this issue Aug 12, 2019 · 14 comments
Labels
plugins Issue relates to a plugin

Comments

@gitmirko
Copy link

Hi,
I've updated my smarthome-ng instance to v1.6. Since than I get the following error in the log:
Generic network acl doesn't permit updates from 10.0.0.X
My plugin.yaml looks like this:

nw:
     class_name: Network
     class_path: plugins.network
     ip: 0.0.0.0
     http: 8765
     http_acl:
        - 10.0.0.X
        - 10.0.0.Y
        - 10.0.0.Z

As a workaround I've removed the http_acl parameter and now it defaults to * and everything is working fine.

Does someone else got a similar problem?

Best regards and keep up the great work
Mirko

@psilo909
Copy link
Contributor

psilo909 commented Aug 12, 2019

was the x, y, z thing EVER been supported by the network plugin?

see e.g. https://github.com/smarthomeNG/plugins/blob/master/network/__init__.py#CL204

i also doubt that the plugin knows about handling different names for the variable part of the ip address like x, y, z... perhaps a "*" could have been a thing for implementing that.

i added the newly logged message 3 years ago when converting to smartplugin. but the logics in the plugin for matching IPs stayed as it is.. how old was your shng / sh.py before upgrading? (https://github.com/smarthomeNG/plugins/blame/80e41af31ed53e405d5e89ed7091ffc4696c71d9/network/__init__.py#L205)

@Tom-Bom-badil
Copy link

@psilo909: Isn't this related to what we discuessed here on Gitter? We were talking about UDP, but if I recall right, your conclusion was that the code indeed needs some review ...
/tom

@psilo909
Copy link
Contributor

psilo909 commented Aug 12, 2019

@Tom-Bom-badil no in this case i really think the X Y Z placeholder causes the problem

@psilo909
Copy link
Contributor

@Tom-Bom-badil by the way, although i wrote in gitter "i noted it" i forgot about it ;-) please create an issue so we can keep track if there was still sth open.

@msinn msinn added the plugins Issue relates to a plugin label Mar 1, 2020
@msinn
Copy link
Member

msinn commented Mar 1, 2020

@gitmirko Is tgis issue still relevant?

@msinn
Copy link
Member

msinn commented Jun 26, 2020

@gitmirko Is this issue still relevant?

@gitmirko
Copy link
Author

gitmirko commented Jul 2, 2020

Hi, I don't know. I've stopped using smarthomeNG.

@gitmirko gitmirko closed this as completed Jul 2, 2020
@ohinckel
Copy link
Member

Just for your information: I tested the network plugin to check how the ACLs are checked. Placeholder are not supported and only full IP addresses works. The code also states checking for IP adresses in ACL lists (like if source not in iacl:).

But if there are other problem which needs to be checked, just told me. I try to check it if we have some problems with the ACL configuration.

@bmxp
Copy link
Member

bmxp commented Sep 26, 2020

It would probably help to allow a certain range of IP Adresses like

  • 192.168.10.* --> substitute * to be 0-255 --> only the adresses starting with 192.168.10 will be valid
  • 192.168.10.0-48 --> only the adresses starting with 192.168.10 and the last octet from 0 to 48 will be valid

I doubt that we need to extend this scheme into a kind of 192.168.*.12 or 192.168.*.*

@Tom-Bom-badil
Copy link

Tom-Bom-badil commented Sep 26, 2020

There seems to be a problem when something like udp_acl: '*' is used in the config. If you just leave the whole acl thing out, it will default back to '*', and everything works fine. But if you explicitely specify udp_acl: '*' (for whatever reason - maybe for readability), the communication won't work.

psilo909 commented on this some time ago:

@Tom-Bom-badil das mit dem stern ist mir jetzt auch klarer. laut plugin.yaml ist das attribut ein list datentyp. daher kommt der stern als array mit 1 element zurück. ich kläre noch wie man das am besten behebt

Full details here and in the chat above.

/tom

@ohinckel
Copy link
Member

In case we want to support different pattern for IP based ACLs it would be helpful to provide such a method in the Network class of lib/network.py. This could do matching IP against a netmask (like 192.168.2.0/24 or 192.168.2.0/255.255.255.0) and matching IP against a pattern (like 192.168.2.*). Just noticed that there are methods in the network.py which are also in utils.py (maybe we should deprecate them?).

I tried to configure the network plugin with udp_acl and * (same with a list, one entry with *) and have no problems updating the items. Is the problem related to the global plugin configuration or item specific configuration (sorry, I can't read the complete thread in the mentioned conversation)?

@Tom-Bom-badil
Copy link

Tom-Bom-badil commented Sep 27, 2020

Long story short: shNG stopped receiving item values through UDP after a regular shNG version update. Before it received well for quite a long time.

nw was defined in plugin.yaml as follows:

nw:
   class_name: Network
   class_path: plugins.network
   ip: 0.0.0.0
   port: 2423
   http: 2422
   tcp: 'yes'
   udp: 'yes'
   udp_acl: '*'

After removing the line udp_acl: '*', shNG started receiving data again. Hope this helps. @psilo909 found out that the behaviour is likely related to the fact that a list with just 1 element ('*') is generated, see above. Hope this helps.

Side note: Again - after removing the line, my shNG is properly receiving the values, so personally this is not an issue anymore. '*' is the default for the acl anyways, so there is no reason to have this line. It just might happen that others will fail at the same place, so maybe it's just a matter of documentation to avoid mentioning '*'.

/tom

@ohinckel
Copy link
Member

shNG stopped receiving item values through UDP after a regular shNG version update.

I can't reproduce this problem (with my plugin repo @ a2bf7e83782ed7a3502bd47b1f03fc59f925d634. I used the network plugin with this configuration:

nw:
    class_name: Network
    class_path: plugins.network
    ip: 0.0.0.0
    port: 2929
    tcp: yes
    udp: yes
    http: 8765
    tcp_acl: '*'
    udp_acl: '*' 
    http_acl: '*'

I also tested with '*' as list option like this:

    udp_acl:
    - '*'

No problems. Update are consumed as expected.

@Tom-Bom-badil
Copy link

Sounds fine, I guess the issue can remain closed then.

/tom

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugins Issue relates to a plugin
Projects
None yet
Development

No branches or pull requests

6 participants