* [Buildroot] [PATCH 1/1] package/quagga: Fix directories and permissions @ 2016-05-11 8:01 Nathaniel Roach 2016-05-11 21:33 ` Thomas Petazzoni 0 siblings, 1 reply; 5+ messages in thread From: Nathaniel Roach @ 2016-05-11 8:01 UTC (permalink / raw) To: buildroot Quagga runs as the "quagga" user, but it also needs to modify files in /etc and /var - config files, pid files and vty sockets for vtysh. Tell the configure script the right folders to use, create the user, fix the permissions, and then let systemd know (if needed). Signed-off-by: Nathaniel Roach <nroach44@gmail.com> --- package/quagga/quagga.mk | 22 +++++++++++++++++++++- package/quagga/quagga_tmpfiles.conf | 2 ++ 2 files changed, 23 insertions(+), 1 deletion(-) create mode 100644 package/quagga/quagga_tmpfiles.conf diff --git a/package/quagga/quagga.mk b/package/quagga/quagga.mk index 6b98367..3592aee 100644 --- a/package/quagga/quagga.mk +++ b/package/quagga/quagga.mk @@ -10,7 +10,11 @@ QUAGGA_SITE = http://download.savannah.gnu.org/releases/quagga QUAGGA_DEPENDENCIES = host-gawk QUAGGA_LICENSE = GPLv2+ QUAGGA_LICENSE_FILES = COPYING -QUAGGA_CONF_OPTS = --program-transform-name='' +QUAGGA_CONF_OPTS = \ + --program-transform-name='' \ + --sysconfdir=/etc/quagga \ + --localstatedir=/var/run/quagga + # 0002-configure-fix-static-linking-with-readline.patch QUAGGA_AUTORECONF = YES @@ -33,6 +37,16 @@ QUAGGA_CONF_OPTS += $(if $(BR2_PACKAGE_QUAGGA_ISISD),--enable-isisd,--disable-is QUAGGA_CONF_OPTS += $(if $(BR2_PACKAGE_QUAGGA_BGP_ANNOUNCE),--enable-bgp-announce,--disable-bgp-announce) QUAGGA_CONF_OPTS += $(if $(BR2_PACKAGE_QUAGGA_TCP_ZERBRA),--enable-tcp-zebra,--disable-tcp-zebra) +define QUAGGA_USERS + quagga -1 quagga -1 * - - - Quagga priv drop user +endef + +define QUAGGA_PERMISSIONS + /etc/quagga r 600 quagga quagga - - - - - + /etc/quagga d 755 quagga quagga - - - - - + /var/run/quagga d 755 quagga quagga - - - - - +endef + ifeq ($(BR2_PACKAGE_QUAGGA_SNMP),y) QUAGGA_CONF_ENV += ac_cv_path_NETSNMP_CONFIG=$(STAGING_DIR)/usr/bin/net-snmp-config QUAGGA_CONF_OPTS += --enable-snmp=agentx @@ -50,4 +64,10 @@ ifeq ($(BR2_arc),y) QUAGGA_CONF_OPTS += --disable-pie endif +define QUAGGA_INSTALL_INIT_SYSTEMD + mkdir -p $(TARGET_DIR)/usr/lib/tmpfiles.d + $(INSTALL) -D -m 644 package/quagga/quagga_tmpfiles.conf \ + $(TARGET_DIR)/usr/lib/tmpfiles.d/quagga.conf +endef + $(eval $(autotools-package)) diff --git a/package/quagga/quagga_tmpfiles.conf b/package/quagga/quagga_tmpfiles.conf new file mode 100644 index 0000000..ad82cc6 --- /dev/null +++ b/package/quagga/quagga_tmpfiles.conf @@ -0,0 +1,2 @@ +d /var/run/quagga/ 1755 quagga quagga - + -- 2.8.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Buildroot] [PATCH 1/1] package/quagga: Fix directories and permissions 2016-05-11 8:01 [Buildroot] [PATCH 1/1] package/quagga: Fix directories and permissions Nathaniel Roach @ 2016-05-11 21:33 ` Thomas Petazzoni 2016-05-12 2:20 ` Nathaniel Roach 0 siblings, 1 reply; 5+ messages in thread From: Thomas Petazzoni @ 2016-05-11 21:33 UTC (permalink / raw) To: buildroot Hello, I'm adding in Cc: Baruch, since he has done most of the recent updates to the Quagga package. Baruch, could you review/test this patch, according to your knowledge of Quagga? I'm also adding some comments below. > Quagga runs as the "quagga" user, but it also needs to modify files > in /etc and /var - config files, pid files and vty sockets for vtysh. Does it really need to write in /etc ? If that's the case, then it seems a bit wrong, and we have a bigger problem. What happens if /etc is read-only ? On Wed, 11 May 2016 16:01:13 +0800, Nathaniel Roach wrote: > diff --git a/package/quagga/quagga.mk b/package/quagga/quagga.mk > index 6b98367..3592aee 100644 > --- a/package/quagga/quagga.mk > +++ b/package/quagga/quagga.mk > @@ -10,7 +10,11 @@ QUAGGA_SITE = http://download.savannah.gnu.org/releases/quagga > QUAGGA_DEPENDENCIES = host-gawk > QUAGGA_LICENSE = GPLv2+ > QUAGGA_LICENSE_FILES = COPYING > -QUAGGA_CONF_OPTS = --program-transform-name='' > +QUAGGA_CONF_OPTS = \ > + --program-transform-name='' \ > + --sysconfdir=/etc/quagga \ > + --localstatedir=/var/run/quagga Indentation should be one tab for those lines. But why isn't sysconfdir=/etc appropriate? Is it because quagga writes to some files in /etc? If that's the case, as said above, I'm believe it's bad. > +define QUAGGA_PERMISSIONS > + /etc/quagga r 600 quagga quagga - - - - - > + /etc/quagga d 755 quagga quagga - - - - - Hum, does this actually work? > + /var/run/quagga d 755 quagga quagga - - - - - > +endef > + > ifeq ($(BR2_PACKAGE_QUAGGA_SNMP),y) > QUAGGA_CONF_ENV += ac_cv_path_NETSNMP_CONFIG=$(STAGING_DIR)/usr/bin/net-snmp-config > QUAGGA_CONF_OPTS += --enable-snmp=agentx > @@ -50,4 +64,10 @@ ifeq ($(BR2_arc),y) > QUAGGA_CONF_OPTS += --disable-pie > endif > > +define QUAGGA_INSTALL_INIT_SYSTEMD > + mkdir -p $(TARGET_DIR)/usr/lib/tmpfiles.d This mkdir -p is useless, as $(INSTALL) -D creates all sub-directories needed to be able to copy to the destination path. > + $(INSTALL) -D -m 644 package/quagga/quagga_tmpfiles.conf \ > + $(TARGET_DIR)/usr/lib/tmpfiles.d/quagga.conf > +endef > + > $(eval $(autotools-package)) > diff --git a/package/quagga/quagga_tmpfiles.conf b/package/quagga/quagga_tmpfiles.conf > new file mode 100644 > index 0000000..ad82cc6 > --- /dev/null > +++ b/package/quagga/quagga_tmpfiles.conf > @@ -0,0 +1,2 @@ > +d /var/run/quagga/ 1755 quagga quagga - > + Thanks! Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 5+ messages in thread
* [Buildroot] [PATCH 1/1] package/quagga: Fix directories and permissions 2016-05-11 21:33 ` Thomas Petazzoni @ 2016-05-12 2:20 ` Nathaniel Roach 2016-05-12 6:58 ` Thomas Petazzoni 0 siblings, 1 reply; 5+ messages in thread From: Nathaniel Roach @ 2016-05-12 2:20 UTC (permalink / raw) To: buildroot Hi Thomas, On 12/05/16 05:33, Thomas Petazzoni wrote: > Hello, > > I'm adding in Cc: Baruch, since he has done most of the recent updates > to the Quagga package. Baruch, could you review/test this patch, > according to your knowledge of Quagga? > > I'm also adding some comments below. > >> Quagga runs as the "quagga" user, but it also needs to modify files >> in /etc and /var - config files, pid files and vty sockets for vtysh. > Does it really need to write in /etc ? If that's the case, then it > seems a bit wrong, and we have a bigger problem. What happens if /etc > is read-only ? If you're using vtysh to configure Quagga, yes, it absolutely needs write permissions to the config folder, as it's more than likely you'd want to save your config. (Running commands in vtysh is very similar to Cisco routers, there's a "running-config" and a "startup-config" - commands are saved into running, but are not copied into startup by default) The daemons themselves don't write to /etc unless you tell it to: $sudo vtysh ... charon# copy run start Building Configuration... Configuration saved to /etc/quagga/zebra.conf Configuration saved to /etc/quagga/ospfd.conf [OK] It needs write permissions to the folder as it moves the old config and writes a new one, rather than just overwriting. In the instance that /etc/ is RO, the user simply won't be able to save an updated configuration. > > > On Wed, 11 May 2016 16:01:13 +0800, Nathaniel Roach wrote: > >> diff --git a/package/quagga/quagga.mk b/package/quagga/quagga.mk >> index 6b98367..3592aee 100644 >> --- a/package/quagga/quagga.mk >> +++ b/package/quagga/quagga.mk >> @@ -10,7 +10,11 @@ QUAGGA_SITE = http://download.savannah.gnu.org/releases/quagga >> QUAGGA_DEPENDENCIES = host-gawk >> QUAGGA_LICENSE = GPLv2+ >> QUAGGA_LICENSE_FILES = COPYING >> -QUAGGA_CONF_OPTS = --program-transform-name='' >> +QUAGGA_CONF_OPTS = \ >> + --program-transform-name='' \ >> + --sysconfdir=/etc/quagga \ >> + --localstatedir=/var/run/quagga > Indentation should be one tab for those lines. But why isn't > sysconfdir=/etc appropriate? Is it because quagga writes to some files > in /etc? If that's the case, as said above, I'm believe it's bad. Ah, editor settings strikes again. > >> +define QUAGGA_PERMISSIONS >> + /etc/quagga r 600 quagga quagga - - - - - >> + /etc/quagga d 755 quagga quagga - - - - - > Hum, does this actually work? Yup, unfortunately wildcards don't, and I didn't feel that adding a line for each daemon was appropriate. (There's one for each daemon, and it's only installed if that daemon is selected, hence why I need to effectively do a wildcard chmod here) > >> + /var/run/quagga d 755 quagga quagga - - - - - >> +endef >> + >> ifeq ($(BR2_PACKAGE_QUAGGA_SNMP),y) >> QUAGGA_CONF_ENV += ac_cv_path_NETSNMP_CONFIG=$(STAGING_DIR)/usr/bin/net-snmp-config >> QUAGGA_CONF_OPTS += --enable-snmp=agentx >> @@ -50,4 +64,10 @@ ifeq ($(BR2_arc),y) >> QUAGGA_CONF_OPTS += --disable-pie >> endif >> >> +define QUAGGA_INSTALL_INIT_SYSTEMD >> + mkdir -p $(TARGET_DIR)/usr/lib/tmpfiles.d > This mkdir -p is useless, as $(INSTALL) -D creates all sub-directories > needed to be able to copy to the destination path. Huh, thanks! I believe I copied this from somewhere else, but I'll take it out in the next revision. > >> + $(INSTALL) -D -m 644 package/quagga/quagga_tmpfiles.conf \ >> + $(TARGET_DIR)/usr/lib/tmpfiles.d/quagga.conf >> +endef >> + >> $(eval $(autotools-package)) >> diff --git a/package/quagga/quagga_tmpfiles.conf b/package/quagga/quagga_tmpfiles.conf >> new file mode 100644 >> index 0000000..ad82cc6 >> --- /dev/null >> +++ b/package/quagga/quagga_tmpfiles.conf >> @@ -0,0 +1,2 @@ >> +d /var/run/quagga/ 1755 quagga quagga - >> + > Thanks! > > Thomas Cheers, Nathaniel ^ permalink raw reply [flat|nested] 5+ messages in thread
* [Buildroot] [PATCH 1/1] package/quagga: Fix directories and permissions 2016-05-12 2:20 ` Nathaniel Roach @ 2016-05-12 6:58 ` Thomas Petazzoni 2016-05-12 7:01 ` Nathaniel Roach 0 siblings, 1 reply; 5+ messages in thread From: Thomas Petazzoni @ 2016-05-12 6:58 UTC (permalink / raw) To: buildroot Hello, On Thu, 12 May 2016 10:20:40 +0800, Nathaniel Roach wrote: > If you're using vtysh to configure Quagga, yes, it absolutely needs > write permissions to the config folder, as it's more than likely you'd > want to save your config. (Running commands in vtysh is very similar to > Cisco routers, there's a "running-config" and a "startup-config" - > commands are saved into running, but are not copied into startup by default) > > The daemons themselves don't write to /etc unless you tell it to: > > $sudo vtysh > ... > charon# copy run start > Building Configuration... > Configuration saved to /etc/quagga/zebra.conf > Configuration saved to /etc/quagga/ospfd.conf > [OK] > > It needs write permissions to the folder as it moves the old config and > writes a new one, rather than just overwriting. > > In the instance that /etc/ is RO, the user simply won't be able to save > an updated configuration. Right, makes sense. Then, perhaps you want to add a comment on top of QUAGGA_CONF_OPTS to indicate why we override localstatedir and sysconfdir. Just something like: # Override localstatedir and sysconfdir so that quagga has its own # directories, which is will access with its own user. or something along those lines (I'm sure a better wording is possible). > >> +define QUAGGA_PERMISSIONS > >> + /etc/quagga r 600 quagga quagga - - - - - > >> + /etc/quagga d 755 quagga quagga - - - - - > > Hum, does this actually work? > Yup, unfortunately wildcards don't, and I didn't feel that adding a line > for each daemon was appropriate. (There's one for each daemon, and it's > only installed if that daemon is selected, hence why I need to > effectively do a wildcard chmod here) So you need the first line to make every file in /etc/quagga owned by quagga, 600, and then the second line to make the /etc/quagga directory owned by the quagga user and 755, so that quagga can create more files in this directory, right? > >> +define QUAGGA_INSTALL_INIT_SYSTEMD > >> + mkdir -p $(TARGET_DIR)/usr/lib/tmpfiles.d > > This mkdir -p is useless, as $(INSTALL) -D creates all sub-directories > > needed to be able to copy to the destination path. > Huh, thanks! I believe I copied this from somewhere else, but I'll take > it out in the next revision. If you've seen it somewhere, try to remember where so that we can fix this place as well :-) So overall, looks good. Just fix the very minor nits that I mentioned, and it's good to go. Thanks! Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 5+ messages in thread
* [Buildroot] [PATCH 1/1] package/quagga: Fix directories and permissions 2016-05-12 6:58 ` Thomas Petazzoni @ 2016-05-12 7:01 ` Nathaniel Roach 0 siblings, 0 replies; 5+ messages in thread From: Nathaniel Roach @ 2016-05-12 7:01 UTC (permalink / raw) To: buildroot On 12/05/16 14:58, Thomas Petazzoni wrote: > Hello, > > On Thu, 12 May 2016 10:20:40 +0800, Nathaniel Roach wrote: > >> If you're using vtysh to configure Quagga, yes, it absolutely needs >> write permissions to the config folder, as it's more than likely you'd >> want to save your config. (Running commands in vtysh is very similar to >> Cisco routers, there's a "running-config" and a "startup-config" - >> commands are saved into running, but are not copied into startup by default) >> >> The daemons themselves don't write to /etc unless you tell it to: >> >> $sudo vtysh >> ... >> charon# copy run start >> Building Configuration... >> Configuration saved to /etc/quagga/zebra.conf >> Configuration saved to /etc/quagga/ospfd.conf >> [OK] >> >> It needs write permissions to the folder as it moves the old config and >> writes a new one, rather than just overwriting. >> >> In the instance that /etc/ is RO, the user simply won't be able to save >> an updated configuration. > Right, makes sense. Then, perhaps you want to add a comment on top of > QUAGGA_CONF_OPTS to indicate why we override localstatedir and > sysconfdir. Just something like: > > # Override localstatedir and sysconfdir so that quagga has its own > # directories, which is will access with its own user. > > or something along those lines (I'm sure a better wording is possible). > >>>> +define QUAGGA_PERMISSIONS >>>> + /etc/quagga r 600 quagga quagga - - - - - >>>> + /etc/quagga d 755 quagga quagga - - - - - >>> Hum, does this actually work? >> Yup, unfortunately wildcards don't, and I didn't feel that adding a line >> for each daemon was appropriate. (There's one for each daemon, and it's >> only installed if that daemon is selected, hence why I need to >> effectively do a wildcard chmod here) > So you need the first line to make every file in /etc/quagga owned by > quagga, 600, and then the second line to make the /etc/quagga directory > owned by the quagga user and 755, so that quagga can create more files > in this directory, right? That's precisely it. I'll comment this too so it's clearer. > >>>> +define QUAGGA_INSTALL_INIT_SYSTEMD >>>> + mkdir -p $(TARGET_DIR)/usr/lib/tmpfiles.d >>> This mkdir -p is useless, as $(INSTALL) -D creates all sub-directories >>> needed to be able to copy to the destination path. >> Huh, thanks! I believe I copied this from somewhere else, but I'll take >> it out in the next revision. > If you've seen it somewhere, try to remember where so that we can fix > this place as well :-) Found it: package/audit/audit.mk:49 I'll send that through in another patch. > > So overall, looks good. Just fix the very minor nits that I mentioned, > and it's good to go. > > Thanks! > > Thomas Cheers! ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-05-12 7:01 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-05-11 8:01 [Buildroot] [PATCH 1/1] package/quagga: Fix directories and permissions Nathaniel Roach 2016-05-11 21:33 ` Thomas Petazzoni 2016-05-12 2:20 ` Nathaniel Roach 2016-05-12 6:58 ` Thomas Petazzoni 2016-05-12 7:01 ` Nathaniel Roach
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox