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

Message ID 20220620122126.2676755-1-wardragon78@gmail.com
State Changes Requested
Headers show
Series [Openvpn-devel] Insert client connection data into PAM environment, upgraded | expand

Commit Message

Paolo Cerrito June 20, 2022, 2:21 a.m. UTC
From: paolo <paolo.cerrito@uniroma2.it>

---
 src/plugins/auth-pam/auth-pam.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

Comments

Antonio Quartulli June 23, 2022, 10:59 p.m. UTC | #1
Hi Paolo,

On 20/06/2022 14:21, Paolo Cerrito wrote:
> From: paolo <paolo.cerrito@uniroma2.it>

Is this a new version of your previous patch having subject "Insert 
client connection data into PAM environment"?

If yes, you should send it as a v2 (i.e. with subject starting with 
"[PATCH v2]") instead of appending ", upgraded".

You can easily create a v2 patch by adding the argument "-v2" when 
running "git format-patch".

On top of that, when sending a v2 it'd be nice if you could specify what 
has changed compared to the original version. Something like:

"Changes from v1:
* this and that
* here and there"

Also, when signing a patch you should put your name and surname, as if 
you are signing a public document.

Last, but not least, please add some text to your commit message, 
explaining why you need to implement this change, what it does (high 
level) and how you are going to do it.


Best Regards,

> 
> ---
>   src/plugins/auth-pam/auth-pam.c | 25 +++++++++++++++++++++----
>   1 file changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/src/plugins/auth-pam/auth-pam.c b/src/plugins/auth-pam/auth-pam.c
> index 70339445..c2e66e5c 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,11 @@ 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 Null, try to take ipv4 if not set ipv6
> +        }
>   
>           /* should we do deferred auth?
>            *  yes, if there is "auth_control_file" and "deferred_auth_pam" env
> @@ -554,7 +560,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 +796,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 +971,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 +986,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
>                   }
>

Patch

diff --git a/src/plugins/auth-pam/auth-pam.c b/src/plugins/auth-pam/auth-pam.c
index 70339445..c2e66e5c 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,11 @@  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 Null, try to take ipv4 if not set ipv6
+        }
 
         /* should we do deferred auth?
          *  yes, if there is "auth_control_file" and "deferred_auth_pam" env
@@ -554,7 +560,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 +796,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 +971,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 +986,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
                 }