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 security@mariadb.org