| Message ID | 20210311232528.338648-1-stijn@linux-ipv6.be |
|---|---|
| Headers | show |
| Series | import libcap from packages feed | expand |
Stijn Tintel <stijn@linux-ipv6.be> writes: > I suspect some people might counter this by saying lldpd belongs in the > packages feed; I strongly disagree as imo LLDP is an essential service > for any network device, and especially switches. Even the cheapest > managed switches support LLDP for more than 5 years already. Yes, and we should really look into doing the 802.3at and bt stuff properly for the switches wich such hardware. This requires lldp. Bjørn
Stijn Tintel <stijn@linux-ipv6.be> [2021-03-12 01:25:24]: Hi, > Having libcap in OpenWrt base allows us to enable libcap support in > other packages in base. there is same functionality available through procd already so essentialy you're throwing away that effort, increasing flash space usage etc. > In lldpd, this would allow the monitor process to drop its privileges > instead of running as root, improving security. It will also allow us to > drop our patch to disable libcap. I assume, that you can do it even better with procd's seccomp/jails and likely confine the master process as well. > I suspect some people might counter this by saying lldpd belongs in the > packages feed; IMO it belongs to packages feed, because currently it's optional package. In other words, it's not included in any of the images by default. > I strongly disagree as imo LLDP is an essential service for any network > device, and especially switches. Even the cheapest managed switches support > LLDP for more than 5 years already. If it's that essential, why it's not enabled and shipped by default? I assume it's because some folks would complain, that LLDP is use case specific, not everybody would like to have another network exposed service running by default, not everybody needs LLDP by default in RX/TX mode as TX mode might be enough etc. Cheers, Petr
On 12/03/2021 10:50, Petr Štetiar wrote: > Stijn Tintel <stijn@linux-ipv6.be> [2021-03-12 01:25:24]: > > Hi, > >> Having libcap in OpenWrt base allows us to enable libcap support in >> other packages in base. > there is same functionality available through procd already so essentialy > you're throwing away that effort, increasing flash space usage etc. Is that documented somewhere? Did you test that lldpd even starts when limiting it like that? I have had issues when I tried to limit the capabilities of infnoised [1] in the systemd unit file; it simply wouldn't start. The flash space usage is a moot point, we no longer care about 4MB devices, and libcap is only 12kb. We can offset that by building lldpd with LTO. > >> In lldpd, this would allow the monitor process to drop its privileges >> instead of running as root, improving security. It will also allow us to >> drop our patch to disable libcap. > I assume, that you can do it even better with procd's seccomp/jails and > likely confine the master process as well. The non-monitor process already runs as the lldp user without libcap. Processes with libcap disabled: 1642 root 4360 S /usr/sbin/lldpd -d -c -f -s -e -M 4 -x -X /var/run/agentx.sock 1693 lldp 4804 S /usr/sbin/lldpd -d -c -f -s -e -M 4 -x -X /var/run/agentx.sock With libcap enabled: 1804 lldp 1216 S /usr/sbin/lldpd -d -c -f -s -e -M 4 1886 lldp 1232 S /usr/sbin/lldpd -d -c -f -s -e -M 4 > >> I suspect some people might counter this by saying lldpd belongs in the >> packages feed; > IMO it belongs to packages feed, because currently it's optional package. In > other words, it's not included in any of the images by default. See Bjorn's reply. It's required to properly support 802.3af/at/bt (PoE). Also, I don't agree that any optional package should be moved to the packages feed. The packages feed is a mess, often PR's are accepted without maintainer approval, sometimes even after explicit NACK by the maintainer or others. For this reason, I'm seriously considering removing myself as maintainer from any packages I maintain there. > >> I strongly disagree as imo LLDP is an essential service for any network >> device, and especially switches. Even the cheapest managed switches support >> LLDP for more than 5 years already. > If it's that essential, why it's not enabled and shipped by default? I assume > it's because some folks would complain, that LLDP is use case specific, not > everybody would like to have another network exposed service running by > default, not everybody needs LLDP by default in RX/TX mode as TX mode might be > enough etc. We're considering enabling it on realtek by default. > > Cheers, > > Petr Stijn [1] https://github.com/13-37-org/infnoise
On Fri, Mar 12, 2021 at 2:10 AM Stijn Tintel <stijn@linux-ipv6.be> wrote: > > On 12/03/2021 10:50, Petr Štetiar wrote: > > Stijn Tintel <stijn@linux-ipv6.be> [2021-03-12 01:25:24]: > > > > Hi, > > > >> Having libcap in OpenWrt base allows us to enable libcap support in > >> other packages in base. > > there is same functionality available through procd already so essentialy > > you're throwing away that effort, increasing flash space usage etc. > > Is that documented somewhere? Did you test that lldpd even starts when > limiting it like that? I have had issues when I tried to limit the > capabilities of infnoised [1] in the systemd unit file; it simply > wouldn't start. > > The flash space usage is a moot point, we no longer care about 4MB > devices, and libcap is only 12kb. We can offset that by building lldpd > with LTO. > > > > >> In lldpd, this would allow the monitor process to drop its privileges > >> instead of running as root, improving security. It will also allow us to > >> drop our patch to disable libcap. > > I assume, that you can do it even better with procd's seccomp/jails and > > likely confine the master process as well. > > The non-monitor process already runs as the lldp user without libcap. > > Processes with libcap disabled: > > 1642 root 4360 S /usr/sbin/lldpd -d -c -f -s -e -M 4 -x -X > /var/run/agentx.sock > 1693 lldp 4804 S /usr/sbin/lldpd -d -c -f -s -e -M 4 -x -X > /var/run/agentx.sock > > With libcap enabled: > > 1804 lldp 1216 S /usr/sbin/lldpd -d -c -f -s -e -M 4 > 1886 lldp 1232 S /usr/sbin/lldpd -d -c -f -s -e -M 4 > > > > >> I suspect some people might counter this by saying lldpd belongs in the > >> packages feed; > > IMO it belongs to packages feed, because currently it's optional package. In > > other words, it's not included in any of the images by default. > > See Bjorn's reply. It's required to properly support 802.3af/at/bt (PoE). > > Also, I don't agree that any optional package should be moved to the > packages feed. The packages feed is a mess, often PR's are accepted > without maintainer approval, sometimes even after explicit NACK by the > maintainer or others. For this reason, I'm seriously considering > removing myself as maintainer from any packages I maintain there. No idea about explicit NACK. I merge mostly complete PRs to avoid the situation in base where hundreds of good PRs pile up only because there is no one to review. > > > > >> I strongly disagree as imo LLDP is an essential service for any network > >> device, and especially switches. Even the cheapest managed switches support > >> LLDP for more than 5 years already. > > If it's that essential, why it's not enabled and shipped by default? I assume > > it's because some folks would complain, that LLDP is use case specific, not > > everybody would like to have another network exposed service running by > > default, not everybody needs LLDP by default in RX/TX mode as TX mode might be > > enough etc. > We're considering enabling it on realtek by default. > > > > Cheers, > > > > Petr > > Stijn > > [1] https://github.com/13-37-org/infnoise > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
On 12/03/21 11:01, Stijn Tintel wrote: > On 12/03/2021 10:50, Petr Štetiar wrote: >> Stijn Tintel <stijn@linux-ipv6.be> [2021-03-12 01:25:24]: > Also, I don't agree that any optional package should be moved to the > packages feed. The packages feed is a mess, often PR's are accepted > without maintainer approval, sometimes even after explicit NACK by the > maintainer or others. For this reason, I'm seriously considering > removing myself as maintainer from any packages I maintain there. > if packages feeds are a mess and people with commit access there do things you don't like then that's an issue that should be discussed and dealt with. Moving packages you care about in core repo is surely a good solution for you and the packages you maintain because you have commit access so you don't need to wait for review but for most other contributors it is only swapping a relatively small problem (package feeds is a mess) for a much bigger one (reviewers are scarce and a lot of stuff especially about "optional" packages no core developer cares much about langushes for months and bitrots). -Alberto
On 3/12/21 12:25 AM, Stijn Tintel wrote: > Having libcap in OpenWrt base allows us to enable libcap support in > other packages in base. > > In lldpd, this would allow the monitor process to drop its privileges > instead of running as root, improving security. It will also allow us to > drop our patch to disable libcap. > > I suspect some people might counter this by saying lldpd belongs in the > packages feed; I strongly disagree as imo LLDP is an essential service > for any network device, and especially switches. Even the cheapest > managed switches support LLDP for more than 5 years already. > > Also see: https://github.com/openwrt/openwrt/pull/3823#issuecomment-795174537 > I'll bump lldpd to the latest version after this series is merged, and > debug the problem reported by John on the realtek target. > > Stijn Tintel (4): > libcap: import from packages feed > libcap: drop invalid copyright header > libcap: bump to 2.48 > lldpd: add libcap dependency > Acked-by: Hauke Mehrtens <hauke@hauke-m.de> I am not a lldpd user so I can not say if we should make it depend on libcap, but I saw some packages which has implicit dependencies. Hauke