Hi Andreas, > Booting an nfsroot with connman requires passing -I eth0 to ignore > the interface. This isn't very nice, for at least the following > reasons: > > * A User interface based on connman is led to believe that there's no > network interface and thus connman seems to be offline when it's not. > * The DHCP lease obtained by the kernel won't get renewed. > * DNS servers won't get obtained from DHCP, thus requiring a workaround > to copy /proc/net/pnp to /etc/resolv.conf and passing -r to connmand.
this sentence makes no sense to me. I do not even know what /proc/net/pnp actually is. > Therefore change behaviour to restrict interfaces passed with -I to > read-only ioctls. > > Signed-off-by: Andreas Oberritter <[email protected]> No signed-off-by in this project. > --- > Makefile.am | 1 + > include/device.h | 3 ++ > src/device.c | 34 +++++++++++--- > src/inet.c | 140 ++++++++++++++++++++++++++++++++++++++++++++++++++----- > src/ipconfig.c | 16 +++++++ > src/rtnl.c | 20 +------- > 6 files changed, 178 insertions(+), 36 deletions(-) > > diff --git a/Makefile.am b/Makefile.am > index 19cc8b1..423261f 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -210,6 +210,7 @@ src_connmand_CFLAGS = @DBUS_CFLAGS@ @GLIB_CFLAGS@ > @XTABLES_CFLAGS@ \ > -DSTORAGEDIR=\""$(storagedir)\"" \ > -DVPN_STORAGEDIR=\""$(vpn_storagedir)\"" \ > -DCONFIGDIR=\""$(configdir)\"" \ > + -DCONNMAND \ We are actually not doing build time options. They need to become runtime options. > -I$(builddir)/src > > EXTRA_DIST = src/genbuiltin src/connman-dbus.conf src/connman-polkit.conf \ > diff --git a/include/device.h b/include/device.h > index 57b925c..3c9615f 100644 > --- a/include/device.h > +++ b/include/device.h > @@ -93,6 +93,9 @@ int connman_device_set_string(struct connman_device *device, > const char *key, const char *value); > const char *connman_device_get_string(struct connman_device *device, > const char *key); > +void connman_device_set_readonly(struct connman_device *device, > + bool readonly); > +bool connman_device_get_readonly(struct connman_device *device); > > int connman_device_add_network(struct connman_device *device, > struct connman_network *network); > diff --git a/src/device.c b/src/device.c > index c0683ab..301e850 100644 > --- a/src/device.c > +++ b/src/device.c > @@ -54,6 +54,7 @@ struct connman_device { > bool powered; > bool scanning; > bool disconnected; > + bool readonly; > char *name; > char *node; > char *address; > @@ -782,6 +783,32 @@ bool connman_device_get_disconnected(struct > connman_device *device) > } > > /** > + * connman_device_set_readonly: > + * @device: device structure > + * @readonly: read-only state > + * > + * Change read-only state of device > + */ > +void connman_device_set_readonly(struct connman_device *device, > + bool readonly) > +{ > + DBG("device %p readonly %d", device, readonly); > + > + device->readonly = readonly; > +} > + > +/** > + * connman_device_get_readonly: > + * @device: device structure > + * > + * Get device read-only state > + */ > +bool connman_device_get_readonly(struct connman_device *device) > +{ > + return device->readonly; > +} > + > +/** > * connman_device_set_string: > * @device: device structure > * @key: unique identifier > @@ -1246,12 +1273,6 @@ struct connman_device > *connman_device_create_from_index(int index) > if (!devname) > return NULL; > > - if (__connman_device_isfiltered(devname)) { > - connman_info("Ignoring interface %s (filtered)", devname); > - g_free(devname); > - return NULL; > - } > - > type = __connman_rtnl_get_device_type(index); > > switch (type) { > @@ -1305,6 +1326,7 @@ struct connman_device > *connman_device_create_from_index(int index) > } > > connman_device_set_string(device, "Address", addr); > + connman_device_set_readonly(device, > __connman_device_isfiltered(devname)); > > done: > g_free(devname); > diff --git a/src/inet.c b/src/inet.c > index cd220ff..ac646c9 100644 > --- a/src/inet.c > +++ b/src/inet.c > @@ -55,6 +55,21 @@ > ((struct rtattr *) (((uint8_t*) (nmsg)) + \ > NLMSG_ALIGN((nmsg)->nlmsg_len))) > > +static inline int __connman_inet_check_write_perm(int ifindex) > +{ > + #if defined(CONNMAND) > + struct connman_device *dev = connman_device_find_by_index(ifindex); > + > + if (!dev) > + return -ENODEV; > + > + if (connman_device_get_readonly(dev)) > + return -EPERM; > + #endif > + > + return 0; > +} > + > int __connman_inet_rtnl_addattr_l(struct nlmsghdr *n, size_t max_length, > int type, const void *data, size_t data_length) > { > @@ -104,6 +119,11 @@ int __connman_inet_modify_address(int cmd, int flags, > if (family != AF_INET && family != AF_INET6) > return -EINVAL; > > + if (__connman_inet_check_write_perm(index) < 0) { > + DBG("insufficient permission, ignoring request"); > + return 0; > + } > + > memset(&request, 0, sizeof(request)); > > header = (struct nlmsghdr *)request; > @@ -245,6 +265,12 @@ int connman_inet_ifup(int index) > struct ifreq ifr; > int sk, err; > > + err = __connman_inet_check_write_perm(index); > + if (err < 0) { > + DBG("insufficient permission"); > + return err; > + } > + > sk = socket(PF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0); > if (sk < 0) > return -errno; > @@ -288,6 +314,12 @@ int connman_inet_ifdown(int index) > struct sockaddr_in *addr; > int sk, err; > > + err = __connman_inet_check_write_perm(index); > + if (err < 0) { > + DBG("insufficient permission"); > + return err; > + } > + > sk = socket(PF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0); > if (sk < 0) > return -errno; > @@ -454,11 +486,17 @@ int connman_inet_add_network_route(int index, const > char *host, > struct ifreq ifr; > struct rtentry rt; > struct sockaddr_in addr; > - int sk, err = 0; > + int sk, err; > > DBG("index %d host %s gateway %s netmask %s", index, > host, gateway, netmask); > > + err = __connman_inet_check_write_perm(index); > + if (err < 0) { > + DBG("insufficient permission"); > + return err; > + } > + > sk = socket(PF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0); > if (sk < 0) { > err = -errno; > @@ -525,10 +563,16 @@ int connman_inet_del_network_route(int index, const > char *host) > struct ifreq ifr; > struct rtentry rt; > struct sockaddr_in addr; > - int sk, err = 0; > + int sk, err; > > DBG("index %d host %s", index, host); > > + err = __connman_inet_check_write_perm(index); > + if (err < 0) { > + DBG("insufficient permission"); > + return err; > + } > + > sk = socket(PF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0); > if (sk < 0) { > err = -errno; > @@ -573,13 +617,19 @@ int connman_inet_del_ipv6_network_route(int index, > const char *host, > unsigned char prefix_len) > { > struct in6_rtmsg rt; > - int sk, err = 0; > + int sk, err; > > DBG("index %d host %s", index, host); > > if (!host) > return -EINVAL; > > + err = __connman_inet_check_write_perm(index); > + if (err < 0) { > + DBG("insufficient permission"); > + return err; > + } > + > memset(&rt, 0, sizeof(rt)); > > rt.rtmsg_dst_len = prefix_len; > @@ -623,13 +673,19 @@ int connman_inet_add_ipv6_network_route(int index, > const char *host, > unsigned char prefix_len) > { > struct in6_rtmsg rt; > - int sk, err = 0; > + int sk, err; > > DBG("index %d host %s gateway %s", index, host, gateway); > > if (!host) > return -EINVAL; > > + err = __connman_inet_check_write_perm(index); > + if (err < 0) { > + DBG("insufficient permission"); > + return err; > + } > + > memset(&rt, 0, sizeof(rt)); > > rt.rtmsg_dst_len = prefix_len; > @@ -677,13 +733,19 @@ int connman_inet_add_ipv6_host_route(int index, const > char *host, > int connman_inet_clear_ipv6_gateway_address(int index, const char *gateway) > { > struct in6_rtmsg rt; > - int sk, err = 0; > + int sk, err; > > DBG("index %d gateway %s", index, gateway); > > if (!gateway) > return -EINVAL; > > + err = __connman_inet_check_write_perm(index); > + if (err < 0) { > + DBG("insufficient permission"); > + return err; > + } > + > memset(&rt, 0, sizeof(rt)); > > if (inet_pton(AF_INET6, gateway, &rt.rtmsg_gateway) < 0) { > @@ -720,10 +782,16 @@ int connman_inet_set_gateway_interface(int index) > struct ifreq ifr; > struct rtentry rt; > struct sockaddr_in addr; > - int sk, err = 0; > + int sk, err; > > DBG("index %d", index); > > + err = __connman_inet_check_write_perm(index); > + if (err < 0) { > + DBG("insufficient permission"); > + return err; > + } > + > sk = socket(PF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0); > if (sk < 0) { > err = -errno; > @@ -773,10 +841,16 @@ int connman_inet_set_ipv6_gateway_interface(int index) > struct rtentry rt; > struct sockaddr_in6 addr; > const struct in6_addr any = IN6ADDR_ANY_INIT; > - int sk, err = 0; > + int sk, err; > > DBG("index %d", index); > > + err = __connman_inet_check_write_perm(index); > + if (err < 0) { > + DBG("insufficient permission"); > + return err; > + } > + > sk = socket(PF_INET6, SOCK_DGRAM | SOCK_CLOEXEC, 0); > if (sk < 0) { > err = -errno; > @@ -825,10 +899,16 @@ int connman_inet_clear_gateway_address(int index, const > char *gateway) > struct ifreq ifr; > struct rtentry rt; > struct sockaddr_in addr; > - int sk, err = 0; > + int sk, err; > > DBG("index %d gateway %s", index, gateway); > > + err = __connman_inet_check_write_perm(index); > + if (err < 0) { > + DBG("insufficient permission"); > + return err; > + } > + > sk = socket(PF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0); > if (sk < 0) { > err = -errno; > @@ -882,10 +962,16 @@ int connman_inet_clear_gateway_interface(int index) > struct ifreq ifr; > struct rtentry rt; > struct sockaddr_in addr; > - int sk, err = 0; > + int sk, err; > > DBG("index %d", index); > > + err = __connman_inet_check_write_perm(index); > + if (err < 0) { > + DBG("insufficient permission"); > + return err; > + } > + > sk = socket(PF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0); > if (sk < 0) { > err = -errno; > @@ -935,10 +1021,16 @@ int connman_inet_clear_ipv6_gateway_interface(int > index) > struct rtentry rt; > struct sockaddr_in6 addr; > const struct in6_addr any = IN6ADDR_ANY_INIT; > - int sk, err = 0; > + int sk, err; > > DBG("index %d", index); > > + err = __connman_inet_check_write_perm(index); > + if (err < 0) { > + DBG("insufficient permission"); > + return err; > + } > + > sk = socket(PF_INET6, SOCK_DGRAM | SOCK_CLOEXEC, 0); > if (sk < 0) { > err = -errno; > @@ -1035,11 +1127,17 @@ bool connman_inet_compare_subnet(int index, const > char *host) > int connman_inet_remove_from_bridge(int index, const char *bridge) > { > struct ifreq ifr; > - int sk, err = 0; > + int sk, err; > > if (!bridge) > return -EINVAL; > > + err = __connman_inet_check_write_perm(index); > + if (err < 0) { > + DBG("insufficient permission"); > + return err; > + } > + > sk = socket(AF_INET, SOCK_STREAM | SOCK_CLOEXEC, 0); > if (sk < 0) { > err = -errno; > @@ -1066,11 +1164,17 @@ out: > int connman_inet_add_to_bridge(int index, const char *bridge) > { > struct ifreq ifr; > - int sk, err = 0; > + int sk, err; > > if (!bridge) > return -EINVAL; > > + err = __connman_inet_check_write_perm(index); > + if (err < 0) { > + DBG("insufficient permission"); > + return err; > + } > + > sk = socket(AF_INET, SOCK_STREAM | SOCK_CLOEXEC, 0); > if (sk < 0) { > err = -errno; > @@ -1099,6 +1203,12 @@ int connman_inet_set_mtu(int index, int mtu) > struct ifreq ifr; > int sk, err; > > + err = __connman_inet_check_write_perm(index); > + if (err < 0) { > + DBG("insufficient permission"); > + return err; > + } > + > sk = socket(AF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0); > if (sk < 0) > return sk; > @@ -2812,6 +2922,12 @@ static int iproute_default_modify(int cmd, uint32_t > table_id, int ifindex, > if (ret <= 0) > return -EINVAL; > > + ret = __connman_inet_check_write_perm(ifindex); > + if (ret < 0) { > + DBG("insufficient permission"); > + return ret; > + } > + > memset(&rth, 0, sizeof(rth)); > > rth.req.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct rtmsg)); > diff --git a/src/ipconfig.c b/src/ipconfig.c > index f8c148b..3ecabfc 100644 > --- a/src/ipconfig.c > +++ b/src/ipconfig.c > @@ -90,6 +90,17 @@ static GHashTable *ipdevice_hash = NULL; > static GList *ipconfig_list = NULL; > static bool is_ipv6_supported = false; > > +static int __connman_ipconfig_check_write_perm(const gchar *ifname) > +{ > + int index = connman_inet_ifindex(ifname); > + struct connman_device *dev = connman_device_find_by_index(index); > + > + if (!dev) > + return -ENODEV; > + > + return connman_device_get_readonly(dev) ? -EPERM : 0; > +} > + > void __connman_ipconfig_clear_address(struct connman_ipconfig *ipconfig) > { > if (!ipconfig) > @@ -210,6 +221,8 @@ static void set_ipv6_state(gchar *ifname, bool enable) > > if (!ifname) > path = g_strdup("/proc/sys/net/ipv6/conf/all/disable_ipv6"); > + else if (__connman_ipconfig_check_write_perm(ifname) < 0) > + return; > else > path = g_strdup_printf( > "/proc/sys/net/ipv6/conf/%s/disable_ipv6", ifname); > @@ -273,6 +286,9 @@ static void set_ipv6_privacy(gchar *ifname, int value) > if (!ifname) > return; > > + if (__connman_ipconfig_check_write_perm(ifname) < 0) > + return; > + > path = g_strdup_printf("/proc/sys/net/ipv6/conf/%s/use_tempaddr", > ifname); > > diff --git a/src/rtnl.c b/src/rtnl.c > index b8b02c4..25da07b 100644 > --- a/src/rtnl.c > +++ b/src/rtnl.c > @@ -83,17 +83,6 @@ static void free_interface(gpointer data) > g_free(interface); > } > > -static bool ether_blacklisted(const char *name) > -{ > - if (!name) > - return true; > - > - if (__connman_device_isfiltered(name)) > - return true; > - > - return false; > -} > - You are taking existing functionality out. That is not really possible and might break existing users. > static bool wext_interface(char *ifname) > { > struct iwreq wrq; > @@ -124,13 +113,8 @@ static void read_uevent(struct interface_data *interface) > > name = connman_inet_ifname(interface->index); > > - if (ether_blacklisted(name)) { > - interface->service_type = CONNMAN_SERVICE_TYPE_UNKNOWN; > - interface->device_type = CONNMAN_DEVICE_TYPE_UNKNOWN; > - } else { > - interface->service_type = CONNMAN_SERVICE_TYPE_ETHERNET; > - interface->device_type = CONNMAN_DEVICE_TYPE_ETHERNET; > - } > + interface->service_type = CONNMAN_SERVICE_TYPE_ETHERNET; > + interface->device_type = CONNMAN_DEVICE_TYPE_ETHERNET; > > filename = g_strdup_printf("/sys/class/net/%s/uevent", name); How is this making it better. It now gets classified as Ethernet, but ConnMan can not do anything with that interface? Who is running DHCP etc. now? Regards Marcel _______________________________________________ connman mailing list [email protected] https://lists.connman.net/mailman/listinfo/connman
