openv / vcontrold

:fire: vcontrold Daemon for control and logging of Viessmann® type heating devices
https://github.com/openv/openv/wiki
GNU General Public License v3.0
102 stars 55 forks source link

issue #100: implementation for the restriction of interfaces vcontrold tries to bind to #101

Closed speters closed 2 years ago

speters commented 2 years ago

ATTN: this is is untested code, just for discussion of

https://github.com/openv/vcontrold/issues/100

speters commented 2 years ago

I only tested it very lazily

./vcontrold -g -n -x ../xml/300/vcontrold.xml -p 3322
...
[83194] Fri Aug 19 21:24:32 2022 : TCP socket 0.0.0.0:3322 opened
[83194] Fri Aug 19 21:24:32 2022 : Not started as root, username/groupname settings ignored
[83194] Fri Aug 19 21:24:35 2022 : Client connected 127.0.0.1:46544 (FD:5)

(connection on external and loopback interfaces possible)

./vcontrold -g -n -x ../xml/300/vcontrold.xml -b "localhost" -p 3322
...
[83351] Fri Aug 19 21:27:02 2022 : TCP socket localhost:3322 opened
[83351] Fri Aug 19 21:27:02 2022 : Not started as root, username/groupname settings ignored
[83351] Fri Aug 19 21:27:07 2022 : Client connected 127.0.0.1:36222 (FD:5)

(now no connection on external interface possible, localhost only)

./vcontrold -g -n -x ../xml/300/vcontrold.xml -b "8.8.8.8" -p 3322
...
[83491] Fri Aug 19 21:30:10 2022 :         Token: 2 Hexlen: 2, Unit: UT
socket error: could not open socket
./vcontrold -g -n -x ../xml/300/vcontrold.xml -b "" -p 3322
...
[83528] Fri Aug 19 21:30:52 2022 : getaddrinfo error: [Name or service not known]
[83528] Fri Aug 19 21:30:52 2022 : Could not start vcontrold on  port 3322
...
[83674] Fri Aug 19 21:31:48 2022 : TCP socket 0:3322 opened
[83674] Fri Aug 19 21:31:48 2022 : Not started as root, username/groupname settings ignored

(this is OK-ish, as 0.0.0.0 is 0 which stands for "all interfaces").

 ./vcontrold -g -n -x ../xml/300/vcontrold.xml -b "-1" -p 3322
...
[83751] Fri Aug 19 21:33:54 2022 : Could not start vcontrold on -1 port 3322

PS: Those exit(1)statements on error in socket.c are questionable. Leave error handling to the caller would be better code IMO. But it's the way it was done, so I leave it for now.

speters commented 2 years ago

I'm not happy with the naming of "bind" config parameter. Any better ideas?

Also: Which default? Safe/localhost only or keep-it-as-it-was/lazy/easy/all interfaces?

l3u commented 2 years ago

What about "IP"? As a socket address consists of an IP and and port, I would call it IP and set it to "0.0.0.0". This way, we get the behavior we have now as the default, and it can easily set to e.g. "127.0.0.1" to only listen on local connections, or to e.g. "192.168.1.10" or such to provide the server in the LAN.

hmueller01 commented 2 years ago

Why not having a "simple" netmask and call it like this?

l3u commented 2 years ago

"listen" sounds nice to me :-)

hmueller01 commented 2 years ago

On my compiler I get a warning

vcontrold/src/xmlconfig.c:504:13: warning: expression result unused [-Wunused-value]
    cfgPtr->listenAddress;
    ~~~~~~  ^~~~~~~~~~~~~
1 warning generated.

Why is line 504

cfgPtr->listenAddress;

needed? I think it can be deleted.

speters commented 2 years ago

Yes, you're right.

hmueller01 commented 2 years ago

I added the removal to #105 ...