[PATCH] Fix pam_conv in pam_wrapper
Andreas Schneider
asn at samba.org
Fri Sep 21 09:11:17 UTC 2018
Hello,
I have a patchset for pam_wrapper to fix the pam conversation and add support
for running with Address Sanitizer.
However, the pam_conv fix is important that I can update pam_wrapper to add
tests for some pam_winbind features:
https://git.samba.org/?p=asn/samba.git;a=shortlog;h=refs/heads/master-pwrap
Please review.
Thanks,
Andreas
--
Andreas Schneider asn at samba.org
Samba Team www.samba.org
GPG-ID: 8DFF53E18F2ABC8D8F3C92237EE0FC4DCC014E3D
-------------- next part --------------
>From a331e42539d54a5cfdb6df41b5ec0d1b60043648 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Mon, 25 Jun 2018 10:39:51 +0200
Subject: [PATCH 1/5] libpamtest: Check that message count matches response
count
Signed-off-by: Andreas Schneider <asn at samba.org>
Reviewed-by: Jakub Hrozek <jakub.hrozek at posteo.se>
---
src/libpamtest.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/src/libpamtest.c b/src/libpamtest.c
index 0a26c19..74da180 100644
--- a/src/libpamtest.c
+++ b/src/libpamtest.c
@@ -214,12 +214,11 @@ static int pamtest_simple_conv(int num_msg,
struct pam_response **response,
void *appdata_ptr)
{
- int i, ri = 0;
+ int i = 0;
int ret;
struct pam_response *reply = NULL;
const char *prompt;
- struct pamtest_conv_ctx *cctx = \
- (struct pamtest_conv_ctx *) appdata_ptr;
+ struct pamtest_conv_ctx *cctx = (struct pamtest_conv_ctx *)appdata_ptr;
if (cctx == NULL) {
return PAM_CONV_ERR;
@@ -241,15 +240,12 @@ static int pamtest_simple_conv(int num_msg,
if (reply != NULL) {
if (prompt != NULL) {
- ret = add_to_reply(&reply[ri], prompt);
+ ret = add_to_reply(&reply[i], prompt);
if (ret != PAM_SUCCESS) {
free_reply(reply, num_msg);
return ret;
}
- } else {
- reply[ri].resp = NULL;
}
- ri++;
}
cctx->echo_off_idx++;
@@ -264,13 +260,12 @@ static int pamtest_simple_conv(int num_msg,
if (reply != NULL) {
if (prompt != NULL) {
- ret = add_to_reply(&reply[ri], prompt);
+ ret = add_to_reply(&reply[i], prompt);
if (ret != PAM_SUCCESS) {
free_reply(reply, num_msg);
return ret;
}
}
- ri++;
}
cctx->echo_on_idx++;
@@ -298,7 +293,7 @@ static int pamtest_simple_conv(int num_msg,
}
}
- if (response && ri > 0) {
+ if (response != NULL) {
*response = reply;
} else {
free(reply);
--
2.18.0
>From a3851def86755da09186622380e9be0b74aa6aea Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Tue, 26 Jun 2018 08:46:44 +0200
Subject: [PATCH 2/5] pwrap: Add PAM_TEXT_INFO and PAM_ERROR_MSG to responses
Signed-off-by: Andreas Schneider <asn at samba.org>
Reviewed-by: Jakub Hrozek <jakub.hrozek at posteo.se>
---
src/libpamtest.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/src/libpamtest.c b/src/libpamtest.c
index 74da180..4474736 100644
--- a/src/libpamtest.c
+++ b/src/libpamtest.c
@@ -271,6 +271,14 @@ static int pamtest_simple_conv(int num_msg,
cctx->echo_on_idx++;
break;
case PAM_ERROR_MSG:
+ if (reply != NULL) {
+ ret = add_to_reply(&reply[i], msgm[i]->msg);
+ if (ret != PAM_SUCCESS) {
+ free_reply(reply, num_msg);
+ return ret;
+ }
+ }
+
if (cctx->data->out_err != NULL) {
memcpy(cctx->data->out_err[cctx->err_idx],
msgm[i]->msg,
@@ -280,6 +288,14 @@ static int pamtest_simple_conv(int num_msg,
}
break;
case PAM_TEXT_INFO:
+ if (reply != NULL) {
+ ret = add_to_reply(&reply[i], msgm[i]->msg);
+ if (ret != PAM_SUCCESS) {
+ free_reply(reply, num_msg);
+ return ret;
+ }
+ }
+
if (cctx->data->out_info != NULL) {
memcpy(cctx->data->out_info[cctx->info_idx],
msgm[i]->msg,
--
2.18.0
>From 66d70c8249e0050dcf594d6bf1b8d79c6e5eab11 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Mon, 25 Jun 2018 11:02:45 +0200
Subject: [PATCH 3/5] pypamtest: Fix number of responses
The number of responses needs to match the number of provided messages.
Signed-off-by: Andreas Schneider <asn at samba.org>
Reviewed-by: Jakub Hrozek <jakub.hrozek at posteo.se>
---
src/python/pypamtest.c | 67 +++++++++++++++++++++++++++---------------
1 file changed, 44 insertions(+), 23 deletions(-)
diff --git a/src/python/pypamtest.c b/src/python/pypamtest.c
index 6bb1e20..905c652 100644
--- a/src/python/pypamtest.c
+++ b/src/python/pypamtest.c
@@ -166,11 +166,13 @@ static char **new_conv_list(const size_t list_size)
return list;
}
-static const char **sequence_as_string_list(PyObject *seq,
- const char *paramname)
+static int sequence_as_string_list(PyObject *seq,
+ const char *paramname,
+ const char **str_list[],
+ size_t *num_str_list)
{
const char *p = paramname ? paramname : "attribute values";
- const char **ret;
+ const char **result;
PyObject *utf_item;
int i;
Py_ssize_t len;
@@ -179,18 +181,18 @@ static const char **sequence_as_string_list(PyObject *seq,
if (!PySequence_Check(seq)) {
PyErr_Format(PyExc_TypeError,
"The object must be a sequence\n");
- return NULL;
+ return -1;
}
len = PySequence_Size(seq);
if (len == -1) {
- return NULL;
+ return -1;
}
- ret = PyMem_New(const char *, (len + 1));
- if (!ret) {
+ result = PyMem_New(const char *, (len + 1));
+ if (result == NULL) {
PyErr_NoMemory();
- return NULL;
+ return -1;
}
for (i = 0; i < len; i++) {
@@ -202,20 +204,24 @@ static const char **sequence_as_string_list(PyObject *seq,
utf_item = get_utf8_string(item, p);
if (utf_item == NULL) {
Py_DECREF(item);
- return NULL;
+ return -1;
}
- ret[i] = py_strdup(PyBytes_AsString(utf_item));
+ result[i] = py_strdup(PyBytes_AsString(utf_item));
Py_DECREF(utf_item);
- if (!ret[i]) {
+ if (result[i] == NULL) {
Py_DECREF(item);
- return NULL;
+ return -1;
}
Py_DECREF(item);
}
- ret[i] = NULL;
- return ret;
+ result[i] = NULL;
+
+ *str_list = result;
+ *num_str_list = (size_t)len;
+
+ return 0;
}
static PyObject *string_list_as_tuple(char **str_list)
@@ -225,7 +231,7 @@ static PyObject *string_list_as_tuple(char **str_list)
PyObject *tup;
PyObject *py_str;
- for (len=0; len < PAM_CONV_MSG_MAX; len++) {
+ for (len=0; str_list[len] != NULL; len++) {
if (str_list[len][0] == '\0') {
/* unused string, stop counting */
break;
@@ -808,31 +814,46 @@ static int fill_conv_data(PyObject *py_echo_off,
PyObject *py_echo_on,
struct pamtest_conv_data *conv_data)
{
+ size_t conv_count = 0;
+ size_t count = 0;
+ int rc;
+
conv_data->in_echo_on = NULL;
conv_data->in_echo_off = NULL;
conv_data->out_err = NULL;
conv_data->out_info = NULL;
if (py_echo_off != NULL) {
- conv_data->in_echo_off = sequence_as_string_list(py_echo_off,
- "echo_off");
- if (conv_data->in_echo_off == NULL) {
+ rc = sequence_as_string_list(py_echo_off,
+ "echo_off",
+ &conv_data->in_echo_off,
+ &count);
+ if (rc != 0) {
free_conv_data(conv_data);
return ENOMEM;
}
+ conv_count += count;
}
if (py_echo_on != NULL) {
- conv_data->in_echo_on = sequence_as_string_list(py_echo_on,
- "echo_on");
- if (conv_data->in_echo_on == NULL) {
+ rc = sequence_as_string_list(py_echo_on,
+ "echo_on",
+ &conv_data->in_echo_on,
+ &count);
+ if (rc != 0) {
free_conv_data(conv_data);
return ENOMEM;
}
+ conv_count += count;
+ }
+
+ if (conv_count > PAM_CONV_MSG_MAX) {
+ free_conv_data(conv_data);
+ return ENOMEM;
}
- conv_data->out_info = new_conv_list(PAM_CONV_MSG_MAX);
- conv_data->out_err = new_conv_list(PAM_CONV_MSG_MAX);
+ conv_data->out_info = new_conv_list(conv_count);
+ conv_data->out_err = new_conv_list(conv_count);
if (conv_data->out_info == NULL || conv_data->out_err == NULL) {
free_conv_data(conv_data);
return ENOMEM;
--
2.18.0
>From 926c10099d43a5f1f1304e9b9c97ade76fdee2e9 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Thu, 20 Sep 2018 10:08:00 +0200
Subject: [PATCH 4/5] pam_set_items: Add logging
Signed-off-by: Andreas Schneider <asn at samba.org>
Reviewed-by: Jakub Hrozek <jakub.hrozek at posteo.se>
---
src/modules/pam_set_items.c | 100 +++++++++++++++++++++++++++++++++++-
1 file changed, 99 insertions(+), 1 deletion(-)
diff --git a/src/modules/pam_set_items.c b/src/modules/pam_set_items.c
index 22c2c56..dd09020 100644
--- a/src/modules/pam_set_items.c
+++ b/src/modules/pam_set_items.c
@@ -19,7 +19,9 @@
#include <stdlib.h>
#include <stdio.h>
+#include <stdarg.h>
#include <string.h>
+#include <unistd.h>
#ifdef HAVE_SECURITY_PAM_APPL_H
#include <security/pam_appl.h>
@@ -30,6 +32,89 @@
#include "config.h"
+/* GCC have printf type attribute check. */
+#ifdef HAVE_FUNCTION_ATTRIBUTE_FORMAT
+#define PRINTF_ATTRIBUTE(a,b) __attribute__ ((__format__ (__printf__, a, b)))
+#else
+#define PRINTF_ATTRIBUTE(a,b)
+#endif /* HAVE_FUNCTION_ATTRIBUTE_FORMAT */
+
+/*****************
+ * LOGGING
+ *****************/
+
+enum pwrap_dbglvl_e {
+ PWRAP_LOG_ERROR = 0,
+ PWRAP_LOG_WARN,
+ PWRAP_LOG_DEBUG,
+ PWRAP_LOG_TRACE
+};
+
+static void pwrap_log(enum pwrap_dbglvl_e dbglvl,
+ const char *function,
+ const char *format, ...) PRINTF_ATTRIBUTE(3, 4);
+# define PWRAP_LOG(dbglvl, ...) pwrap_log((dbglvl), __func__, __VA_ARGS__)
+
+static void pwrap_vlog(enum pwrap_dbglvl_e dbglvl,
+ const char *function,
+ const char *format,
+ va_list args) PRINTF_ATTRIBUTE(3, 0);
+
+static void pwrap_vlog(enum pwrap_dbglvl_e dbglvl,
+ const char *function,
+ const char *format,
+ va_list args)
+{
+ char buffer[1024];
+ const char *d;
+ unsigned int lvl = 0;
+ const char *prefix = "PWRAP";
+
+ d = getenv("PAM_WRAPPER_DEBUGLEVEL");
+ if (d != NULL) {
+ lvl = atoi(d);
+ }
+
+ if (lvl < dbglvl) {
+ return;
+ }
+
+ vsnprintf(buffer, sizeof(buffer), format, args);
+
+ switch (dbglvl) {
+ case PWRAP_LOG_ERROR:
+ prefix = "PWRAP_ERROR";
+ break;
+ case PWRAP_LOG_WARN:
+ prefix = "PWRAP_WARN";
+ break;
+ case PWRAP_LOG_DEBUG:
+ prefix = "PWRAP_DEBUG";
+ break;
+ case PWRAP_LOG_TRACE:
+ prefix = "PWRAP_TRACE";
+ break;
+ }
+
+ fprintf(stderr,
+ "%s(%d) - PAM_SET_ITEM %s: %s\n",
+ prefix,
+ (int)getpid(),
+ function,
+ buffer);
+}
+
+static void pwrap_log(enum pwrap_dbglvl_e dbglvl,
+ const char *function,
+ const char *format, ...)
+{
+ va_list va;
+
+ va_start(va, format);
+ pwrap_vlog(dbglvl, function, format, va);
+ va_end(va);
+}
+
#define ITEM_FILE_KEY "item_file="
static const char *envs[] = {
@@ -83,6 +168,8 @@ static void pam_setitem_env(pam_handle_t *pamh)
continue;
}
+ PWRAP_LOG(PWRAP_LOG_TRACE, "%s=%s", envs[i], v);
+
rv = pam_set_item(pamh, items[i], v);
if (rv != PAM_SUCCESS) {
continue;
@@ -98,6 +185,8 @@ pam_sm_authenticate(pam_handle_t *pamh, int flags,
(void) argc; /* unused */
(void) argv; /* unused */
+ PWRAP_LOG(PWRAP_LOG_TRACE, "AUTHENTICATE");
+
pam_setitem_env(pamh);
return PAM_SUCCESS;
}
@@ -110,6 +199,8 @@ pam_sm_setcred(pam_handle_t *pamh, int flags,
(void) argc; /* unused */
(void) argv; /* unused */
+ PWRAP_LOG(PWRAP_LOG_TRACE, "SETCRED");
+
pam_setitem_env(pamh);
return PAM_SUCCESS;
}
@@ -122,6 +213,8 @@ pam_sm_acct_mgmt(pam_handle_t *pamh, int flags,
(void) argc; /* unused */
(void) argv; /* unused */
+ PWRAP_LOG(PWRAP_LOG_TRACE, "ACCT_MGMT");
+
pam_setitem_env(pamh);
return PAM_SUCCESS;
}
@@ -134,6 +227,8 @@ pam_sm_open_session(pam_handle_t *pamh, int flags,
(void) argc; /* unused */
(void) argv; /* unused */
+ PWRAP_LOG(PWRAP_LOG_TRACE, "OPEN_SESSION");
+
pam_setitem_env(pamh);
return PAM_SUCCESS;
}
@@ -146,6 +241,8 @@ pam_sm_close_session(pam_handle_t *pamh, int flags,
(void) argc; /* unused */
(void) argv; /* unused */
+ PWRAP_LOG(PWRAP_LOG_TRACE, "CLOSE_SESSION");
+
pam_setitem_env(pamh);
return PAM_SUCCESS;
}
@@ -158,7 +255,8 @@ pam_sm_chauthtok(pam_handle_t *pamh, int flags,
(void) argc; /* unused */
(void) argv; /* unused */
+ PWRAP_LOG(PWRAP_LOG_TRACE, "CHAUTHTOK");
+
pam_setitem_env(pamh);
return PAM_SUCCESS;
}
-
--
2.18.0
>From aaeb4549a7b5605faeb5384e9c56db11ba503e99 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Sat, 1 Sep 2018 19:46:36 +0200
Subject: [PATCH 5/5] pwrap: Don't do a deep bind if we run with libasan
Signed-off-by: Andreas Schneider <asn at samba.org>
Reviewed-by: Jakub Hrozek <jakub.hrozek at posteo.se>
---
src/pam_wrapper.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/src/pam_wrapper.c b/src/pam_wrapper.c
index 230a45d..48d2c2a 100644
--- a/src/pam_wrapper.c
+++ b/src/pam_wrapper.c
@@ -300,7 +300,15 @@ static void *pwrap_load_lib_handle(enum pwrap_lib lib)
void *handle = NULL;
#ifdef RTLD_DEEPBIND
- flags |= RTLD_DEEPBIND;
+ const char *env = getenv("LD_PRELOAD");
+
+ /* Don't do a deepbind if we run with libasan */
+ if (env != NULL && strlen(env) < PATH_MAX) {
+ const char *p = strstr(env, "libasan.so");
+ if (p == NULL) {
+ flags |= RTLD_DEEPBIND;
+ }
+ }
#endif
switch (lib) {
--
2.18.0
More information about the samba-technical
mailing list