Hi Martin,
no problem for the delay :-)
I have changed the license to MIT and moved the service lookup into the listener construction.
Best regards
Andrea

On 17/01/14 12:32, Martin Willi wrote:
Andrea,

the following patch is a small improvement for the xauth_pam plugin.
The actual plugin performs the XAuth users authentication though PAM
but don't open/close the user sessions with the PAM
pam_open_session/pam_close_session methods.
Sorry for the loooong delay, I finally had a chance to take a closer
look at your patch.

I haven't tested it yet, but overall it looks very good, thanks a lot.

+    service = lib->settings->get_str(lib->settings,
+                "%s.plugins.xauth-pam.pam_service",
+                    lib->settings->get_str(lib->settings,
+                        "%s.plugins.eap-gtc.pam_service",
+                        "login", charon->name),
+                charon->name);
That service lookup could actually be done during listener construction,
avoiding it for each client. Just a minor nitpick, not really important.

Unfortunately, your mailer messed up to patch. Can you resend it without
unintended line breaks? As attachement is ok as well.

Further, would you mind to contribute this patch under the MIT license
instead of GPLv2? Have a look at [1] for the reasons.

Best Regards
Martin

[1]http://wiki.strongswan.org/projects/strongswan/wiki/Contributions



From b5b4ce763ba0fc6bfa5ec4073da26a54a5563cb0 Mon Sep 17 00:00:00 2001
From: Andrea Bonomi <[email protected]>
Date: Tue, 21 Jan 2014 09:54:20 +0100
Subject: [PATCH] [PATCH] xauth_pam don't open/close PAM sessions


Signed-off-by: Andrea Bonomi <[email protected]>
---
 src/libcharon/plugins/xauth_pam/Makefile.am        |    3 +-
 .../plugins/xauth_pam/xauth_pam_listener.c         |  167 ++++++++++++++++++++
 .../plugins/xauth_pam/xauth_pam_listener.h         |   58 +++++++
 src/libcharon/plugins/xauth_pam/xauth_pam_plugin.c |   60 +++++--
 4 files changed, 277 insertions(+), 11 deletions(-)
 create mode 100644 src/libcharon/plugins/xauth_pam/xauth_pam_listener.c
 create mode 100644 src/libcharon/plugins/xauth_pam/xauth_pam_listener.h

diff --git a/src/libcharon/plugins/xauth_pam/Makefile.am 
b/src/libcharon/plugins/xauth_pam/Makefile.am
index a7d4f64..370661b 100644
--- a/src/libcharon/plugins/xauth_pam/Makefile.am
+++ b/src/libcharon/plugins/xauth_pam/Makefile.am
@@ -14,6 +14,7 @@ endif
 
 libstrongswan_xauth_pam_la_SOURCES = \
        xauth_pam_plugin.h xauth_pam_plugin.c \
-       xauth_pam.h xauth_pam.c
+       xauth_pam.h xauth_pam.c \
+       xauth_pam_listener.h xauth_pam_listener.c
 
 libstrongswan_xauth_pam_la_LDFLAGS = -module -avoid-version -lpam
diff --git a/src/libcharon/plugins/xauth_pam/xauth_pam_listener.c 
b/src/libcharon/plugins/xauth_pam/xauth_pam_listener.c
new file mode 100644
index 0000000..ee4bfd7
--- /dev/null
+++ b/src/libcharon/plugins/xauth_pam/xauth_pam_listener.c
@@ -0,0 +1,167 @@
+/*
+ * Copyright (C) 2013 Endian srl
+ * Author: Andrea Bonomi - <[email protected]>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#define _GNU_SOURCE
+#include <stdio.h>
+
+#include "xauth_pam_listener.h"
+
+#include <daemon.h>
+#include <library.h>
+
+#include <security/pam_appl.h>
+
+typedef struct private_xauth_pam_listener_t private_xauth_pam_listener_t;
+
+/**
+ * Private data of an xauth_pam_listener_t object.
+ */
+struct private_xauth_pam_listener_t {
+
+       /**
+        * Public xauth_pam_listener_t interface.
+        */
+       xauth_pam_listener_t public;
+
+       /**
+        * PAM service
+        */
+       char *service = NULL;
+
+};
+
+static struct pam_conv null_conv = {
+       NULL
+};
+
+static void open_close_session(private_xauth_pam_listener_t *this, ike_sa_t 
*ike_sa, bool up)
+{
+       pam_handle_t *pamh = NULL;
+       char *user = NULL;
+       int ret;
+       enumerator_t *enumerator;
+       host_t *vip;
+
+       enumerator = ike_sa->create_virtual_ip_enumerator(ike_sa, FALSE);
+       while (enumerator->enumerate(enumerator, &vip))
+       {
+               if (asprintf(&user, "%Y", ike_sa->get_other_eap_id(ike_sa)) == 
-1)
+               {
+                       user = NULL;
+                       continue;
+               }
+               ret = pam_start(this->service, user, &null_conv, &pamh);
+               if (ret != PAM_SUCCESS)
+               {
+                       DBG1(DBG_IKE, "XAuth pam_start for '%s' failed: %s",
+                               user, pam_strerror(pamh, ret));
+               }
+               else if (up)
+               {
+                       ret = pam_open_session(pamh, 0);
+                       if (ret != PAM_SUCCESS)
+                       {
+                               DBG1(DBG_IKE, "XAuth pam_open_session for '%s' 
failed: %s",
+                                       user, pam_strerror(pamh, ret));
+                       }
+               }
+               else
+               {
+                       ret = pam_close_session(pamh, 0);
+                       if (ret != PAM_SUCCESS)
+                       {
+                               DBG1(DBG_IKE, "XAuth pam_close_session for '%s' 
failed: %s",
+                                       user, pam_strerror(pamh, ret));
+                       }
+               }
+               pam_end(pamh, ret);
+               free(user);
+       }
+       enumerator->destroy(enumerator);
+
+       return TRUE;
+}
+
+METHOD(listener_t, message_hook, bool,
+       private_xauth_pam_listener_t *this, ike_sa_t *ike_sa,
+       message_t *message, bool incoming, bool plain)
+{
+       if (plain && ike_sa->get_state(ike_sa) == IKE_ESTABLISHED &&
+               !incoming && !message->get_request(message))
+       {
+               if (ike_sa->get_version(ike_sa) == IKEV1 &&
+                       message->get_exchange_type(message) == TRANSACTION)
+               {
+                       open_close_session(this, ike_sa, TRUE);
+               }
+               if (ike_sa->get_version(ike_sa) == IKEV2 &&
+                       message->get_exchange_type(message) == IKE_AUTH)
+               {
+                       open_close_session(this, ike_sa, TRUE);
+               }
+       }
+       return TRUE;
+}
+
+METHOD(listener_t, ike_updown, bool,
+       private_xauth_pam_listener_t *this, ike_sa_t *ike_sa, bool up)
+{
+       if (!up)
+       {
+               open_close_session(this, ike_sa, FALSE);
+       }
+       return TRUE;
+}
+
+METHOD(xauth_pam_listener_t, listener_destroy, void,
+       private_xauth_pam_listener_t *this)
+{
+       free(this);
+}
+
+xauth_pam_listener_t *xauth_pam_listener_create()
+{
+       private_xauth_pam_listener_t *this;
+
+       INIT(this,
+               .public = {
+                       .listener = {
+                               .message = _message_hook,
+                               .ike_updown = _ike_updown,
+                       },
+                       .destroy = _listener_destroy,
+               },
+       );
+
+       /* Look for PAM service, with a legacy fallback for the eap-gtc plugin.
+        * Default to "login". */
+       this->service = lib->settings->get_str(lib->settings,
+                               "%s.plugins.xauth-pam.pam_service",
+                                       lib->settings->get_str(lib->settings,
+                                               
"%s.plugins.eap-gtc.pam_service",
+                                               "login", charon->name),
+                               charon->name);
+
+       return &this->public;
+}
+
diff --git a/src/libcharon/plugins/xauth_pam/xauth_pam_listener.h 
b/src/libcharon/plugins/xauth_pam/xauth_pam_listener.h
new file mode 100644
index 0000000..5b15410
--- /dev/null
+++ b/src/libcharon/plugins/xauth_pam/xauth_pam_listener.h
@@ -0,0 +1,58 @@
+/*
+ * Copyright (C) 2013 Endian srl
+ * Author: Andrea Bonomi - <[email protected]>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+/**
+ * @defgroup xauth_pam_i xauth_pam
+ * @{ @ingroup xauth_pam
+ */
+
+#ifndef XAUTH_PAM_LISENER_H_
+#define XAUTH_PAM_LISTENER_H_
+
+typedef struct xauth_pam_listener_t xauth_pam_listener_t;
+
+#include <bus/listeners/listener.h>
+
+/**
+ * Listener
+ */
+struct xauth_pam_listener_t {
+
+       /**
+        * Implements listener_t interface.
+        */
+       listener_t listener;
+
+       /**
+        * Destroy a xauth_pam_listener_t.
+        */
+       void (*destroy)(xauth_pam_listener_t *this);
+};
+
+/**
+ * Create a xauth_pam_listener instance.
+ */
+xauth_pam_listener_t *xauth_pam_listener_create();
+
+
+#endif /** XAUTH_PAM_LISTENER_H_ @}*/
diff --git a/src/libcharon/plugins/xauth_pam/xauth_pam_plugin.c 
b/src/libcharon/plugins/xauth_pam/xauth_pam_plugin.c
index 2ef9a6c..3e6402c 100644
--- a/src/libcharon/plugins/xauth_pam/xauth_pam_plugin.c
+++ b/src/libcharon/plugins/xauth_pam/xauth_pam_plugin.c
@@ -15,6 +15,7 @@
 
 #include "xauth_pam_plugin.h"
 #include "xauth_pam.h"
+#include "xauth_pam_listener.h"
 
 #include <daemon.h>
 
@@ -22,17 +23,53 @@
 #define CAP_AUDIT_WRITE 29
 #endif
 
+typedef struct private_xauth_pam_plugin_t private_xauth_pam_plugin_t;
+
+/**
+ * private data of xauth_pam plugin
+ */
+struct private_xauth_pam_plugin_t {
+
+       /**
+        * implements plugin interface
+        */
+       xauth_pam_plugin_t public;
+
+       /**
+        * Listener
+        */
+       xauth_pam_listener_t *listener;
+
+};
+
+bool xauth_pam_method_register(private_xauth_pam_plugin_t *this, 
plugin_feature_t *feature,
+                                                bool reg, void *data)
+{
+    /* (un-)register XAuth methods from plugin features */
+    xauth_method_register(this, feature, reg, data);
+       if (reg)
+       {
+               charon->bus->add_listener(charon->bus, 
&this->listener->listener);
+       }
+       else
+       {
+               charon->bus->remove_listener(charon->bus, 
&this->listener->listener);
+       }
+       return TRUE;
+}
+
+
 METHOD(plugin_t, get_name, char*,
-       xauth_pam_plugin_t *this)
+       private_xauth_pam_plugin_t *this)
 {
        return "xauth-pam";
 }
 
 METHOD(plugin_t, get_features, int,
-       xauth_pam_plugin_t *this, plugin_feature_t *features[])
+       private_xauth_pam_plugin_t *this, plugin_feature_t *features[])
 {
        static plugin_feature_t f[] = {
-               PLUGIN_CALLBACK(xauth_method_register, xauth_pam_create_server),
+               PLUGIN_CALLBACK(xauth_pam_method_register, 
xauth_pam_create_server),
                        PLUGIN_PROVIDE(XAUTH_SERVER, "pam"),
        };
        *features = f;
@@ -40,7 +77,7 @@ METHOD(plugin_t, get_features, int,
 }
 
 METHOD(plugin_t, destroy, void,
-       xauth_pam_plugin_t *this)
+       private_xauth_pam_plugin_t *this)
 {
        free(this);
 }
@@ -50,7 +87,7 @@ METHOD(plugin_t, destroy, void,
  */
 plugin_t *xauth_pam_plugin_create()
 {
-       xauth_pam_plugin_t *this;
+       private_xauth_pam_plugin_t *this;
 
        /* required for PAM authentication */
        if (!lib->caps->keep(lib->caps, CAP_AUDIT_WRITE))
@@ -60,12 +97,15 @@ plugin_t *xauth_pam_plugin_create()
        }
 
        INIT(this,
-               .plugin = {
-                       .get_name = _get_name,
-                       .get_features = _get_features,
-                       .destroy = _destroy,
+               .public = {
+                   .plugin = {
+                           .get_name = _get_name,
+                           .get_features = _get_features,
+                           .destroy = _destroy,
+            },
                },
+               .listener = xauth_pam_listener_create(),
        );
 
-       return &this->plugin;
+       return &this->public.plugin;
 }
-- 
1.7.9.5

_______________________________________________
Dev mailing list
[email protected]
https://lists.strongswan.org/mailman/listinfo/dev

Reply via email to