RFC changes to the auth-pam plugin.

PATCH Superseded Tim Small 1 participant 12 messages Jul 8, 2015 – Jul 13, 2015

Share & tools

Tim Small

author

2015-07-08 21:37 UTC #0 lore raw

cover RFC changes to the auth-pam plugin.

I wanted to use OpenVPN with PAM whilst enforcing the use of the TLS client cert common name in place of the user-supplied user name. This wouldn't work with the PAM plugin I was using, and in the process of debugging this I've made a few changes to the auth-pam plugin which I hope make it easier to use and less 'brittle' in that it should now be possible to configure customised behaviour whilst minimising the reliance on matching against strings which are only intended to be parsed by humans, not other software.

For my use case, I can now obtain the custom behaviour I wanted without having to rely on any such matching against human-readable messages generated by PAM modules (which may be subject to changes due to software updates, changes in default system modules and PAM config, as well as localisation etc.).

The changes should be backward compatible with existing deployments and I've also made some changes to the README as I felt it was a bit difficult to follow in places...

I write very little C, so please excuse me if the following are a bit rough.

Tim.

Tim Small (4):
Add explicit debug statements for pam auth and account steps. Factor out name/val list walk, add default password reply. Allow administrator to supply a user to pam_start(). Improve docs and also detail new functionality.

 src/plugins/auth-pam/README.auth-pam | 152 +++++++++++++++++++++++++++++-----
 src/plugins/auth-pam/auth-pam.c      | 154 +++++++++++++++++++++++------------
 2 files changed, 236 insertions(+), 70 deletions(-)

Signature · 1 line

2.1.4

Tim Small

author

2015-07-08 21:37 UTC #1 lore raw

PATCH 1/4 Add explicit debug statements for pam auth and account steps.

The plugin carries out separate checks on authorisation and account validity, but only prints a single "user X failed to authenticate" message, even if the PAM authenticate tests pass, but the PAM account check fails.

Also log common name as well as user name.

Signed-off-by Tim Small <[email protected]...>

---
 src/plugins/auth-pam/auth-pam.c | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

src/plugins/auth-pam/auth-pam.c +20 ~~-13~~

index bd71792..fd46b99 100644
@@ -668,16 +668,22 @@ pam_auth (const char *service, const struct user_pass *up)
       /* Call PAM to verify username/password */
       status = pam_authenticate(pamh, 0);
       if (status == PAM_SUCCESS)
-	status = pam_acct_mgmt (pamh, 0);
-      if (status == PAM_SUCCESS)
-	ret = 1;
-
-      /* Output error message if failed */
-      if (!ret)
-	{
-	  fprintf (stderr, "AUTH-PAM: BACKGROUND: user '%s' failed to authenticate: %s\n",
-		   up->username,
-		   pam_strerror (pamh, status));
+        {
+          status = pam_acct_mgmt (pamh, 0);
+          if (status == PAM_SUCCESS)
+            ret = 1; /* Passed pam authenticate and account management steps */
+	  else
+            fprintf (stderr, "AUTH-PAM: BACKGROUND: user '%s' / commonname '%s' "
+                     "passed pam credential authentication, but failed account validation: %s\n"
+                     "... check system auth log for details?\n" ,
+                     up->username, up->common_name, pam_strerror (pamh, status));
+        }
+      else
+        {
+          fprintf (stderr, "AUTH-PAM: BACKGROUND: user '%s' / commonname '%s' "
+                   "failed to authenticate: %s\n"
+		   "... check system auth log for details?\n" ,
+		   up->username, up->common_name, pam_strerror (pamh, status));
 	}
 
       /* Close PAM */

@@ -756,10 +762,11 @@ pam_server (int fd, const char *service, int verb, const struct name_value_list
 	  if (DEBUG (verb))
 	    {
 #if 0
-	      fprintf (stderr, "AUTH-PAM: BACKGROUND: USER/PASS: %s/%s\n",
-		       up.username, up.password);
+	      fprintf (stderr, "AUTH-PAM: BACKGROUND: USER/COMMON_NAME/PASS: %s/%s/%s\n",
+		       up.username, up.common_name, up.password);
 #else
-	      fprintf (stderr, "AUTH-PAM: BACKGROUND: USER: %s\n", up.username);
+	      fprintf (stderr, "AUTH-PAM: BACKGROUND: USER/COMMON_NAME: %s/%s\n",
+		       up.username, up.common_name);
 #endif
 	    }

Signature · 1 line

2.1.4

Tim Small

author

2015-07-08 21:37 UTC #2 lore raw

PATCH 2/4 Factor out name/val list walk, add default password reply.

In reply to Tim Small

If the user doesn't supply any arguments to the plugin in the OpenVPN configuration file, then it defaults to answering any PAM conversation 'questions' where the pam module sets the message style to PAM_PROMPT_ECHO_OFF with the password, and where the style is set to PAM_PROMPT_ECHO_ON with the username.

If the administrator supplies one or more arguments to the auth-pam plugin, then this simple but robust default behaviour is abandoned, and the user must supply a match for the message text content (NOT the message style) for ALL messages. Non-matching messages are answered with a NULL.

This patch restores the default behaviour for PAM_PROMPT_ECHO_OFF style questions, and returns the password if no prior match has been found from the explicit list which the administrator specified. The aim is to make the default behaviour less surprising for the user, and also to make deployments less likely to break from changes in what are indended to be human-readable messages (and which may change due to e.g. localisation coverage improvements).

Signed-off-by Tim Small <[email protected]...>

---
 src/plugins/auth-pam/auth-pam.c | 107 ++++++++++++++++++++++++++--------------
 1 file changed, 71 insertions(+), 36 deletions(-)

src/plugins/auth-pam/auth-pam.c +71 ~~-36~~

index fd46b99..e0dc029 100644
@@ -538,6 +538,53 @@ openvpn_plugin_abort_v1 (openvpn_plugin_handle_t handle)
 }
 
 /*
+ * Key value pair list search with USERNAME/COMMONNAME/PASSWORD
+ * substitution.  Callee must arrange to free() the response.
+ */
+
+int
+get_value_with_subst (struct user_pass const *up, const char * const search_term, char **response)
+{
+  int ret = PAM_CONV_ERR;
+  int j;
+  const struct name_value_list *list = up->name_value_list;
+  /* loop through name/value pairs */
+  for (j = 0; j < list->len; ++j)
+    {
+      const char *item_key = list->data[j].name;
+      const char *item_value = list->data[j].value;
+
+      if (name_value_match (search_term, item_key))
+	{
+	  /* found name/value match */
+	  *response = NULL;
+
+	  if (DEBUG (up->verb))
+	    fprintf (stderr, "AUTH-PAM: BACKGROUND: name match found, query/match-string ['%s', '%s'] = '%s'\n",
+		     search_term,
+		     item_key,
+		     item_value);
+
+	  if (strstr(item_value, "USERNAME"))
+	    *response = searchandreplace(item_value, "USERNAME", up->username);
+	  else if (strstr(item_value, "PASSWORD"))
+	    *response = searchandreplace(item_value, "PASSWORD", up->password);
+	  else if (strstr(item_value, "COMMONNAME"))
+	    *response = searchandreplace(item_value, "COMMONNAME", up->common_name);
+	  else
+	    *response = strdup (item_value);
+
+	  if (*response != NULL)
+	    ret = PAM_SUCCESS;
+	  break;
+	}
+    }
+  return ret;
+}
+
+
+
+/*
  * PAM conversation function
  */
 static int

@@ -574,47 +621,35 @@ my_conv (int n, const struct pam_message **msg_array,
       if (up->name_value_list && up->name_value_list->len > 0)
 	{
 	  /* use name/value list match method */
-	  const struct name_value_list *list = up->name_value_list;
-	  int j;
-
-	  /* loop through name/value pairs */
-	  for (j = 0; j < list->len; ++j)
-	    {
-	      const char *match_name = list->data[j].name;
-	      const char *match_value = list->data[j].value;
-
-	      if (name_value_match (msg->msg, match_name))
-		{
-		  /* found name/value match */
-		  aresp[i].resp = NULL;
-
-		  if (DEBUG (up->verb))
-		    fprintf (stderr, "AUTH-PAM: BACKGROUND: name match found, query/match-string ['%s', '%s'] = '%s'\n",
-			     msg->msg,
-			     match_name,
-			     match_value);
-
-		  if (strstr(match_value, "USERNAME"))
-		    aresp[i].resp = searchandreplace(match_value, "USERNAME", up->username);
-		  else if (strstr(match_value, "PASSWORD"))
-		    aresp[i].resp = searchandreplace(match_value, "PASSWORD", up->password);
-		  else if (strstr(match_value, "COMMONNAME"))
-		    aresp[i].resp = searchandreplace(match_value, "COMMONNAME", up->common_name);
-		  else
-		    aresp[i].resp = strdup (match_value);
-
-		  if (aresp[i].resp == NULL)
-		    ret = PAM_CONV_ERR;
-		  break;
+	  ret = get_value_with_subst (up, msg->msg, &(aresp[i].resp));
+	  if (ret == PAM_CONV_ERR)
+            {
+	      /*
+	       * No match for requested message, but the pam module asked
+	       * that the user-entered-data for this response shouldn't be
+	       * displayed, so we will provide the password.  If the user
+	       * doesn't want this behaviour they can add an explicit
+	       * match for this pam message, and return something else.
+	       */
+	      if (msg->msg_style == PAM_PROMPT_ECHO_OFF)
+	        {
+                  if (DEBUG (up->verb))
+                    fprintf (stderr, "AUTH-PAM: BACKGROUND: Got non-matching request for "
+			     "'%s' with style set to PAM_PROMPT_ECHO_OFF - "
+			     "sending password...\n", msg->msg);
+                  aresp[i].resp = strdup (up->password);
+		  if (aresp[i].resp != NULL)
+		    ret = PAM_SUCCESS;
+		} else {
+                  if (DEBUG (up->verb))
+	            fprintf (stderr, "AUTH-PAM: BACKGROUND: Failed to find a match for "
+			     "'%s' which was requested by pam module.\n", msg->msg);
 		}
 	    }
-
-	  if (j == list->len)
-	    ret = PAM_CONV_ERR;
 	}
       else
 	{
-	  /* use PAM_PROMPT_ECHO_x hints */
+	  /* no name/value list provided, rely on PAM_PROMPT_ECHO_x hints */
 	  switch (msg->msg_style)
 	    {
 	    case PAM_PROMPT_ECHO_OFF:

Signature · 1 line

2.1.4

Tim Small

author

2015-07-08 21:37 UTC #3 lore raw

PATCH 3/4 Allow administrator to supply a user to pam_start().

In reply to Tim Small

If the user doesn't supply any arguments to the plugin in the OpenVPN configuration file, then it defaults to setting the second argument to the pam_start() function with the username that the other end of the vpn supplied. The pam libraries use this as the initial value for the PAM_USER pam 'item'.

If the administrator supplies one or more arguments to the auth-pam plugin, then this default behaviour is abandoned, and the user argument is set to NULL. The administrator must then supply a match for the message text content which the pam module will (hopefully) supply in order to try to discover the message (some pam modules will just give up at this stage, and fail the authentication e.g. Google Authenticator will fail if PAM_USER is set to NULL).

The message text which the pam module supplies is intended to be read by a human, not parsed by other code (as is the current behaviour of the plugin), and as such may be subject to change (e.g. because of localisation changes etc.).

This patch allows the user to supply a value for the second argument of pam_start, if they have supplied any arguments (by using the special value of 'PAM_USER' as the question message). This allows operation with inflexible pam modules, and also decreases the likelihood that system configuration changes (e.g. localisation changes, package updates etc.) will break the OpenVPN setup by causing existing matches to fail.

Signed-off-by Tim Small <[email protected]...>

---
 src/plugins/auth-pam/auth-pam.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

src/plugins/auth-pam/auth-pam.c +12 -2

index e0dc029..b14cf06 100644
@@ -693,11 +693,21 @@ pam_auth (const char *service, const struct user_pass *up)
   int status = PAM_SUCCESS;
   int ret = 0;
   const int name_value_list_provided = (up->name_value_list && up->name_value_list->len > 0);
-
+  char *pam_user = NULL;
+  if (name_value_list_provided)
+    {
+      ret = get_value_with_subst (up, "PAM_USER", &pam_user);
+      if (ret == PAM_SUCCESS)
+	fprintf (stderr, "Passing user '%s' in pam_start\n", pam_user);
+      else
+	fprintf (stderr, "User didn't specify value for user argument to pam_start()\n");
+    }
   /* Initialize PAM */
   conv.conv = my_conv;
   conv.appdata_ptr = (void *)up;
-  status = pam_start (service, name_value_list_provided ? NULL : up->username, &conv, &pamh);
+  status = pam_start (service, name_value_list_provided ? pam_user : up->username, &conv, &pamh);
+  if (pam_user)
+    free(pam_user);
   if (status == PAM_SUCCESS)
     {
       /* Call PAM to verify username/password */

Signature · 1 line

2.1.4

Tim Small

author

2015-07-08 21:37 UTC #4 lore raw

PATCH 4/4 Improve docs and also detail new functionality.

In reply to Tim Small

Hopefully clarify usage, and fix spelling errors.

Document new functionality WRT PAM_USER and implicit password responses when a non-matching non-echoing PROMPT is made by a PAM module.

Signed-off-by Tim Small <[email protected]...>

---
 src/plugins/auth-pam/README.auth-pam | 152 ++++++++++++++++++++++++++++++-----
 1 file changed, 133 insertions(+), 19 deletions(-)

src/plugins/auth-pam/README.auth-pam +133 ~~-19~~

index e123690..5f55857 100644
@@ -7,7 +7,7 @@ authentication via PAM, and essentially allows any authentication
 method supported by PAM (such as LDAP, RADIUS, or Linux Shadow
 passwords) to be used with OpenVPN.  While PAM supports
 username/password authentication, this can be combined with X509
-certificates to provide two indepedent levels of authentication.
+certificates to provide two independent levels of authentication.
 
 This module uses a split privilege execution model which will
 function even if you drop openvpn daemon privileges using the user,

@@ -31,32 +31,133 @@ The required service-type parameter corresponds to
 the PAM service definition file usually found
 in /etc/pam.d.
 
+The plugin first uses PAM to authenticate the user, if this succeeds it
+then verifies that the users' account is valid via PAM as well.  Only
+if both of these steps succeeds does the plugin approve the OpenVPN
+authentication attempt.
+
+In PAM terminology, the openvpn-plugin-auth-pam.so acts as a PAM
+"application" - that is to say it is an OpenVPN plugin and it makes use
+of the PAM API to authenticate users.  openvpn-plugin-auth-pam.so is
+NOT a PAM module itself.
+
+Similarly it is also NOT possible to merely use the OpenVPN plugin
+syntax to directly specify the use of a particular PAM module in
+isolation...  It IS however possible to arrange for such use of a
+particular PAM module (if desired without changing the behaviour of
+other applications which make use of PAM) by the usual means i.e.
+creating a custom PAM service definition.
+
+
+DEFAULT BEHAVIOUR
+
+By default the plugin will supply the username to the PAM modules
+(by setting the 'user' field of the pam_start() function).  It
+will also attempt to answer 'questions' which individual PAM modules
+ask using the PAM 'conversation' mechanism.
+
+The content of the conversation "message" - see pam_conv(3) - is
+ignored, but each conversation call which has its style set to
+'PAM_PROMPT_ECHO_ON' (i.e. those where the user's response isn't
+intended to be hidden on the devices display etc.) are answered
+with the username.  Calls with the style set to 'PAM_PROMPT_ECHO_OFF'
+are (i.e. the user's response should be hidden) are answered with
+the password.
+
+
+CUSTOM BEHAVIOUR
+
 This plugin also supports the usage of a list of name/value
-pairs to answer PAM module queries.
+pairs to answer PAM module queries using the following syntax:
+
+  plugin openvpn-auth-pam.so "PAMSERVICE [PROMPT ANSWER]..."
+
+These PAM queries take the form of text prompts which individual
+PAM modules supply.
+
+OpenVPN currently lacks a mechanism to supply these prompts (or related
+error messages etc.) to the user, but instead the plugin can be
+configured to respond to them on the users' behalf.
+
+If a PAM module prompts for a reply and supplies a message which matches
+the text PROMPT, then the plugin will reply with ANSWER.
+
+The match is case insensitive and the match will succeed if the PAM
+module's message includes trailing characters e.g. The PROMPT 'Log' will
+match the PAM message 'login:'.
+
+As a special case, if a PROMPT value of 'PAM_USER' is given, then the
+corresponding ANSWER value will be supplied to the PAM service via
+the second argument of the pam_start() function.  When this is supplied
+to a PAM module, they do not need to request the user name of the user
+via the pam_conv(3) mechanism.  This is the recommended way of
+supplying the user name to a PAM service.
+
+If no match is found for a prompt message, but the message style is
+set to 'PAM_PROMPT_ECHO_OFF', then the plugin will supply the password
+given by the OpenVPN client.
+
+
+Example 1:
 
-For example:
+  plugin openvpn-auth-pam.so "login PAM_USER COMMONNAME"
 
-  plugin openvpn-auth-pam.so "login login USERNAME password PASSWORD"
+tells the plugin to (a) use the "login" PAM service and (b) supply
+the commonname (as supplied to the plugin by OpenVPN) to the PAM
+service as the user name.  Most PAM services will request a secret in
+order to authorise the user, and as there are no further explicit
+PROMPT values specified by the example plugin definition, then any
+such password style (non-echoing input) requests made by the PAM
+modules which make up the PAM service definition will be automatically
+answered using the password given which was supplied by the OpenVPN
+client.
 
-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
-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":
+n.b. Typically this would be used to enforce a 1:1 mapping between
+users' common names as defined by their TLS client certificate, and
+their system login account.  The username supplied by the end-user will
+not be passed to PAM in this example, and will effectively be ignored
+in favour of this client certificate common name.
+
+
+Example 2:
+
+  plugin openvpn-auth-pam.so "openvpn login USERNAME password PASSWORD"
+
+tells the plugin to (a) use the "openvpn" PAM service, (b) answer any
+PAM "conversation" call which has a message which begins 'login' with
+the username given by the OpenVPN client, and (c) answer a PAM prompt
+which begins with the text 'password' with the password given by the
+OpenVPN client.  This provides some flexibility in dealing with the
+different types of query strings which different PAM modules might
+generate.
+
+n.b. An "openvpn" PAM service definition must exist on the system in
+order to use the above declaration successfully.
+
+
+Example 3:
+
+Suppose you were using a PAM service called "test".  One of the
+modules defined in this service prompts with the message "Name Please:"
+rather than "Login :" - you could use:
 
   plugin openvpn-auth-pam.so "test name USERNAME password PASSWORD"
 
-While "USERNAME" "COMMONNAME" and "PASSWORD" 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
-is to be answered with the constant value "mydomain.com":
 
-  plugin openvpn-auth-pam.so "login login USERNAME password PASSWORD domain mydomain.com"
+Example 4:
+
+While "USERNAME" "COMMONNAME" and "PASSWORD" 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 prompted for a "domain" which should be
+answered with the constant value "mydomain.com":
 
-The following OpenVPN directives can also influence
+  plugin openvpn-auth-pam.so "test login USERNAME password PASSWORD domain mydomain.com"
+
+
+ENVIRONMENT
+
+The following OpenVPN configuration directives can also influence
 the operation of this plugin:
 
   client-cert-not-required

@@ -64,11 +165,24 @@ the operation of this plugin:
 
 Run OpenVPN with --verb 7 or higher to get debugging output from
 this plugin, including the list of queries presented by the
-underlying PAM module.  This is a useful debugging tool to figure
+underlying PAM modules.  This is a useful debugging tool to figure
 out which queries a given PAM module is making, so that you can
 craft the appropriate plugin directive to answer it.
 
+
 CAVEATS
 
 This module will only work on *nix systems which support PAM,
 not Windows.
+
+
+DEBUGGING
+
+Debugging information from PAM services may be logged using different
+syslog facilities than OpenVPN, and so may end up in different
+locations (e.g. /var/log/auth.log etc. depending on system
+configuration).
+
+The administrator is advised to consult the many PAM tutorials and
+'howto' documents in order to learn about the debugging on PAM, and
+the creation and testing of custom PAM service definitions etc.

Signature · 1 line

2.1.4

Tim Small

author

2015-07-13 15:57 UTC #5 lore raw

v2 cover RFC changes to the auth-pam plugin.

In reply to Tim Small

Take 2 - sorry for the noise - this update to the series splits some patches down further, fixes a security bug vs the first iteration, and hopefully makes various improvements to patches.

The changes should be backward compatible with existing deployments and I've also made some changes to the README as I felt it was a bit difficult to follow in places...

I write very little C, so please excuse me if the following are a bit rough.

Tim.

Tim Small (6):
Separate error logging for pam auth and account Log common name as well as username for pam auth Refactor name/value list search and substitution code. Add default password reply with name_value_list Allow administrator to supply a user to pam_start Improve docs and detail new functionality

 src/plugins/auth-pam/README.auth-pam | 152 +++++++++++++++++++++++++----
 src/plugins/auth-pam/auth-pam.c      | 182 ++++++++++++++++++++++++-----------
 2 files changed, 259 insertions(+), 75 deletions(-)

Signature · 1 line

2.1.4

Tim Small

author

2015-07-13 15:57 UTC #6 lore raw

PATCH v2 1/6 Separate error logging for pam auth and account

Print separate error messages if failure occurs in either step.

Signed-off-by Tim Small <[email protected]...>

---
 src/plugins/auth-pam/auth-pam.c | 42 ++++++++++++++++++++++++++---------------
 1 file changed, 27 insertions(+), 15 deletions(-)

src/plugins/auth-pam/auth-pam.c +27 ~~-15~~

index bd71792..1fa73c1 100644
@@ -655,7 +655,7 @@ pam_auth (const char *service, const struct user_pass *up)
 {
   struct pam_conv conv;
   pam_handle_t *pamh = NULL;
-  int status = PAM_SUCCESS;
+  int status;
   int ret = 0;
   const int name_value_list_provided = (up->name_value_list && up->name_value_list->len > 0);

@@ -663,23 +663,35 @@ pam_auth (const char *service, const struct user_pass *up)
   conv.conv = my_conv;
   conv.appdata_ptr = (void *)up;
   status = pam_start (service, name_value_list_provided ? NULL : up->username, &conv, &pamh);
-  if (status == PAM_SUCCESS)
+  if (status != PAM_SUCCESS)
     {
-      /* Call PAM to verify username/password */
+      fprintf (stderr, "AUTH-PAM: BACKGROUND: user '%s' / commonname '%s' "
+          "call to pam_start() didn't return PAM_SUCCESS\n",
+          up->username, up->common_name);
+    }
+  else
+    {
+      /* Call PAM to authenticate the username */
       status = pam_authenticate(pamh, 0);
-      if (status == PAM_SUCCESS)
-	status = pam_acct_mgmt (pamh, 0);
-      if (status == PAM_SUCCESS)
-	ret = 1;
-
-      /* Output error message if failed */
-      if (!ret)
-	{
-	  fprintf (stderr, "AUTH-PAM: BACKGROUND: user '%s' failed to authenticate: %s\n",
-		   up->username,
-		   pam_strerror (pamh, status));
+      if (status != PAM_SUCCESS)
+        {
+          fprintf (stderr, "AUTH-PAM: BACKGROUND: user '%s' / commonname '%s' "
+                   "failed to authenticate: %s\n"
+                   "... check system auth log for details.\n",
+                   up->username, up->common_name, pam_strerror (pamh, status));
 	}
-
+      else
+        {
+          /* Call PAM to check user's account is valid */
+          status = pam_acct_mgmt (pamh, 0);
+          if (status != PAM_SUCCESS)
+            fprintf (stderr, "AUTH-PAM: BACKGROUND: user '%s' / commonname '%s' "
+                     "passed pam credential authentication, but failed account validation: %s\n"
+                     "... check system auth log for details.\n",
+                     up->username, up->common_name, pam_strerror (pamh, status));
+          else
+            ret = 1; /* Passed pam authenticate and account management steps */
+        }
       /* Close PAM */
       pam_end (pamh, status);      
     }

Signature · 1 line

2.1.4

Tim Small

author

2015-07-13 15:57 UTC #7 lore raw

PATCH v2 2/6 Log common name as well as username for pam auth

In reply to Tim Small

Improve debugability by logging the common name as well as the user name.

Signed-off-by Tim Small <[email protected]...>

---
 src/plugins/auth-pam/auth-pam.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

src/plugins/auth-pam/auth-pam.c +4 -3

index 1fa73c1..d6fcda9 100644
@@ -768,10 +768,11 @@ pam_server (int fd, const char *service, int verb, const struct name_value_list
 	  if (DEBUG (verb))
 	    {
 #if 0
-	      fprintf (stderr, "AUTH-PAM: BACKGROUND: USER/PASS: %s/%s\n",
-		       up.username, up.password);
+	      fprintf (stderr, "AUTH-PAM: BACKGROUND: USER/COMMON_NAME/PASS: %s/%s/%s\n",
+		       up.username, up.common_name, up.password);
 #else
-	      fprintf (stderr, "AUTH-PAM: BACKGROUND: USER: %s\n", up.username);
+	      fprintf (stderr, "AUTH-PAM: BACKGROUND: USER/COMMON_NAME: %s/%s\n",
+		       up.username, up.common_name);
 #endif
 	    }

Signature · 1 line

2.1.4

Tim Small

author

2015-07-13 15:57 UTC #8 lore raw

PATCH v2 3/6 Refactor name/value list search and substitution code.

In reply to Tim Small

Pull out the code which searches the name_value_list for a matching prompt, and also substitutes keywords (USERNAME etc.) into a separate function, for clarity and to support forthcoming changes.

Signed-off-by Tim Small <[email protected]...>

---
 src/plugins/auth-pam/auth-pam.c | 96 +++++++++++++++++++++++++----------------
 1 file changed, 59 insertions(+), 37 deletions(-)

src/plugins/auth-pam/auth-pam.c +59 ~~-37~~

index d6fcda9..722d0cf 100644
@@ -538,6 +538,58 @@ openvpn_plugin_abort_v1 (openvpn_plugin_handle_t handle)
 }
 
 /*
+ * Key value pair list search with USERNAME/COMMONNAME/PASSWORD
+ * substitution.
+ *
+ * Returns 1 if match (with/without substitution) and the first
+ * match (substituted if necessary) in the list is returned in
+ * response. The callee must arrange to free() the response.
+ *
+ * Returns 0 if no match for search_term found in the user_pass
+ * name_value_list, in this case response is left unmodified.
+ */
+
+int
+get_value_with_subst (struct user_pass const *up, const char * const search_term, char **response)
+{
+  int ret = 0;
+  int j;
+  const struct name_value_list *list = up->name_value_list;
+  /* loop through name/value pairs */
+  for (j = 0; j < list->len; ++j)
+    {
+      const char *item_key = list->data[j].name;
+      const char *item_value = list->data[j].value;
+
+      if (name_value_match (search_term, item_key))
+	{
+	  /* found name/value match */
+	  *response = NULL;
+
+	  if (DEBUG (up->verb))
+	    fprintf (stderr, "AUTH-PAM: BACKGROUND: name match found, query/match-string ['%s', '%s'] = '%s'\n",
+		     search_term,
+		     item_key,
+		     item_value);
+
+	  if (strstr(item_value, "USERNAME"))
+	    *response = searchandreplace(item_value, "USERNAME", up->username);
+	  else if (strstr(item_value, "PASSWORD"))
+	    *response = searchandreplace(item_value, "PASSWORD", up->password);
+	  else if (strstr(item_value, "COMMONNAME"))
+	    *response = searchandreplace(item_value, "COMMONNAME", up->common_name);
+	  else
+	    *response = strdup (item_value);
+
+	  if (*response != NULL)
+	    ret = 1;
+	  break;
+	}
+    }
+  return ret;
+}
+
+/*
  * PAM conversation function
  */
 static int

@@ -574,47 +626,17 @@ my_conv (int n, const struct pam_message **msg_array,
       if (up->name_value_list && up->name_value_list->len > 0)
 	{
 	  /* use name/value list match method */
-	  const struct name_value_list *list = up->name_value_list;
-	  int j;
-
-	  /* loop through name/value pairs */
-	  for (j = 0; j < list->len; ++j)
-	    {
-	      const char *match_name = list->data[j].name;
-	      const char *match_value = list->data[j].value;
-
-	      if (name_value_match (msg->msg, match_name))
-		{
-		  /* found name/value match */
-		  aresp[i].resp = NULL;
-
-		  if (DEBUG (up->verb))
-		    fprintf (stderr, "AUTH-PAM: BACKGROUND: name match found, query/match-string ['%s', '%s'] = '%s'\n",
-			     msg->msg,
-			     match_name,
-			     match_value);
-
-		  if (strstr(match_value, "USERNAME"))
-		    aresp[i].resp = searchandreplace(match_value, "USERNAME", up->username);
-		  else if (strstr(match_value, "PASSWORD"))
-		    aresp[i].resp = searchandreplace(match_value, "PASSWORD", up->password);
-		  else if (strstr(match_value, "COMMONNAME"))
-		    aresp[i].resp = searchandreplace(match_value, "COMMONNAME", up->common_name);
-		  else
-		    aresp[i].resp = strdup (match_value);
-
-		  if (aresp[i].resp == NULL)
-		    ret = PAM_CONV_ERR;
-		  break;
-		}
+          if (!get_value_with_subst (up, msg->msg, &(aresp[i].resp)))
+            {
+              ret = PAM_CONV_ERR;
+              if (DEBUG (up->verb))
+                fprintf (stderr, "AUTH-PAM: BACKGROUND: Didn't find a match for prompt "
+                             "'%s' which was requested by pam module.\n", msg->msg);
 	    }
-
-	  if (j == list->len)
-	    ret = PAM_CONV_ERR;
 	}
       else
 	{
-	  /* use PAM_PROMPT_ECHO_x hints */
+          /* no name/value list provided, rely on PAM_PROMPT_ECHO_x hints alone */
 	  switch (msg->msg_style)
 	    {
 	    case PAM_PROMPT_ECHO_OFF:

Signature · 1 line

2.1.4

Tim Small

author

2015-07-13 15:57 UTC #9 lore raw

PATCH v2 4/6 Add default password reply with name_value_list

In reply to Tim Small

This patch returns the password for PAM_PROMPT_ECHO_OFF style questions, if no prior match has been found from the explicit list which the administrator specified. The aim is to make the default behaviour less surprising for the user, and also to make deployments less likely to break from changes in what are indended to be human-readable messages (and which may change due to e.g. localisation coverage improvements).

Signed-off-by Tim Small <[email protected]...>

---
 src/plugins/auth-pam/auth-pam.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

src/plugins/auth-pam/auth-pam.c +20 -1

index 722d0cf..48279cd 100644
@@ -628,10 +628,29 @@ my_conv (int n, const struct pam_message **msg_array,
 	  /* use name/value list match method */
           if (!get_value_with_subst (up, msg->msg, &(aresp[i].resp)))
             {
-              ret = PAM_CONV_ERR;
               if (DEBUG (up->verb))
                 fprintf (stderr, "AUTH-PAM: BACKGROUND: Didn't find a match for prompt "
                              "'%s' which was requested by pam module.\n", msg->msg);
+              /*
+               * No match, fall back to responding with the password if the caller is
+               * prompting for a value, and has requested that the user's answer
+               * should be displayed.  Won't happen if the system defines an explicit
+               * match for the pam message in question.
+               */
+              if (msg->msg_style != PAM_PROMPT_ECHO_OFF)
+                {
+                  ret = PAM_CONV_ERR;
+                }
+              else
+                {
+                  aresp[i].resp = strdup (up->password);
+                  if (aresp[i].resp == NULL)
+                    ret = PAM_CONV_ERR;
+                  if (DEBUG (up->verb))
+                    fprintf (stderr, "AUTH-PAM: BACKGROUND: ...but the prompt style "
+                             "was set to PAM_PROMPT_ECHO_OFF - so responding with "
+                             "password.\n");
+                }
 	    }
 	}
       else

Signature · 1 line

2.1.4

Tim Small

author

2015-07-13 15:57 UTC #10 lore raw

PATCH v2 5/6 Allow administrator to supply a user to pam_start

In reply to Tim Small

This patch allows the user to supply a value for the second argument of pam_start, if they have supplied any arguments (by using the special value of 'PAM_USER' as the question message). This allows operation with inflexible pam modules, and also decreases the likelihood that other system changes (e.g. localisation changes, package updates etc.) will break an existing OpenVPN setup.

Signed-off-by Tim Small <[email protected]...>

---
 src/plugins/auth-pam/auth-pam.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

src/plugins/auth-pam/auth-pam.c +17 -1

index 48279cd..2d0f4d0 100644
@@ -699,11 +699,27 @@ pam_auth (const char *service, const struct user_pass *up)
   int status;
   int ret = 0;
   const int name_value_list_provided = (up->name_value_list && up->name_value_list->len > 0);
+  char *pam_user = NULL;
+  if (name_value_list_provided)
+    {
+      if (get_value_with_subst (up, "PAM_USER", &pam_user))
+        {
+          if (DEBUG (up->verb))
+            fprintf (stderr, "Passing user '%s' in pam_start\n", pam_user);
+        }
+      else
+        {
+          if (DEBUG (up->verb))
+            fprintf (stderr, "User didn't specify value for user argument to pam_start()\n");
+        }
+    }
 
   /* Initialize PAM */
   conv.conv = my_conv;
   conv.appdata_ptr = (void *)up;
-  status = pam_start (service, name_value_list_provided ? NULL : up->username, &conv, &pamh);
+  status = pam_start (service, name_value_list_provided ? pam_user : up->username, &conv, &pamh);
+  if (pam_user)
+    free(pam_user);
   if (status != PAM_SUCCESS)
     {
       fprintf (stderr, "AUTH-PAM: BACKGROUND: user '%s' / commonname '%s' "

Signature · 1 line

2.1.4

Tim Small

author

2015-07-13 15:57 UTC #11 lore raw

PATCH v2 6/6 Improve docs and detail new functionality

In reply to Tim Small

Hopefully clarify usage, and fix spelling errors.

Document new functionality WRT PAM_USER and implicit password responses when a non-matching non-echoing PROMPT is made by a PAM module.

Signed-off-by Tim Small <[email protected]...>

---
 src/plugins/auth-pam/README.auth-pam | 152 ++++++++++++++++++++++++++++++-----
 1 file changed, 133 insertions(+), 19 deletions(-)

src/plugins/auth-pam/README.auth-pam +133 ~~-19~~

index e123690..5f55857 100644
@@ -7,7 +7,7 @@ authentication via PAM, and essentially allows any authentication
 method supported by PAM (such as LDAP, RADIUS, or Linux Shadow
 passwords) to be used with OpenVPN.  While PAM supports
 username/password authentication, this can be combined with X509
-certificates to provide two indepedent levels of authentication.
+certificates to provide two independent levels of authentication.
 
 This module uses a split privilege execution model which will
 function even if you drop openvpn daemon privileges using the user,

@@ -31,32 +31,133 @@ The required service-type parameter corresponds to
 the PAM service definition file usually found
 in /etc/pam.d.
 
+The plugin first uses PAM to authenticate the user, if this succeeds it
+then verifies that the users' account is valid via PAM as well.  Only
+if both of these steps succeeds does the plugin approve the OpenVPN
+authentication attempt.
+
+In PAM terminology, the openvpn-plugin-auth-pam.so acts as a PAM
+"application" - that is to say it is an OpenVPN plugin and it makes use
+of the PAM API to authenticate users.  openvpn-plugin-auth-pam.so is
+NOT a PAM module itself.
+
+Similarly it is also NOT possible to merely use the OpenVPN plugin
+syntax to directly specify the use of a particular PAM module in
+isolation...  It IS however possible to arrange for such use of a
+particular PAM module (if desired without changing the behaviour of
+other applications which make use of PAM) by the usual means i.e.
+creating a custom PAM service definition.
+
+
+DEFAULT BEHAVIOUR
+
+By default the plugin will supply the username to the PAM modules
+(by setting the 'user' field of the pam_start() function).  It
+will also attempt to answer 'questions' which individual PAM modules
+ask using the PAM 'conversation' mechanism.
+
+The content of the conversation "message" - see pam_conv(3) - is
+ignored, but each conversation call which has its style set to
+'PAM_PROMPT_ECHO_ON' (i.e. those where the user's response isn't
+intended to be hidden on the devices display etc.) are answered
+with the username.  Calls with the style set to 'PAM_PROMPT_ECHO_OFF'
+are (i.e. the user's response should be hidden) are answered with
+the password.
+
+
+CUSTOM BEHAVIOUR
+
 This plugin also supports the usage of a list of name/value
-pairs to answer PAM module queries.
+pairs to answer PAM module queries using the following syntax:
+
+  plugin openvpn-auth-pam.so "PAMSERVICE [PROMPT ANSWER]..."
+
+These PAM queries take the form of text prompts which individual
+PAM modules supply.
+
+OpenVPN currently lacks a mechanism to supply these prompts (or related
+error messages etc.) to the user, but instead the plugin can be
+configured to respond to them on the users' behalf.
+
+If a PAM module prompts for a reply and supplies a message which matches
+the text PROMPT, then the plugin will reply with ANSWER.
+
+The match is case insensitive and the match will succeed if the PAM
+module's message includes trailing characters e.g. The PROMPT 'Log' will
+match the PAM message 'login:'.
+
+As a special case, if a PROMPT value of 'PAM_USER' is given, then the
+corresponding ANSWER value will be supplied to the PAM service via
+the second argument of the pam_start() function.  When this is supplied
+to a PAM module, they do not need to request the user name of the user
+via the pam_conv(3) mechanism.  This is the recommended way of
+supplying the user name to a PAM service.
+
+If no match is found for a prompt message, but the message style is
+set to 'PAM_PROMPT_ECHO_OFF', then the plugin will supply the password
+given by the OpenVPN client.
+
+
+Example 1:
 
-For example:
+  plugin openvpn-auth-pam.so "login PAM_USER COMMONNAME"
 
-  plugin openvpn-auth-pam.so "login login USERNAME password PASSWORD"
+tells the plugin to (a) use the "login" PAM service and (b) supply
+the commonname (as supplied to the plugin by OpenVPN) to the PAM
+service as the user name.  Most PAM services will request a secret in
+order to authorise the user, and as there are no further explicit
+PROMPT values specified by the example plugin definition, then any
+such password style (non-echoing input) requests made by the PAM
+modules which make up the PAM service definition will be automatically
+answered using the password given which was supplied by the OpenVPN
+client.
 
-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
-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":
+n.b. Typically this would be used to enforce a 1:1 mapping between
+users' common names as defined by their TLS client certificate, and
+their system login account.  The username supplied by the end-user will
+not be passed to PAM in this example, and will effectively be ignored
+in favour of this client certificate common name.
+
+
+Example 2:
+
+  plugin openvpn-auth-pam.so "openvpn login USERNAME password PASSWORD"
+
+tells the plugin to (a) use the "openvpn" PAM service, (b) answer any
+PAM "conversation" call which has a message which begins 'login' with
+the username given by the OpenVPN client, and (c) answer a PAM prompt
+which begins with the text 'password' with the password given by the
+OpenVPN client.  This provides some flexibility in dealing with the
+different types of query strings which different PAM modules might
+generate.
+
+n.b. An "openvpn" PAM service definition must exist on the system in
+order to use the above declaration successfully.
+
+
+Example 3:
+
+Suppose you were using a PAM service called "test".  One of the
+modules defined in this service prompts with the message "Name Please:"
+rather than "Login :" - you could use:
 
   plugin openvpn-auth-pam.so "test name USERNAME password PASSWORD"
 
-While "USERNAME" "COMMONNAME" and "PASSWORD" 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
-is to be answered with the constant value "mydomain.com":
 
-  plugin openvpn-auth-pam.so "login login USERNAME password PASSWORD domain mydomain.com"
+Example 4:
+
+While "USERNAME" "COMMONNAME" and "PASSWORD" 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 prompted for a "domain" which should be
+answered with the constant value "mydomain.com":
 
-The following OpenVPN directives can also influence
+  plugin openvpn-auth-pam.so "test login USERNAME password PASSWORD domain mydomain.com"
+
+
+ENVIRONMENT
+
+The following OpenVPN configuration directives can also influence
 the operation of this plugin:
 
   client-cert-not-required

@@ -64,11 +165,24 @@ the operation of this plugin:
 
 Run OpenVPN with --verb 7 or higher to get debugging output from
 this plugin, including the list of queries presented by the
-underlying PAM module.  This is a useful debugging tool to figure
+underlying PAM modules.  This is a useful debugging tool to figure
 out which queries a given PAM module is making, so that you can
 craft the appropriate plugin directive to answer it.
 
+
 CAVEATS
 
 This module will only work on *nix systems which support PAM,
 not Windows.
+
+
+DEBUGGING
+
+Debugging information from PAM services may be logged using different
+syslog facilities than OpenVPN, and so may end up in different
+locations (e.g. /var/log/auth.log etc. depending on system
+configuration).
+
+The administrator is advised to consult the many PAM tutorials and
+'howto' documents in order to learn about the debugging on PAM, and
+the creation and testing of custom PAM service definitions etc.

Signature · 1 line

2.1.4