Hi,

sorry for not handling this in a more timely fashion.  It needs some
changes, though.

On Fri, Jun 24, 2022 at 12:49:41PM +0200, Paolo Cerrito wrote:
> From: paolo <[email protected]>
> 
> "Changes from v1:
> changed sprintf for logging to plugin_log
> "
> 
> change to reflect current head openvpn repository
> 
> this patch put remote host ip into pam environment, so this make pam
> module able to use it.
> 
> in simple, this patch get ip (ipv4 and ipv6) from openvpn, put into pam
> environment and log this operation.

When committing, please use "git commit -s", so the patch gets your
signed-off-by: line.

> ---
>  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
> +        }

Please follow the OpenVPN coding style:

+        if (remote == NULL)
+        {
+            /*  if Null, try to take ipv4 if not set ipv6 */
+            remote = get_env("untrusted_ip", envp);
+        }

(opening "{" is on a new line, always, and 4 space indent, and no // C++
comments)

This also needs some way to handle the case that neither untrusted_ip6
nor untrusted_ip is set (which might be due to changes on the OpenVPN
side) - with your v2 patch, this will pass NULL to send_string(), 
and then it will crash.

You could do

  remote = "";

to make it point to an empty string...

> @@ -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);
> +        }

... which would work nicely together with this check for a non-empty
string on the other end of the pipeline.

gert
-- 
"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
                             Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany                             [email protected]

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to