Merge lp:~sergei.glushchenko/percona-pam-for-mysql/BT32086-bug1160348 into lp:percona-pam-for-mysql

[Sergei Glushchenko]

This adds support for supplementary groups to PAM plugin user mapping. getgrouplist is used to obtain full list of groups user belongs to. If user is a member of several mapped groups, the group which is listed in authentication string first will be taken.

Also minor issues were addressed, such as copyright changed in files, broken link was removed, libmysqlclient is taken from mysql_config output.

Tested on CentOS 5 with PAM Unix and Mac OS X 10.7 with Open Directory.

[Laurynas Biveinis (laurynas-biveinis)]

Looks good.

Please convert bug 1160348 to a blueprint for Percona Server, and please consider expanding the PAM Jenkins test setup to handle this.

review: Approve

[Sergei Glushchenko (sergei.glushchenko)]

Laurynas,

I don't like current concept of test cases for PAM. They are very intrusive. They add users and groups, create files in /etc and so on. Is there any chance that we revisit it in future?

I believe that tests should be easy to run by every user who downloaded and built software. Current approach makes it very hard to do. One should either run test suite as root or manually setup users and groups. Also there is no cleanup at all. It means that created user accounts remain after test is finished which is not good at all.

Can't we use some kind of mocks or stubs for NSS lookups and PAM functions?
We also could easily cover large of the code by unit tests.

[Laurynas Biveinis (laurynas-biveinis)]

The current test cases are meant to be run in VMs, and as such they get their job done. I don't see a problem with extending the current testcases first, I don't think the current MP is the right place/time to pursue the alternatives.

For alternatives in general we can do what MariaDB did, i.e. a mock PAM module.

[Sergei Glushchenko (sergei.glushchenko)]

So, for every new test case I have to create issue in "Percona Dev Services" board to change Jenkins configuration. In this case I need a user to be created and added to several groups. I find that it is not convenient.

[Laurynas Biveinis (laurynas-biveinis)]

Sergei -

Please create an issue "consider migrating away from VM-based PAM testing" instead.

To work with the current tests, please have a look at test/dbqp/percona_tests/percona_pam. I don't think there is any need to change Jenkins configuration itself.

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2012-06-13 04:13:02 +0000
+++ CMakeLists.txt	2013-06-06 14:09:25 +0000
@@ -1,4 +1,4 @@
-# (C) 2011 Percona Inc.
+# (C) 2011-2013 Percona Ireland Ltd.
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -23,7 +23,7 @@
 IF(HAVE_PAM AND HAVE_MYSQLCLIENT AND HAVE_GETPWNAM_R AND HAVE_GETGRGID_R)
   SET(AUTH_PAM_COMMON_SOURCES 
     src/auth_pam_common.c src/lib_auth_pam_client.c src/lib_auth_pam_client.h
-    src/auth_mapping.h src/auth_mapping.c)
+    src/auth_mapping.h src/auth_mapping.c src/groups.c src/groups.h)
   SET(AUTH_PAM_SOURCES ${AUTH_PAM_COMMON_SOURCES} src/auth_pam.c)
   SET(AUTH_PAM_COMPAT_SOURCES ${AUTH_PAM_COMMON_SOURCES} src/auth_pam_compat.c)
   MYSQL_ADD_PLUGIN(auth_pam ${AUTH_PAM_SOURCES} LINK_LIBRARIES pam)

=== modified file 'configure.ac'
--- configure.ac	2012-02-10 04:21:32 +0000
+++ configure.ac	2013-06-06 14:09:25 +0000
@@ -1,5 +1,5 @@
 #                                               -*- Autoconf -*-
-# (C) 2011 Percona Inc.
+# (C) 2011-2013 Percona Ireland Ltd.
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -46,10 +46,7 @@
     AC_MSG_ERROR(
         [Unable to find PAM. Please install the PAM development libraries])
 )
-AC_CHECK_LIB([mysqlclient], [mysql_init], [DIALOG_LIBS="$DIALOG_LIBS -lmysqlclient"],
-    AC_MSG_ERROR(
-        [Unable to find libmysqlclient. Please install the mysql development libraries])
-)
+DIALOG_LIBS="$DIALOG_LIBS "`${MYSQL_CONFIG} --libs_r`
 
 # Replace -I with -isystem for the MySQL header include GCC option to silence 
 # warnings originating from them

=== modified file 'src/Makefile.am'
--- src/Makefile.am	2012-02-13 06:19:11 +0000
+++ src/Makefile.am	2013-06-06 14:09:25 +0000
@@ -1,4 +1,4 @@
-# (C) 2011 Percona Inc.
+# (C) 2011-2013 Percona Ireland Ltd.
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -20,11 +20,14 @@
 plugin_CPPFLAGS = -DMYSQL_DYNAMIC_PLUGIN
 plugin_LDFLAGS = -module -avoid-version -shared
 
-auth_pam_la_SOURCES = auth_pam_common.c auth_pam.c lib_auth_pam_client.h lib_auth_pam_client.c auth_mapping.c
+auth_pam_la_SOURCES = auth_pam_common.c auth_pam.c lib_auth_pam_client.h \
+                      lib_auth_pam_client.c auth_mapping.c groups.c
 auth_pam_la_CPPFLAGS = $(plugin_CPPFLAGS)
 auth_pam_la_LDFLAGS = $(plugin_LDFLAGS) $(AUTH_PAM_LIBS)
 
-auth_pam_compat_la_SOURCES = auth_pam_common.c auth_pam_compat.c lib_auth_pam_client.h lib_auth_pam_client.c auth_mapping.c
+auth_pam_compat_la_SOURCES = auth_pam_common.c auth_pam_compat.c \
+                             lib_auth_pam_client.h lib_auth_pam_client.c \
+                             auth_mapping.c groups.c
 auth_pam_compat_la_CPPFLAGS = $(plugin_CPPFLAGS)
 auth_pam_compat_la_LDFLAGS = $(plugin_LDFLAGS) $(AUTH_PAM_LIBS)
 

=== modified file 'src/auth_mapping.c'
--- src/auth_mapping.c	2012-02-09 16:25:22 +0000
+++ src/auth_mapping.c	2013-06-06 14:09:25 +0000
@@ -1,5 +1,5 @@
 /*
-(C) 2012 Percona Inc.
+(C) 2012, 2013 Percona Ireland Ltd.
 
 This program is free software; you can redistribute it and/or modify
 it under the terms of the GNU General Public License as published by
@@ -16,19 +16,13 @@
 */
 
 #include "auth_mapping.h"
-#include <pwd.h>
-#include <grp.h>
+#include "groups.h"
 #include <string.h>
 #include <stdlib.h>
 #include <ctype.h>
 
 #define MIN(X,Y) ((X) < (Y) ? (X) : (Y))
 
-/** The maximum length of buffer for storing NSS record. NSS will store in
-    buffer the whole result of lookup request including user name,
-    gecos, etc. */
-enum { max_nss_name_len = 10240 };
-
 /** Token representation:
     token type, string repr, length of token */
 struct token
@@ -50,43 +44,6 @@
   const char *ptr;
 };
 
-
-/** Lookup NSS database for group name by specified user name.
-    On sucess user_group returned, otherwise NULL */
-char *lookup_user_group (const char *user_name,
-                         char *user_group, int user_group_len)
-{
-  struct passwd pwd, *pwd_result;
-  struct group grp, *grp_result;
-  char *buf;
-  int error;
-
-  buf= malloc(max_nss_name_len);
-  if (buf == NULL)
-    return NULL;
-
-  error= getpwnam_r(user_name, &pwd, buf, max_nss_name_len, &pwd_result);
-  if (error != 0 || pwd_result == NULL)
-  {
-    free(buf);
-    return NULL;
-  }
-
-  error= getgrgid_r(pwd_result->pw_gid,
-                    &grp, buf, max_nss_name_len, &grp_result);
-  if (error != 0 || grp_result == NULL)
-  {
-    free(buf);
-    return NULL;
-  }
-
-  strncpy(user_group, grp_result->gr_name, user_group_len);
-  user_group[user_group_len]= '\0';
-  free(buf);
-
-  return user_group;
-}
-
 /** Get next token from buf. Returns new buf position. */
 static const char *get_token(struct token *token,
                              const char *buf)
@@ -147,7 +104,7 @@
     otherwise NULL */
 const char *mapping_iter_next(struct mapping_iter *it)
 {
-  struct token token[4];
+  struct token token[4]= {{0}};
 
   /* read next 4 tokens */
   it->ptr= get_token(token + 3,
@@ -182,28 +139,63 @@
   free(it);
 }
 
-/** Get value by given key. On success value_buf returned,
-    otherwise NULL */
-char *mapping_get_value(const char *key, char *value_buf, int value_buf_len,
-                        const char *mapping_string)
+/** Get mapped value for given user name.
+    Value is looked up by using all user groups as a key.
+    Auth string is iterated only once, while groups are iterated
+    for every key-value pair. This is mean than auth string order
+    is dominant.
+
+    Example:
+
+    given:
+      user "foo" is the member of "wheel", "staff" and "bar".
+      auth string is "mysql, root=user1, bar=user2, staff=user3"
+
+    result is "user2".
+
+    On success value_buf returned, otherwise NULL */
+char *mapping_lookup_user(const char *user_name,
+                          char *value_buf, int value_buf_len,
+                          const char *mapping_string)
 {
-  struct mapping_iter *it= mapping_iter_new(mapping_string);
-  int key_len= strlen(key);
+  /* Iterate through the key-value list stored in auth_string and
+  find key (which is interpreted as group name) in the list of groups
+  for specified user. If match is found, store appropriate value in
+  the authenticated_as field. */
+  struct groups_iter *group_it;
+  struct mapping_iter *keyval_it;
+  const char *key;
+  const char *group;
 
-  if (it == NULL)
+  keyval_it= mapping_iter_new(mapping_string);
+  if (keyval_it == NULL)
     return NULL;
 
-  while (mapping_iter_next(it))
+  group_it= groups_iter_new(user_name);
+  if (group_it == NULL)
   {
-    if (key_len == it->key_len && strncmp(key, it->key, key_len) == 0)
-    {
-      memcpy(value_buf, it->value, MIN(value_buf_len, it->value_len));
-      value_buf[MIN(value_buf_len, it->value_len)]= '\0';
-      mapping_iter_free(it);
-      return value_buf;
+    mapping_iter_free(keyval_it);
+    return NULL;
+  }
+
+  while ((key= mapping_iter_next(keyval_it)) != NULL) {
+    while ((group= groups_iter_next(group_it)) != NULL) {
+      if (keyval_it->key_len == strlen(group) &&
+          strncmp(key, group, keyval_it->key_len) == 0) {
+        /* match is found */
+        memcpy(value_buf, keyval_it->value,
+               MIN(value_buf_len, keyval_it->value_len));
+        value_buf[MIN(value_buf_len, keyval_it->value_len)]= '\0';
+        groups_iter_free(group_it);
+        mapping_iter_free(keyval_it);
+        return value_buf;
+      }
     }
+    groups_iter_reset(group_it);
   }
-  mapping_iter_free(it);
+
+  groups_iter_free(group_it);
+  mapping_iter_free(keyval_it);
 
   return NULL;
 }

=== modified file 'src/auth_mapping.h'
--- src/auth_mapping.h	2012-02-13 06:19:11 +0000
+++ src/auth_mapping.h	2013-06-06 14:09:25 +0000
@@ -1,7 +1,7 @@
 #ifndef AUTH_MAPPING_INCLUDED
 #define AUTH_MAPPING_INCLUDED
 /*
-  (C) 2012 Percona Inc.
+  (C) 2012, 2013 Percona Ireland Ltd.
 
   This program is free software; you can redistribute it and/or modify
   it under the terms of the GNU General Public License as published by
@@ -31,11 +31,6 @@
 /** Mapping iterator. It's not exposed outsude */
 struct mapping_iter;
 
-/** Lookup NSS database for group name by specified user name.
-    On sucess user_group returned, otherwise NULL */
-char *lookup_user_group (const char *user_name,
-                         char *user_group, int user_group_len);
-
 /** Create iterator through mapping string.
     Initially iterator set to position before first
     key-value pair. On success non-NULL pointer returned, otherwise NULL */
@@ -59,7 +54,7 @@
 
 /** Get value by given key. On success value_buf returned,
     otherwise NULL */
-char *mapping_get_value(const char *key, char *value_buf, int value_buf_len,
+char *mapping_lookup_user(const char *key, char *value_buf, int value_buf_len,
                         const char *mapping_string);
 
 /** Get service name for auth_string. On success buf returned,

=== modified file 'src/auth_pam.c'
--- src/auth_pam.c	2012-02-13 06:19:11 +0000
+++ src/auth_pam.c	2013-06-06 14:09:25 +0000
@@ -1,5 +1,5 @@
 /*
-(C) 2012 Percona Inc.
+(C) 2012, 2013 Percona Ireland Ltd.
 
 This program is free software; you can redistribute it and/or modify
 it under the terms of the GNU General Public License as published by
@@ -54,6 +54,7 @@
 #include "config.h"
 #endif
 
+#include <string.h>
 #include "auth_pam_common.h"
 
 /** The maximum length of buffered PAM messages, i.e. any messages up to the

=== modified file 'src/auth_pam_common.c'
--- src/auth_pam_common.c	2012-02-13 06:19:11 +0000
+++ src/auth_pam_common.c	2013-06-06 14:09:25 +0000
@@ -1,5 +1,5 @@
 /*
-(C) 2011 Percona Inc.
+(C) 2011-2013 Percona Ireland Ltd.
 
 This program is free software; you can redistribute it and/or modify
 it under the terms of the GNU General Public License as published by
@@ -19,8 +19,10 @@
 #include "config.h"
 #endif
 
+#include <string.h>
 #include "auth_pam_common.h"
 #include "auth_mapping.h"
+#include "groups.h"
 
 /* The server plugin */
 
@@ -110,7 +112,6 @@
   struct pam_conv conv_func_info= { &vio_server_conv, &data };
   int error;
   char *pam_mapped_user_name;
-  char user_group[MYSQL_USERNAME_LENGTH];
   char service_name[max_pam_service_name_len];
 
   /* Set service name as specified in auth_string. If no auth_string
@@ -173,13 +174,10 @@
     info->authenticated_as[MYSQL_USERNAME_LENGTH]= '\0';
   }
 
-  /* If auth_string specified, then lookup user group,
-  get mapped MySQL user name and use it as CURRENT_USER() value */
-  if (info->auth_string &&
-      lookup_user_group(pam_mapped_user_name, user_group, sizeof(user_group)))
+  if (info->auth_string)
   {
-    mapping_get_value(user_group, info->authenticated_as,
-                      MYSQL_USERNAME_LENGTH, info->auth_string);
+    mapping_lookup_user(pam_mapped_user_name, info->authenticated_as,
+                        MYSQL_USERNAME_LENGTH, info->auth_string);
   }
 
   error= pam_end(pam_handle, error);

=== modified file 'src/auth_pam_compat.c'
--- src/auth_pam_compat.c	2012-02-13 06:19:11 +0000
+++ src/auth_pam_compat.c	2013-06-06 14:09:25 +0000
@@ -1,5 +1,5 @@
 /*
-(C) 2012 Percona Inc.
+(C) 2012, 2013 Percona Ireland Ltd.
 
 This program is free software; you can redistribute it and/or modify
 it under the terms of the GNU General Public License as published by
@@ -52,6 +52,7 @@
 #include "config.h"
 #endif
 
+#include <string.h>
 #include "auth_pam_common.h"
 
 int auth_pam_client_talk_init(void **talk_data)

=== added file 'src/groups.c'
--- src/groups.c	1970-01-01 00:00:00 +0000
+++ src/groups.c	2013-06-06 14:09:25 +0000
@@ -0,0 +1,98 @@
+/*
+(C) 2013 Percona Ireland Ltd.
+
+This program is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; version 2 of the License.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with this program; if not, write to the Free Software
+Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+*/
+
+#include <pwd.h>
+#include <grp.h>
+#include <string.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+enum { max_nss_name_len = 10240 };
+enum { max_number_of_groups = 1024 };
+
+/** Groups iterator. It's not exposed outsude */
+struct groups_iter {
+  char buf[max_nss_name_len];
+  gid_t groups[max_number_of_groups];
+  int current_group;
+  int ngroups;
+};
+
+/** Create iterator through user groups.
+    Initially iterator set to position before first
+    group. On success non-NULL pointer returned, otherwise NULL */
+struct groups_iter *groups_iter_new(const char *user_name)
+{
+  struct passwd pwd, *pwd_result;
+  int error;
+  struct groups_iter *it;
+
+  it= calloc(1, sizeof(struct groups_iter));
+  if (it == NULL)
+    return NULL;
+
+  error= getpwnam_r(user_name, &pwd, it->buf, max_nss_name_len, &pwd_result);
+  if (error != 0 || pwd_result == NULL)
+  {
+    free(it);
+    return NULL;
+  }
+
+  it->ngroups= max_number_of_groups;
+  error= getgrouplist(user_name, pwd_result->pw_gid, it->groups, &it->ngroups);
+  if (error == -1)
+  {
+    free(it);
+    return NULL;
+  }
+
+  return it;
+}
+
+/** Move iterator to next group.
+    On success group name is returned,
+    otherwise NULL */
+const char *groups_iter_next(struct groups_iter *it)
+{
+  struct group grp, *grp_result;
+  int error;
+
+  if (it->current_group >= it->ngroups)
+    return NULL;
+
+  error= getgrgid_r(it->groups[it->current_group++],
+                    &grp, it->buf, max_nss_name_len, &grp_result);
+  if (error != 0 || grp_result == NULL)
+  {
+    return NULL;
+  }
+
+  return grp_result->gr_name;
+}
+
+/** Make iterator to point to beginning again */
+void groups_iter_reset(struct groups_iter *it)
+{
+  it->current_group= 0;
+}
+
+/** Finish iteration and release iterator */
+void groups_iter_free(struct groups_iter *it)
+{
+  free(it);
+}
+

=== added file 'src/groups.h'
--- src/groups.h	1970-01-01 00:00:00 +0000
+++ src/groups.h	2013-06-06 14:09:25 +0000
@@ -0,0 +1,53 @@
+#ifndef AUTH_PAM_GROUPS_INCLUDED
+#define AUTH_PAM_GROUPS_INCLUDED
+/*
+  (C) 2013 Percona Ireland Ltd.
+
+  This program is free software; you can redistribute it and/or modify
+  it under the terms of the GNU General Public License as published by
+  the Free Software Foundation; version 2 of the License.
+
+  This program is distributed in the hope that it will be useful,
+  but WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+  GNU General Public License for more details.
+
+  You should have received a copy of the GNU General Public License
+  along with this program; if not, write to the Free Software
+  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+*/
+
+/**
+ @file
+
+ PAM authentication for MySQL, interface for groups enumeration.
+
+*/
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+struct groups_iter;
+
+/** Create iterator through user groups.
+    Initially iterator set to position before first
+    group. On success non-NULL pointer returned, otherwise NULL */
+struct groups_iter *groups_iter_new(const char *user_name);
+
+/** Move iterator to next group.
+    On success group name is returned,
+    otherwise NULL */
+const char *groups_iter_next(struct groups_iter *it);
+
+/** Make iterator to point to beginning again */
+void groups_iter_reset(struct groups_iter *it);
+
+/** Finish iteration and release iterator */
+void groups_iter_free(struct groups_iter *it);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif

=== removed symlink 'test/dbqp/workdir'
=== target was u'/dev/shm/qp_workdir_root_e5f64ede-8707-4589-9c7a-fc598c0e2f6f'