diff mbox series

[Openvpn-devel,v3] Insert client connection data into PAM environment

Message ID 20221010122745.19809-1-wardragon78@gmail.com
State Accepted
Headers show
Series [Openvpn-devel,v3] Insert client connection data into PAM environment | expand

Commit Message

Paolo Cerrito Oct. 10, 2022, 1:27 a.m. UTC 
From: paolo <paolo.cerrito@uniroma2.it>

- styled code as openvpn 
- added check for remote, if NULL after all get_env, put to point to empy string

Signed-off-by: Paolo Cerrito <wardragon78@gmail.com>
---
 src/plugins/auth-pam/auth-pam.c | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

Comments

Gert Doering Oct. 10, 2022, 3:37 a.m. UTC | #1 
Acked-by: Gert Doering <gert@greenie.muc.de>

Thanks for providing a v3, and following our sometimes difficult rules
for patch submission.

I have tested this with v4 and v6 connections, and it looks good:

   PLUGIN AUTH-PAM: BACKGROUND: REMOTE: 2001:608:0:814::f000:21
   PLUGIN AUTH-PAM: BACKGROUND: REMOTE: 194.97.140.21

(I have not tested with "both environment variables are unset" but I
think the code in v3 is good.  I have not tested with a PAM module that
actually makes uses of this - but the pam_set_item() and PAM_RHOST matche
the linux manpage, so "it should be fine").

Unfortunately, v3 still had a coding style problem - we require that
if() statements always - no exceptions - have {} brackets, so the
"empty string" part looks like this now:

+        if (remote == NULL)
+        {
+            remote = "";
+        }

(for "core developers", running uncrustify is required, but since this
was taking so long already, I ran the uncrustify for you and committed
the result)

I have also added a bit more of commit message to explain what the
patch does - how OpenVPN sends this information to the plugin interface,
and how the PAM stack needs it.  This helps future contributors to
understand why certain code lines are the way they are.

Your patch has been applied to the master branch.

commit 8e9f9d031f7f2dbf2a505af297b808f22430a381
Author: Paolo Cerrito
Date:   Mon Oct 10 14:27:46 2022 +0200

     Insert client connection data into PAM environment

     Signed-off-by: Paolo Cerrito <wardragon78@gmail.com>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20221010122745.19809-1-wardragon78@gmail.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25375.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/plugins/auth-pam/auth-pam.c b/src/plugins/auth-pam/auth-pam.c
index 70339445..f90ffc5c 100644
--- a/src/plugins/auth-pam/auth-pam.c
+++ b/src/plugins/auth-pam/auth-pam.c
@@ -49,7 +49,7 @@ 
 #include <syslog.h>
 #include <limits.h>
 #include "utils.h"
-
+#include <arpa/inet.h>
 #include <openvpn-plugin.h>
 
 #define DEBUG(verb) ((verb) >= 4)
@@ -121,6 +121,7 @@  struct user_pass {
     char password[128];
     char common_name[128];
     char response[128];
+    char remote[INET6_ADDRSTRLEN];
 
     const struct name_value_list *name_value_list;
 };
@@ -529,6 +530,14 @@  openvpn_plugin_func_v1(openvpn_plugin_handle_t handle, const int type, const cha
         const char *username = get_env("username", envp);
         const char *password = get_env("password", envp);
         const char *common_name = get_env("common_name", envp) ? get_env("common_name", envp) : "";
+        const char *remote = get_env("untrusted_ip6", envp);
+
+        if (remote == NULL)
+        {
+            remote = get_env("untrusted_ip", envp);
+        }
+	
+	if (remote == NULL) remote="";
 
         /* should we do deferred auth?
          *  yes, if there is "auth_control_file" and "deferred_auth_pam" env
@@ -554,7 +563,8 @@  openvpn_plugin_func_v1(openvpn_plugin_handle_t handle, const int type, const cha
                 || send_string(context->foreground_fd, username) == -1
                 || send_string(context->foreground_fd, password) == -1
                 || send_string(context->foreground_fd, common_name) == -1
-                || send_string(context->foreground_fd, auth_control_file) == -1)
+                || send_string(context->foreground_fd, auth_control_file) == -1
+                || send_string(context->foreground_fd, remote) == -1)
             {
                 plugin_log(PLOG_ERR|PLOG_ERRNO, MODULE, "Error sending auth info to background process");
             }
@@ -789,8 +799,16 @@  pam_auth(const char *service, const struct user_pass *up)
     status = pam_start(service, name_value_list_provided ? NULL : up->username, &conv, &pamh);
     if (status == PAM_SUCCESS)
     {
+        /* Set PAM_RHOST environment variable */
+        if (*(up->remote))
+        {
+            status = pam_set_item(pamh, PAM_RHOST, up->remote);
+        }
         /* Call PAM to verify username/password */
-        status = pam_authenticate(pamh, 0);
+        if (status == PAM_SUCCESS)
+        {
+            status = pam_authenticate(pamh, 0);
+        }
         if (status == PAM_SUCCESS)
         {
             status = pam_acct_mgmt(pamh, 0);
@@ -956,7 +974,8 @@  pam_server(int fd, const char *service, int verb, const struct name_value_list *
                 if (recv_string(fd, up.username, sizeof(up.username)) == -1
                     || recv_string(fd, up.password, sizeof(up.password)) == -1
                     || recv_string(fd, up.common_name, sizeof(up.common_name)) == -1
-                    || recv_string(fd, ac_file_name, sizeof(ac_file_name)) == -1)
+                    || recv_string(fd, ac_file_name, sizeof(ac_file_name)) == -1
+                    || recv_string(fd, up.remote, sizeof(up.remote)) == -1)
                 {
                     plugin_log(PLOG_ERR|PLOG_ERRNO, MODULE, "BACKGROUND: read error on command channel: code=%d, exiting",
                                command);
@@ -970,6 +989,7 @@  pam_server(int fd, const char *service, int verb, const struct name_value_list *
                                up.username, up.password);
 #else
                     plugin_log(PLOG_NOTE, MODULE, "BACKGROUND: USER: %s", up.username);
+                    plugin_log(PLOG_NOTE, MODULE, "BACKGROUND: REMOTE: %s", up.remote);
 #endif
                 }