| Message ID | 1532486093-24793-1-git-send-email-selva.nair@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [Openvpn-devel,v3] Parse static challenge response in auth-pam plugin | expand |
I don't know if it is appropriate to reply to this post in this manner, but Selva's static challenge response in the PAM plugin would be a great addition; I've applied this and the base64 patch and can successfully use the implementation with Tunnelblick (which is supporting static-challenge as of 3.7.7beta03, as well as the current version of the Windows OpenVPN client (I'm using Google Authenticator for the OTP). If there is anything I can do to provide a list of testcases, write-up on its configuration and usage, etc., in aide of getting the patch merged and released, please let me know. Many thanks, Joe On Tue, Jul 24, 2018 at 9:34 PM, <selva.nair@gmail.com> wrote: > From: Selva Nair <selva.nair@gmail.com> > > If static challenge is in use, the password passed to the plugin by openvpn > is of the form "SCRV1:base64-pass:base64-response". Parse this string to > separate it into password and response and use them to respond to queries > in the pam conversation function. > > On the plugin parameters line the substitution keyword for the static > challenge response is "OTP". For example, for pam config named "test" that > prompts for "user", "password" and "pin", use > > plugin openvpn-auth-pam.so "test user USERNAME password PASSWORD pin OTP" > > Signed-off-by: Selva Nair <selva.nair@gmail.com> > > --- > v2: Depends on the base64 export patch > v3: match password string with "SCRV1:" instead of "SCRV1" > (pointed out by Joe Bell <joeaintexas@gmail.com>) > > src/plugins/auth-pam/README.auth-pam | 15 +++++--- > src/plugins/auth-pam/auth-pam.c | 75 ++++++++++++++++++++++++++++++ > +++++- > 2 files changed, 84 insertions(+), 6 deletions(-) > > diff --git a/src/plugins/auth-pam/README.auth-pam > b/src/plugins/auth-pam/README.auth-pam > index e123690..9081565 100644 > --- a/src/plugins/auth-pam/README.auth-pam > +++ b/src/plugins/auth-pam/README.auth-pam > @@ -36,19 +36,20 @@ pairs to answer PAM module queries. > > For example: > > - plugin openvpn-auth-pam.so "login login USERNAME password PASSWORD" > + plugin openvpn-auth-pam.so "login login USERNAME password PASSWORD pin > OTP" > > tells auth-pam to (a) use the "login" PAM module, (b) answer a > -"login" query with the username given by the OpenVPN client, and > -(c) answer a "password" query with the password given by the > -OpenVPN client. This provides flexibility in dealing with the different > +"login" query with the username given by the OpenVPN client, > +(c) answer a "password" query with the password, and (d) answer a > +"pin" query with the OTP given by the OpenVPN client. > +This provides flexibility in dealing with different > types of query strings which different PAM modules might generate. > For example, suppose you were using a PAM module called > "test" which queried for "name" rather than "login": > > plugin openvpn-auth-pam.so "test name USERNAME password PASSWORD" > > -While "USERNAME" "COMMONNAME" and "PASSWORD" are special strings which > substitute > +While "USERNAME" "COMMONNAME" "PASSWORD" and "OTP" are special strings > which substitute > to client-supplied values, it is also possible to name literal values > to use as PAM module query responses. For example, suppose that the > login module queried for a third parameter, "domain" which > @@ -61,6 +62,10 @@ the operation of this plugin: > > client-cert-not-required > username-as-common-name > + static-challenge > + > +Use of --static challenege is required to pass a pin (represented by > "OTP" in > +parameter substituion) or a second password. > > Run OpenVPN with --verb 7 or higher to get debugging output from > this plugin, including the list of queries presented by the > diff --git a/src/plugins/auth-pam/auth-pam.c b/src/plugins/auth-pam/auth- > pam.c > index 26b0eeb..e22ce5f 100644 > --- a/src/plugins/auth-pam/auth-pam.c > +++ b/src/plugins/auth-pam/auth-pam.c > @@ -6,6 +6,7 @@ > * packet compression. > * > * Copyright (C) 2002-2018 OpenVPN Inc <sales@openvpn.net> > + * Copyright (C) 2016-2018 Selva Nair <selva.nair@gmail.com> > * > * This program is free software; you can redistribute it and/or modify > * it under the terms of the GNU General Public License version 2 > @@ -64,6 +65,7 @@ > > /* Pointers to functions exported from openvpn */ > static plugin_secure_memzero_t plugin_secure_memzero = NULL; > +static plugin_base64_decode_t plugin_base64_decode = NULL; > > /* > * Plugin state, used by foreground > @@ -87,6 +89,7 @@ struct auth_pam_context > * "USERNAME" -- substitute client-supplied username > * "PASSWORD" -- substitute client-specified password > * "COMMONNAME" -- substitute client certificate common name > + * "OTP" -- substitute static challenge response if available > */ > > #define N_NAME_VALUE 16 > @@ -111,6 +114,7 @@ struct user_pass { > char username[128]; > char password[128]; > char common_name[128]; > + char response[128]; > > const struct name_value_list *name_value_list; > }; > @@ -276,6 +280,66 @@ name_value_match(const char *query, const char *match) > return strncasecmp(match, query, strlen(match)) == 0; > } > > +/* > + * Split and decode up->password in the form SCRV1:base64_pass:base64_ > response > + * into pass and response and save in up->password and up->response. > + * If the password is not in the expected format, input is not changed. > + */ > +static void > +split_scrv1_password(struct user_pass *up) > +{ > + const int skip = strlen("SCRV1:"); > + if (strncmp(up->password, "SCRV1:", skip) != 0) > + { > + return; > + } > + > + char *tmp = strdup(up->password); > + if (!tmp) > + { > + fprintf(stderr, "AUTH-PAM: out of memory parsing static challenge > password\n"); > + goto out; > + } > + > + char *pass = tmp + skip; > + char *resp = strchr(pass, ':'); > + if (!resp) /* string not in SCRV1:xx:yy format */ > + { > + goto out; > + } > + *resp++ = '\0'; > + > + int n = plugin_base64_decode(pass, up->password, > sizeof(up->password)-1); > + if (n > 0) > + { > + up->password[n] = '\0'; > + n = plugin_base64_decode(resp, up->response, > sizeof(up->response)-1); > + if (n > 0) > + { > + up->response[n] = '\0'; > + if (DEBUG(up->verb)) > + { > + fprintf(stderr, "AUTH-PAM: BACKGROUND: parsed static > challenge password\n"); > + } > + goto out; > + } > + } > + > + /* decode error: reinstate original value of up->password and return > */ > + plugin_secure_memzero(up->password, sizeof(up->password)); > + plugin_secure_memzero(up->response, sizeof(up->response)); > + strcpy(up->password, tmp); /* tmp is guaranteed to fit in > up->password */ > + > + fprintf(stderr, "AUTH-PAM: base64 decode error while parsing static > challenge password\n"); > + > +out: > + if (tmp) > + { > + plugin_secure_memzero(tmp, strlen(tmp)); > + free(tmp); > + } > +} > + > OPENVPN_EXPORT int > openvpn_plugin_open_v3(const int v3structver, > struct openvpn_plugin_args_open_in const *args, > @@ -316,6 +380,7 @@ openvpn_plugin_open_v3(const int v3structver, > > /* Save global pointers to functions exported from openvpn */ > plugin_secure_memzero = args->callbacks->plugin_secure_memzero; > + plugin_base64_decode = args->callbacks->plugin_base64_decode; > > /* > * Make sure we have two string arguments: the first is the .so name, > @@ -599,6 +664,10 @@ my_conv(int n, const struct pam_message **msg_array, > { > aresp[i].resp = searchandreplace(match_value, > "COMMONNAME", up->common_name); > } > + else if (strstr(match_value, "OTP")) > + { > + aresp[i].resp = searchandreplace(match_value, > "OTP", up->response); > + } > else > { > aresp[i].resp = strdup(match_value); > @@ -787,6 +856,9 @@ pam_server(int fd, const char *service, int verb, > const struct name_value_list * > #endif > } > > + /* If password is of the form SCRV1:base64:base64 split > it up */ > + split_scrv1_password(&up); > + > if (pam_auth(service, &up)) /* Succeeded */ > { > if (send_control(fd, RESPONSE_VERIFY_SUCCEEDED) == -1) > @@ -818,10 +890,11 @@ pam_server(int fd, const char *service, int verb, > const struct name_value_list * > command); > goto done; > } > + plugin_secure_memzero(up.response, sizeof(up.response)); > } > done: > - > plugin_secure_memzero(up.password, sizeof(up.password)); > + plugin_secure_memzero(up.response, sizeof(up.response)); > #ifdef USE_PAM_DLOPEN > dlclose_pam(); > #endif > -- > 2.1.4 > > > ------------------------------------------------------------ > ------------------ > Check out the vibrant tech community on one of the world's most > engaging tech sites, Slashdot.org! http://sdm.link/slashdot > _______________________________________________ > Openvpn-devel mailing list > Openvpn-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/openvpn-devel > <div dir="ltr">I don't know if it is appropriate to reply to this post in this manner, but Selva's static challenge response in the PAM plugin would be a great addition; I've applied this and the base64 patch and can successfully use the implementation with Tunnelblick (which is supporting static-challenge as of 3.7.7beta03, as well as the current version of the Windows OpenVPN client (I'm using Google Authenticator for the OTP).<div><br></div><div>If there is anything I can do to provide a list of testcases, write-up on its configuration and usage, etc., in aide of getting the patch merged and released, please let me know.</div><div><br></div><div>Many thanks,</div><div>Joe</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jul 24, 2018 at 9:34 PM, <span dir="ltr"><<a href="mailto:selva.nair@gmail.com" target="_blank">selva.nair@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">From: Selva Nair <<a href="mailto:selva.nair@gmail.com">selva.nair@gmail.com</a>><br> <br> If static challenge is in use, the password passed to the plugin by openvpn<br> is of the form "SCRV1:base64-pass:base64-<wbr>response". Parse this string to<br> separate it into password and response and use them to respond to queries<br> in the pam conversation function.<br> <br> On the plugin parameters line the substitution keyword for the static<br> challenge response is "OTP". For example, for pam config named "test" that<br> prompts for "user", "password" and "pin", use<br> <br> plugin openvpn-auth-pam.so "test user USERNAME password PASSWORD pin OTP"<br> <br> Signed-off-by: Selva Nair <<a href="mailto:selva.nair@gmail.com">selva.nair@gmail.com</a>><br> <br> ---<br> v2: Depends on the base64 export patch<br> v3: match password string with "SCRV1:" instead of "SCRV1"<br> (pointed out by Joe Bell <<a href="mailto:joeaintexas@gmail.com">joeaintexas@gmail.com</a>>)<br> <br> src/plugins/auth-pam/README.<wbr>auth-pam | 15 +++++---<br> src/plugins/auth-pam/auth-pam.<wbr>c | 75 ++++++++++++++++++++++++++++++<wbr>+++++-<br> 2 files changed, 84 insertions(+), 6 deletions(-)<br> <br> diff --git a/src/plugins/auth-pam/README.<wbr>auth-pam b/src/plugins/auth-pam/README.<wbr>auth-pam<br> index e123690..9081565 100644<br> --- a/src/plugins/auth-pam/README.<wbr>auth-pam<br> +++ b/src/plugins/auth-pam/README.<wbr>auth-pam<br> @@ -36,19 +36,20 @@ pairs to answer PAM module queries.<br> <br> For example:<br> <br> - plugin openvpn-auth-pam.so "login login USERNAME password PASSWORD"<br> + plugin openvpn-auth-pam.so "login login USERNAME password PASSWORD pin OTP"<br> <br> tells auth-pam to (a) use the "login" PAM module, (b) answer a<br> -"login" query with the username given by the OpenVPN client, and<br> -(c) answer a "password" query with the password given by the<br> -OpenVPN client. This provides flexibility in dealing with the different<br> +"login" query with the username given by the OpenVPN client,<br> +(c) answer a "password" query with the password, and (d) answer a<br> +"pin" query with the OTP given by the OpenVPN client.<br> +This provides flexibility in dealing with different<br> types of query strings which different PAM modules might generate.<br> For example, suppose you were using a PAM module called<br> "test" which queried for "name" rather than "login":<br> <br> plugin openvpn-auth-pam.so "test name USERNAME password PASSWORD"<br> <br> -While "USERNAME" "COMMONNAME" and "PASSWORD" are special strings which substitute<br> +While "USERNAME" "COMMONNAME" "PASSWORD" and "OTP" are special strings which substitute<br> to client-supplied values, it is also possible to name literal values<br> to use as PAM module query responses. For example, suppose that the<br> login module queried for a third parameter, "domain" which<br> @@ -61,6 +62,10 @@ the operation of this plugin:<br> <br> client-cert-not-required<br> username-as-common-name<br> + static-challenge<br> +<br> +Use of --static challenege is required to pass a pin (represented by "OTP" in<br> +parameter substituion) or a second password.<br> <br> Run OpenVPN with --verb 7 or higher to get debugging output from<br> this plugin, including the list of queries presented by the<br> diff --git a/src/plugins/auth-pam/auth-<wbr>pam.c b/src/plugins/auth-pam/auth-<wbr>pam.c<br> index 26b0eeb..e22ce5f 100644<br> --- a/src/plugins/auth-pam/auth-<wbr>pam.c<br> +++ b/src/plugins/auth-pam/auth-<wbr>pam.c<br> @@ -6,6 +6,7 @@<br> * packet compression.<br> *<br> * Copyright (C) 2002-2018 OpenVPN Inc <<a href="mailto:sales@openvpn.net">sales@openvpn.net</a>><br> + * Copyright (C) 2016-2018 Selva Nair <<a href="mailto:selva.nair@gmail.com">selva.nair@gmail.com</a>><br> *<br> * This program is free software; you can redistribute it and/or modify<br> * it under the terms of the GNU General Public License version 2<br> @@ -64,6 +65,7 @@<br> <br> /* Pointers to functions exported from openvpn */<br> static plugin_secure_memzero_t plugin_secure_memzero = NULL;<br> +static plugin_base64_decode_t plugin_base64_decode = NULL;<br> <br> /*<br> * Plugin state, used by foreground<br> @@ -87,6 +89,7 @@ struct auth_pam_context<br> * "USERNAME" -- substitute client-supplied username<br> * "PASSWORD" -- substitute client-specified password<br> * "COMMONNAME" -- substitute client certificate common name<br> + * "OTP" -- substitute static challenge response if available<br> */<br> <br> #define N_NAME_VALUE 16<br> @@ -111,6 +114,7 @@ struct user_pass {<br> char username[128];<br> char password[128];<br> char common_name[128];<br> + char response[128];<br> <br> const struct name_value_list *name_value_list;<br> };<br> @@ -276,6 +280,66 @@ name_value_match(const char *query, const char *match)<br> return strncasecmp(match, query, strlen(match)) == 0;<br> }<br> <br> +/*<br> + * Split and decode up->password in the form SCRV1:base64_pass:base64_<wbr>response<br> + * into pass and response and save in up->password and up->response.<br> + * If the password is not in the expected format, input is not changed.<br> + */<br> +static void<br> +split_scrv1_password(struct user_pass *up)<br> +{<br> + const int skip = strlen("SCRV1:");<br> + if (strncmp(up->password, "SCRV1:", skip) != 0)<br> + {<br> + return;<br> + }<br> +<br> + char *tmp = strdup(up->password);<br> + if (!tmp)<br> + {<br> + fprintf(stderr, "AUTH-PAM: out of memory parsing static challenge password\n");<br> + goto out;<br> + }<br> +<br> + char *pass = tmp + skip;<br> + char *resp = strchr(pass, ':');<br> + if (!resp) /* string not in SCRV1:xx:yy format */<br> + {<br> + goto out;<br> + }<br> + *resp++ = '\0';<br> +<br> + int n = plugin_base64_decode(pass, up->password, sizeof(up->password)-1);<br> + if (n > 0)<br> + {<br> + up->password[n] = '\0';<br> + n = plugin_base64_decode(resp, up->response, sizeof(up->response)-1);<br> + if (n > 0)<br> + {<br> + up->response[n] = '\0';<br> + if (DEBUG(up->verb))<br> + {<br> + fprintf(stderr, "AUTH-PAM: BACKGROUND: parsed static challenge password\n");<br> + }<br> + goto out;<br> + }<br> + }<br> +<br> + /* decode error: reinstate original value of up->password and return */<br> + plugin_secure_memzero(up-><wbr>password, sizeof(up->password));<br> + plugin_secure_memzero(up-><wbr>response, sizeof(up->response));<br> + strcpy(up->password, tmp); /* tmp is guaranteed to fit in up->password */<br> +<br> + fprintf(stderr, "AUTH-PAM: base64 decode error while parsing static challenge password\n");<br> +<br> +out:<br> + if (tmp)<br> + {<br> + plugin_secure_memzero(tmp, strlen(tmp));<br> + free(tmp);<br> + }<br> +}<br> +<br> OPENVPN_EXPORT int<br> openvpn_plugin_open_v3(const int v3structver,<br> struct openvpn_plugin_args_open_in const *args,<br> @@ -316,6 +380,7 @@ openvpn_plugin_open_v3(const int v3structver,<br> <br> /* Save global pointers to functions exported from openvpn */<br> plugin_secure_memzero = args->callbacks->plugin_<wbr>secure_memzero;<br> + plugin_base64_decode = args->callbacks->plugin_<wbr>base64_decode;<br> <br> /*<br> * Make sure we have two string arguments: the first is the .so name,<br> @@ -599,6 +664,10 @@ my_conv(int n, const struct pam_message **msg_array,<br> {<br> aresp[i].resp = searchandreplace(match_value, "COMMONNAME", up->common_name);<br> }<br> + else if (strstr(match_value, "OTP"))<br> + {<br> + aresp[i].resp = searchandreplace(match_value, "OTP", up->response);<br> + }<br> else<br> {<br> aresp[i].resp = strdup(match_value);<br> @@ -787,6 +856,9 @@ pam_server(int fd, const char *service, int verb, const struct name_value_list *<br> #endif<br> }<br> <br> + /* If password is of the form SCRV1:base64:base64 split it up */<br> + split_scrv1_password(&up);<br> +<br> if (pam_auth(service, &up)) /* Succeeded */<br> {<br> if (send_control(fd, RESPONSE_VERIFY_SUCCEEDED) == -1)<br> @@ -818,10 +890,11 @@ pam_server(int fd, const char *service, int verb, const struct name_value_list *<br> command);<br> goto done;<br> }<br> + plugin_secure_memzero(up.<wbr>response, sizeof(up.response));<br> }<br> done:<br> -<br> plugin_secure_memzero(up.<wbr>password, sizeof(up.password));<br> + plugin_secure_memzero(up.<wbr>response, sizeof(up.response));<br> #ifdef USE_PAM_DLOPEN<br> dlclose_pam();<br> #endif<br> <span class="HOEnZb"><font color="#888888">-- <br> 2.1.4<br> <br> <br> ------------------------------<wbr>------------------------------<wbr>------------------<br> Check out the vibrant tech community on one of the world's most<br> engaging tech sites, Slashdot.org! <a href="http://sdm.link/slashdot" rel="noreferrer" target="_blank">http://sdm.link/slashdot</a><br> ______________________________<wbr>_________________<br> Openvpn-devel mailing list<br> <a href="mailto:Openvpn-devel@lists.sourceforge.net">Openvpn-devel@lists.<wbr>sourceforge.net</a><br> <a href="https://lists.sourceforge.net/lists/listinfo/openvpn-devel" rel="noreferrer" target="_blank">https://lists.sourceforge.net/<wbr>lists/listinfo/openvpn-devel</a><br> </font></span></blockquote></div><br></div> ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Hi, On Wed, Jul 25, 2018 at 04:31:05PM -0500, Joe Bell wrote: > I don't know if it is appropriate to reply to this post in this manner, It is and it helps :-) We want test reports, as in "I am using this, because it is a useful feature for me, and it works fine!" - not as strong as a full code review, but an indication that something is beneficial at least. [..] > If there is anything I can do to provide a list of testcases, write-up on > its configuration and usage, etc., in aide of getting the patch merged and > released, please let me know. If you could find a few spare weeks for me, that would help quite a bit :-) Jokes aside, a bit of writeup how this looks in the client config, how the client prompts will then look, and how you configure the server would be nice - just here on the list, so google can find it. Actually, this is something useful for $payingcustomer, so I hope to find time to review and test it myself "soonish"... unfortunately, $otherpayingcustomers are screaming more loudly for me to get other stuff done first, with a hard deadline next Tuesday... gert
On Wed, Jul 25, 2018 at 4:43 PM, Gert Doering <gert@greenie.muc.de> wrote: > Hi, > > On Wed, Jul 25, 2018 at 04:31:05PM -0500, Joe Bell wrote: > > I don't know if it is appropriate to reply to this post in this manner, > > It is and it helps :-) > > We want test reports, as in "I am using this, because it is a useful > feature for me, and it works fine!" - not as strong as a full code > review, but an indication that something is beneficial at least. > Terrific. Indeed, I am using this now, and quite successfully in a staging environment. > > [..] > > If there is anything I can do to provide a list of testcases, write-up on > > its configuration and usage, etc., in aide of getting the patch merged > and > > released, please let me know. > > If you could find a few spare weeks for me, that would help quite a bit :-) > > Jokes aside, a bit of writeup how this looks in the client config, how > the client prompts will then look, and how you configure the server would > be nice - just here on the list, so google can find it. > > I will do a full writeup, but the magic comes from plugin /usr/lib/openvpn/plugins/openvpn-plugin-auth-pam.so "openvpn login: USERNAME Password: PASSWORD Verification OTP" in the OpenVPN server.conf, as well as: auth required /lib/security/pam_google_authenticator.so secret=/etc/openvpn/google-authenticator/${USER} user=gauth auth required pam_ldap.so for the contents of /etc/pam.d/openvpn. (Note that I'm using LDAP for username/password, this should be replaced by the appropriate PAM module for other types.) Then, on the client configuration (.ovpn): auth-user-pass static-challenge "Enter Google Authenticator Token" 1 Now obviously there are a lot of details i've left out (setting up Google Authenticator, etc.) but once I compiled OpenVPN with the base64 patch and the plugin patch, it was smiles all around. I will work to put together a full write-up of zero to sixty with screenshots, etc. I've been delighted to cobble this together with my previous knowledge of OpenVPN configurations, finding Selva's patches, and pointers on how to pull this all together, but without a doubt it would have been delightful to have it out-of-the-box. Thanks for the feedback and encouragement to push for this. Joe <div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Jul 25, 2018 at 4:43 PM, Gert Doering <span dir="ltr"><<a href="mailto:gert@greenie.muc.de" target="_blank">gert@greenie.muc.de</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi,<br> <span class="gmail-"><br> On Wed, Jul 25, 2018 at 04:31:05PM -0500, Joe Bell wrote:<br> > I don't know if it is appropriate to reply to this post in this manner, <br> <br> </span>It is and it helps :-)<br> <br> We want test reports, as in "I am using this, because it is a useful<br> feature for me, and it works fine!" - not as strong as a full code<br> review, but an indication that something is beneficial at least.<br></blockquote><div><br></div><div>Terrific. Indeed, I am using this now, and quite successfully in a staging environment.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <br> [..]<br> <span class="gmail-">> If there is anything I can do to provide a list of testcases, write-up on<br> > its configuration and usage, etc., in aide of getting the patch merged and<br> > released, please let me know.<br> <br> </span>If you could find a few spare weeks for me, that would help quite a bit :-)<br> <br> Jokes aside, a bit of writeup how this looks in the client config, how<br> the client prompts will then look, and how you configure the server would<br> be nice - just here on the list, so google can find it.<br> <br></blockquote><div><br></div><div>I will do a full writeup, but the magic comes from</div><div><br></div><div><font face="monospace, monospace">plugin /usr/lib/openvpn/plugins/openvpn-plugin-auth-pam.so "openvpn login: USERNAME Password: PASSWORD Verification OTP"</font></div><div><br></div><div>in the OpenVPN server.conf, as well as:</div><div><br></div><div><div><font face="monospace, monospace">auth required /lib/security/pam_google_authenticator.so secret=/etc/openvpn/google-authenticator/${USER} user=gauth</font></div><div><font face="monospace, monospace">auth required pam_ldap.so</font></div></div><div><br></div><div>for the contents of <span style="font-family:monospace,monospace;font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">/etc/pam.d/openvpn</span><span style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><font face="arial, helvetica, sans-serif">. (Note that I'm using LDAP for username/password, this should be replaced by the appropriate PAM module for other types.)</font></span></div><div><span style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><font face="arial, helvetica, sans-serif"><br></font></span></div><div><span style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><font face="arial, helvetica, sans-serif">Then, on the client configuration (.ovpn):</font></span></div><div><span style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><font face="arial, helvetica, sans-serif"><br></font></span></div><div><span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><font face="monospace, monospace"><div style="">auth-user-pass</div><div style="">static-challenge "Enter Google Authenticator Token" 1</div></font></span></div><div><span style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><font face="arial, helvetica, sans-serif"><br></font></span></div><div><span style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><font face="arial, helvetica, sans-serif">Now obviously there are a lot of details i've left out (setting up Google Authenticator, etc.) but once I compiled OpenVPN with the base64 patch and the plugin patch, it was smiles all around. I will work to put together a full write-up of zero to sixty with screenshots, etc.</font></span></div><div><span style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><font face="arial, helvetica, sans-serif"><br></font></span></div><div><span style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><font face="arial, helvetica, sans-serif">I've been delighted to cobble this together with my previous knowledge of OpenVPN configurations, finding Selva's patches, and pointers on how to pull this all together, but without a doubt it would have been delightful to have it out-of-the-box. Thanks for the feedback and encouragement to push for this.</font></span></div><div><span style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><font face="arial, helvetica, sans-serif"><br></font></span></div><div><span style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><font face="arial, helvetica, sans-serif">Joe</font></span></div><div><span style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><font face="arial, helvetica, sans-serif"><br></font></span></div></div></div></div> ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Hi, On Tue, Jul 24, 2018 at 10:34:53PM -0400, selva.nair@gmail.com wrote: > From: Selva Nair <selva.nair@gmail.com> > > If static challenge is in use, the password passed to the plugin by openvpn > is of the form "SCRV1:base64-pass:base64-response". Parse this string to > separate it into password and response and use them to respond to queries > in the pam conversation function. > > On the plugin parameters line the substitution keyword for the static > challenge response is "OTP". For example, for pam config named "test" that > prompts for "user", "password" and "pin", use > > plugin openvpn-auth-pam.so "test user USERNAME password PASSWORD pin OTP" > > Signed-off-by: Selva Nair <selva.nair@gmail.com> > > --- > v2: Depends on the base64 export patch > v3: match password string with "SCRV1:" instead of "SCRV1" > (pointed out by Joe Bell <joeaintexas@gmail.com>) I'm a bit confused about the prerequisites for this - if I found the right "base64 export patch", this is still pending some reworking because you and David disagreed on details, and then David got sucked into "Client for OpenVPN 3 on Linux" and had no time working on the plugin API for v2.x anymore. Or is there a different base64 export patch which is "just" pending review and merge? I want *this* (so, feature-ACK, haven't stared at the code yet or tested it) - so I want all the prerequisites too :-) gert
Hi, On Sun, Jul 29, 2018 at 3:34 PM, Gert Doering <gert@greenie.muc.de> wrote: > Hi, > > On Tue, Jul 24, 2018 at 10:34:53PM -0400, selva.nair@gmail.com wrote: >> From: Selva Nair <selva.nair@gmail.com> >> >> If static challenge is in use, the password passed to the plugin by openvpn >> is of the form "SCRV1:base64-pass:base64-response". Parse this string to >> separate it into password and response and use them to respond to queries >> in the pam conversation function. >> >> On the plugin parameters line the substitution keyword for the static >> challenge response is "OTP". For example, for pam config named "test" that >> prompts for "user", "password" and "pin", use >> >> plugin openvpn-auth-pam.so "test user USERNAME password PASSWORD pin OTP" >> >> Signed-off-by: Selva Nair <selva.nair@gmail.com> >> >> --- >> v2: Depends on the base64 export patch >> v3: match password string with "SCRV1:" instead of "SCRV1" >> (pch ointed out by Joe Bell <joeaintexas@gmail.com>) > > I'm a bit confused about the prerequisites for this - if I found the > right "base64 export patch", this is still pending some reworking > because you and David disagreed on details, and then David got sucked > into "Client for OpenVPN 3 on Linux" and had no time working on the > plugin API for v2.x anymore. Yes that's the base64 patch. What is stopping it is not the disagreement on that patch but an "error" [*] in the plugin header that I had discovered. David wants to fix that before this one, but it seems he is too busy with other things. And there is a pending patch to fix that : https://patchwork.openvpn.net/patch/87/ Actually we can get base64 export merged without "fixing" the API (header) as the only place it is referred to in David's patch is in the accompanying sample that shows how to use the exported function. If David is okay with it I can volunteer to split his patch into two and get the base64 export merged sooner as its generally useful. Selva [*] A function signature uses a pointer to an opaque handle (a void *) while it should be just the handle. It generates no warning as it is void * vs void ** and all existing codes out there must be correctly passing the pointer (handle) ignoring the signature in the header -- else they wont work. I wanted the header to be fixed and David seems to agree with that. ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Hi, On 30/07/18 04:16, Selva Nair wrote: > Yes that's the base64 patch. What is stopping it is not the > disagreement on that patch but an "error" [*] in the plugin header > that I had discovered. David wants to fix that before this one, but > it seems he is too busy with other things. > > And there is a pending patch to fix that : > https://patchwork.openvpn.net/patch/87/ > [CUT] > > [*] A function signature uses a pointer to an opaque handle (a void *) > while it should be just the handle. It generates no warning as it is > void * vs void ** and all existing codes out there must be correctly > passing the pointer (handle) ignoring the signature in the header -- > else they wont work. I wanted the header to be fixed and David seems > to agree with that. > I remember discussing this patch with David as well and the general feeling was that the patch was correct. Maybe David wanted to spend some more time on this patch, but it slipped off the plate. As Selva said, if users of that function were following the header they would see a lot of explosions, while this is not the case. Therefore it should be happily applied with no risk. Cheers,
Hi, On Mon, Jul 30, 2018 at 10:31 AM, Antonio Quartulli <a@unstable.cc> wrote: > Hi, > > On 30/07/18 04:16, Selva Nair wrote: >> Yes that's the base64 patch. What is stopping it is not the >> disagreement on that patch but an "error" [*] in the plugin header >> that I had discovered. David wants to fix that before this one, but >> it seems he is too busy with other things. >> >> And there is a pending patch to fix that : >> https://patchwork.openvpn.net/patch/87/ >> > > [CUT] > > >> >> [*] A function signature uses a pointer to an opaque handle (a void *) >> while it should be just the handle. It generates no warning as it is >> void * vs void ** and all existing codes out there must be correctly >> passing the pointer (handle) ignoring the signature in the header -- >> else they wont work. I wanted the header to be fixed and David seems >> to agree with that. >> > > I remember discussing this patch with David as well and the general > feeling was that the patch was correct. > > Maybe David wanted to spend some more time on this patch, but it slipped > off the plate. > > As Selva said, if users of that function were following the header they > would see a lot of explosions, while this is not the case. > Therefore it should be happily applied with no risk. Antonio, thanks for recalling your discussion and for the reassurance. My description of this in the previous mail was not entirely correct: it has been a long time, and I was a bit rusty on the details -- the pointer in question is a member of "struct openvpn_plugin_args_open_return" where its wrongly declared. Selva ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
On 30/07/18 16:58, Selva Nair wrote: > Hi, > > On Mon, Jul 30, 2018 at 10:31 AM, Antonio Quartulli <a@unstable.cc> wrote: >> Hi, >> >> On 30/07/18 04:16, Selva Nair wrote: >>> Yes that's the base64 patch. What is stopping it is not the >>> disagreement on that patch but an "error" [*] in the plugin header >>> that I had discovered. David wants to fix that before this one, but >>> it seems he is too busy with other things. >>> >>> And there is a pending patch to fix that : >>> https://patchwork.openvpn.net/patch/87/ >>> >> >> [CUT] >> >> >>> >>> [*] A function signature uses a pointer to an opaque handle (a void *) >>> while it should be just the handle. It generates no warning as it is >>> void * vs void ** and all existing codes out there must be correctly >>> passing the pointer (handle) ignoring the signature in the header -- >>> else they wont work. I wanted the header to be fixed and David seems >>> to agree with that. >>> >> >> I remember discussing this patch with David as well and the general >> feeling was that the patch was correct. >> >> Maybe David wanted to spend some more time on this patch, but it slipped >> off the plate. >> >> As Selva said, if users of that function were following the header they >> would see a lot of explosions, while this is not the case. >> Therefore it should be happily applied with no risk. > > Antonio, thanks for recalling your discussion and for the reassurance. > > My description of this in the previous mail was not entirely correct: > it has been a long time, and I was a bit rusty on the details -- the > pointer in question is a member of "struct openvpn_plugin_args_open_return" > where its wrongly declared. > Hi all, and sorry for letting this one (with many others) fall through the cracks. And quickly responding from a holiday now (planning to be mostly disconnected until mid-August; first real holiday in 2 years now). I remember patch was fine but never got around to fix the header file. So if we get the header file fixed, all should be good for getting the base64 stuff added; including this patch. I have only glared at the code quickly of this last v3 patch, so it just needs to be well tested before we add it to git master. Next cool thing to get added would be dynamic challenge, but that will require quite some extensions in the plug-in API as well. Use case: systems which uses sssd to do the authentication via PAM, where OTP may or may not be activated for some user accounts - or temporarily disabled if the backend sssd depends on is unavailable (where sssd can do a local off-line password auth only).
HI On Tue, Jul 31, 2018 at 3:07 AM, David Sommerseth <openvpn@sf.lists.topphemmelig.net> wrote: > On 30/07/18 16:58, Selva Nair wrote: >> Hi, >> >> On Mon, Jul 30, 2018 at 10:31 AM, Antonio Quartulli <a@unstable.cc> wrote: >>> Hi, >>> >>> On 30/07/18 04:16, Selva Nair wrote: >>>> Yes that's the base64 patch. What is stopping it is not the >>>> disagreement on that patch but an "error" [*] in the plugin header >>>> that I had discovered. David wants to fix that before this one, but >>>> it seems he is too busy with other things. >>>> snip >>> I remember discussing this patch with David as well and the general >>> feeling was that the patch was correct. >>> >>> Maybe David wanted to spend some more time on this patch, but it slipped >>> off the plate. >>> >>> As Selva said, if users of that function were following the header they >>> would see a lot of explosions, while this is not the case. >>> Therefore it should be happily applied with no risk. >> >> Antonio, thanks for recalling your discussion and for the reassurance. >> snip > > Hi all, and sorry for letting this one (with many others) fall through the > cracks. And quickly responding from a holiday now (planning to be mostly > disconnected until mid-August; first real holiday in 2 years now). I remember > patch was fine but never got around to fix the header file. Agreed :) The header patch is short and benign, and it seems more than a couple of pairs of eyes have viewed and "approved" it. So can we get an ACK? Here is a link to the patch. https://patchwork.openvpn.net/patch/87/ Once this is accepted, the rest should be straightforward. > Next cool thing to get added would be dynamic challenge, but that will require > quite some extensions in the plug-in API as well. What is stopping this is really the same old problem with sending an AUTH_FAILED reason back to the client -- once that is solved easy to support this with minimal changes to the plugin code. I have some ideas on how to solve the former without too much refactoring, but let's get the static challenge support in first. Thanks, Selva ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Hi, On Tue, Jul 24, 2018 at 10:34:53PM -0400, selva.nair@gmail.com wrote: > From: Selva Nair <selva.nair@gmail.com> > > If static challenge is in use, the password passed to the plugin by openvpn > is of the form "SCRV1:base64-pass:base64-response". Parse this string to > separate it into password and response and use them to respond to queries > in the pam conversation function. > > On the plugin parameters line the substitution keyword for the static > challenge response is "OTP". For example, for pam config named "test" that > prompts for "user", "password" and "pin", use > > plugin openvpn-auth-pam.so "test user USERNAME password PASSWORD pin OTP" > > Signed-off-by: Selva Nair <selva.nair@gmail.com> > > --- > v2: Depends on the base64 export patch > v3: match password string with "SCRV1:" instead of "SCRV1" > (pointed out by Joe Bell <joeaintexas@gmail.com>) Nicely works and does what it says on the lid. So... Acked-By: Gert Doering <gert@greenie.muc.de> I've tried to make it work for our use case, but in combination with pam_auth_linotp, it actually doesn't work - the latter wants pin+otp to be "one string" (to be parsed by the server), so there is no separate "OTP" challenge. I've made it work by hacking a "combine password+otp together" substitution + else if (strstr(match_value, "PASS+OTP")) + { + char temp[100]; + snprintf( temp, sizeof(temp), "%s%s", up->password, up->response ); + aresp[i].resp = searchandreplace(match_value, "PASS+OTP", temp); + } ... but that's not exactly pretty. It works, though :-) Plugin logging is also something we should improve. The current approach "just log to stderr" is failing if the main openvpn process is started by init/rc and stderr is not available... we have logging functions in the v3 api nowadays, so maybe we should use them :-) * * plugin_log * plugin_vlog : Use these functions to add information to the OpenVPN log file. * Messages will only be displayed if the plugin_name parameter * is set. PLOG_DEBUG messages will only be displayed with plug-in * debug log verbosity (at the time of writing that's verb >= 7). There's another catch which we might want to at least document: if you build this plugin and run it from a slightly older openvpn binary which doesn't export the base64 functions, it will core dump most ungraciously Aug 7 20:17:19 openvpn-tcp kernel: pid 49459 (openvpn), uid 0: exited on signal 11 (core dumped) (unsurprisingly, jumping over an uninitialized function pointer) So, shall we bump the plugin API version "with base64" to 5, and change this plugin to actually check it? (Applying it anyway, because this can all be addressed in a subsequent patch) gert
Your patch has been applied to the master branch.
(I'm a bit undecided about release/2.4 - this is in "new feature!" land,
and all the challenge stuff is "master" territory. OTOH, it's not openvpn
main code, and the code is sane enough - so if folks think it should be
in release/2.4, tell me)
commit 7369d01bf360bcfa02f26c05b86dde5496d120f6 (master)
Author: Selva Nair
Date: Tue Jul 24 22:34:53 2018 -0400
Parse static challenge response in auth-pam plugin
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1532486093-24793-1-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17307.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Hi, On Tue, Aug 07, 2018 at 08:59:37PM +0200, Gert Doering wrote: > > v2: Depends on the base64 export patch > > v3: match password string with "SCRV1:" instead of "SCRV1" > > (pointed out by Joe Bell <joeaintexas@gmail.com>) > > Nicely works and does what it says on the lid. So... Talking to myself a lot, lately... Found an interesting caveat which should be addressed, I think. Our system (LinOTP) knows "PIN+OTP" or "PIN" as valid input, the latter leading to "send me a token by SMS/e-mail/...". If I press return at the challenge prompt, it seems the SCRV1: string is not formed the way the plugin wants it, and I end up with pass=SCRV1%3AMTE5NQ%3D%3D in the LinOTP URL - so, it didn't decode it, because the second ':' was missing (if I put a blank in there, I get pass=mypin%20). Is this intentional? Should it be that way? gert
Hi, On Tue, Aug 7, 2018 at 2:59 PM, Gert Doering <gert@greenie.muc.de> wrote: ...some good comments snipped... > > There's another catch which we might want to at least document: if you > build this plugin and run it from a slightly older openvpn binary which > doesn't export the base64 functions, it will core dump most ungraciously > > Aug 7 20:17:19 openvpn-tcp kernel: pid 49459 (openvpn), uid 0: exited on signal 11 (core dumped) > > (unsurprisingly, jumping over an uninitialized function pointer) > > > So, shall we bump the plugin API version "with base64" to 5, and change > this plugin to actually check it? > > (Applying it anyway, because this can all be addressed in a subsequent > patch) Oh, the version bump was overlooked... David's export patch was obviously written assuming it will get merged soon after the secure_memzero export patch but we took a year and some to get there :) Let's bump the plugin struct version and add a check for it in this plugin. Selva ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Hi, On Tue, Aug 7, 2018 at 3:07 PM, Gert Doering <gert@greenie.muc.de> wrote: > Hi, > > On Tue, Aug 07, 2018 at 08:59:37PM +0200, Gert Doering wrote: >> > v2: Depends on the base64 export patch >> > v3: match password string with "SCRV1:" instead of "SCRV1" >> > (pointed out by Joe Bell <joeaintexas@gmail.com>) >> >> Nicely works and does what it says on the lid. So... > > Talking to myself a lot, lately... > > Found an interesting caveat which should be addressed, I think. > > Our system (LinOTP) knows "PIN+OTP" or "PIN" as valid input, the > latter leading to "send me a token by SMS/e-mail/...". > > If I press return at the challenge prompt, it seems the SCRV1: string > is not formed the way the plugin wants it, and I end up with > > pass=SCRV1%3AMTE5NQ%3D%3D How to format this if response is empty is not clearly documented but my impression was that the second ':' (%3A) is required. management-notes.txt specifies the format as password "Auth" "SCRV1:<BASE64_PASSWORD>:<BASE64_RESPONSE>" The Windows GUI prints it using a template "SCRV1:%s:%s", so the second colon will be present if response is empty -- if that's not happening I would consider that a bug in the GUI. When password is read from stdin, its formatted as (from misc.c line 358) buf_printf(&packed_resp, "SCRV1:%s:%s", pw64, resp64); So that should also contain the second colon. > > in the LinOTP URL - so, it didn't decode it, because the second ':' > was missing (if I put a blank in there, I get pass=mypin%20). > > Is this intentional? Should it be that way? If you are constructing the SCRV1: line using a custom UI, I would suggest to add the second colon. If using Windows-GUI or running OpenVPN from command line we'll need to fix this one place or the other. Selva ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Hi, Correcting myself... >> Found an interesting caveat which should be addressed, I think. >> >> Our system (LinOTP) knows "PIN+OTP" or "PIN" as valid input, the >> latter leading to "send me a token by SMS/e-mail/...". >> >> If I press return at the challenge prompt, it seems the SCRV1: string >> is not formed the way the plugin wants it, and I end up with >> >> pass=SCRV1%3AMTE5NQ%3D%3D > > How to format this if response is empty is not clearly documented > but my impression was that the second ':' (%3A) is required. > > management-notes.txt specifies the format as > > password "Auth" "SCRV1:<BASE64_PASSWORD>:<BASE64_RESPONSE>" > > The Windows GUI prints it using a template "SCRV1:%s:%s", so the second > colon will be present if response is empty -- if that's not happening > I would consider that a bug in the GUI. > > When password is read from stdin, its formatted as (from misc.c line 358) > > buf_printf(&packed_resp, "SCRV1:%s:%s", pw64, resp64); > > So that should also contain the second colon. However. just realized that my plugin patch assumes a non-empty password and response after base64 decode. Else it gives up on unpacking and treats it as a regular password. E.g., in auth-pamc.c (line 316) n = plugin_base64_decode(resp, up->response, sizeof(up->response)-1); if (n > 0) I think we could and should accept empty strings for both -- let's fix it. Selva ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Hi, On Tue, Aug 07, 2018 at 03:38:43PM -0400, Selva Nair wrote: > > If I press return at the challenge prompt, it seems the SCRV1: string > > is not formed the way the plugin wants it, and I end up with > > > > pass=SCRV1%3AMTE5NQ%3D%3D > > How to format this if response is empty is not clearly documented > but my impression was that the second ':' (%3A) is required. > > management-notes.txt specifies the format as > > password "Auth" "SCRV1:<BASE64_PASSWORD>:<BASE64_RESPONSE>" OK, so something is bugged, and it seems it's not the plugin. > When password is read from stdin, its formatted as (from misc.c line 358) > > buf_printf(&packed_resp, "SCRV1:%s:%s", pw64, resp64); > > So that should also contain the second colon. This is a bit surprising. So "something" is eating it between "openvpn command line client", "openvpn server" and "plugin-auth-pam". Interesting. > > in the LinOTP URL - so, it didn't decode it, because the second ':' > > was missing (if I put a blank in there, I get pass=mypin%20). > > > > Is this intentional? Should it be that way? > > If you are constructing the SCRV1: line using a custom UI, > I would suggest to add the second colon. If using Windows-GUI or running > OpenVPN from command line we'll need to fix this one place > or the other. Command line client (git:master/5961250e776194a4, what I happened to have lying around), run with a config file that has auth-user-pass auth-nocache auth-retry interact static-challenge "token value: " 1 in it, and pressing <return> at the CHALLENGE: token value: _ prompt. gert
Hi On Tue, Aug 7, 2018 at 5:01 PM, Gert Doering <gert@greenie.muc.de> wrote: > >> > in the LinOTP URL - so, it didn't decode it, because the second ':' >> > was missing (if I put a blank in there, I get pass=mypin%20). >> > >> > Is this intentional? Should it be that way? >> >> If you are constructing the SCRV1: line using a custom UI, >> I would suggest to add the second colon. If using Windows-GUI or running >> OpenVPN from command line we'll need to fix this one place >> or the other. > > Command line client (git:master/5961250e776194a4, what I happened to > have lying around), run with a config file that has > > auth-user-pass > auth-nocache > auth-retry interact > static-challenge "token value: " 1 > > in it, and pressing <return> at the > > CHALLENGE: token value: _ > > prompt. I cannot reproduce this but the plugin was not able to handle an empty challenge response. The patch sent to the list should fix that. With that just pressing <return> at the challenge prompt behaves as expected (on unpack up->password gets the user input, up->response stays empty). I haven't tested this using LinOTP, but should work. Selva ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Hi, On Tue, Aug 7, 2018 at 3:01 PM Gert Doering <gert@greenie.muc.de> wrote: > > Your patch has been applied to the master branch. > > (I'm a bit undecided about release/2.4 - this is in "new feature!" land, > and all the challenge stuff is "master" territory. OTOH, it's not openvpn > main code, and the code is sane enough - so if folks think it should be > in release/2.4, tell me) > > commit 7369d01bf360bcfa02f26c05b86dde5496d120f6 (master) I failed to respond to this that time. Although this is a new feature, I too think its safe and useful to include this in 2.4. If we do, we'll need this one (commit 7369d01bf360bcfa02f26c05b86dde5496d120f6) and the followup one 7a8109023f4c345fe12f23421c5fa7e88e1ea85b Both should cherry-pick without conflicts. See also Trac #1275 https://community.openvpn.net/openvpn/ticket/1275 Thanks, Selva
Hi, On Thu, Apr 23, 2020 at 10:02:10AM -0400, Selva Nair wrote: > On Tue, Aug 7, 2018 at 3:01 PM Gert Doering <gert@greenie.muc.de> wrote: > > Your patch has been applied to the master branch. > > > > (I'm a bit undecided about release/2.4 - this is in "new feature!" land, > > and all the challenge stuff is "master" territory. OTOH, it's not openvpn > > main code, and the code is sane enough - so if folks think it should be > > in release/2.4, tell me) > > > > commit 7369d01bf360bcfa02f26c05b86dde5496d120f6 (master) > > I failed to respond to this that time. > > Although this is a new feature, I too think its safe and useful to > include this in 2.4. > If we do, we'll need this one (commit > 7369d01bf360bcfa02f26c05b86dde5496d120f6) and the followup one > 7a8109023f4c345fe12f23421c5fa7e88e1ea85b > > Both should cherry-pick without conflicts. If you tell me we should have it, I'm with you :-) (Given that this is only affecting the auth-pam plugin, and that this is a very useful addition, I think it makes sense) commit cab48ad43eaba51c54fa23e55b0b2eb436dd921f (HEAD -> release/2.4) Author: Selva Nair <selva.nair@gmail.com> Date: Tue Aug 7 22:44:31 2018 -0400 Accept empty password and/or response in auth-pam plugin (cherry picked from commit 7a8109023f4c345fe12f23421c5fa7e88e1ea85b) commit b89e48b015e581a4a0f5c306e2ab20da34c862ea Author: Selva Nair <selva.nair@gmail.com> Date: Tue Jul 24 22:34:53 2018 -0400 Parse static challenge response in auth-pam plugin (cherry picked from commit 7369d01bf360bcfa02f26c05b86dde5496d120f6) I've only done very limited testing - quick look at the changes (nothing conflicting or "funky" here), and a test build. > See also Trac #1275 https://community.openvpn.net/openvpn/ticket/1275 Noted! gert
diff --git a/src/plugins/auth-pam/README.auth-pam b/src/plugins/auth-pam/README.auth-pam index e123690..9081565 100644 --- a/src/plugins/auth-pam/README.auth-pam +++ b/src/plugins/auth-pam/README.auth-pam @@ -36,19 +36,20 @@ pairs to answer PAM module queries. For example: - plugin openvpn-auth-pam.so "login login USERNAME password PASSWORD" + plugin openvpn-auth-pam.so "login login USERNAME password PASSWORD pin OTP" tells auth-pam to (a) use the "login" PAM module, (b) answer a -"login" query with the username given by the OpenVPN client, and -(c) answer a "password" query with the password given by the -OpenVPN client. This provides flexibility in dealing with the different +"login" query with the username given by the OpenVPN client, +(c) answer a "password" query with the password, and (d) answer a +"pin" query with the OTP given by the OpenVPN client. +This provides flexibility in dealing with different types of query strings which different PAM modules might generate. For example, suppose you were using a PAM module called "test" which queried for "name" rather than "login": plugin openvpn-auth-pam.so "test name USERNAME password PASSWORD" -While "USERNAME" "COMMONNAME" and "PASSWORD" are special strings which substitute +While "USERNAME" "COMMONNAME" "PASSWORD" and "OTP" are special strings which substitute to client-supplied values, it is also possible to name literal values to use as PAM module query responses. For example, suppose that the login module queried for a third parameter, "domain" which @@ -61,6 +62,10 @@ the operation of this plugin: client-cert-not-required username-as-common-name + static-challenge + +Use of --static challenege is required to pass a pin (represented by "OTP" in +parameter substituion) or a second password. Run OpenVPN with --verb 7 or higher to get debugging output from this plugin, including the list of queries presented by the diff --git a/src/plugins/auth-pam/auth-pam.c b/src/plugins/auth-pam/auth-pam.c index 26b0eeb..e22ce5f 100644 --- a/src/plugins/auth-pam/auth-pam.c +++ b/src/plugins/auth-pam/auth-pam.c @@ -6,6 +6,7 @@ * packet compression. * * Copyright (C) 2002-2018 OpenVPN Inc <sales@openvpn.net> + * Copyright (C) 2016-2018 Selva Nair <selva.nair@gmail.com> * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 @@ -64,6 +65,7 @@ /* Pointers to functions exported from openvpn */ static plugin_secure_memzero_t plugin_secure_memzero = NULL; +static plugin_base64_decode_t plugin_base64_decode = NULL; /* * Plugin state, used by foreground @@ -87,6 +89,7 @@ struct auth_pam_context * "USERNAME" -- substitute client-supplied username * "PASSWORD" -- substitute client-specified password * "COMMONNAME" -- substitute client certificate common name + * "OTP" -- substitute static challenge response if available */ #define N_NAME_VALUE 16 @@ -111,6 +114,7 @@ struct user_pass { char username[128]; char password[128]; char common_name[128]; + char response[128]; const struct name_value_list *name_value_list; }; @@ -276,6 +280,66 @@ name_value_match(const char *query, const char *match) return strncasecmp(match, query, strlen(match)) == 0; } +/* + * Split and decode up->password in the form SCRV1:base64_pass:base64_response + * into pass and response and save in up->password and up->response. + * If the password is not in the expected format, input is not changed. + */ +static void +split_scrv1_password(struct user_pass *up) +{ + const int skip = strlen("SCRV1:"); + if (strncmp(up->password, "SCRV1:", skip) != 0) + { + return; + } + + char *tmp = strdup(up->password); + if (!tmp) + { + fprintf(stderr, "AUTH-PAM: out of memory parsing static challenge password\n"); + goto out; + } + + char *pass = tmp + skip; + char *resp = strchr(pass, ':'); + if (!resp) /* string not in SCRV1:xx:yy format */ + { + goto out; + } + *resp++ = '\0'; + + int n = plugin_base64_decode(pass, up->password, sizeof(up->password)-1); + if (n > 0) + { + up->password[n] = '\0'; + n = plugin_base64_decode(resp, up->response, sizeof(up->response)-1); + if (n > 0) + { + up->response[n] = '\0'; + if (DEBUG(up->verb)) + { + fprintf(stderr, "AUTH-PAM: BACKGROUND: parsed static challenge password\n"); + } + goto out; + } + } + + /* decode error: reinstate original value of up->password and return */ + plugin_secure_memzero(up->password, sizeof(up->password)); + plugin_secure_memzero(up->response, sizeof(up->response)); + strcpy(up->password, tmp); /* tmp is guaranteed to fit in up->password */ + + fprintf(stderr, "AUTH-PAM: base64 decode error while parsing static challenge password\n"); + +out: + if (tmp) + { + plugin_secure_memzero(tmp, strlen(tmp)); + free(tmp); + } +} + OPENVPN_EXPORT int openvpn_plugin_open_v3(const int v3structver, struct openvpn_plugin_args_open_in const *args, @@ -316,6 +380,7 @@ openvpn_plugin_open_v3(const int v3structver, /* Save global pointers to functions exported from openvpn */ plugin_secure_memzero = args->callbacks->plugin_secure_memzero; + plugin_base64_decode = args->callbacks->plugin_base64_decode; /* * Make sure we have two string arguments: the first is the .so name, @@ -599,6 +664,10 @@ my_conv(int n, const struct pam_message **msg_array, { aresp[i].resp = searchandreplace(match_value, "COMMONNAME", up->common_name); } + else if (strstr(match_value, "OTP")) + { + aresp[i].resp = searchandreplace(match_value, "OTP", up->response); + } else { aresp[i].resp = strdup(match_value); @@ -787,6 +856,9 @@ pam_server(int fd, const char *service, int verb, const struct name_value_list * #endif } + /* If password is of the form SCRV1:base64:base64 split it up */ + split_scrv1_password(&up); + if (pam_auth(service, &up)) /* Succeeded */ { if (send_control(fd, RESPONSE_VERIFY_SUCCEEDED) == -1) @@ -818,10 +890,11 @@ pam_server(int fd, const char *service, int verb, const struct name_value_list * command); goto done; } + plugin_secure_memzero(up.response, sizeof(up.response)); } done: - plugin_secure_memzero(up.password, sizeof(up.password)); + plugin_secure_memzero(up.response, sizeof(up.response)); #ifdef USE_PAM_DLOPEN dlclose_pam(); #endif