Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 16 Oct 2002 23:44:22 -0400
From:      "Danny J. Zerkel" <dzerkel@columbus.rr.com>
To:        Robert Watson <rwatson@FreeBSD.ORG>, Terry Lambert <tlambert2@mindspring.com>
Cc:        Maxim Sobolev <sobomax@FreeBSD.ORG>, "Vladimir B. Grebenschikov,Moscow,408-7227,123-4567,Some-info" <vova@express.ru>, freebsd-arch@FreeBSD.ORG, freebsd-current@FreeBSD.ORG
Subject:   Re: short uid/gid
Message-ID:  <200210162344.22969.dzerkel@columbus.rr.com>
In-Reply-To: <Pine.NEB.3.96L.1021016125106.24763B-100000@fledge.watson.org>
References:  <Pine.NEB.3.96L.1021016125106.24763B-100000@fledge.watson.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wednesday 16 October 2002 13:01, Robert Watson wrote:
> On Wed, 16 Oct 2002, Terry Lambert wrote:
> > Robert Watson wrote:
> > > I'm not convinced there's any value to providing the backward
> > > compatibility that has to be asked for: the only benefit to the current
> > > short-based API is that it allow serious security holes while not
> > > following the standard API offered by other platforms (except Linux).
> >
> > The main benefit, to my mind, is standards compliance.  The secondary
> > benefit is ABI similarity for the purposes of supporting ABIs to run
> > non-native software (e.g. Linux prior to 2.4).
>
> Could you point me at the standard that indicates these fields should be
> shorts?  We're all advocating switching to uid_t/gid_t/id_t here, the only
> question is with regard to compatibility.  Platforms such as Solaris
> already use uid_t/gid_t for these fields, and we break portable
> applications because applications assume that uid_t fits into ipc_perm.uid
> (for example).
>
> > Binary backward compatability *demands* that you limit the range of your
> > UIDs and GIDs to what will fit in a uint16_t, just as the recent issue
> > with a pid_t exceeeding this size has damaged binary backward
> > compatability with third party a.out binaries.
>
> This has to do with the size of the field.  uid_t and gid_t are 32-bit on
> most relevant platforms, although there are lingering software
> compatibility issues (such as the Sysem V IPC issue).  The goal now is to
> correct lingering incompatibilities so that we can properly support these
> ranges.
>
> > > Freshly compiled applications should be using the proper types to
> > > represent uid's and gid's -- if they're not doing that in the existing
> > > code, they'll get truncated to the right size for "bug compatibility". 
> > > If they are using the correct size, they'll work correctly.  To be able
> > > to run properly on other platforms (vis Solaris), they already should
> > > be using those types.
> >
> > Truncating of oversized values for credentials is Bad(tm), in that what
> > you get is a different credential.  The constraint has to be a priori
> > enforced, or it's meaningless.
>
> I think we're in agreement here.  Unfortunately, because of the "short"
> uid/gid/cuid/cgid fields, truncation is *already* happening.  That's what
> we're trying to fix.  If you try to crush a uid_t into ipc_perm.uid on
> FreeBSD, the field is truncated, resulting in a vulnerability.  What
> pushing the type checking into the application space does is generate
> warnings for applications making bogus assumptions, and corrects the
> truncation performed by the kernel in the current scenario.  I.e., when a
> SysVIPC shm segment is created by a user with a uid that doesn't fit in
> short, the kernel gets it "wrong".  The change we're talking about will
> fix the kernel, and make type problems visible to the applications if they
> make incorrect assumptions.  Of course, past applications assuming 16-bit
> uids and gids will still be wrong, nothing changes that, but we do fix
> current applications.
>
> > > And it's not like the approach you've described makes it any easier to
> > > implement: you still have to break out the old and new structures since
> > > changing ipc_perm breaks the ABI for all of the System V interfaces,
> > > rewrite the kernel code, etc. You might as well have added the
> > > compatibility system calls since you still have to do all the mapping.
> >
> > His approach avoids the proliferation of system call entry points, which
> > may conflict with those of other BSDs (among other things).  BSDI
> > limited the number of available additional FreeBSD system calls in a
> > standardization effort a while back, if you'll remember, so a
> > proliferation of calls is Bad.  In addition, the compatability required
> > this proliferation going forward.
>
> I suspect that this may end up being a red herring.  You do realize that
> OpenBSD and NetBSD have *already* made precisely the change I am
> describing?  Here's the exerpt from OpenBSD's ipc.h:
>
> struct ipc_perm {
>         uid_t           cuid;   /* creator user id */
>         gid_t           cgid;   /* creator group id */
>         uid_t           uid;    /* user id */
>         gid_t           gid;    /* group id */
>         mode_t          mode;   /* r/w permission */
>         unsigned short  seq;    /* sequence # (to generate unique
> msg/sem/shm id) */
>         key_t           key;    /* user specified msg/sem/shm key */
> };
>
> #ifdef _KERNEL
> struct oipc_perm {
>         unsigned short  cuid;   /* creator user id */
>         unsigned short  cgid;   /* creator group id */
>         unsigned short  uid;    /* user id */
>         unsigned short  gid;    /* group id */
>         unsigned short  mode;   /* r/w permission */
>         unsigned short  seq;    /* sequence # (to generate unique
> msg/sem/shm id) */
>         key_t           key;    /* user specified msg/sem/shm key */
> };
> #endif
>
> We're *improve* our compatibility with other BSD platforms by making these
> changes.
>
> > FWIW, I would agree with you, if you were advocating a cross-OS ABI
> > definition of manifest constants, so that FreeBSD's new ABI, and, say,
> > Solaris or Linux's ABI, were more congruent (you could get this without
> > a new sstem call vector entry point, by using a different "ELF brand"
> > value), but that's not what you are advocating.
>
> I'm advocating we adopt pretty much the ABI for these calls that Solaris
> uses.  If you inspect their ipc.h, you'll see that they define two
> versions of the structure, ipc_perm, and ipc_perm32.  ipc_perm is the
> version that makes use of types such as uid_t and gid_t; ipc_perm32
> hard-codes these fields to 32-bit and can be used for compatibility.
>
> > Also FWIW, my personal preference would be to follow the Solaris ABI;
> > it's the least volatile of all ABI's in this area.  In terms of the
> > available applications, Linux is probably a better choice, but in terms
> > of the amount of maintenance work required, Solaris is far and away the
> > leader.
>
> We already have the SysVIPC wrappers for the Linux code, but those are
> actually broken because they don't take into account our truncation bugs.
>
> Robert N M Watson             FreeBSD Core Team, TrustedBSD Projects
> robert@fledge.watson.org      Network Associates Laboratories
>
>
> To Unsubscribe: send mail to majordomo@FreeBSD.org
> with "unsubscribe freebsd-current" in the body of the message

To quote the POSIX standard:

The ipc_perm structure shall contain the following members:

uid_t    uid    Owner's user ID. 
gid_t    gid    Owner's group ID. 
uid_t    cuid   Creator's user ID. 
gid_t    cgid   Creator's group ID. 
mode_t   mode   Read/write permission. 

I don't think size is an issue.  At least not within a given machine.
I think OpenBSD and NetBSD have already taken the correct path.
We just need to have a compatibility interface (automatic extra
flag: pretty cheap; or alternate syscalls: probably a waste).

Danny J. Zerkel
dzerkel@columbus.rr.com

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-arch" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200210162344.22969.dzerkel>