Unless I've missed it, there's no privlege dropping, and I'd like wget
to run as a normal user (specifically: nobody).  This should be easily
implemented, as the script is just writing to /tmp/.

Well, that's not entirely true. It's not writing to /tmp/, it's writing
to /tmp/msttcorefonts/, which is root.root 755.  If wget were to switch
uid to nobody, it would not be able to put its output in this directory.

I don't think it would be more secure to switch the download directory
be writable by user nobody.  This may open up some of this issues
raised in bug #98408, which was solved by making sure that only root
could write to the download directory.

Do you have a way around this?

Eric

Well, that's not entirely true. It's not writing to /tmp/, it's writing
to /tmp/msttcorefonts/, which is root.root 755.  If wget were to switch
uid to nobody, it would not be able to put its output in this directory.

I don't think it would be more secure to switch the download directory
be writable by user nobody.  This may open up some of this issues
raised in bug #98408, which was solved by making sure that only root
could write to the download directory.

Do you have a way around this?

Eric

Debian's tempfile(1) should do it.  tempfile -n $name will request a
name.  But it'd be better to not specify -n actually, because otherwise
you'd have a security race condition if you didn't deal with the return
value anyway.

file=`tempfile`;
wget -O $file;

Cheers,
Justin

Have you given any thought to this bug?  It seems like it would be
easy and desirable to avoid running wget as root.  Indeed, *two* of
the outstanding RC bugs in the months leading up to the etch release
were security problems with wget.

* Justin Pryzby

| Have you given any thought to this bug?  It seems like it would be
| easy and desirable to avoid running wget as root.  Indeed, *two* of
| the outstanding RC bugs in the months leading up to the etch release
| were security problems with wget.

If so, I don't think I would want it to run as nobody, but rather
having a special user created for this sole task, which seems rather
overkill.  Else, any other process running as nobody could subvert the
download and possibly make root run arbitrary code.

I don't suppose md5sums are available?  Does the file change
sufficiently often such that its not reasonable to hardcode an MD5?

Though I'm not sure what the requirements are for this kind of user,
is there any problem adding a user, running wget as that user, and
then removing the user?

Ah, maybe this bug is moot anyway.  I seem to recall a thread on LKML
(or was it a debian bug log?) where it was pointed out that there is
no way to drop privileges in such a way that you can't get them back
with seteuid(getsuid()).  In which case, this bug lies in your hands,
since I can't think of a clean way to have it run as a user which
cannot regain privileges.

* Justin Pryzby

| I don't suppose md5sums are available?  Does the file change
| sufficiently often such that its not reasonable to hardcode an MD5?

It doesn't change, and I have the hashes, so that might be a workable
approach.

| Ah, maybe this bug is moot anyway.  I seem to recall a thread on LKML
| (or was it a debian bug log?) where it was pointed out that there is
| no way to drop privileges in such a way that you can't get them back
| with seteuid(getsuid()).  In which case, this bug lies in your hands,
| since I can't think of a clean way to have it run as a user which
| cannot regain privileges.

setuid(2) seems to disagree with you (for suid root, and we wouldn't
be suid, we would be run by root).

Bah, I don't know.

Hows this?

  md5good="d41d8cd98f00b204e9800998ecf8427e" # NEEDS CHANGING
  f=`mktemp` || exit 1;
  su -c "wget -O $f http://..." nobody;
  echo "$md5good  $f" |md5sum -c || { echo >&2 $0: MD5Sum failed!!; exit 1; }

I don't know if it works in posix sh like posh or dash..

Can I expect to see this bug fixed for etch?

http://bugs.debian.org/248122

Justin

As root (assuming running with set -e):

  d=`mktemp -d`
  install -d -m 700 -o nobody "$d"/writable
  (cd "$d"/writable && su nobody -c 'wget ...')

User `nobody' can write into this `writable' directory, but only for a
process that has already cd'd into it as root before becoming nobody:
the "$d" directory is executable only by root.

pjrm.

I think it is intended that "nobody" never owns any files.  So the
right way to do it probably involves dynamically creating a user, or
using some user guaranteed to exist, like "sys" or "operator".

Justin

I believe that the reason for this is so that no `nobody'-owned process
can read/write non-world-accessible files other than its own.  The above
approach does achieve this result even though it does literally create a
file as `nobody': no other non-root process can access the file.

I believe that many things would benefit at least slightly in security
from such a facility: even things that don't need to read/write files
would still be less exposed to denial of service by being killed by
other nobody-owned processes.

However, in the short term, I suggest using the only-root-readable
directory approach: it already gets us most of the way there, avoiding
giving the wget executable privileges of an important user like
root (or sys or operator).

pjrm.

* Peter Moulder

| I believe that the reason for this is so that no `nobody'-owned process
| can read/write non-world-accessible files other than its own.  The above
| approach does achieve this result even though it does literally create a
| file as `nobody': no other non-root process can access the file.

That's not strictly true.  Another process running as nobody could
ptrace the wget process and do harm.

I'm going to make it download as nobody and check the md5sums as root
and throw a big warning if the md5sum doesn't match.  This should make
everybody happy, I just need the get a round tuit first.

Hi Justin,

No, I'm sorry it's too late for that in the release cycle now. I've only
just taken over the package and I'll make any non-trivial fixes only
after etch's release.


Thijs

Hi,

I've seen the discussion in this bug, and I wonder whether it makes sense
to actually go the way to drop these privileges. A user running apt-get
update or apt-get upgrade is already performing many HTTP requests and
downloading numerous files from relatively untrusted sources (they are
verified after downloading), as root.

Would it make sense to change msttcorefonts for this while an admin will
already be doing this with APT?


Thijs

Hi,

I will not fix this bug as per the reasoning in my previous message,
some months ago.


Thijs

Hi, Will you at least consider using checksums?

I still think running wget as root should be avoided.  The apt code
was written with this in mind, but wget is typically run by normal
users.

Anyway, I know more about setuid and friends now.  A root process can
(and will by default) change all its UIDs with setuid.  Using
setresuid you can control all the UIDs independantly.  Anyway
something like this should be sufficient.

f=`tempfile`
adduser mstwget
su -c "wget -O '$f'" mstwget
chown root:root "$f"
deluser msgwget
for m in $validmd5
do
	[ "$m" = "`md5sum "$f" |sed -e 's/ .*//'`" ] && break
done

[ $? -eq 0 ] || {
	echo "$0: error: md5sum failed to match any correct value"
	exit 1
} >&2

Justin

reopen 248122
tags 248122 wontfix
thanks

Because there's still discussion in this bug I'm reopening it, but
tagging it wontfix for now since that's my current opinion.

I don't understand... we're already using checksums to verify the
downloaded files. Or am I missing something?

this needs a `chown mstwget $f` I think.

I'm not opposed to running wget non-privileged in principle, but am
concerned about extra fragility introduced in the process, offset
against the risk that running wget as root could bring. Since wget is
very widely used, and also in quite some packages as root automatically,
I think the risk of wget being faulty is relatively small.

msttcorefonts is not the only 'downloader' package. What I think would
be a nice solution, is if some system is developed that serves as common
downloader-package code, and that code is used by the downloading
packages. While I don't feel like maintaining my own system for
privilege dropping when other packages are not doing that, or doing it
differently, I am open to use a system as long as this is common for all
downloaders, hence increasing the testing and reducing duplication.

Maybe some kind of "downloaders-common" like "dbconfig-common"?


Thijs

Great; I didn't realize that this was implemented.  It seems that the
md5sum happens only for the extracted files; shouldn't it happen for
the originally downloaded file?
Yea, prolly; it wasn't actually meant to be working code, but I got
carried away.
Do you know where it's run as root?  This might be a nonissue if it's
really a common assumption.
This would be neat indeed.

Justin

Ah, in that sense. It's only checked currently when (re)using previously
downloaded files. I'll see whether I can easily expand that to checking
the .cabs before extracting them, which sounds like a good feature.

Since the package is undergoing some real work now, I think this can be
included before the next upgrade.

From the top of my head at least flashplugin-nonfree and quake2-data.
But I really doubt that those are the only other ones. Maybe a package
search on "downloader" would yield some more results.


Thijs

Hi Thijs,

I'm sorry to resurrect this from the dead. I came across this bug
looking for something completely different...

APT uses its own much smaller special-purpose HTTP implementation. It
also spawns a sub-process just for the HTTP method which I think used to
run as an unprivileged user. On a jessie system the latter doesn't
currently happen any more but that would be a bug in APT.

As for msttcorefonts, a straightforward approach would be to have wget
output to stdout and avoid file system access by wget altogether:

# su - wgetuser -c "wget -O - $url/$file" > ./$file

I haven't tested it but this should run wget as wgetuser yet write to
./$file as root while the destination path is controlled by the shell
not wget.

Cheers,

Rene

(reposting, I think my last comment may have been blocked for using a fake email)

Hi,

I too noticed that wget STILL runs as root:root.
Nothing with root privileges should ever connect to the internet unless absolutely necessary.

Fixing this is as simple as changing two lines in ttf-mscorefonts-installer.postinst:

	SCRATCHDIR=`mktemp -t -d ttf-mscorefonts-installer.XXXXXX
becomes
	SCRATCHDIR=`sudo -u _apt mktemp -t -d ttf-mscorefonts-installer.XXXXXX

and
	if ! wget --continue --tries=1 --connect-timeout=60 [...]
becomes
	if ! sudo -u _apt wget --continue --tries=1 --connect-timeout=60 [...]

I don't know if _apt user is standard debian or just part of the distro I'm using.
sudo -u nobody worked just as well.

This is a serious and completely unnecessary security threat where a remote code execution vulnerability could get an attacker full root access. Why???