Re: [Samba] WARNING to those running Samba on OpenIndiana or other Illumos based systems with 16 groups

2013-07-28 Thread Ira Cooper 
On Sun, Jul 14, 2013 at 8:23 AM, Andrew Bartlett abart...@samba.org wrote:

 On Wed, 2013-04-24 at 10:31 +1000, Andrew Bartlett wrote:
  Just a heads-up, because this bug took me absolutely ages to chase down,
  and I want to save others the same pain.
 
  Samba is perhaps the most prominent reason why you might find a user in
  more than 16 groups on a Unix system, and so this bug may at first
  appear to be a 'Samba issue' (that certainly is why it found it's way to
  my attention :-)
 
  https://www.illumos.org/issues/3691
 
  In short, unless the group list we supply to setgroups() is sorted, if
  there are more than 16 groups, the Illumos kernel fails to honour some
  of the groups.  Presumably there is a bisection search being done.
 
  The symptom for Samba users is that as a user is added to more groups,
  they loose access to folders they previously had access too.
 
  Attached is a total hack that appears to resolve the issue, but the real
  fix needs to be in glibc or the kernel.

 Just as a follow-up, if you experience this please also see
 https://www.illumos.org/issues/3577 and
 https://bugzilla.samba.org/show_bug.cgi?id=7588 for WORKAROUNDS if you
 cannot fix/change your host OS.  There is a patch for nss_winbind and
 smbd attached to that bug, both of which are required to ensure both
 Samba and other unix applications see all the windows groups.

 As we have now had success getting this fixed upstream I've not had time
 to get back to applying these to Samba when we run on Solaris, but the
 view was that for the small cost of a qsort we probably should.  If a
 DENY ACL is involved, this may also be a SECURITY issue, which is how we
 finally got the route cause addressed upstream.



Andrew,

As the upstream developer who fixed the issue: The fix had nothing to do
with security.  It had to do with Bjorn posting the root cause, and that
frankly I found sorting the list in samba beyond fugly.

I look at the fact you sorted the list in samba and just shake my head...
 The same qsort put in the illumos kernel fixes the issue for good.

Given our past history with such bugs, I'd expect we'll tell people to
upgrade their OS.

Thanks,

-Ira
-- 
To unsubscribe from this list go to the following URL and read the
instructions:  https://lists.samba.org/mailman/options/samba

Re: [Samba] WARNING to those running Samba on OpenIndiana or other Illumos based systems with 16 groups

2013-07-15 Thread Jeremy Allison 
On Sun, Jul 14, 2013 at 09:50:29AM -0400, Ira Cooper wrote:
 On Sun, Jul 14, 2013 at 8:23 AM, Andrew Bartlett abart...@samba.org wrote:
 
  On Wed, 2013-04-24 at 10:31 +1000, Andrew Bartlett wrote:
   Just a heads-up, because this bug took me absolutely ages to chase down,
   and I want to save others the same pain.
  
   Samba is perhaps the most prominent reason why you might find a user in
   more than 16 groups on a Unix system, and so this bug may at first
   appear to be a 'Samba issue' (that certainly is why it found it's way to
   my attention :-)
  
   https://www.illumos.org/issues/3691
  
   In short, unless the group list we supply to setgroups() is sorted, if
   there are more than 16 groups, the Illumos kernel fails to honour some
   of the groups.  Presumably there is a bisection search being done.
  
   The symptom for Samba users is that as a user is added to more groups,
   they loose access to folders they previously had access too.
  
   Attached is a total hack that appears to resolve the issue, but the real
   fix needs to be in glibc or the kernel.
 
  Just as a follow-up, if you experience this please also see
  https://www.illumos.org/issues/3577 and
  https://bugzilla.samba.org/show_bug.cgi?id=7588 for WORKAROUNDS if you
  cannot fix/change your host OS.  There is a patch for nss_winbind and
  smbd attached to that bug, both of which are required to ensure both
  Samba and other unix applications see all the windows groups.
 
  As we have now had success getting this fixed upstream I've not had time
  to get back to applying these to Samba when we run on Solaris, but the
  view was that for the small cost of a qsort we probably should.  If a
  DENY ACL is involved, this may also be a SECURITY issue, which is how we
  finally got the route cause addressed upstream.
 
 
 
 Andrew,
 
 As the upstream developer who fixed the issue: The fix had nothing to do
 with security.  It had to do with Bjorn posting the root cause, and that
 frankly I found sorting the list in samba beyond fugly.

May be beyong fugly, but I think Andrew was perfectly correct in
doing so :-).

 I look at the fact you sorted the list in samba and just shake my head...
  The same qsort put in the illumos kernel fixes the issue for good.

Not everyone has the same familiarity with kernel programming as you :-).

 Given our past history with such bugs, I'd expect we'll tell people to
 upgrade their OS.

Yeah, but not everyone can do that easily. Having a fix for Samba only
is A GOOD THING (tm) even if you think it's horrible :-).

Jeremy.
-- 
To unsubscribe from this list go to the following URL and read the
instructions:  https://lists.samba.org/mailman/options/samba

Re: [Samba] WARNING to those running Samba on OpenIndiana or other Illumos based systems with 16 groups

2013-07-14 Thread Andrew Bartlett 
On Wed, 2013-04-24 at 10:31 +1000, Andrew Bartlett wrote:
 Just a heads-up, because this bug took me absolutely ages to chase down,
 and I want to save others the same pain.
 
 Samba is perhaps the most prominent reason why you might find a user in
 more than 16 groups on a Unix system, and so this bug may at first
 appear to be a 'Samba issue' (that certainly is why it found it's way to
 my attention :-)
 
 https://www.illumos.org/issues/3691
 
 In short, unless the group list we supply to setgroups() is sorted, if
 there are more than 16 groups, the Illumos kernel fails to honour some
 of the groups.  Presumably there is a bisection search being done. 
 
 The symptom for Samba users is that as a user is added to more groups,
 they loose access to folders they previously had access too. 
 
 Attached is a total hack that appears to resolve the issue, but the real
 fix needs to be in glibc or the kernel. 

Just as a follow-up, if you experience this please also see 
https://www.illumos.org/issues/3577 and
https://bugzilla.samba.org/show_bug.cgi?id=7588 for WORKAROUNDS if you
cannot fix/change your host OS.  There is a patch for nss_winbind and
smbd attached to that bug, both of which are required to ensure both
Samba and other unix applications see all the windows groups. 

As we have now had success getting this fixed upstream I've not had time
to get back to applying these to Samba when we run on Solaris, but the
view was that for the small cost of a qsort we probably should.  If a
DENY ACL is involved, this may also be a SECURITY issue, which is how we
finally got the route cause addressed upstream.

Thanks,

Andrew Bartlett
-- 
Andrew Bartletthttp://samba.org/~abartlet/
Authentication Developer, Samba Team   http://samba.org

-- 
To unsubscribe from this list go to the following URL and read the
instructions:  https://lists.samba.org/mailman/options/samba

Re: [Samba] WARNING to those running Samba on OpenIndiana or other Illumos based systems with 16 groups

2013-04-29 Thread Ira Cooper 
Hey Volker, is this familiar?  (I've actually used this exact example in
presentations!)

I think this is the 1st or 2nd issue I tried to address.  It turns out
there is a *MUCH* simpler fix.

For modern enough Illumos/Solaris systems you can really fix this, for
the most part.

Put set ngroups_max = 1024 in your /etc/system.  (On less modern systems
you may have to use 128...)

If your user is in over 1024 groups... Well.. Then you need a patch to cap
it.  But in my environment, it doesn't happen.

I suspect with a recompile that 1024 can be bumped, though I haven't
researched it.

Note on my 1st systems, I couldn't do this, so I clamped using NGROUPS_MAX
as defined by POSIX.  That stopped the process death, but it didn't deal
with the security issue, that users can't access files in some of the
groups they should be in... (For me, a working system was more important, I
didn't need all the groups.  I moved on.)

Thanks,

-Ira


On Tue, Apr 23, 2013 at 8:31 PM, Andrew Bartlett abart...@samba.org wrote:

 Just a heads-up, because this bug took me absolutely ages to chase down,
 and I want to save others the same pain.

 Samba is perhaps the most prominent reason why you might find a user in
 more than 16 groups on a Unix system, and so this bug may at first
 appear to be a 'Samba issue' (that certainly is why it found it's way to
 my attention :-)

 https://www.illumos.org/issues/3691

 In short, unless the group list we supply to setgroups() is sorted, if
 there are more than 16 groups, the Illumos kernel fails to honour some
 of the groups.  Presumably there is a bisection search being done.

 The symptom for Samba users is that as a user is added to more groups,
 they loose access to folders they previously had access too.

 Attached is a total hack that appears to resolve the issue, but the real
 fix needs to be in glibc or the kernel.

 Andrew Bartlett
 --
 Andrew Bartletthttp://samba.org/~abartlet/
 Authentication Developer, Samba Team   http://samba.org


-- 
To unsubscribe from this list go to the following URL and read the
instructions:  https://lists.samba.org/mailman/options/samba

Re: [Samba] WARNING to those running Samba on OpenIndiana or other Illumos based systems with 16 groups

2013-04-24 Thread Volker Lendecke 
On Wed, Apr 24, 2013 at 10:31:20AM +1000, Andrew Bartlett wrote:
 Just a heads-up, because this bug took me absolutely ages to chase down,
 and I want to save others the same pain.

Yep, same here. A customer ran into this and we stared at
that for ages. Björn Jacke figured this out together with
someone from Oracle. It is a Solaris issue as well, fixed
with current Solaris patchsets.

It's one of the very, very rare cases where we actually have
a kernel bug.

Volker

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-37-0, fax: +49-551-37-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kont...@sernet.de
-- 
To unsubscribe from this list go to the following URL and read the
instructions:  https://lists.samba.org/mailman/options/samba

[Samba] WARNING to those running Samba on OpenIndiana or other Illumos based systems with 16 groups

2013-04-23 Thread Andrew Bartlett 
Just a heads-up, because this bug took me absolutely ages to chase down,
and I want to save others the same pain.

Samba is perhaps the most prominent reason why you might find a user in
more than 16 groups on a Unix system, and so this bug may at first
appear to be a 'Samba issue' (that certainly is why it found it's way to
my attention :-)

https://www.illumos.org/issues/3691

In short, unless the group list we supply to setgroups() is sorted, if
there are more than 16 groups, the Illumos kernel fails to honour some
of the groups.  Presumably there is a bisection search being done. 

The symptom for Samba users is that as a user is added to more groups,
they loose access to folders they previously had access too. 

Attached is a total hack that appears to resolve the issue, but the real
fix needs to be in glibc or the kernel. 

Andrew Bartlett
-- 
Andrew Bartletthttp://samba.org/~abartlet/
Authentication Developer, Samba Team   http://samba.org

diff --git a/source3/lib/system.c b/source3/lib/system.c
index 7c0bb3f..be7d94d 100644
--- a/source3/lib/system.c
+++ b/source3/lib/system.c
@@ -1174,14 +1174,14 @@ int groups_max(void)
  returning an array of gid_t, but actuall return an array of int.
 /
 
-#if defined(HAVE_BROKEN_GETGROUPS)
-
 #ifdef HAVE_BROKEN_GETGROUPS
 #define GID_T int
 #else
 #define GID_T gid_t
 #endif
 
+#if defined(HAVE_BROKEN_GETGROUPS)
+
 static int sys_broken_getgroups(int setlen, gid_t *gidset)
 {
 	GID_T gid;
@@ -1224,6 +1224,13 @@ static int sys_broken_getgroups(int setlen, gid_t *gidset)
 	return ngroups;
 }
 
+#endif /* HAVE_BROKEN_GETGROUPS */
+
+static int gid_compare(void *gid_1, void *gid_2)
+{
+	return (*(GID_T *)gid_1 - *(GID_T *)gid_2);
+}
+
 static int sys_broken_setgroups(int setlen, gid_t *gidset)
 {
 	GID_T *group_list;
@@ -1250,6 +1257,8 @@ static int sys_broken_setgroups(int setlen, gid_t *gidset)
 	for(i = 0; i  setlen; i++) 
 		group_list[i] = (GID_T) gidset[i]; 
 
+	TYPESAFE_QSORT(group_list, setlen, gid_compare);
+
 	if(setgroups(setlen, group_list) != 0) {
 		int saved_errno = errno;
 		SAFE_FREE(group_list);
@@ -1261,8 +1270,6 @@ static int sys_broken_setgroups(int setlen, gid_t *gidset)
 	return 0 ;
 }
 
-#endif /* HAVE_BROKEN_GETGROUPS */
-
 /* This is a list of systems that require the first GID passed to setgroups(2)
  * to be the effective GID. If your system is one of these, add it here.
  */
@@ -1353,11 +1360,8 @@ int sys_setgroups(gid_t UNUSED(primary_gid), int setlen, gid_t *gidset)
 
 #if defined(USE_BSD_SETGROUPS)
 	return sys_bsd_setgroups(primary_gid, setlen, gidset);
-#elif defined(HAVE_BROKEN_GETGROUPS)
-	return sys_broken_setgroups(setlen, gidset);
-#else
-	return setgroups(setlen, gidset);
 #endif
+	return sys_broken_setgroups(setlen, gidset);
 }
 
 /**
-- 
To unsubscribe from this list go to the following URL and read the
instructions:  https://lists.samba.org/mailman/options/samba

Re: [Samba] WARNING to those running Samba on OpenIndiana or other Illumos based systems with 16 groups

2013-04-23 Thread Andrew Bartlett 
On Tue, 2013-04-23 at 20:44 -0400, Ira Cooper wrote:
 Hey Volker, is this familiar?  (I've actually used this exact
 example in presentations!)
 
 
 I think this is the 1st or 2nd issue I tried to address.  It turns out
 there is a *MUCH* simpler fix.
 
 
 For modern enough Illumos/Solaris systems you can really fix this,
 for the most part.
 
 
 Put set ngroups_max = 1024 in your /etc/system.  (On less modern
 systems you may have to use 128...)
 
 
 If your user is in over 1024 groups... Well.. Then you need a patch to
 cap it.  But in my environment, it doesn't happen.
 
 
 I suspect with a recompile that 1024 can be bumped, though I haven't
 researched it.
 
 
 Note on my 1st systems, I couldn't do this, so I clamped using
 NGROUPS_MAX as defined by POSIX.  That stopped the process death, but
 it didn't deal with the security issue, that users can't access files
 in some of the groups they should be in... (For me, a working system
 was more important, I didn't need all the groups.  I moved on.)

I should have made clear, ngroups_max = 1024 on these systems already.
That's the easy part, for which we have a PANIC in the code.

This issue is that changing ngroups_max is essentially untested in this
kernel.  Later Oracle Solaris releases allegedly already have the fix. 

I had this IRC conversation on #openindiana on irc.freenode.net on  09
Apr 2013:

(15:22:16) abartlet: anyone here deep enough into the kernel to comment
on setgroups() and if the group list must be sorted?
(15:22:39) abartlet: I've been chasing for some time a really odd issue,
which seems to be that if more than 16 groups are specified, they only
work if sorted...

(17:17:47) MarcelT: abartlet: it is a bug in illumos. The groups should
be sorted automatically, but they are not. Feel free to file a bug.

(17:26:13) abartlet: MarcelT: done: https://www.illumos.org/issues/3691
(17:26:28) abartlet: MarcelT: it sounds like it's been around for a
while then?

(18:35:21) abartlet: is there something about this that makes it
particularly fiendish to fix, or do folks just pretend the old 16 group
limit still exists?
(18:35:36) easye [~user@213.33.70.157] entered the room.
(18:36:06) MarcelT: I do not remember details. I just know that there is
a bug related to (un)sorted groups
(18:36:17) MarcelT: IIRC it was fixed in Solaris 11 in 2011
(18:37:01) MarcelT: and probably backported to Solaris 10 too, but I
(18:37:08) MarcelT: 'am not sure about that
(18:37:41) abartlet: BTW, my background is that I'm an upstream dev on
Samba (which is one of the best ways to get lots of groups onto a box,
because of windows nested groups), working for a NAS vendor 
(18:39:15) MarcelT: ... and maybe I backported the fix to Amber Road
about a year ago :-)
(18:39:57) MarcelT: I did something related to 16 groups in amber road
and I backported a bunch o bugs there... :-)
(18:41:01) abartlet: so, I take it this isn't the kind of thing where I
grab the Solaris 11 git tree, and cherry-pick out a fix?
(18:41:31) ***abartlet assumes not, given Oracle's reputation, but
anyway...
(18:41:57) MarcelT: heh, do you have access to Solaris 11 sources? :-)
(18:42:10) MarcelT: BTW, the amber road stuff is here:
(18:42:12) MarcelT:
https://wikis.oracle.com/display/FishWorks/ak-2011.04.24.4.0+Release
+Notes
(18:42:27) MarcelT: 6199185 netname2user() code has a limit for the
number of groups
(18:42:33) MarcelT: 6949066 User can't belong to more than 16 groups.
Impacts AUTH_SYS authentication
(18:42:50) abartlet: MarcelT: that's different, I think...
(18:43:04) MarcelT: 7044547 kernel rpc should call KEY_GETCRED_3 and get
all available gids
(18:43:12) MarcelT: 7044600 keyserv dumps core when the remote procedure
KEY_GETCRED_3 is called
(18:43:20) MarcelT: 7044891 groups aren't always sorted in the
credential
(18:43:26) MarcelT: 7047829 AUTH_LOOPBACK corrupts data when  32 groups
are available
(18:43:31) abartlet: that sounds more like it 7044891
(18:43:33) MarcelT: 7052192 Several parts of the kernel are inefficient
when using multiple groups
(18:43:40) MarcelT: 7052195 The backend can call netname2user with an
improperly sized array
(18:45:51) MarcelT: this is more-or-less the complete list of 16 groups
related fixes I backported to amber road
(18:46:09) MarcelT: but since I just backported them, I do not remember
details about the fix
(18:46:19) abartlet: ok, so how do I get a kernel with that in it to
test with?
(18:46:32) MarcelT: try Solaris 11 :-)
(18:46:40) MarcelT: or maybe Solaris 11.1
(18:47:08) MarcelT: or, try to fix it in illumos :-)
(18:47:28) abartlet: ahh, so you were doing this inside Oracle?
(18:47:30) MarcelT: you have a hint from the Sun bugs synopses above :-)
(18:47:38) MarcelT: sure :-)
(18:47:57) abartlet: sorry, don't know folks here (yet)
(18:48:06) MarcelT: no problem

Andrew Bartlett


-- 
Andrew Bartletthttp://samba.org/~abartlet/
Authentication Developer, Samba Team   http://samba.org


-- 
To unsubscribe from this