Re: Possible bug when /var/lib/iwd is a symlink

Re: Possible bug when /var/lib/iwd is a symlink

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 2816 bytes --]

Hi Joakim,

On 4/14/22 03:17, Joakim Lotsengård wrote:
> Hi,
> 
> Possible bug when /var/lib/iwd is a symlink:

iwd assumes that it will create the storage directory on first start.  So it 
should never be a symlink.  How/why are you creating the symlink in the first place?

> 
> I ran across a problem when the ending part of /var/lib/iwd is a symlink.
> In my case it points to /mnt/persistent/iwd.  We are an IoT device that runs
> on UBIFS (+ tmpfs) and the entire rootfs partition of the NAND will be
> updated on FOTA.  Hence anything we want to save in an upgrade must be on a
> persistent partition.  Our target is a Freescale i.MX6 Ultralite module
> running an ARM A7 core (if that matters).

Can you mount /var/lib/iwd directly?

> 
> Problem seems to be that the l_dir_watch_new() call in knownnetworks.c never
> follows symlink.  l_dir_watch_new() uses IN_DONT_FOLLOW in its call to
> inotify_add_watch() in ell.

Right, along with some other flags that are on purpose for security reasons.

> 
> This will make the known network list misbehave.  If one forgets a
> known-network via iwctl, the list doesn't get updated.  The same goes for
> connecting to a not already known network.  It is easy to reproduce.  Just
> symlink /var/lib/iwd to another location and run some connect + forget in
> iwctl.  One can also note that adding a new /var/lib/iwd/my_ssid.psk, by
> manually creating the file, is not picked up by iwd automatically.

Looks like we do not check that the inotify watch creation failed.  So we should 
fix that.

> 
> I've only tested this with iwd 1.17 and ell 0.43 due to the complexity to
> build another version using Yocto.  However, I've checked the source code
> for iwd on master and they don't seem to handle the fact that /var/lib/iwd
> is a symlink.  I've tried to search the mailing-list and google, but can't
> find anything about this.  It seems I can change the /var/lib/iwd directory
> during compilation by setting DAEMON_STORAGEDIR during configure/build.
> However, that is a bit unflexible as my system is Yocto based, and hacking
> the Yocto recipes for this seems wrong.  Normally you symlink things in your
> file tree in IoT devices to point to persistent storage areas.
> 
> I don't have a solution, but at least iwd should detect that /var/lib/iwd is
> a symlink and warn the user.  The implication is that the known-network list
> doesn't work at all.  The internal known-network list should work even if
> inotify doesn't.

I think iwd should fail to start if /var/lib/iwd is a symlink...

> 
> Perhaps also it should be allowed to change the default storage dir in
> /etc/iwd/main.conf.
> 

I think that is probably the best approach.  Patches are always welcome of course.

Regards,
-Denis

Re: Possible bug when /var/lib/iwd is a symlink

[@ 2022-04-20 7:33 UTC (permalink / raw)]

[-- Attachment #1: Type: text/plain, Size: 4479 bytes --]

Hi,

No, I have not tested STATE_DIRECTORY. I used bind mount (as
suggested) instead, and before that the patch with realname() I sent
in as a demo-fix.

See this rather as a bug report. You get an incorrect behaviour from
iwd if /var/lib/iwd is a symlink. It will not update the known network
list correctly. Sadly I do not have a patch for it as it might be a
discussion on how to solve it. I am not part of iwd core team and I
feel I have no say in how to solve it.

The problem is very easy to reproduce. Just point /var/lib/iwd with a
symlink to somewhere else. Run iwctl and do some "station wlanX
connect Y" and "known-networks X forget" and watch how the
known-networks list won't get updated correctly. In fact, the easiest
way to reproduce is probably to just copy any ssid.psk file to
ssid_copy.psk. Start iwd (it will then find both 'ssid' and
'ssid_copy'). Now run a "known-networks ssid_copy forget". It will
remove the file in /var/lib/iwd but it will not remove it from the
'known-networks list'.

[iwd]# known-networks list
                                 Known Networks
--------------------------------------------------------------------------------
  Name                            Security   Hidden   Last connected
--------------------------------------------------------------------------------
  Orbital                         psk                 Apr 20,  7:29 AM
  Copy                            psk                 Apr 20,  7:29 AM

[iwd]# known-networks Copy forget
[iwd]# known-networks list
                                 Known Networks
--------------------------------------------------------------------------------
  Name                            Security   Hidden   Last connected
--------------------------------------------------------------------------------
  Orbital                         psk                 Apr 20,  7:29 AM
  Copy                            psk                 Apr 20,  7:29 AM
   <----- Should now be removed, but is still here!

On Tue, 19 Apr 2022 at 10:53, Marcel Holtmann <marcel(a)holtmann.org> wrote:
>
> Hi Joakim,
>
> > Possible bug when /var/lib/iwd is a symlink:
> >
> > I ran across a problem when the ending part of /var/lib/iwd is a symlink.
> > In my case it points to /mnt/persistent/iwd.  We are an IoT device that runs
> > on UBIFS (+ tmpfs) and the entire rootfs partition of the NAND will be
> > updated on FOTA.  Hence anything we want to save in an upgrade must be on a
> > persistent partition.  Our target is a Freescale i.MX6 Ultralite module
> > running an ARM A7 core (if that matters).
> >
> > Problem seems to be that the l_dir_watch_new() call in knownnetworks.c never
> > follows symlink.  l_dir_watch_new() uses IN_DONT_FOLLOW in its call to
> > inotify_add_watch() in ell.
> >
> > This will make the known network list misbehave.  If one forgets a
> > known-network via iwctl, the list doesn't get updated.  The same goes for
> > connecting to a not already known network.  It is easy to reproduce.  Just
> > symlink /var/lib/iwd to another location and run some connect + forget in
> > iwctl.  One can also note that adding a new /var/lib/iwd/my_ssid.psk, by
> > manually creating the file, is not picked up by iwd automatically.
> >
> > I've only tested this with iwd 1.17 and ell 0.43 due to the complexity to
> > build another version using Yocto.  However, I've checked the source code
> > for iwd on master and they don't seem to handle the fact that /var/lib/iwd
> > is a symlink.  I've tried to search the mailing-list and google, but can't
> > find anything about this.  It seems I can change the /var/lib/iwd directory
> > during compilation by setting DAEMON_STORAGEDIR during configure/build.
> > However, that is a bit unflexible as my system is Yocto based, and hacking
> > the Yocto recipes for this seems wrong.  Normally you symlink things in your
> > file tree in IoT devices to point to persistent storage areas.
> >
> > I don't have a solution, but at least iwd should detect that /var/lib/iwd is
> > a symlink and warn the user.  The implication is that the known-network list
> > doesn't work at all.  The internal known-network list should work even if
> > inotify doesn't.
> >
> > Perhaps also it should be allowed to change the default storage dir in
> > /etc/iwd/main.conf.
>
> have you tried to set the STATE_DIRECTORY environment variable?
>
> Regards
>
> Marcel
>

Re: Possible bug when /var/lib/iwd is a symlink

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 2279 bytes --]

Hi Joakim,

> Possible bug when /var/lib/iwd is a symlink:
> 
> I ran across a problem when the ending part of /var/lib/iwd is a symlink.
> In my case it points to /mnt/persistent/iwd.  We are an IoT device that runs
> on UBIFS (+ tmpfs) and the entire rootfs partition of the NAND will be
> updated on FOTA.  Hence anything we want to save in an upgrade must be on a
> persistent partition.  Our target is a Freescale i.MX6 Ultralite module
> running an ARM A7 core (if that matters).
> 
> Problem seems to be that the l_dir_watch_new() call in knownnetworks.c never
> follows symlink.  l_dir_watch_new() uses IN_DONT_FOLLOW in its call to
> inotify_add_watch() in ell.
> 
> This will make the known network list misbehave.  If one forgets a
> known-network via iwctl, the list doesn't get updated.  The same goes for
> connecting to a not already known network.  It is easy to reproduce.  Just
> symlink /var/lib/iwd to another location and run some connect + forget in
> iwctl.  One can also note that adding a new /var/lib/iwd/my_ssid.psk, by
> manually creating the file, is not picked up by iwd automatically.
> 
> I've only tested this with iwd 1.17 and ell 0.43 due to the complexity to
> build another version using Yocto.  However, I've checked the source code
> for iwd on master and they don't seem to handle the fact that /var/lib/iwd
> is a symlink.  I've tried to search the mailing-list and google, but can't
> find anything about this.  It seems I can change the /var/lib/iwd directory
> during compilation by setting DAEMON_STORAGEDIR during configure/build.
> However, that is a bit unflexible as my system is Yocto based, and hacking
> the Yocto recipes for this seems wrong.  Normally you symlink things in your
> file tree in IoT devices to point to persistent storage areas.
> 
> I don't have a solution, but at least iwd should detect that /var/lib/iwd is
> a symlink and warn the user.  The implication is that the known-network list
> doesn't work at all.  The internal known-network list should work even if
> inotify doesn't.
> 
> Perhaps also it should be allowed to change the default storage dir in
> /etc/iwd/main.conf.

have you tried to set the STATE_DIRECTORY environment variable?

Regards

Marcel

Re: Possible bug when /var/lib/iwd is a symlink

[@ 2022-04-14 12:20 UTC (permalink / raw)]

[-- Attachment #1: Type: text/plain, Size: 3228 bytes --]

Hi Joakim,

On Thu, Apr 14, 2022 at 10:17:26AM +0200, Joakim LotsengÄrd wrote:
> Hi,
> 
> Possible bug when /var/lib/iwd is a symlink:
> 
> I ran across a problem when the ending part of /var/lib/iwd is a symlink.
> In my case it points to /mnt/persistent/iwd.  We are an IoT device that runs
> on UBIFS (+ tmpfs) and the entire rootfs partition of the NAND will be
> updated on FOTA.  Hence anything we want to save in an upgrade must be on a
> persistent partition.  Our target is a Freescale i.MX6 Ultralite module
> running an ARM A7 core (if that matters).
> 
> Problem seems to be that the l_dir_watch_new() call in knownnetworks.c never
> follows symlink.  l_dir_watch_new() uses IN_DONT_FOLLOW in its call to
> inotify_add_watch() in ell.
> 
> This will make the known network list misbehave.  If one forgets a
> known-network via iwctl, the list doesn't get updated.  The same goes for
> connecting to a not already known network.  It is easy to reproduce.  Just
> symlink /var/lib/iwd to another location and run some connect + forget in
> iwctl.  One can also note that adding a new /var/lib/iwd/my_ssid.psk, by
> manually creating the file, is not picked up by iwd automatically.
> 
> I've only tested this with iwd 1.17 and ell 0.43 due to the complexity to
> build another version using Yocto.  However, I've checked the source code

You can just keep a recipe in your own custom layer with a higher version.
Yocto actually makes this really easy - in practice all we do when bumping iwd
is to update the SRC_URI checksum and PV (in the filename), while the recipe is
otherwise the same as in meta-oe.

> for iwd on master and they don't seem to handle the fact that /var/lib/iwd
> is a symlink.  I've tried to search the mailing-list and google, but can't
> find anything about this.  It seems I can change the /var/lib/iwd directory
> during compilation by setting DAEMON_STORAGEDIR during configure/build.
> However, that is a bit unflexible as my system is Yocto based, and hacking
> the Yocto recipes for this seems wrong.  Normally you symlink things in your
> file tree in IoT devices to point to persistent storage areas.
> 
> I don't have a solution, but at least iwd should detect that /var/lib/iwd is
> a symlink and warn the user.  The implication is that the known-network list
> doesn't work at all.  The internal known-network list should work even if
> inotify doesn't.

The way we solve this problem in general is to bind mount a directory on our rw
filesystem to /var/lib/iwd. We also use Yocto, although we have a custom recipe
for setting up our bind mounts. For inspiration, you can look at the
volatile-binds.bb recipe in poky, which does pretty much the same.

Perhaps iwd should tolerate symlinks, but iwd is not the only program that has
issues with symlinks, so often a bind mount is more robust :-)

Kind regards,
Alvin

> 
> Perhaps also it should be allowed to change the default storage dir in
> /etc/iwd/main.conf.
> 
> -- 
> Best regards,
> Joakim LotsengÄrd
> _______________________________________________
> iwd mailing list -- iwd(a)lists.01.org
> To unsubscribe send an email to iwd-leave(a)lists.01.org

Re: Possible bug when /var/lib/iwd is a symlink

[@ 2022-04-14 8:54 UTC (permalink / raw)]

[-- Attachment #1: Type: text/plain, Size: 1392 bytes --]

On Thu, 14 Apr 2022 at 10:17, Joakim Lotsengård
<joakim.l(a)orbital-systems.com> wrote:
>
> Possible bug when /var/lib/iwd is a symlink:
>

Follow up with a demo-patch that solves the problem, but probably isn't the
correct solution. I have not thought about any security implications this might
have.

Subject: [PATCH]: Resolve /var/lib/iwd before inotify

l_dir_watch_new() in knownnetworks.c will not follow symlink.

Quick hack to realname() resolve before adding the inotify.

If not done, the known network list will misbehave. Forgotten
networks will not be removed, and connected networks won't
get added to the known network list.
---
 src/knownnetworks.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/knownnetworks.c b/src/knownnetworks.c
index 487b7017..89c0efcc 100644
--- a/src/knownnetworks.c
+++ b/src/knownnetworks.c
@@ -1102,9 +1102,13 @@ static int known_networks_init(void)

  closedir(dir);

- storage_dir_watch = l_dir_watch_new(storage_dir,
+ // HAX: Resolve the storage_dir as l_dir_watch_new() won't follow symlinks
+ char *storage_dir_real = realpath(storage_dir, NULL);
+ storage_dir_watch = l_dir_watch_new(storage_dir_real,
  known_networks_watch_cb, NULL,
  known_networks_watch_destroy);
+ free(storage_dir_real);
+
  watchlist_init(&known_network_watches, NULL);

  return 0;
-- 
2.25.1

Possible bug when /var/lib/iwd is a symlink

[@ 2022-04-14 8:17 UTC (permalink / raw)]

[-- Attachment #1: Type: text/plain, Size: 2150 bytes --]

Hi,

Possible bug when /var/lib/iwd is a symlink:

I ran across a problem when the ending part of /var/lib/iwd is a symlink.
In my case it points to /mnt/persistent/iwd.  We are an IoT device that runs
on UBIFS (+ tmpfs) and the entire rootfs partition of the NAND will be
updated on FOTA.  Hence anything we want to save in an upgrade must be on a
persistent partition.  Our target is a Freescale i.MX6 Ultralite module
running an ARM A7 core (if that matters).

Problem seems to be that the l_dir_watch_new() call in knownnetworks.c never
follows symlink.  l_dir_watch_new() uses IN_DONT_FOLLOW in its call to
inotify_add_watch() in ell.

This will make the known network list misbehave.  If one forgets a
known-network via iwctl, the list doesn't get updated.  The same goes for
connecting to a not already known network.  It is easy to reproduce.  Just
symlink /var/lib/iwd to another location and run some connect + forget in
iwctl.  One can also note that adding a new /var/lib/iwd/my_ssid.psk, by
manually creating the file, is not picked up by iwd automatically.

I've only tested this with iwd 1.17 and ell 0.43 due to the complexity to
build another version using Yocto.  However, I've checked the source code
for iwd on master and they don't seem to handle the fact that /var/lib/iwd
is a symlink.  I've tried to search the mailing-list and google, but can't
find anything about this.  It seems I can change the /var/lib/iwd directory
during compilation by setting DAEMON_STORAGEDIR during configure/build.
However, that is a bit unflexible as my system is Yocto based, and hacking
the Yocto recipes for this seems wrong.  Normally you symlink things in your
file tree in IoT devices to point to persistent storage areas.

I don't have a solution, but at least iwd should detect that /var/lib/iwd is
a symlink and warn the user.  The implication is that the known-network list
doesn't work at all.  The internal known-network list should work even if
inotify doesn't.

Perhaps also it should be allowed to change the default storage dir in
/etc/iwd/main.conf.

-- 
Best regards,
Joakim Lotsengård