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

[Dan Williams]

On Fri, 2014-11-07 at 09:20 -0800, Roshan Pius 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)

Looks OK to me.

Dan

> ---
>  configure.ac                  | 11 +++++++++++
>  src/libmbim-glib/mbim-proxy.c | 38 ++++++++++++++++++++++++++++++++------
>  2 files changed, 43 insertions(+), 6 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..d3d32e0 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,17 @@ 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);
> +        /* Falling back to check for root user if the configured user is unknown */
> +        if (uid != 0) {
> +            g_warning ("Client not allowed: Not enough privileges");
> +            return;
> +        }
> +    }
> +    else if (uid != expected_usr->pw_uid) {
> +        g_warning ("Client not allowed: Not the expected user: %s", MBIM_PROXY_USERNAME);
>          return;
>      }
>  
> @@ -1213,13 +1226,26 @@ MbimProxy *
>  mbim_proxy_new (GError **error)
>  {
>      MbimProxy *self;
> -
> -    /* Only root can run the mbim-proxy */
> -    if (getuid () != 0) {
> +    struct passwd *expected_usr = NULL;
> +
> +    /* 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);
> +        /* Falling back to check for root user if the configured user is unknown */
> +        if (getuid () != 0) {
> +            g_set_error (error,
> +                         MBIM_CORE_ERROR,
> +                         MBIM_CORE_ERROR_FAILED,
> +                          "Not enough privileges");
> +            return NULL;
> +        }
> +    }
> +    else 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;
>      }
>