Message ID | 20200330125719.766763-1-wardragon78@gmail.com |
---|---|
State | Changes Requested |
Headers | show |
Series | [Openvpn-devel,v5] Insert client connection data into PAM environment | expand |
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 <<a href="mailto:wardragon78@gmail.com">wardragon78@gmail.com</a>> 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 <signal.h><br> #include <syslog.h><br> #include "utils.h"<br> -<br> +#include <arpa/inet.h> </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> #include <openvpn-plugin.h><br> <br> #define DEBUG(verb) ((verb) >= 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 <msgid> referring to the previous version.</div><div><br></div><div>Submitting incremental pieces of fixup commits doesn'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->foreground_fd, > common_name) == -1<br> > +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â || send_string(context->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
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 const struct name_value_list *name_value_list; }; @@ -518,12 +518,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); //try to take ipv4 if not set ipv6 + 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