Hi, Alexey! Looks good, of course. See few minor comments below.
On Mar 18, Alexey Botchkov wrote: > revision-id: 108cdbf854df6af1ed2c0fc3a63e1786f6b4b7b4 > (mariadb-10.1.31-61-g108cdbf) > parent(s): a0c722d853071e605ea025168da1b01bfe21092c > committer: Alexey Botchkov > timestamp: 2018-03-18 12:03:59 +0400 > message: > > MDEV-10871 Add logging capability to pam_user_map.c. > > 'debug' option added to the pam_user_map.so. > It writes the verbose report to the syslog. don't indent the commit comment, please > > --- > plugin/auth_pam/mapper/pam_user_map.c | 74 > +++++++++++++++++++++++++++++++++-- > 1 file changed, 71 insertions(+), 3 deletions(-) > > diff --git a/plugin/auth_pam/mapper/pam_user_map.c > b/plugin/auth_pam/mapper/pam_user_map.c > index e62be94..1706630 100644 > --- a/plugin/auth_pam/mapper/pam_user_map.c > +++ b/plugin/auth_pam/mapper/pam_user_map.c > @@ -26,10 +26,13 @@ top: accounting > > #include <stdlib.h> > #include <stdio.h> > +#include <ctype.h> > +#include <string.h> > #include <syslog.h> > #include <grp.h> > #include <pwd.h> > > +#include <security/pam_ext.h> > #include <security/pam_modules.h> > > #define FILENAME "/etc/security/user_map.conf" > @@ -90,9 +93,40 @@ static int user_in_group(const gid_t *user_groups, int > ng,const char *group) > } > > > +static void print_groups(pam_handle_t *pamh, const gid_t *user_groups, int > ng) > +{ > + char buf[256]; > + char *c_buf= buf, *buf_end= buf+sizeof(buf)-1; > + struct group *gr; > + > + for (; ng > 0; user_groups++, ng--) > + { > + char *c; > + if (!(gr= getgrgid(*user_groups)) || > + !(c= gr->gr_name)) > + continue; > + while (*c) > + { > + if (c_buf == buf_end) > + break; > + *(c_buf++)= *c; > + } > + if (c_buf == buf_end) > + break; > + *(c_buf++)= ','; > + } > + *c_buf= 0; > + pam_syslog(pamh, LOG_DEBUG, "User belongs to groups [%s].\n", buf); s/pam_syslog/SYSLOG_DEBUG/ ? > +} > + > + > +static const char debug_keyword[]= "debug"; > +#define SYSLOG_DEBUG if (mode_debug) pam_syslog > + > int pam_sm_authenticate(pam_handle_t *pamh, int flags, > int argc, const char *argv[]) > { > + int mode_debug= 0; > int pam_err, line= 0; > const char *username; > char buf[256]; > @@ -101,6 +135,14 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags, > gid_t *groups= group_buffer; > int n_groups= -1; > > + for (; argc > 0; argv++) > + { > + if (strcasecmp(*argv, debug_keyword) == 0) > + mode_debug= 1; > + } can there be many arguments? > + > + SYSLOG_DEBUG(pamh, LOG_DEBUG, "Opening file '%s'.\n", FILENAME); > > + > f= fopen(FILENAME, "r"); > if (f == NULL) > { > @@ -110,12 +152,18 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags, > > pam_err = pam_get_item(pamh, PAM_USER, (const void**)&username); > if (pam_err != PAM_SUCCESS) > + { > + pam_syslog(pamh, LOG_ERR, "Cannot get username.\n"); > goto ret; > + } > + > + SYSLOG_DEBUG(pamh, LOG_DEBUG, "Incoming username '%s'.\n", username); > > while (fgets(buf, sizeof(buf), f) != NULL) > { > char *s= buf, *from, *to, *end_from, *end_to; > int check_group; > + int cmp_result; > > line++; > > @@ -124,7 +172,11 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags, > if ((check_group= *s == '@')) > { > if (n_groups < 0) > + { > n_groups= populate_user_groups(username, &groups); > + if (mode_debug) > + print_groups(pamh, groups, n_groups); > + } > s++; > } > from= s; > @@ -139,14 +191,29 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags, > if (end_to == to) goto syntax_error; > > *end_from= *end_to= 0; > - if (check_group ? > - user_in_group(groups, n_groups, from) : > - (strcmp(username, from) == 0)) > + > + cmp_result= 0; looks redundant ^^^ > + if (check_group) > + { > + cmp_result= user_in_group(groups, n_groups, from); > + SYSLOG_DEBUG(pamh, LOG_DEBUG, "Check if user is in gourp '%s': %s\n", s/gourp/group/ > + from, cmp_result ? "YES":"NO"); > + } > + else > + { > + cmp_result= (strcmp(username, from) == 0); > + SYSLOG_DEBUG(pamh, LOG_DEBUG, "Check if username '%s': %s\n", > + from, cmp_result ? "YES":"NO"); > + } > + if (cmp_result) > { > pam_err= pam_set_item(pamh, PAM_USER, to); > + SYSLOG_DEBUG(pamh, LOG_DEBUG, "User authenticated as '%s'\n", to); may be "user mapped to %s" ? > goto ret; > } > } > + > + SYSLOG_DEBUG(pamh, LOG_DEBUG, "User not found in the list.\n"); > pam_err= PAM_AUTH_ERR; > goto ret; > > @@ -162,6 +229,7 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags, > return pam_err; > } > > + > int pam_sm_setcred(pam_handle_t *pamh, int flags, > int argc, const char *argv[]) > { Regards, Sergei Chief Architect MariaDB and secur...@mariadb.org _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp