tags 314949 patch upstream
thanks
On Tue, Jun 21, 2005 at 11:32:31AM +0200, Thomas Hood wrote:
> On Sun, Jun 19, 2005 at 05:50:42PM +0200, Thomas Hood wrote:
> > I get an error message from sudo even though I have
> >
> > Defaults !fqdn
> >
> > in /etc/sudoers. If I change this to
> >
> > Defaults fqdn
> >
> > then I get the error message twice.
>
> I'm not familiar with sudo's code but at the first glance it seems that
> all calls to gethostbyname() are protected by the FQDN flag. One
> possibility I can think of is that the config file is parsed _after_ the
> first hostname check but I do not have enough time to check that.
Well, the config file and the file having hostname-restricted
definitions for sudo rights is one and the same file: /etc/sudoers. The
current logic is simply to query the FQDN always
(./configure-controlled), and only later read /etc/sudoers. Disabling
fqdn in that file will prevent any second and further query (it is not
remembered whether the FQDN has already been queried at all, either).
Right, so this logic needs a serious recosideration. Please find
attached a patch which changes the logic as follows:
- Querying of the FQDN will happen at most once, by remembering whether
set_fqdn() has been succesfully called already
- Every time a sudoers line is encountered which involves a hostname (so
not 'ALL' or an IP address), the current configuration status is
consulted (default is still 'true', but Defaults !fqdn might have
disabled it), and a FQDN is performed if it hasn't been done already
and the configuration allows it
- If the 'fqdn' flag is explicitely set in /etc/sudoers, a FQDN lookup
is forced if none has been done yet (this code was already
present).[1]
As a result, this means that no DNS query is done by default, and no DNS
query is done at all as long as you don't have any non-ALL/non-IP
'hosts' statements. Note that my patch also does a FQDN if an alias is
used, regardless whether that's needed.
[1] For logging, in the logfile it is tried to use the full hostname.
This is a trade-off, a lot of error conditions can happen before
defaults line are encountered. I don't think logging should be with
FQDN, no other deamon/service does that. For people who want FQDN
logging, I left the 'force FQDN lookup' upon setting the fqdn
option. There is now no way anymore to get FQDN-enhanced logging
before the first line of /etc/sudoers is parsed.
Note that I didn't update documentation, that still needs to happen, but
I've spent enough time right now on this patch to not do it right now,
anyone who reads this is invited to give it go, otherwise, I'll do it
later.
Please don't forget to re-yacc (byacc seems to have been used, not
bison).
--Jeroen
--
Jeroen van Wolffelaar
[EMAIL PROTECTED]
http://jeroen.A-Eskwadraat.nl
diff -ur sudo-1.6.8p12.old/parse.yacc sudo-1.6.8p12/parse.yacc
--- sudo-1.6.8p12.old/parse.yacc 2005-06-19 18:24:32.000000000 +0000
+++ sudo-1.6.8p12/parse.yacc 2006-03-31 20:41:36.000000000 +0000
@@ -396,6 +396,7 @@
free($1);
}
| NETGROUP {
+ set_fqdn();
if (netgr_matches($1, user_host, user_shost, NULL))
$$ = TRUE;
else
@@ -403,6 +404,7 @@
free($1);
}
| WORD {
+ set_fqdn();
if (hostname_matches(user_shost, user_host, $1) ==
0)
$$ = TRUE;
else
@@ -412,6 +414,7 @@
| ALIAS {
aliasinfo *aip = find_alias($1, HOST_ALIAS);
+ set_fqdn();
/* could be an all-caps hostname */
if (aip)
$$ = aip->val;
diff -ur sudo-1.6.8p12.old/sudo.c sudo-1.6.8p12/sudo.c
--- sudo-1.6.8p12.old/sudo.c 2005-06-19 20:35:46.000000000 +0000
+++ sudo-1.6.8p12/sudo.c 2006-03-31 21:12:45.000000000 +0000
@@ -510,22 +510,18 @@
* "host" is the (possibly fully-qualified) hostname and
* "shost" is the unqualified form of the hostname.
*/
+ sudo_user.host_fqdn_queried = FALSE;
nohostname = gethostname(thost, sizeof(thost));
if (nohostname)
user_host = user_shost = "localhost";
else {
user_host = estrdup(thost);
- if (def_fqdn) {
- /* Defer call to set_fqdn() until log_error() is safe. */
- user_shost = user_host;
+ if ((p = strchr(user_host, '.'))) {
+ *p = '\0';
+ user_shost = estrdup(user_host);
+ *p = '.';
} else {
- if ((p = strchr(user_host, '.'))) {
- *p = '\0';
- user_shost = estrdup(user_host);
- *p = '.';
- } else {
- user_shost = user_host;
- }
+ user_shost = user_host;
}
}
@@ -566,12 +562,12 @@
/* It is now safe to use log_error() and set_perms() */
- if (def_fqdn)
- set_fqdn(); /* may call log_error() */
-
if (nohostname)
log_error(USE_ERRNO|MSG_ONLY, "can't get hostname");
+ /* We don't query FQDN yet, it might get disabled later. Querying is done
+ * when host matching is executed and def_fqdn still true */
+
set_runaspw(*user_runas); /* may call log_error() */
if (*user_runas[0] == '#' && runas_pw->pw_name && runas_pw->pw_name[0])
*user_runas = estrdup(runas_pw->pw_name);
@@ -1010,6 +1006,11 @@
struct hostent *hp;
char *p;
+ if (!def_fqdn || sudo_user.host_fqdn_queried) {
+ /* Only querying just once is good enough */
+ return;
+ }
+
if (!(hp = gethostbyname(user_host))) {
log_error(MSG_ONLY|NO_EXIT,
"unable to lookup %s via gethostbyname()", user_host);
@@ -1026,6 +1027,7 @@
} else {
user_shost = user_host;
}
+ sudo_user.host_fqdn_queried = TRUE;
}
/*
diff -ur sudo-1.6.8p12.old/sudo.h sudo-1.6.8p12/sudo.h
--- sudo-1.6.8p12.old/sudo.h 2005-03-23 23:44:46.000000000 +0000
+++ sudo-1.6.8p12/sudo.h 2006-03-31 20:32:21.000000000 +0000
@@ -42,6 +42,7 @@
char cwd[PATH_MAX];
char *host;
char *shost;
+ int host_fqdn_queried;
char **runas;
char *prompt;
char *cmnd;
diff -ur sudo-1.6.8p12.old/sudo.tab.c sudo-1.6.8p12/sudo.tab.c
