diff mbox series

[Openvpn-devel] Add common_name to the conv method. This allows the common_name to be accessible in PAM.

Message ID 1513450642-29687-1-git-send-email-Michael.Karvan@gmail.com
State Changes Requested
Headers
Series [Openvpn-devel] Add common_name to the conv method. This allows the common_name to be accessible in PAM. |

Commit Message

Michael Karvan Dec. 16, 2017, 7:57 a.m. UTC 
  ---
 src/plugins/auth-pam/auth-pam.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Comments

Selva Nair Dec. 16, 2017, 10:48 a.m. UTC | #1 
Hi,

On Sat, Dec 16, 2017 at 1:57 PM, Michael Karvan
<michael.karvan@gmail.com> wrote:
> ---
>  src/plugins/auth-pam/auth-pam.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/src/plugins/auth-pam/auth-pam.c b/src/plugins/auth-pam/auth-pam.c
> index ae514d7..c64e14b 100644
> --- a/src/plugins/auth-pam/auth-pam.c
> +++ b/src/plugins/auth-pam/auth-pam.c
> @@ -637,9 +637,16 @@ my_conv(int n, const struct pam_message **msg_array,
>                          ret = PAM_CONV_ERR;
>                      }
>                      break;
> +
> +                case PAM_TEXT_INFO:
> +                    aresp[i].resp = strdup(up->common_name);
> +                    if (aresp[i].resp == NULL)
> +                    {
> +                        ret = PAM_CONV_ERR;
> +                    }
> +                    break;

The purpose of PAM_TEXT_INFO is for the module to send an info message
to the user. Using it to send the common name back to the module is
hackish. Yes, it can work in a custom module but its not right to
interpret every
PAM_TEXT_INFO msg as a request for common name.

Why not prompt for it just like username and have the plugin return it?

Selva

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Selva Nair Jan. 20, 2018, 9:28 a.m. UTC | #2 
Hi,

On Sat, Dec 16, 2017 at 4:48 PM, Selva Nair <selva.nair@gmail.com> wrote:
> Hi,
>
> On Sat, Dec 16, 2017 at 1:57 PM, Michael Karvan
> <michael.karvan@gmail.com> wrote:
>> ---
>>  src/plugins/auth-pam/auth-pam.c | 9 ++++++++-
>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/plugins/auth-pam/auth-pam.c b/src/plugins/auth-pam/auth-pam.c
>> index ae514d7..c64e14b 100644
>> --- a/src/plugins/auth-pam/auth-pam.c
>> +++ b/src/plugins/auth-pam/auth-pam.c
>> @@ -637,9 +637,16 @@ my_conv(int n, const struct pam_message **msg_array,
>>                          ret = PAM_CONV_ERR;
>>                      }
>>                      break;
>> +
>> +                case PAM_TEXT_INFO:
>> +                    aresp[i].resp = strdup(up->common_name);
>> +                    if (aresp[i].resp == NULL)
>> +                    {
>> +                        ret = PAM_CONV_ERR;
>> +                    }
>> +                    break;
>
> The purpose of PAM_TEXT_INFO is for the module to send an info message
> to the user. Using it to send the common name back to the module is
> hackish. Yes, it can work in a custom module but its not right to
> interpret every
> PAM_TEXT_INFO msg as a request for common name.
>
> Why not prompt for it just like username and have the plugin return it?

In case I was not clear enough. NAK on the current patch. A modified one
could be considered.

Selva

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
diff mbox series

Patch

diff --git a/src/plugins/auth-pam/auth-pam.c b/src/plugins/auth-pam/auth-pam.c
index ae514d7..c64e14b 100644
--- a/src/plugins/auth-pam/auth-pam.c
+++ b/src/plugins/auth-pam/auth-pam.c
@@ -637,9 +637,16 @@  my_conv(int n, const struct pam_message **msg_array,
                         ret = PAM_CONV_ERR;
                     }
                     break;
+                
+                case PAM_TEXT_INFO:
+                    aresp[i].resp = strdup(up->common_name);
+                    if (aresp[i].resp == NULL)
+                    {
+                        ret = PAM_CONV_ERR;
+                    }
+                    break;
 
                 case PAM_ERROR_MSG:
-                case PAM_TEXT_INFO:
                     break;
 
                 default: