On Fri, Jul 07, 2006 at 11:32:43PM +0200, Marco d'Itri wrote: > FYI, I do not feel like working on this now. So if you want to see > progress before a few months, I suggest you debug it by yourself.
OK, I've figured out what's going on, I think...
hosts_access.c matches the entries in /etc/hosts.{allow,deny} against
the name of the daemon running the service. This daemon name is
derived in tcpd.c by truncating argv[0] at the final / .
In the case of dovecot, the daemon responding to the incoming imap
(or pop3) connection is named "imap-login" (or "pop3-login").
Therefore, unless the entries in /etc/hosts.{allow,deny} use the
daemon name "imap-login" instead of the obvious "imap" or "imap2" that
one might deduce from reading /etc/services, they will not match the
daemon name provided by tcpd.c.
My symlink workaround works because it effectively renames the daemon
executable to "imap" and therefore it does match the entries in
/etc/hosts.{allow,deny}. However, as I later found, it is still not a
complete solution as it does not produce the required result where
IMAP-over-SSL is concerned. dovecot uses the same daemon to handle
both IMAP and IMAP-over-SSL connections, passing it the "--ssl"
parameter in the case of an IMAP-over-SSL connection. Since the daemon
name is the same in both cases, permissions for both services are set
by the entry for the plain IMAP service. It is not possible to, for
instance, set more restrictive permissions for IMAP and less
restrictive ones for IMAP-over-SSL.
Similar considerations apply to dovecot's secure and insecure pop3
services, or indeed to any package which uses the same daemon to
provide more than one service according to a command-line parameter or
the port of the incoming connection or whatever.
It is not possible to extend my symlink workaround to cater properly
for IMAP-over-SSL by creating another symlink named "imaps" without
modifying dovecot, because dovecot works out what service to provide
based on the stem of the daemon name, and only recognises "imap" and
"pop3". The workaround also completely fails to address the problem of
what happens if any other packages behave similarly to dovecot. IIRC
the courier-imap daemon has a not dissimilar problem.
Now, there is much generic Linux documentation on the net that says
that the daemon names in /etc/hosts.{allow,deny} should correspond to
those in /etc/services. Certainly the dovecot wiki entry I referred to
seems to be written on that assumption, and there are many other
documents written similarly. However, none of the Debian-specific
documentation links the daemon names in /etc/hosts.{allow,deny} and
/etc/services, and grepping the tcp-wrappers source code reveals no
references to /etc/services, or to getservbyport() or its relations.
But there is nothing in the Debian changelogs to indicate that support
for /etc/services has been removed from the Debian package for any
reason - no indication as to why the Debian package doesn't behave as
the generic, non-Debian-specific information suggests it might.
Since I couldn't find any reason for it not to be there, and if it was
there it would fix the problem, I decided to add support for
/etc/services to tcpd.
I modified tcpd.h to include an extra member,
"struct servent service_entry", to struct request_info, along with a
"#define RQ_SERVENT 10" for use wht request_init() and request_set().
I modified update.c to support updating the service_entry member of
struct request_info from the struct servent * returned by
getservbyfoo().
I modified hosts_access.3 to describe this extra member.
I modified tcpd.c to fill in the extra member with information about
the incoming connection read from /etc/services using getservbyport().
I modified hosts_access.c to match the server daemon names from
/etc/hosts.{allow,deny} against the daemon name and any aliases
obtained from /etc/services in addition to argv[0]. If the daemon name
from /etc/hosts.{allow,deny} matches argv[0] OR the daemon name from
/etc/services OR any aliases from /etc/services, a positive match is
returned. If the /etc/services data is not present, it simply behaves
as it did before.
I modified debian/rules as a workaround to allow me to experiment with
building the modified package - see below. This modification is not
part of the bug fix but it is included in my patch - again see below.
I encountered a certain amount of difficulty in experimentally
building the package because of the awkward tarball format it's in.
The usual "dpkg-buildpackage -b -rfakeroot -us -uc" incantation is no
good because it unpacks the secondary tarballs all over again and
wipes out the changes I've just made. "fakeroot debian/rules build"
doesn't work either because the "build" target depends only on
"unpack" - if the secondary tarballs have already been unpacked then
"make" reports "Nothing to do for build.", with the result that
"fakeroot debian/rules build" only does something the first time you
run it. I confess that picking through nested complex Makefiles tends
to melt my brain, so my solution was perhaps not entirely elegant:
- Unpack the source (primary stage) with dpkg-source -x.
- cd to the source directory.
- Do an initial build of the unmodified package, which unpacks the
secondary tarballs and has the useful side effect of confirming that
everything's OK as regards building the unmodified package.
- Modify the source code.
- In debian/rules, comment out the dependency of "build" on "unpack",
so that "fakeroot debian/rules build" always builds the
already-unpacked package (but does not check it's unpacked first,
which doesn't matter in this particular case). This is the
modification referred to above.
- Run "fakeroot debian/rules build" to build the modified package.
- Run "fakeroot debian/rules binary-arch" to build the .debs.
This awkwardness makes me somewhat unsure how to submit my patch... I
can't submit it as a patch against the raw source tree of an unpacked
package because the "raw source tree" as produced by "dpkg-source -x"
doesn't have any source code in it, only secondary tarballs. So I've
submitted it as a patch that works in the "Modify the source code"
stage of the above-described procedure, along with a very minimal
script to automatically build a patched package. I would appreciate
some guidance on what I should have done :-)
--
Pigeon
Be kind to pigeons
Get my GPG key here:
http://pgp.mit.edu:11371/pks/lookup?op=get&search=0x21C61F7F
diff -Nru tcp-wrappers-7.6.dbs/build-tree/tcp_wrappers_7.6/hosts_access.3
tcp-wrappers-7.6.dbs.modified/build-tree/tcp_wrappers_7.6/hosts_access.3
--- tcp-wrappers-7.6.dbs/build-tree/tcp_wrappers_7.6/hosts_access.3
2006-07-10 05:11:05.000000000 +0100
+++ tcp-wrappers-7.6.dbs.modified/build-tree/tcp_wrappers_7.6/hosts_access.3
2006-07-10 04:19:12.000000000 +0100
@@ -57,6 +57,10 @@
The name of the daemon process running on the server host.
.IP "RQ_USER (char *)"
The name of the user on whose behalf the client host makes the request.
+.IP "RQ_SERVENT (struct servent *)"
+The daemon name and aliases associated with the server port and socket
+type (tcp or udp) as listed in /etc/services. This is filled in
+automatically if it is empty when hosts_access() is called.
.PP
hosts_access() consults the access control tables described in the
\fIhosts_access(5)\fR manual page. When internal endpoint information
diff -Nru tcp-wrappers-7.6.dbs/build-tree/tcp_wrappers_7.6/hosts_access.c
tcp-wrappers-7.6.dbs.modified/build-tree/tcp_wrappers_7.6/hosts_access.c
--- tcp-wrappers-7.6.dbs/build-tree/tcp_wrappers_7.6/hosts_access.c
2006-07-10 05:11:06.000000000 +0100
+++ tcp-wrappers-7.6.dbs.modified/build-tree/tcp_wrappers_7.6/hosts_access.c
2006-07-10 05:07:56.000000000 +0100
@@ -28,9 +28,9 @@
typedef uint32_t u_int32_t;
#endif
#include <sys/param.h>
-#ifdef INET6
+/*#ifdef INET6*/
#include <sys/socket.h>
-#endif
+/*#endif*/
#include <netinet/in.h>
#include <arpa/inet.h>
#include <stdio.h>
@@ -39,9 +39,9 @@
#include <errno.h>
#include <setjmp.h>
#include <string.h>
-#ifdef INET6
+/*#ifdef INET6*/
#include <netdb.h>
-#endif
+/*#endif*/
extern char *fgets();
extern int errno;
@@ -125,13 +125,18 @@
if (resident <= 0)
resident++;
+
verdict = setjmp(tcpd_buf);
+
if (verdict != 0)
return (verdict == AC_PERMIT);
+
if (table_match(hosts_allow_table, request))
return (YES);
+
if (table_match(hosts_deny_table, request) == NO)
return (YES);
+
return (NO);
}
@@ -171,7 +176,7 @@
}
sh_cmd = split_at(cl_list, ':');
match = list_match(sv_list, request, server_match)
- && list_match(cl_list, request, client_match);
+ && list_match(cl_list, request, client_match);
}
(void) fclose(fp);
} else if (errno != ENOENT) {
@@ -223,6 +228,23 @@
return (NO);
}
+/* server_match_aliases - match ssrver information including
+ * daemon executable name, and service name and aliases from /etc/services */
+
+static int server_match_aliases(char *tok, struct request_info *request) {
+ int x;
+ int result;
+
+ result = string_match(tok, eval_daemon(request));
+ if (request->service_entry.s_name) {
+ result |= string_match(tok, request->service_entry.s_name);
+ for (x=0;request->service_entry.s_aliases[x];x++) {
+ result |= string_match(tok,
request->service_entry.s_aliases[x]);
+ }
+ }
+ return(result);
+}
+
/* server_match - match server information */
static int server_match(tok, request)
@@ -232,9 +254,11 @@
char *host;
if ((host = split_at(tok + 1, '@')) == 0) { /* plain daemon */
- return (string_match(tok, eval_daemon(request)));
+/* return (string_match(tok, eval_daemon(request)));*/
+ return (server_match_aliases(tok, request));
} else { /* [EMAIL PROTECTED] */
- return (string_match(tok, eval_daemon(request))
+/* return (string_match(tok, eval_daemon(request))*/
+ return (server_match_aliases(tok, request)
&& host_match(host, request->server));
}
}
diff -Nru tcp-wrappers-7.6.dbs/build-tree/tcp_wrappers_7.6/tcpd.c
tcp-wrappers-7.6.dbs.modified/build-tree/tcp_wrappers_7.6/tcpd.c
--- tcp-wrappers-7.6.dbs/build-tree/tcp_wrappers_7.6/tcpd.c 2006-07-10
05:11:05.000000000 +0100
+++ tcp-wrappers-7.6.dbs.modified/build-tree/tcp_wrappers_7.6/tcpd.c
2006-07-10 05:06:46.000000000 +0100
@@ -47,6 +47,11 @@
{
struct request_info request;
char path[MAXPATHNAMELEN];
+ int port;
+ int conntype;
+ char *conntype_str;
+ int x;
+ struct servent *serv_entry;
/* Attempt to prevent the creation of world-writable files. */
@@ -114,6 +119,34 @@
*/
#ifdef HOSTS_ACCESS
+
+ /*
+ * Get information about the request from /etc/aliases
+ */
+
+ /* Determine whether connection is tcp, udp or unknown */
+ x=sizeof(conntype);
+ if (getsockopt(request.fd, SOL_SOCKET, SO_TYPE, &conntype, &x)==0) {
+ switch (conntype) {
+ case SOCK_STREAM: conntype_str="tcp";
+ break;
+ case SOCK_DGRAM: conntype_str="udp";
+ break;
+ default: conntype_str=NULL;
+ break;
+ }
+ } else {
+ conntype_str=NULL;
+ }
+
+ /* Read /etc/services entry for this port and connection type */
+ if (conntype_str) {
+ port=((struct sockaddr_in *)(request.server->sin))->sin_port;
+ if ((serv_entry=getservbyport(port,conntype_str))!=NULL) {
+ request_set(&request, RQ_SERVENT, serv_entry, 0);
+ }
+ }
+
if (!hosts_access(&request))
refuse(&request);
#endif
diff -Nru tcp-wrappers-7.6.dbs/build-tree/tcp_wrappers_7.6/tcpd.h
tcp-wrappers-7.6.dbs.modified/build-tree/tcp_wrappers_7.6/tcpd.h
--- tcp-wrappers-7.6.dbs/build-tree/tcp_wrappers_7.6/tcpd.h 2006-07-10
05:11:05.000000000 +0100
+++ tcp-wrappers-7.6.dbs.modified/build-tree/tcp_wrappers_7.6/tcpd.h
2006-07-10 04:19:12.000000000 +0100
@@ -11,6 +11,9 @@
#include <netinet/in.h>
#include <stdio.h>
+/* Also need definition of struct servent */
+#include <netdb.h>
+
__BEGIN_DECLS
/* Structure to describe one communications endpoint. */
@@ -43,6 +46,7 @@
void (*hostaddr) (struct host_info *); /* address to printable address */
void (*cleanup) (struct request_info *); /* cleanup function or 0 */
struct netconfig *config; /* netdir handle */
+ struct servent service_entry;
};
/* Common string operations. Less clutter should be more readable. */
@@ -134,6 +138,7 @@
#define RQ_SERVER_NAME 7 /* server host name */
#define RQ_SERVER_ADDR 8 /* server host address */
#define RQ_SERVER_SIN 9 /* server endpoint (internal) */
+#define RQ_SERVENT 10 /* data from /etc/services */
/*
* Routines for delayed evaluation of request attributes. Each attribute
diff -Nru tcp-wrappers-7.6.dbs/build-tree/tcp_wrappers_7.6/update.c
tcp-wrappers-7.6.dbs.modified/build-tree/tcp_wrappers_7.6/update.c
--- tcp-wrappers-7.6.dbs/build-tree/tcp_wrappers_7.6/update.c 2006-07-10
05:11:05.000000000 +0100
+++ tcp-wrappers-7.6.dbs.modified/build-tree/tcp_wrappers_7.6/update.c
2006-07-10 04:19:12.000000000 +0100
@@ -22,6 +22,7 @@
#include <stdio.h>
#include <syslog.h>
#include <string.h>
+#include <netdb.h>
/* Local stuff. */
@@ -42,6 +43,12 @@
default:
tcpd_warn("request_fill: invalid key: %d", key);
return (request);
+
+ case RQ_SERVENT:
+ memcpy(&(request->service_entry), va_arg(ap, struct servent *),
+ sizeof(struct servent));
+ continue;
+
case RQ_FILE:
request->fd = va_arg(ap, int);
continue;
@@ -95,7 +102,7 @@
static struct request_info default_info;
struct request_info *r;
va_list ap;
-
+
/*
* Initialize data members. We do not assign default function pointer
* members, to avoid pulling in the whole socket module when it is not
diff -Nru tcp-wrappers-7.6.dbs/debian/rules
tcp-wrappers-7.6.dbs.modified/debian/rules
--- tcp-wrappers-7.6.dbs/debian/rules 2006-07-10 05:10:57.000000000 +0100
+++ tcp-wrappers-7.6.dbs.modified/debian/rules 2006-07-10 04:19:12.000000000
+0100
@@ -38,7 +38,7 @@
touch $@
build: $(STAMP_DIR)/build
-$(STAMP_DIR)/build: $(STAMP_DIR)/unpack
+#$(STAMP_DIR)/build: $(STAMP_DIR)/unpack
dh_testdir
$(MAKE) -f debian/sys-build.mk source.command SOURCE_CMD=" \
#!/bin/bash
dpkg-source -x tcp-wrappers_7.6.dbs-8.dsc
cd tcp-wrappers-7.6.dbs
fakeroot debian/rules build
for y in '*.a' '*.o' '*.so' '*.so.*' 'access*.gz'; do
for x in `find . -name "$y"`; do
rm $x
done
done
cd ..
patch -p0 < search.etc.services.patch
cd tcp-wrappers-7.6.dbs
fakeroot debian/rules build
fakeroot debian/rules binary-arch
cd ..
signature.asc
Description: Digital signature
