You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Post Master <da...@izhnet.ru> on 2014/04/16 19:47:27 UTC
using system groups in svnaccess.conf
subversion need to specify group members in the 'groups'
section of
svnaccess.conf configuration file.
external access control system like LDAP, AD, &etc.
requires to syncronize group
members to svnaccess.conf (for example:
http://thoughtspark.org/node/26/).
subversion must query operating system for group members
directly.
for example: on posix systems from nss (ldap, nis,
/etc/group ...). on windows:
from AD or local groups.
authz_posixgroup_contains_user.patch is a prototype for
posix system (getgrnam).
svnaccess.conf may be like that:
[repos1:/]
%wheel = rw
%members.test.bla-bla-bla = r
'%'-prefix means system group
http://subversion.tigris.org/issues/show_bug.cgi?id=4489
Re: using system groups in svnaccess.conf
Posted by Stefan Sperling <st...@elego.de>.
On Thu, Apr 17, 2014 at 03:09:47PM +0100, Philip Martin wrote:
> Stefan Sperling <st...@elego.de> writes:
>
> > The 'pool' parmeter is unused.
> >
> >> + struct group *grp;
> >> + char **gmem;
> >> +
> >> + if ((grp = getgrnam(group)) == NULL)
> >
> > It would be nice if APR offered an interface to this function.
> > I checked but couldn't find one.
>
> APR has apr_gid_t, apr_gid_get(), apr_gid_name_get(), etc. Could we use
> those?
I don't think we can.
I cannot find a function to determine whether a user is a member
of a given a group. APR uses getgrnam_r() internally but doesn't
expose the 'struct group' result it gets from getgrnam_r().
We could add a new function to APR which this patch could then make use of.
But until an APR version with such functionality is released we'll have
to call getgrnam_r() ourselves.
Re: using system groups in svnaccess.conf
Posted by Dmitry Bakshaev <da...@izhnet.ru>.
new version of authz_posixgroup_contains_user.patch.
+treadsafe (getgrnam_r)
+AC_CHECK_FUNCS
-windows code still missing
using system groups in svnaccess.conf
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Thursday, 17 April 2014, Philip Martin
<philip.martin@wandisco.com<javascript:_e(%7B%7D,'cvml','philip.martin@wandisco.com');>>
wrote:
> Philip Martin <ph...@wandisco.com> writes:
>
> > Does Windows offer a comparable concept? If so then we should attempt
> > to write the Subversion code using a generic API we can push to APR.
>
> Is CheckTokenMembership the concept?
>
>
> http://msdn.microsoft.com/en-us/library/windows/desktop/aa376389%28v=vs.85%29.aspx
Yes, it is. But you need authentication token to check user membership, not
user id. Authentication token populated with list of all groups that user
is member of on logon.
--
Ivan Zhakov
CTO | VisualSVN | http://www.visualsvn.com
Re: using system groups in svnaccess.conf
Posted by Philip Martin <ph...@wandisco.com>.
Philip Martin <ph...@wandisco.com> writes:
> Does Windows offer a comparable concept? If so then we should attempt
> to write the Subversion code using a generic API we can push to APR.
Is CheckTokenMembership the concept?
http://msdn.microsoft.com/en-us/library/windows/desktop/aa376389%28v=vs.85%29.aspx
Rather than returning a list of users in the group perhaps the APR API
would accept apr_uid_t and apr_gid_t and return a status? Or perhaps
the APR API accepts username and groupname?
--
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*
Re: using system groups in svnaccess.conf
Posted by Philip Martin <ph...@wandisco.com>.
Philip Martin <ph...@wandisco.com> writes:
> Stefan Sperling <st...@elego.de> writes:
>
>> The 'pool' parmeter is unused.
>>
>>> + struct group *grp;
>>> + char **gmem;
>>> +
>>> + if ((grp = getgrnam(group)) == NULL)
>>
>> It would be nice if APR offered an interface to this function.
>> I checked but couldn't find one.
>
> APR has apr_gid_t, apr_gid_get(), apr_gid_name_get(), etc. Could we use
> those?
No, probably not. We could get the user's group ID and match that, but
I don't think we could ask whether the user is part of an arbitrary
group.
Does Windows offer a comparable concept? If so then we should attempt
to write the Subversion code using a generic API we can push to APR.
--
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*
Re: using system groups in svnaccess.conf
Posted by Philip Martin <ph...@wandisco.com>.
Stefan Sperling <st...@elego.de> writes:
> The 'pool' parmeter is unused.
>
>> + struct group *grp;
>> + char **gmem;
>> +
>> + if ((grp = getgrnam(group)) == NULL)
>
> It would be nice if APR offered an interface to this function.
> I checked but couldn't find one.
APR has apr_gid_t, apr_gid_get(), apr_gid_name_get(), etc. Could we use
those?
--
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*
Re: using system groups in svnaccess.conf
Posted by Stefan Sperling <st...@elego.de>.
On Wed, Apr 16, 2014 at 09:47:27PM +0400, Post Master wrote:
>
> subversion need to specify group members in the 'groups' section of
> svnaccess.conf configuration file.
> external access control system like LDAP, AD, &etc. requires to syncronize
> group
> members to svnaccess.conf (for example: http://thoughtspark.org/node/26/).
> subversion must query operating system for group members directly.
> for example: on posix systems from nss (ldap, nis, /etc/group ...). on
> windows:
> from AD or local groups.
>
> authz_posixgroup_contains_user.patch is a prototype for posix system
> (getgrnam).
>
> svnaccess.conf may be like that:
>
> [repos1:/]
> %wheel = rw
> %members.test.bla-bla-bla = r
>
> '%'-prefix means system group
>
> http://subversion.tigris.org/issues/show_bug.cgi?id=4489
I like this idea.
What will happen if someone is already using group names that
start with '%'? I think this feature should be backwards compatible.
Perhaps we could add an option to the authz file format that enables
the new '%' syntax, and disable the new syntax by default.
Something like this:
[groups]
use_posix_groups = true
[repos1:/]
%wheel = rw
%members.test.bla-bla-bla = r
I doubt anyone has a group named 'use_posix_groups', so this should
be fairly safe. Or we could add a new [options] section for this
purpose instead:
[options]
use_posix_groups = true
What do you think?
> --- ./subversion/libsvn_repos/authz.c.orig 2013-05-04 01:21:54.000000000 +0400
> +++ ./subversion/libsvn_repos/authz.c 2014-04-06 17:18:40.000000000 +0400
> @@ -25,6 +25,7 @@
>
> #include <apr_pools.h>
> #include <apr_file_io.h>
You need to #include <sys/types.h> here, too.
> +#include <grp.h>
>
> #include "svn_hash.h"
> #include "svn_pools.h"
> @@ -197,6 +198,25 @@
> return FALSE;
> }
>
> +static svn_boolean_t
> +authz_posixgroup_contains_user(svn_config_t *cfg,
> + const char *group,
> + const char *user,
> + apr_pool_t *pool)
> +{
The 'pool' parmeter is unused.
> + struct group *grp;
> + char **gmem;
> +
> + if ((grp = getgrnam(group)) == NULL)
It would be nice if APR offered an interface to this function.
I checked but couldn't find one.
Please use getgrgid_r() instead. getgrnam isn't thread-safe.
We might want to use conditional compilation to support systems
where getgrnam_r() is not available. Can you add configure script
checks for grp.h and getgrnam_r(), and only call the getgrnam_r
function if it is available?
> + perror("getgrnam() error");
Please don't write anything to stderr.
libsvn_repos is a library, it should not use stdio.
Just return FALSE in this case.
> + else
> + for (gmem=grp->gr_mem; *gmem != NULL; gmem++)
> + if (strcmp(*gmem, user) == 0)
> + return TRUE;
> +
> + return FALSE;
> +}
> +
>
> /* Determines whether an authz rule applies to the current
> * user, given the name part of the rule's name-value pair
> @@ -242,6 +262,9 @@
> if (rule_match_string[0] == '@')
> return authz_group_contains_user(
> b->config, &rule_match_string[1], b->user, pool);
> + else if (rule_match_string[0] == '%')
> + return authz_posixgroup_contains_user(
> + b->config, &rule_match_string[1], b->user, pool);
> else if (rule_match_string[0] == '&')
> return authz_alias_is_user(
> b->config, &rule_match_string[1], b->user, pool);