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

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

Commit Message

Paolo Cerrito June 26, 2019, 10:26 p.m. UTC
From: paolo <paolo.cerrito@uniroma2.it>

Signed-off-by: Paolo Cerrito <paolo.cerrito@uniroma2.it>
---
 src/plugins/auth-pam/auth-pam.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

Comments

Antonio Quartulli June 26, 2019, 11:07 p.m. UTC | #1
Hi,

On 27/06/2019 10:26, Paolo Cerrito wrote:
> From: paolo <paolo.cerrito@uniroma2.it>
> 
> Signed-off-by: Paolo Cerrito <paolo.cerrito@uniroma2.it>

Why do we need this change?
What benefit does it give us?
How can it be used?

IMHO it would be nice to add these pieces of information to the commit
message (right now it feels .. "empty" ;-) )

Regards,

> ---
>  src/plugins/auth-pam/auth-pam.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/src/plugins/auth-pam/auth-pam.c b/src/plugins/auth-pam/auth-pam.c
> index 88b53204..9d8dfb95 100644
> --- a/src/plugins/auth-pam/auth-pam.c
> +++ b/src/plugins/auth-pam/auth-pam.c
> @@ -115,6 +115,7 @@ struct user_pass {
>      char password[128];
>      char common_name[128];
>      char response[128];
> +    char remote[128];
>  
>      const struct name_value_list *name_value_list;
>  };
> @@ -517,13 +518,15 @@ 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_ip", envp) ? get_env("untrusted_ip", envp) : get_env("untrusted_ip6", envp);
>  
>          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 +753,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 +850,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 +865,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
>                  }
>  
>
Paolo Cerrito June 26, 2019, 11:48 p.m. UTC | #2
Hi,

this change is needed to pass remote ip address to pam environment. I
try to explain by an example, so this is the case for us.

I would use pam_recent module to make dynamic ip firewalling. User can
try to login some times, after for example 3 times, pam_recent could
block ip using iptables dynamic rules for about an hour. But pam module
need not only username for logging, but remote ip for firewalling.

This is just an example about the use. But other modules can make other
things using remote ip.




Il 27/06/19 11:07, Antonio Quartulli ha scritto:
> Hi,
>
> On 27/06/2019 10:26, Paolo Cerrito wrote:
>> From: paolo <paolo.cerrito@uniroma2.it>
>>
>> Signed-off-by: Paolo Cerrito <paolo.cerrito@uniroma2.it>
> Why do we need this change?
> What benefit does it give us?
> How can it be used?
>
> IMHO it would be nice to add these pieces of information to the commit
> message (right now it feels .. "empty" ;-) )
>
> Regards,
>
>> ---
>>  src/plugins/auth-pam/auth-pam.c | 19 ++++++++++++++++---
>>  1 file changed, 16 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/plugins/auth-pam/auth-pam.c b/src/plugins/auth-pam/auth-pam.c
>> index 88b53204..9d8dfb95 100644
>> --- a/src/plugins/auth-pam/auth-pam.c
>> +++ b/src/plugins/auth-pam/auth-pam.c
>> @@ -115,6 +115,7 @@ struct user_pass {
>>      char password[128];
>>      char common_name[128];
>>      char response[128];
>> +    char remote[128];
>>  
>>      const struct name_value_list *name_value_list;
>>  };
>> @@ -517,13 +518,15 @@ 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_ip", envp) ? get_env("untrusted_ip", envp) : get_env("untrusted_ip6", envp);
>>  
>>          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 +753,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 +850,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 +865,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
>>                  }
>>  
>>
Paolo Cerrito June 26, 2019, 11:52 p.m. UTC | #3
Hi,

another example (and the simplest case) is:

support to set remote client data into PAM environment, in turn
correctly allow PAM logging the client address to syslog

Paolo Cerrito

Il 27/06/19 11:07, Antonio Quartulli ha scritto:
> Hi,
>
> On 27/06/2019 10:26, Paolo Cerrito wrote:
>> From: paolo <paolo.cerrito@uniroma2.it>
>>
>> Signed-off-by: Paolo Cerrito <paolo.cerrito@uniroma2.it>
> Why do we need this change?
> What benefit does it give us?
> How can it be used?
>
> IMHO it would be nice to add these pieces of information to the commit
> message (right now it feels .. "empty" ;-) )
>
> Regards,
>
>> ---
>>  src/plugins/auth-pam/auth-pam.c | 19 ++++++++++++++++---
>>  1 file changed, 16 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/plugins/auth-pam/auth-pam.c b/src/plugins/auth-pam/auth-pam.c
>> index 88b53204..9d8dfb95 100644
>> --- a/src/plugins/auth-pam/auth-pam.c
>> +++ b/src/plugins/auth-pam/auth-pam.c
>> @@ -115,6 +115,7 @@ struct user_pass {
>>      char password[128];
>>      char common_name[128];
>>      char response[128];
>> +    char remote[128];
>>  
>>      const struct name_value_list *name_value_list;
>>  };
>> @@ -517,13 +518,15 @@ 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_ip", envp) ? get_env("untrusted_ip", envp) : get_env("untrusted_ip6", envp);
>>  
>>          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 +753,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 +850,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 +865,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
>>                  }
>>  
>>

Patch

diff --git a/src/plugins/auth-pam/auth-pam.c b/src/plugins/auth-pam/auth-pam.c
index 88b53204..9d8dfb95 100644
--- a/src/plugins/auth-pam/auth-pam.c
+++ b/src/plugins/auth-pam/auth-pam.c
@@ -115,6 +115,7 @@  struct user_pass {
     char password[128];
     char common_name[128];
     char response[128];
+    char remote[128];
 
     const struct name_value_list *name_value_list;
 };
@@ -517,13 +518,15 @@  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_ip", envp) ? get_env("untrusted_ip", envp) : get_env("untrusted_ip6", envp);
 
         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 +753,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 +850,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 +865,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
                 }