Re: svn commit: r332559 - head/usr.sbin/mountd
On 2018-04-16 8:02 PM, Rick Macklem wrote:
I wrote:
Julian Elischer wrote:
On 16/4/18 6:56 pm, Konstantin Belousov wrote:
[stuff snipped]
+ngroups =3D XU_NGROUPS + 1;
Why XU_NGROUPS and not the value of sysctl("kern.ngroups") ?
valid question.. because that is how many are allocated?
it was a "minimally invasive patch".. whoever used XU_NGROUPS before
should have fixed it.
Having said that, thanks for drawing out attention to it.. will
probably fix.
16 is the limit specified in the RFCs for Sun RPC, so that is the "on the wire"
limit.
I haven't looked at the code. It might make sense to handle more here and then
set the limit at 16 after getting rid of duplicates, but I have no idea if =
it matters?
rick
Correcting my own post. Now that I've looked at the code, this doesn't go on
the wire. It does go in the exports structure, which means that this structure
would have to be revised (along with the syscall and VOP calls and the kernel
code that uses it). These credentials are for the "maproot/mapall" export
option and revising the export structure seems like quite a bit of work for this
case. (Until revised XU_NGROUPS is the correct value to set it to, since there
is a "struct xucred" in the exports structure.)
Since Julian Elischer has been emailing me about adding a "fsid" export option
which allows /etc/exports to set the FSID of the exported fs (which would also
need to go in the exports structure), it might be about time to rev. the exports
structure?
rick
I second the request for FSID as an export option. It is important for
having smooth failover between NFS servers.
--
Allan Jude
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r332559 - head/usr.sbin/mountd
On 19/4/18 5:15 am, Rick Macklem wrote: Julian Elischer wrote: [stuff snipped] our issue is that we make a server that combines CIFS/SMB access (via samba), credential setting from a company wide AD server (windows) via winbindd (samba) via nsswitch.. and NFS. The problem is that when one looks up a user name from the AD server One can get back a credential with a large number of groups, because some companies use windows groups extensively. SO a sinel user may be in a group for every project they are involved with and a method of giving them access to files related to a project. In this scenario a group manager may be given access to a lot of groups. A user looking at a file via NFS needs to be able to see what he needs and still be blocked as per company policy. I am investigating the new user-manager daemon may help but I don't fully understand it yet. I gather it maps an incoming request to a set of groups as defined on the server rather than on the client, but I'm not sure yet how that relates to mountd. I am happy to say I know nothing about AD, but I thought it included an LDAP service? yes and this what is used when one uses ldap against an AD server. (which seems to work) If there is a way to configure FreeBSD so that getgrouplist(3) gets this list of AD groups, then "nfsuserd -manage-gids" on the NFS server should do what you want. (It takes the "uid" from the AUTH_SYS RPC request header and then creates a list of groups for that "uid" via getgrouplist(3). It basically does a getpwuid() and then uses the pw_name as the first arg to getgrouplist(3). It ignores the list of groups in the RPC header and, therefore, is not limited to 16.) yes that is what I was referring to in my previous email getgrouplist(3) does the right thing as far as I know. If getgrouplist(3) can't see the set of AD groups, then something needs to be done to make that work. rick ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r332559 - head/usr.sbin/mountd
Julian Elischer wrote: [stuff snipped] >our issue is that we make a server that combines CIFS/SMB access (via >samba), credential setting from a company wide AD server (windows) >via winbindd (samba) via nsswitch.. and NFS. > >The problem is that when one looks up a user name from the AD server >One can get back a credential with a large number of groups, because >some companies use windows groups extensively. SO a sinel user may be >in a group for every project they are involved with and a method of >giving them access to files related to a project. >In this scenario a group manager may be given access to a lot of groups. > >A user looking at a file via NFS needs to be able to see what he needs >and still be blocked as per company policy. >I am investigating the new user-manager daemon may help but I don't >fully understand it yet. >I gather it maps an incoming request to a set of groups as defined on >the server rather than on the client, but I'm not sure yet how that >relates to mountd. I am happy to say I know nothing about AD, but I thought it included an LDAP service? If there is a way to configure FreeBSD so that getgrouplist(3) gets this list of AD groups, then "nfsuserd -manage-gids" on the NFS server should do what you want. (It takes the "uid" from the AUTH_SYS RPC request header and then creates a list of groups for that "uid" via getgrouplist(3). It basically does a getpwuid() and then uses the pw_name as the first arg to getgrouplist(3). It ignores the list of groups in the RPC header and, therefore, is not limited to 16.) If getgrouplist(3) can't see the set of AD groups, then something needs to be done to make that work. rick ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r332559 - head/usr.sbin/mountd
On 17/4/18 8:32 pm, Konstantin Belousov wrote:
On Tue, Apr 17, 2018 at 12:02:04AM +, Rick Macklem wrote:
I wrote:
Julian Elischer wrote:
On 16/4/18 6:56 pm, Konstantin Belousov wrote:
[stuff snipped]
+ngroups =3D XU_NGROUPS + 1;
Why XU_NGROUPS and not the value of sysctl("kern.ngroups") ?
valid question.. because that is how many are allocated?
it was a "minimally invasive patch".. whoever used XU_NGROUPS before
should have fixed it.
Having said that, thanks for drawing out attention to it.. will
probably fix.
16 is the limit specified in the RFCs for Sun RPC, so that is the "on the wire"
limit.
I haven't looked at the code. It might make sense to handle more here and then
set the limit at 16 after getting rid of duplicates, but I have no idea if =
it matters?
rick
Correcting my own post. Now that I've looked at the code, this doesn't go on
the wire. It does go in the exports structure, which means that this structure
would have to be revised (along with the syscall and VOP calls and the kernel
code that uses it). These credentials are for the "maproot/mapall" export
option and revising the export structure seems like quite a bit of work for this
case. (Until revised XU_NGROUPS is the correct value to set it to, since there
is a "struct xucred" in the exports structure.)
Since Julian Elischer has been emailing me about adding a "fsid" export option
which allows /etc/exports to set the FSID of the exported fs (which would also
need to go in the exports structure), it might be about time to rev. the exports
structure?
Probably yes, we would need a new variant of the nmount(2) syscall.
Existing syscall should use the old layout for compatibility (we care
about nmount and COMPAT32 as well).
our issue is that we make a server that combines CIFS/SMB access (via
samba), credential setting from a company wide AD server (windows)
via winbindd (samba) via nsswitch.. and NFS.
The problem is that when one looks up a user name from the AD server
One can get back a credential with a large number of groups, because
some companies use windows groups extensively. SO a sinel user may be
in a group for every project they are involved with and a method of
giving them access to files related to a project.
In this scenario a group manager may be given access to a lot of groups.
A user looking at a file via NFS needs to be able to see what he needs
and still be blocked as per company policy.
I am investigating the new user-manager daemon may help but I don't
fully understand it yet.
I gather it maps an incoming request to a set of groups as defined on
the server rather than on the client, but I'm not sure yet how that
relates to mountd.
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r332559 - head/usr.sbin/mountd
On Tue, Apr 17, 2018 at 12:02:04AM +, Rick Macklem wrote:
> I wrote:
> >Julian Elischer wrote:
> >>On 16/4/18 6:56 pm, Konstantin Belousov wrote:
> >[stuff snipped]
> +ngroups =3D XU_NGROUPS + 1;
> >>> Why XU_NGROUPS and not the value of sysctl("kern.ngroups") ?
> >>valid question.. because that is how many are allocated?
> >>it was a "minimally invasive patch".. whoever used XU_NGROUPS before
> >>should have fixed it.
> >>Having said that, thanks for drawing out attention to it.. will
> >>probably fix.
> >16 is the limit specified in the RFCs for Sun RPC, so that is the "on the
> >wire" limit.
> >I haven't looked at the code. It might make sense to handle more here and
> >then
> >set the limit at 16 after getting rid of duplicates, but I have no idea if =
> >it matters?
> >
> >rick
> Correcting my own post. Now that I've looked at the code, this doesn't go on
> the wire. It does go in the exports structure, which means that this structure
> would have to be revised (along with the syscall and VOP calls and the kernel
> code that uses it). These credentials are for the "maproot/mapall" export
> option and revising the export structure seems like quite a bit of work for
> this
> case. (Until revised XU_NGROUPS is the correct value to set it to, since there
> is a "struct xucred" in the exports structure.)
>
> Since Julian Elischer has been emailing me about adding a "fsid" export option
> which allows /etc/exports to set the FSID of the exported fs (which would also
> need to go in the exports structure), it might be about time to rev. the
> exports
> structure?
Probably yes, we would need a new variant of the nmount(2) syscall.
Existing syscall should use the old layout for compatibility (we care
about nmount and COMPAT32 as well).
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r332559 - head/usr.sbin/mountd
I wrote:
>Julian Elischer wrote:
>>On 16/4/18 6:56 pm, Konstantin Belousov wrote:
>[stuff snipped]
+ngroups =3D XU_NGROUPS + 1;
>>> Why XU_NGROUPS and not the value of sysctl("kern.ngroups") ?
>>valid question.. because that is how many are allocated?
>>it was a "minimally invasive patch".. whoever used XU_NGROUPS before
>>should have fixed it.
>>Having said that, thanks for drawing out attention to it.. will
>>probably fix.
>16 is the limit specified in the RFCs for Sun RPC, so that is the "on the
>wire" limit.
>I haven't looked at the code. It might make sense to handle more here and then
>set the limit at 16 after getting rid of duplicates, but I have no idea if =
>it matters?
>
>rick
Correcting my own post. Now that I've looked at the code, this doesn't go on
the wire. It does go in the exports structure, which means that this structure
would have to be revised (along with the syscall and VOP calls and the kernel
code that uses it). These credentials are for the "maproot/mapall" export
option and revising the export structure seems like quite a bit of work for this
case. (Until revised XU_NGROUPS is the correct value to set it to, since there
is a "struct xucred" in the exports structure.)
Since Julian Elischer has been emailing me about adding a "fsid" export option
which allows /etc/exports to set the FSID of the exported fs (which would also
need to go in the exports structure), it might be about time to rev. the exports
structure?
rick
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r332559 - head/usr.sbin/mountd
Julian Elischer wrote:
>On 16/4/18 6:56 pm, Konstantin Belousov wrote:
[stuff snipped]
>>> +ngroups = XU_NGROUPS + 1;
>> Why XU_NGROUPS and not the value of sysctl("kern.ngroups") ?
>valid question.. because that is how many are allocated?
>it was a "minimally invasive patch".. whoever used XU_NGROUPS before
>should have fixed it.
>Having said that, thanks for drawing out attention to it.. will
>probably fix.
16 is the limit specified in the RFCs for Sun RPC, so that is the "on the wire"
limit.
I haven't looked at the code. It might make sense to handle more here and then
set the limit at 16 after getting rid of duplicates, but I have no idea if it
matters?
rick
[more stuff snipped]
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r332559 - head/usr.sbin/mountd
On 16/4/18 6:56 pm, Konstantin Belousov wrote:
On Mon, Apr 16, 2018 at 09:17:36AM +, Andriy Gapon wrote:
Author: avg
Date: Mon Apr 16 09:17:36 2018
New Revision: 332559
URL: https://svnweb.freebsd.org/changeset/base/332559
Log:
mountd: fix a crash when getgrouplist reports too many groups
Previously the code only warned about the condition and then happily
proceeded to use the too large value resulting in the array
out-of-bounds access.
Obtained from: Panzura (Chuanbo Zheng)
MFC after: 10 days
Sponsored by:Panzura
Modified:
head/usr.sbin/mountd/mountd.c
Modified: head/usr.sbin/mountd/mountd.c
==
--- head/usr.sbin/mountd/mountd.c Mon Apr 16 08:41:44 2018
(r332558)
+++ head/usr.sbin/mountd/mountd.c Mon Apr 16 09:17:36 2018
(r332559)
@@ -2915,8 +2915,11 @@ parsecred(char *namelist, struct xucred *cr)
}
cr->cr_uid = pw->pw_uid;
ngroups = XU_NGROUPS + 1;
- if (getgrouplist(pw->pw_name, pw->pw_gid, groups, &ngroups))
+ if (getgrouplist(pw->pw_name, pw->pw_gid, groups, &ngroups)) {
syslog(LOG_ERR, "too many groups");
+ ngroups = XU_NGROUPS + 1;
Why XU_NGROUPS and not the value of sysctl("kern.ngroups") ?
valid question.. because that is how many are allocated?
it was a "minimally invasive patch".. whoever used XU_NGROUPS before
should have fixed it.
Having said that, thanks for drawing out attention to it.. will
probably fix.
+ }
+
/*
* Compress out duplicate.
*/
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r332559 - head/usr.sbin/mountd
On 16/04/2018 13:56, Konstantin Belousov wrote:
> On Mon, Apr 16, 2018 at 09:17:36AM +, Andriy Gapon wrote:
>> Author: avg
>> Date: Mon Apr 16 09:17:36 2018
>> New Revision: 332559
>> URL: https://svnweb.freebsd.org/changeset/base/332559
>>
>> Log:
>> mountd: fix a crash when getgrouplist reports too many groups
>>
>> Previously the code only warned about the condition and then happily
>> proceeded to use the too large value resulting in the array
>> out-of-bounds access.
>>
>> Obtained from: Panzura (Chuanbo Zheng)
>> MFC after: 10 days
>> Sponsored by: Panzura
>>
>> Modified:
>> head/usr.sbin/mountd/mountd.c
>>
>> Modified: head/usr.sbin/mountd/mountd.c
>> ==
>> --- head/usr.sbin/mountd/mountd.cMon Apr 16 08:41:44 2018
>> (r332558)
>> +++ head/usr.sbin/mountd/mountd.cMon Apr 16 09:17:36 2018
>> (r332559)
>> @@ -2915,8 +2915,11 @@ parsecred(char *namelist, struct xucred *cr)
>> }
>> cr->cr_uid = pw->pw_uid;
>> ngroups = XU_NGROUPS + 1;
>> -if (getgrouplist(pw->pw_name, pw->pw_gid, groups, &ngroups))
>> +if (getgrouplist(pw->pw_name, pw->pw_gid, groups, &ngroups)) {
>> syslog(LOG_ERR, "too many groups");
>> +ngroups = XU_NGROUPS + 1;
> Why XU_NGROUPS and not the value of sysctl("kern.ngroups") ?
Two reasons:
1. it's what the code already used
2. the groups are placed into struct xucred and later that struct is passed to
kernel, so in my opinion it's xucred that defines the limit in this case
--
Andriy Gapon
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r332559 - head/usr.sbin/mountd
On Mon, Apr 16, 2018 at 09:17:36AM +, Andriy Gapon wrote:
> Author: avg
> Date: Mon Apr 16 09:17:36 2018
> New Revision: 332559
> URL: https://svnweb.freebsd.org/changeset/base/332559
>
> Log:
> mountd: fix a crash when getgrouplist reports too many groups
>
> Previously the code only warned about the condition and then happily
> proceeded to use the too large value resulting in the array
> out-of-bounds access.
>
> Obtained from: Panzura (Chuanbo Zheng)
> MFC after: 10 days
> Sponsored by: Panzura
>
> Modified:
> head/usr.sbin/mountd/mountd.c
>
> Modified: head/usr.sbin/mountd/mountd.c
> ==
> --- head/usr.sbin/mountd/mountd.c Mon Apr 16 08:41:44 2018
> (r332558)
> +++ head/usr.sbin/mountd/mountd.c Mon Apr 16 09:17:36 2018
> (r332559)
> @@ -2915,8 +2915,11 @@ parsecred(char *namelist, struct xucred *cr)
> }
> cr->cr_uid = pw->pw_uid;
> ngroups = XU_NGROUPS + 1;
> - if (getgrouplist(pw->pw_name, pw->pw_gid, groups, &ngroups))
> + if (getgrouplist(pw->pw_name, pw->pw_gid, groups, &ngroups)) {
> syslog(LOG_ERR, "too many groups");
> + ngroups = XU_NGROUPS + 1;
Why XU_NGROUPS and not the value of sysctl("kern.ngroups") ?
> + }
> +
> /*
>* Compress out duplicate.
>*/
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
svn commit: r332559 - head/usr.sbin/mountd
Author: avg
Date: Mon Apr 16 09:17:36 2018
New Revision: 332559
URL: https://svnweb.freebsd.org/changeset/base/332559
Log:
mountd: fix a crash when getgrouplist reports too many groups
Previously the code only warned about the condition and then happily
proceeded to use the too large value resulting in the array
out-of-bounds access.
Obtained from:Panzura (Chuanbo Zheng)
MFC after:10 days
Sponsored by: Panzura
Modified:
head/usr.sbin/mountd/mountd.c
Modified: head/usr.sbin/mountd/mountd.c
==
--- head/usr.sbin/mountd/mountd.c Mon Apr 16 08:41:44 2018
(r332558)
+++ head/usr.sbin/mountd/mountd.c Mon Apr 16 09:17:36 2018
(r332559)
@@ -2915,8 +2915,11 @@ parsecred(char *namelist, struct xucred *cr)
}
cr->cr_uid = pw->pw_uid;
ngroups = XU_NGROUPS + 1;
- if (getgrouplist(pw->pw_name, pw->pw_gid, groups, &ngroups))
+ if (getgrouplist(pw->pw_name, pw->pw_gid, groups, &ngroups)) {
syslog(LOG_ERR, "too many groups");
+ ngroups = XU_NGROUPS + 1;
+ }
+
/*
* Compress out duplicate.
*/
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
