[PATCHv2] libmbim-glib,proxy: add a configure flag to set the user ID of MBIM proxy

[Aleksander Morgado]

On Fri, Nov 7, 2014 at 5:42 PM, Roshan Pius <rpius at chromium.org> wrote:
> Currently, the MBIM proxy process assumes that it is run as root user and
> that all incoming client connection users are also root.
> However, it's not always preferable to run the MBIM proxy as root for
> security reasons. On some platforms, the MBIM proxy could be constrained
> to run as a less-privileged user and specially granted the permission to
> access the MBIM device. So, adding a compile time flag in libmbim to check
> for the specified user, rather than assume it to be the root user. If the flag is
> not sent, it'll revert to the existing behaviour of checking for
> user=root(i.e UID=0)
>

Also, if the flag is set in configure, but when running we don't find
the specified user, we should warn (that, you already do), but also
then still let user=root use the proxy. It's just a fallback for
misconfigured systems, but I think we should do it instead of
returning an error. What do you think?


> ---
>  configure.ac                  | 11 +++++++++++
>  src/libmbim-glib/mbim-proxy.c | 25 ++++++++++++++++++++-----
>  2 files changed, 31 insertions(+), 5 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 27f82c9..3e0ede2 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -96,6 +96,17 @@ AC_SUBST(GLIB_MKENUMS)
>  dnl Documentation
>  GTK_DOC_CHECK(1.0)
>
> +# MBIM proxy UID
> +AC_ARG_ENABLE(mbim-proxy-username,
> +              AS_HELP_STRING([--enable-mbim-proxy-username=<username>], [where mbim proxy username is]),
> +              mbim_proxy_username=$enableval,
> +              mbim_proxy_username="")
> +if ! test x"$mbim_proxy_username" = x""; then
> +  AC_DEFINE_UNQUOTED(MBIM_PROXY_USERNAME, $mbim_proxy_username, [Define the MBIM Proxy username])
> +else
> +  AC_DEFINE(MBIM_PROXY_USERNAME, "root", [Define the MBIM Proxy username])
> +fi
> +
>  dnl Man page
>  AC_PATH_PROG(HELP2MAN, help2man, false)
>  AM_CONDITIONAL(BUILDOPT_MAN, test x$HELP2MAN != xfalse)
> diff --git a/src/libmbim-glib/mbim-proxy.c b/src/libmbim-glib/mbim-proxy.c
> index 7677cc6..6f709c1 100644
> --- a/src/libmbim-glib/mbim-proxy.c
> +++ b/src/libmbim-glib/mbim-proxy.c
> @@ -25,12 +25,15 @@
>  #include <string.h>
>  #include <ctype.h>
>  #include <sys/file.h>
> +#include <sys/types.h>
>  #include <errno.h>
> +#include <pwd.h>
>
>  #include <glib.h>
>  #include <glib/gstdio.h>
>  #include <gio/gunixsocketaddress.h>
>
> +#include "config.h"
>  #include "mbim-device.h"
>  #include "mbim-utils.h"
>  #include "mbim-proxy.h"
> @@ -1041,6 +1044,7 @@ incoming_cb (GSocketService *service,
>      Client *client;
>      GCredentials *credentials;
>      GError *error = NULL;
> +    struct passwd *expected_usr = NULL;
>      uid_t uid;
>
>      g_debug ("Client (%d) connection open...", g_socket_get_fd (g_socket_connection_get_socket (connection)));
> @@ -1060,8 +1064,13 @@ incoming_cb (GSocketService *service,
>          return;
>      }
>
> -    if (uid != 0) {
> -        g_warning ("Client not allowed: Not enough privileges");
> +    expected_usr = getpwnam (MBIM_PROXY_USERNAME);
> +    if (!expected_usr) {
> +        g_warning ("Unknown user configured: %s", MBIM_PROXY_USERNAME);
> +        return;
> +    }
> +    if (uid != expected_usr->pw_uid) {
> +        g_warning ("Client not allowed: Not the expected user: %s", MBIM_PROXY_USERNAME);
>          return;
>      }
>
> @@ -1213,13 +1222,19 @@ MbimProxy *
>  mbim_proxy_new (GError **error)
>  {
>      MbimProxy *self;
> +    struct passwd *expected_usr = NULL;
>
> -    /* Only root can run the mbim-proxy */
> -    if (getuid () != 0) {
> +    /* Only the specified user can run the mbim-proxy */
> +    expected_usr = getpwnam (MBIM_PROXY_USERNAME);
> +    if (!expected_usr) {
> +        g_warning ("Unknown user configured: %s", MBIM_PROXY_USERNAME);
> +        return NULL;
> +    }
> +    if (getuid () != expected_usr->pw_uid) {
>          g_set_error (error,
>                       MBIM_CORE_ERROR,
>                       MBIM_CORE_ERROR_FAILED,
> -                     "Not enough privileges");
> +                     "Not started with the expected user: %s", MBIM_PROXY_USERNAME);
>          return NULL;
>      }
>
> --
> 2.1.0.rc2.206.gedb03e5
>



-- 
Aleksander
https://aleksander.es