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

  1) so remote was set to the maxlenght of ipv6 address defined into
arpa/inet.h  + 1 for string terminator

2) I refactored the call to get_env to take first ipv6 address, then
   only if it is NULL, i make a call for ipv4
---
 src/plugins/auth-pam/auth-pam.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
  

Hi,

On Mon, Mar 30, 2020 at 8:59 AM Paolo Cerrito <wardragon78@gmail.com> wrote:

> 1) so remote was set to the maxlenght of ipv6 address defined into
> arpa/inet.h  + 1 for string terminator
>
> 2) I refactored the call to get_env to take first ipv6 address, then
>    only if it is NULL, i make a call for ipv4
> ---
>  src/plugins/auth-pam/auth-pam.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/src/plugins/auth-pam/auth-pam.c
> b/src/plugins/auth-pam/auth-pam.c
> index ae0d495a..cd91a33c 100644
> --- a/src/plugins/auth-pam/auth-pam.c
> +++ b/src/plugins/auth-pam/auth-pam.c
> @@ -48,7 +48,7 @@
>  #include <signal.h>
>  #include <syslog.h>
>  #include "utils.h"
> -
> +#include <arpa/inet.h>

 #include <openvpn-plugin.h>
>
>  #define DEBUG(verb) ((verb) >= 4)
> @@ -115,7 +115,7 @@ struct user_pass {
>      char password[128];
>      char common_name[128];
>      char response[128];
> -    char remote[46]; //46 as ipv6 form n:n:n:n:n:n:d.d.d.d and +
> terminator \0
> +    char remote[INET6_ADDRSTRLEN+1]; //INET6_ADDRSTRLEN  is the lenght of
> ipv6 + terminator \0
>

INET6_ADDRSTRLEN = 46 already includes space for nul termination

More importantly, you have to provide a single updated patch
preferably with version indicated in the subject and sent out with
--in-reply-to <msgid> referring to the previous version.

Submitting incremental pieces of fixup commits doesn't help. And please
wait for review before making changes unless correcting a critical error.

Thanks,

Selva
<div dir="ltr"><div>Hi,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Mar 30, 2020 at 8:59 AM Paolo Cerrito &lt;<a href="mailto:wardragon78@gmail.com">wardragon78@gmail.com</a>&gt; wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">1) so remote was set to the maxlenght of ipv6 address defined into<br>
arpa/inet.h  + 1 for string terminator<br>
<br>
2) I refactored the call to get_env to take first ipv6 address, then<br>
   only if it is NULL, i make a call for ipv4<br>
---<br>
 src/plugins/auth-pam/auth-pam.c | 8 +++++---<br>
 1 file changed, 5 insertions(+), 3 deletions(-)<br>
<br>
diff --git a/src/plugins/auth-pam/auth-pam.c b/src/plugins/auth-pam/auth-pam.c<br>
index ae0d495a..cd91a33c 100644<br>
--- a/src/plugins/auth-pam/auth-pam.c<br>
+++ b/src/plugins/auth-pam/auth-pam.c<br>
@@ -48,7 +48,7 @@<br>
 #include &lt;signal.h&gt;<br>
 #include &lt;syslog.h&gt;<br>
 #include &quot;utils.h&quot;<br>
-<br>
+#include &lt;arpa/inet.h&gt; </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
 #include &lt;openvpn-plugin.h&gt;<br>
<br>
 #define DEBUG(verb) ((verb) &gt;= 4)<br>
@@ -115,7 +115,7 @@ struct user_pass {<br>
     char password[128];<br>
     char common_name[128];<br>
     char response[128];<br>
-    char remote[46]; //46 as ipv6 form n:n:n:n:n:n:d.d.d.d and + terminator \0<br>
+    char remote[INET6_ADDRSTRLEN+1]; //INET6_ADDRSTRLEN  is the lenght of ipv6 + terminator \0<br></blockquote><div><br></div><div>INET6_ADDRSTRLEN = 46 already includes space for nul termination </div><div><br></div><div>More importantly, you have to provide a single updated patch </div><div>preferably with version indicated in the subject and sent out with</div><div>--in-reply-to &lt;msgid&gt; referring to the previous version.</div><div><br></div><div>Submitting incremental pieces of fixup commits doesn&#39;t help. And please</div><div>wait for review before making changes unless correcting a critical error.</div><div><br></div><div>Thanks,</div><div><br></div><div>Selva</div></div></div>
  

So,
as you asked, now a make the complete patch from diff from master and my
branch, making the change to remote lenght.

--- a/src/plugins/auth-pam/auth-pam.c
+++ b/src/plugins/auth-pam/auth-pam.c
@@ -48,7 +48,7 @@
 #include <signal.h>
 #include <syslog.h>
 #include "utils.h"
-
+#include <arpa/inet.h>
 #include <openvpn-plugin.h>
 
 #define DEBUG(verb) ((verb) >= 4)
@@ -115,6 +115,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;
 };
@@ -517,13 +518,19 @@ 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
+        }
 
         if (username && strlen(username) > 0 && password)
         {
             if (send_control(context->foreground_fd, COMMAND_VERIFY) == -1
                 || 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, common_name) == -1
+                || send_string(context->foreground_fd, remote) == -1)
             {
                 fprintf(stderr, "AUTH-PAM: Error sending auth info to
background process\n");
             }
@@ -750,8 +757,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);
@@ -839,7 +854,8 @@ pam_server(int fd, const char *service, int verb,
const struct name_value_list *
             case COMMAND_VERIFY:
                 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, up.common_name,
sizeof(up.common_name)) == -1
+                    || recv_string(fd, up.remote, sizeof(up.remote)) == -1)
                 {
                     fprintf(stderr, "AUTH-PAM: BACKGROUND: read error
on command channel: code=%d, exiting\n",
                             command);
@@ -853,6 +869,7 @@ pam_server(int fd, const char *service, int verb,
const struct name_value_list *
                             up.username, up.password);
 #else
                     fprintf(stderr, "AUTH-PAM: BACKGROUND: USER: %s\n",
up.username);
+                    fprintf(stderr, "AUTH-PAM: BACKGROUND: REMOTE:
%s\n", up.remote);
 #endif
                 }


Il 30/03/20 22:58, Selva Nair ha scritto:
> Hi,
>
> On Mon, Mar 30, 2020 at 8:59 AM Paolo Cerrito <wardragon78@gmail.com
> <mailto:wardragon78@gmail.com>> wrote:
>
>     1) so remote was set to the maxlenght of ipv6 address defined into
>     arpa/inet.h  + 1 for string terminator
>
>     2) I refactored the call to get_env to take first ipv6 address, then
>        only if it is NULL, i make a call for ipv4
>     ---
>      src/plugins/auth-pam/auth-pam.c | 8 +++++---
>      1 file changed, 5 insertions(+), 3 deletions(-)
>
>     diff --git a/src/plugins/auth-pam/auth-pam.c
>     b/src/plugins/auth-pam/auth-pam.c
>     index ae0d495a..cd91a33c 100644
>     --- a/src/plugins/auth-pam/auth-pam.c
>     +++ b/src/plugins/auth-pam/auth-pam.c
>     @@ -48,7 +48,7 @@
>      #include <signal.h>
>      #include <syslog.h>
>      #include "utils.h"
>     -
>     +#include <arpa/inet.h> 
>
>      #include <openvpn-plugin.h>
>
>      #define DEBUG(verb) ((verb) >= 4)
>     @@ -115,7 +115,7 @@ struct user_pass {
>          char password[128];
>          char common_name[128];
>          char response[128];
>     -    char remote[46]; //46 as ipv6 form n:n:n:n:n:n:d.d.d.d and +
>     terminator \0
>     +    char remote[INET6_ADDRSTRLEN+1]; //INET6_ADDRSTRLEN  is the
>     lenght of ipv6 + terminator \0
>
>
> INET6_ADDRSTRLEN = 46 already includes space for nul termination 
>
> More importantly, you have to provide a single updated patch 
> preferably with version indicated in the subject and sent out with
> --in-reply-to <msgid> referring to the previous version.
>
> Submitting incremental pieces of fixup commits doesn't help. And please
> wait for review before making changes unless correcting a critical error.
>
> Thanks,
>
> Selva
  

Am 31.03.20 um 09:34 schrieb Paolo:
> So,
> as you asked, now a make the complete patch from diff from master and my
> branch, making the change to remote lenght.
> 

Your patch got mangled very badly for me:

>     -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  || send_string(context->foreground_fd,
>       common_name) == -1)<br>
>       +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  || send_string(context-&gt;foreground_fd,
>       common_name) == -1<br>
>       +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  || send_string(context-&gt;foreground_fd, remote)

Can you try to resend it with git send email or even as an attachment
that does not get reformatted would help.

Arne