Date: Wed, 16 Oct 2002 13:01:50 -0400 (EDT) From: Robert Watson <rwatson@FreeBSD.ORG> To: Terry Lambert <tlambert2@mindspring.com> Cc: Maxim Sobolev <sobomax@FreeBSD.ORG>, "Danny J. Zerkel" <dzerkel@columbus.rr.com>, "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: <Pine.NEB.3.96L.1021016125106.24763B-100000@fledge.watson.org> In-Reply-To: <3DAD97D1.DB378D6B@mindspring.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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-arch" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.NEB.3.96L.1021016125106.24763B-100000>