Hello,
I received feedback from deraadt that the first two unveil(2) calls were
unnecessary because pledge(2) automatically unveils "/usr/share/zoneinfo".
This updated patch removes the unneeded unveil(2) calls.
Best regards,
Stephen
---
usr.sbin/dhcpd/confpars.c | 41 ++++++++++++++++++++++++++-------------
usr.sbin/dhcpd/db.c | 19 ++++++++++++++++++
usr.sbin/dhcpd/dhcpd.c | 30 ++++++++++++++++++++++++++--
usr.sbin/dhcpd/dhcpd.h | 2 ++
4 files changed, 76 insertions(+), 16 deletions(-)
diff --git usr.sbin/dhcpd/confpars.c usr.sbin/dhcpd/confpars.c
index 1bdffb38842..bb245fc663e 100644
--- usr.sbin/dhcpd/confpars.c
+++ usr.sbin/dhcpd/confpars.c
@@ -57,6 +57,8 @@
#include "dhctoken.h"
#include "log.h"
+FILE *db_file_ro;
+
int parse_cidr(FILE *, unsigned char *, unsigned char *);
/*
@@ -106,18 +108,11 @@ readconf(void)
}
/*
- * lease-file :== lease-declarations EOF
- * lease-statments :== <nil>
- * | lease-declaration
- * | lease-declarations lease-declaration
+ * Open and retain a read-only file descriptor to the lease database file.
*/
void
-read_leases(void)
+open_leases(void)
{
- FILE *cfile;
- char *val;
- int token;
-
new_parse(path_dhcpd_db);
/*
@@ -131,23 +126,40 @@ read_leases(void)
* thinking that no leases have been assigned to anybody, which
* could create severe network chaos.
*/
- if ((cfile = fopen(path_dhcpd_db, "r")) == NULL) {
+ if ((db_file_ro = fopen(path_dhcpd_db, "r")) == NULL) {
log_warn("Can't open lease database (%s)", path_dhcpd_db);
log_warnx("check for failed database rewrite attempt!");
log_warnx("Please read the dhcpd.leases manual page if you");
fatalx("don't know what to do about this.");
}
+}
+
+/*
+ * lease-file :== lease-declarations EOF
+ * lease-statments :== <nil>
+ * | lease-declaration
+ * | lease-declarations lease-declaration
+ */
+void
+read_leases(void)
+{
+ char *val;
+ int token;
+
+ if (!db_file_ro) {
+ fatalx("db_file_ro is NULL");
+ }
do {
- token = next_token(&val, cfile);
+ token = next_token(&val, db_file_ro);
if (token == EOF)
break;
if (token != TOK_LEASE) {
log_warnx("Corrupt lease file - possible data loss!");
- skip_to_semi(cfile);
+ skip_to_semi(db_file_ro);
} else {
struct lease *lease;
- lease = parse_lease_declaration(cfile);
+ lease = parse_lease_declaration(db_file_ro);
if (lease)
enter_lease(lease);
else
@@ -155,7 +167,8 @@ read_leases(void)
}
} while (1);
- fclose(cfile);
+ fclose(db_file_ro);
+ db_file_ro = NULL;
}
/*
diff --git usr.sbin/dhcpd/db.c usr.sbin/dhcpd/db.c
index 295e522b1ce..634ec8a593a 100644
--- usr.sbin/dhcpd/db.c
+++ usr.sbin/dhcpd/db.c
@@ -190,6 +190,16 @@ commit_leases(void)
return (1);
}
+/*
+ * Open and retain two file descriptors to the lease database file:
+ *
+ * - A read-only fd for learning existing leases
+ * - A write-only fd for writing new leases
+ *
+ * Reading and parsing data from the read-only fd is done separately.
+ * This allows privilege drop to happen *before* parsing untrusted
+ * client data from the lease database file.
+ */
void
db_startup(void)
{
@@ -202,6 +212,15 @@ db_startup(void)
if ((db_file = fdopen(db_fd, "w")) == NULL)
fatalx("Can't fdopen new lease file!");
+ open_leases();
+}
+
+/*
+ * Read and parse the existing leases from the lease database file.
+ */
+void
+db_parse(void)
+{
/* Read in the existing lease file... */
read_leases();
time(&write_time);
diff --git usr.sbin/dhcpd/dhcpd.c usr.sbin/dhcpd/dhcpd.c
index b3562dce34f..7759f7839e0 100644
--- usr.sbin/dhcpd/dhcpd.c
+++ usr.sbin/dhcpd/dhcpd.c
@@ -244,8 +244,6 @@ main(int argc, char *argv[])
icmp_startup(1, lease_pinged);
- if (chroot(pw->pw_dir) == -1)
- fatal("chroot %s", pw->pw_dir);
if (chdir("/") == -1)
fatal("chdir(\"/\")");
if (setgroups(1, &pw->pw_gid) ||
@@ -253,6 +251,34 @@ main(int argc, char *argv[])
setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid))
fatal("can't drop privileges");
+ /*
+ * The lease database parsing logic needs the zoneinfo files
+ * for the lease begin / end date strings. These files are
+ * automatically unveiled by pledge(2). It also opens the
+ * /etc/ethers file using ether_hostton(3).
+ */
+ if (unveil("/etc/ethers", "r") == -1)
+ err(1, "unveil /etc/ethers");
+ if (unveil(NULL, NULL) == -1)
+ err(1, "unveil");
+
+ /*
+ * Apply initial pledge(2) promises - we will drop "rpath" after
+ * parsing the lease database (we need it for ether_hostton(3)).
+ */
+ if (udpsockmode) {
+ if (pledge("rpath stdio inet route sendfd", NULL) == -1)
+ err(1, "pledge init");
+ } else {
+ if (pledge("rpath stdio inet sendfd", NULL) == -1)
+ err(1, "pledge init");
+ }
+
+ db_parse();
+
+ /*
+ * Drop the "rpath" promise.
+ */
if (udpsockmode) {
if (pledge("stdio inet route sendfd", NULL) == -1)
err(1, "pledge");
diff --git usr.sbin/dhcpd/dhcpd.h usr.sbin/dhcpd/dhcpd.h
index 0903c244ddd..8c52b6603fc 100644
--- usr.sbin/dhcpd/dhcpd.h
+++ usr.sbin/dhcpd/dhcpd.h
@@ -343,6 +343,7 @@ int peek_token(char **, FILE *);
/* confpars.c */
int readconf(void);
+void open_leases(void);
void read_leases(void);
int parse_statement(FILE *, struct group *, int, struct host_decl *, int);
void parse_allow_deny(FILE *, struct group *, int);
@@ -489,6 +490,7 @@ char *piaddr(struct iaddr);
int write_lease(struct lease *);
int commit_leases(void);
void db_startup(void);
+void db_parse(void);
void new_lease_file(void);
/* packet.c */
