[
https://issues.apache.org/jira/browse/TS-165?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12833621#action_12833621
]
Leif Hedstrom commented on TS-165:
----------------------------------
So, this is similar to what we talked about on the IRC chat, but I'm not sure I
understand the need for testing on
+#ifdef _POSIX_SAVED_ID
what platform(s) that we support would not have seteuid ? If this is a "just in
case" check, I'd rather just get rid of it for now, and/or make tests for
seteuid() vs setreuid() in configure.ac. I guess what I'm really saying is,
what is the relationship between this posix define, and the existence of
seteuid() (those seem orthogonal to me, unless there's something else
overloaded in the POSIX_SAVED_ID define)?
I'm also not sure I see the need for doing it this "complex", those two
functions really aren't what their names indicate any more. removeRootPriv now
is really just a wrapper over seteuid and/or setreuid(). If we are going this
route, just eliminate these functions completely, and just call seteuid()
directly wherever we need to 'restore' the euid.
I guess I had imagined something just as simple as the below, which preserves
the semantics of those two existing functions. But, I'm OK with the above, but
my votes would be for a) Just call seteuid() always, and b) eliminate the
'wrapper' function and just call seteuid() directly.
-- Leif
Index: proxy/mgmt2/LocalManager.cc
===================================================================
--- proxy/mgmt2/LocalManager.cc (revision 909713)
+++ proxy/mgmt2/LocalManager.cc (working copy)
@@ -1323,15 +1323,16 @@ LocalManager::listenForProxy()
// - Returns true on success
// and false on failure
// - no-op on WIN32
+static uid_t saved_euid = 0;
bool
removeRootPriv(void)
{
- if (seteuid(getuid()) < 0) {
- Debug("lm", "[restoreRootPiv] seteuid root failed : %s\n",
strerror(errno));
+ if (seteuid(saved_euid) < 0) {
+ Debug("lm", "[restoreRootPriv] seteuid failed : %s\n", strerror(errno));
return false;
}
- Debug("lm", "[removeRootPiv] removed root privileges. Euid is %d\n",
geteuid());
+ Debug("lm", "[removeRootPriv] removed root privileges. Euid is %d\n",
saved_euid);
return true;
}
@@ -1345,12 +1346,13 @@ removeRootPriv(void)
bool
restoreRootPriv(void)
{
+ saved_euid = geteuid();
if (seteuid(0) < 0) {
- Debug("lm", "[restoreRootPiv] seteuid root failed : %s\n",
strerror(errno));
+ Debug("lm", "[restoreRootPriv] seteuid root failed : %s\n",
strerror(errno));
return false;
}
- Debug("lm", "[restoreRootPiv] restored root privileges. Euid is %d\n",
geteuid());
+ Debug("lm", "[restoreRootPriv] restored root privileges. Euid is %d\n",
geteuid());
return true;
}
Index: proxy/mgmt2/RecordsConfig.cc
> Config files (records.config at least) can get wrong ownership
> --------------------------------------------------------------
>
> Key: TS-165
> URL: https://issues.apache.org/jira/browse/TS-165
> Project: Traffic Server
> Issue Type: Bug
> Components: Config
> Reporter: Leif Hedstrom
> Assignee: George Paul
> Priority: Critical
> Fix For: 2.0.0a
>
> Attachments: 0001-TS165_patch1.diff.patch
>
>
> With the following steps, I get records.config to become owned by root, when
> it should stay owned by nobody:
> (04:43:38 PM) zwoop: Ok, so this reproduces it every time on my fedora box,
> gonna try it on ubuntu next
> (04:44:41 PM) zwoop: This is what I did
> (04:44:41 PM) zwoop: 1) rm -rf local
> (04:44:41 PM) zwoop: 2) sudo gmake install
> (04:44:41 PM) zwoop: 3) emacs local/etc/trafficserver/records.config
> (04:44:41 PM) zwoop: change port from 8080 to 80, and change eth0 to
> eth1 (I have to do the later, or it'll fail)
> (04:44:41 PM) zwoop: 4) local/bin/trafficserver start
> (04:44:41 PM) zwoop: 5) Wait 10-20 seconds (at least, maybe longer)
> (04:44:41 PM) zwoop: 6) ls -lrt local/etc/trafficserver
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
