pam_access / group supprt

[Thorsten Kukuk]

Hi,

On Wed, Jun 13, Julien Lecomte wrote:

> 
> Hi all,
> 
> This is my first contribution to pam: this patch adds direct support for 
> groups for pam_access.
> 
> - A new option "nodefgroup" has been added to remove previous default 
> behavior of supporting groups as a last resort.

For what is this good, or else, which problem should this solve?
I don't see a real value, you can get the same behavior if you don't
use group names there.

> - Groups can be explicitly written under the form "(group_name)"; the 
> use of parenthesis has been chosen to not conflict with netgroups.

A few comments from me:

> # Group 'users' may switch to any other member of 'users'.
> + : (users) : (users)

This use case and your implementation will only work for tools like
"su", but not from tools which allow login from remote (telnet, rlogin,
ssh, ftp, ...), because you use as "remote user" the UID under which
the application is running. But this does not need to have anything
to do with the remote user.

> +    } else if (tok[0] == '(' && tok[strlen(tok) - 1] == ')') { /* local group */
> +      /* get calling user's main group */
> +      return group_match(pamh, tok, getpwuid(getuid())->pw_name);

This is a pretty bad idea. You are assuming there is always
a passwd entry for the UID the application is running with,
but this don't need to be true, or there could be other
problems why getpwuid can return NULL. In that case your
application will crash, something never should happen.

  Thorsten

-- 
Thorsten Kukuk, Project Manager/Release Manager SLES
SUSE LINUX Products GmbH, Maxfeldstr. 5, D-90409 Nuernberg
GF: Markus Rex, HRB 16746 (AG Nuernberg)