Hello tech@!
As per Theo's request I'm sending both patches attached below for spamd(8) and
spamd-setup(8) in order to pledge them and for spamd(8) I will just transcribe
the same text I sent before, but with an addition spotted by himself after I
ran a patch on libc.
spamd(8):
Hoist the sync_init() part to the top since it calls a setsockopt(2) forbidden
by pledge(2). Then in the default mode (greylisting) spamd(8) needs
"rpath wpath" to at least open "/dev/pf", "flock" to lock the database
/var/db/spamd each time it needs to update it, "proc id" for chroot(2) and
the privdrop section. Finally it also needs "exec" since it will call pfctl(8)
to update the tables with the information it gets from the database. If not in
greylist mode (blacklist) then it only needs "rpath proc id" to chroot(2) and
privdrop. Both modes always needs "inet", and also "getpw" due to YP
environments.
After privdrop the child only needs "stdio inet" since it only accepts
connections and makes decisions on that, although doesn't seem to need more
than "inet" for that.
spamd-setup(8):
Now this one will really need someone to give an extra careful look at it since
I may, I'm almost 100% sure I did, have screw up somewhere big time. So please
review it and let me know where I got it wrong and why so I don't make the same
mistakes again (I'm still trying to revive my poor C skills from last century,
and learning new stuff as I go along, so be patient with me, although you can
still beat me with the clue stick).
First, currently the parsing of PATH_SPAMD_CONF file is done inside a while
loop and at the same time it proceeds with the loading of the black/whitelists
in the same step. In order to pledge it correctly I separated this logic by
first parsing the entire file and only afterwards getting the black/whitelists
and sending them to pfctl(8).
I created a new struct called conffile with members capability, name, message,
method and file (see spamd.conf(5)), then I basically took out all the code
referring to the parsing of the file that was within getlist() (emulating as
much as possible of code that was there) and put it in a new function called
parsefile() which parses PATH_SPAMD_CONF and adds the contents into an array
of struct conffile. I allocated memory for at most 255 objects of this type,
which I still think is overkill, so please comment this, and finally the
function returns a struct conffile*
I also had to change getlist() parameters to take a struct conffile* so I could
access its members, then the function behaves the same as before but without
all parsing that was already done in parsefile().
In main() then I call parsefile(), assign its result to "cf" object, run a loop
to check the number of members of the array, and if any of the methods are http
or ftp then assign flag=1.
If flag=1 then the program will need "rpath" to read PATH_SPAMD_CONF,
"inet dns" to download the lists, and "proc exec" for fork, running pfctl and
ftp. If flag=0 then it doesn't need "inet dns" since all methods parsed don't
need network access.
I'm truly sorry for the long email, I will try to be more succinct next time,
so without further ado:
Index: libexec/spamd/spamd.c
===================================================================
RCS file: /cvs/src/libexec/spamd/spamd.c,v
retrieving revision 1.130
diff -u -p -u -r1.130 spamd.c
--- libexec/spamd/spamd.c 10 Sep 2015 13:56:12 -0000 1.130
+++ libexec/spamd/spamd.c 1 Dec 2015 11:15:43 -0000
@@ -1335,9 +1335,22 @@ main(int argc, char *argv[])
greylist ? " (greylist)" : "",
(syncrecv || syncsend) ? " (sync)" : "");
- if (!greylist)
+ if (syncsend || syncrecv) {
+ syncfd = sync_init(sync_iface, sync_baddr, sync_port);
+ if (syncfd == -1)
+ err(1, "sync init");
+ }
+
+ if (pledge("stdio rpath wpath flock inet getpw proc exec id", NULL) ==
-1)
+ err(1, "pledge");
+
+ if (!greylist) {
maxblack = maxcon;
- else if (maxblack > maxcon)
+
+ if (pledge("stdio rpath inet getpw proc id", NULL) == -1)
+ err(1, "pledge");
+
+ } else if (maxblack > maxcon)
usage();
rlp.rlim_cur = rlp.rlim_max = maxcon + 15;
@@ -1403,12 +1416,6 @@ main(int argc, char *argv[])
if (bind(conflisten, (struct sockaddr *)&lin, sizeof lin) == -1)
err(1, "bind local");
- if (syncsend || syncrecv) {
- syncfd = sync_init(sync_iface, sync_baddr, sync_port);
- if (syncfd == -1)
- err(1, "sync init");
- }
-
if ((pw = getpwnam("_spamd")) == NULL)
errx(1, "no such user _spamd");
@@ -1492,6 +1499,9 @@ jail:
setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid))
err(1, "failed to drop privs");
+ if (pledge("stdio inet", NULL) == -1)
+ err(1, "pledge");
+
if (listen(smtplisten, 10) == -1)
err(1, "listen");
Index: libexec/spamd-setup/spamd-setup.c
===================================================================
RCS file: /cvs/src/libexec/spamd-setup/spamd-setup.c,v
retrieving revision 1.46
diff -u -p -u -r1.46 spamd-setup.c
--- libexec/spamd-setup/spamd-setup.c 3 Jun 2015 02:24:36 -0000 1.46
+++ libexec/spamd-setup/spamd-setup.c 1 Dec 2015 11:15:46 -0000
@@ -61,6 +61,14 @@ struct blacklist {
u_int8_t black;
};
+struct conffile {
+ int capability;
+ char *name;
+ char *message;
+ char *method;
+ char *file;
+};
+
u_int32_t imask(u_int8_t);
u_int8_t maxblock(u_int32_t, u_int8_t);
u_int8_t maxdiff(u_int32_t, u_int32_t);
@@ -79,7 +87,8 @@ int cmpbl(const void *, const void *);
struct cidr *collapse_blacklist(struct bl *, size_t, u_int *);
int configure_spamd(u_short, char *, char *, struct cidr *, u_int);
int configure_pf(struct cidr *);
-int getlist(char **, char *, struct blacklist *, struct blacklist
*);
+int getlist(struct conffile *, struct blacklist *, struct
blacklist *);
+struct conffile *parsefile(void);
__dead void usage(void);
int debug;
@@ -669,84 +678,52 @@ configure_pf(struct cidr *blacklists)
}
int
-getlist(char ** db_array, char *name, struct blacklist *blist,
+getlist(struct conffile *cflist, struct blacklist *blist,
struct blacklist *blistnew)
{
- char *buf, *method, *file, *message;
- int fd, black = 0, serror;
+ int fd, serror;
size_t blc, bls;
struct bl *bl = NULL;
gzFile gzf;
- if (cgetent(&buf, db_array, name) != 0)
- err(1, "Can't find \"%s\" in spamd config", name);
- buf = fix_quoted_colons(buf);
- if (cgetcap(buf, "black", ':') != NULL) {
+ if (cflist->capability == 1) {
/* use new list */
- black = 1;
blc = blistnew->blc;
bls = blistnew->bls;
bl = blistnew->bl;
- } else if (cgetcap(buf, "white", ':') != NULL) {
+ } else if (cflist->capability == 0) {
/* apply to most recent blacklist */
- black = 0;
blc = blist->blc;
bls = blist->bls;
bl = blist->bl;
- } else
- errx(1, "Must have \"black\" or \"white\" in %s", name);
-
- switch (cgetstr(buf, "msg", &message)) {
- case -1:
- if (black)
- errx(1, "No msg for blacklist \"%s\"", name);
- break;
- case -2:
- err(1, NULL);
- }
-
- switch (cgetstr(buf, "method", &method)) {
- case -1:
- method = NULL;
- break;
- case -2:
- err(1, NULL);
- }
-
- switch (cgetstr(buf, "file", &file)) {
- case -1:
- errx(1, "No file given for %slist %s",
- black ? "black" : "white", name);
- case -2:
- err(1, NULL);
- default:
- fd = open_file(method, file);
- if (fd == -1)
- err(1, "Can't open %s by %s method",
- file, method ? method : "file");
- free(method);
- free(file);
- gzf = gzdopen(fd, "r");
- if (gzf == NULL)
- errx(1, "gzdopen");
}
- free(buf);
- bl = add_blacklist(bl, &blc, &bls, gzf, !black);
+
+ fd = open_file(cflist->method, cflist->file);
+ if (fd == -1)
+ err(1, "Can't open %s by %s method",
+ cflist->file, cflist->method ? cflist->method : "file");
+ free(cflist->method);
+ free(cflist->file);
+ gzf = gzdopen(fd, "r");
+ if (gzf == NULL)
+ errx(1, "gzdopen");
+
+ bl = add_blacklist(bl, &blc, &bls, gzf, !cflist->capability);
serror = errno;
gzclose(gzf);
if (bl == NULL) {
errno = serror;
- warn("Could not add %slist %s", black ? "black" : "white",
- name);
+ warn("Could not add %slist %s", cflist->capability ? "black" :
"white",
+ cflist->name);
return (0);
}
- if (black) {
+ if (cflist->capability) {
if (debug)
fprintf(stderr, "blacklist %s %zu entries\n",
- name, blc / 2);
- blistnew->message = message;
- blistnew->name = name;
- blistnew->black = black;
+ cflist->name, blc / 2);
+ blistnew->message = cflist->message;
+ blistnew->name = cflist->name;
+ blistnew->black = cflist->capability;
blistnew->bl = bl;
blistnew->blc = blc;
blistnew->bls = bls;
@@ -754,12 +731,12 @@ getlist(char ** db_array, char *name, st
/* whitelist applied to last active blacklist */
if (debug)
fprintf(stderr, "whitelist %s %zu entries\n",
- name, (blc - blist->blc) / 2);
+ cflist->name, (blc - blist->blc) / 2);
blist->bl = bl;
blist->blc = blc;
blist->bls = bls;
}
- return (black);
+ return (cflist->capability);
}
void
@@ -796,11 +773,11 @@ usage(void)
int
main(int argc, char *argv[])
{
- size_t blc, bls, black, white;
- char *db_array[2], *buf, *name;
+ size_t blc, bls, black, white, nmembers;
struct blacklist *blists;
+ struct conffile *cf;
struct servent *ent;
- int daemonize = 0, ch;
+ int daemonize = 0, ch, i, flag = 0;
while ((ch = getopt(argc, argv, "bdDn")) != -1) {
switch (ch) {
@@ -826,6 +803,29 @@ main(int argc, char *argv[])
if (argc != 0)
usage();
+ cf = parsefile();
+
+ for (i = 0; cf[i].name != NULL; ++i) {
+ if (strcmp(cf[i].method, "http") ||
+ strcmp(cf[i].method, "ftp")) {
+ flag = 1;
+ }
+ }
+ nmembers = i;
+
+ /*
+ * if flag = 1, it means that there's at least one of the methods in
+ * PATH_SPAMD_CONF that is either http or ftp and therefore needs
+ * "inet dns" annotations to get file from network, otherwise reduce
them
+ */
+ if (flag) {
+ if (pledge("stdio rpath inet dns proc exec", NULL) == -1)
+ err(1, "pledge");
+ } else {
+ if (pledge("stdio rpath proc exec", NULL) == -1)
+ err(1, "pledge");
+ }
+
if (daemonize)
daemon(0, 0);
@@ -833,45 +833,119 @@ main(int argc, char *argv[])
errx(1, "cannot find service \"spamd-cfg\" in /etc/services");
ent->s_port = ntohs(ent->s_port);
+ blists = NULL;
+ blc = bls = 0;
+ for (i = 0; i < nmembers; ++i) {
+ if (blc == bls) {
+ struct blacklist *tmp;
+
+ bls += 32;
+ tmp = reallocarray(blists, bls, sizeof(struct
blacklist));
+ if (tmp == NULL)
+ err(1, NULL);
+ blists = tmp;
+ }
+ if (blc == 0)
+ black = white = 0;
+ else {
+ white = blc - 1;
+ black = blc;
+ }
+ memset(&blists[black], 0, sizeof(struct blacklist));
+ black = getlist(&cf[i], &blists[white], &blists[black]);
+ if (black && blc > 0) {
+ /* collapse and free previous blacklist */
+ send_blacklist(&blists[blc - 1], ent->s_port);
+ }
+ blc += black;
+ }
+
+ /* collapse and free last blacklist */
+ if (blc > 0)
+ send_blacklist(&blists[blc - 1], ent->s_port);
+ return (0);
+}
+
+/*
+ * Parse PATH_SPAMD_CONF file to check for black and white lists
+ * Returns a struct conffile* with all the lists parsed from the file
+ */
+struct conffile*
+parsefile(void)
+{
+ char *db_array[2], *buf, *tmpbuf, *name, *message, *method, *file;
+ const size_t MAX_OBJECTS = 255;
+ int i = 0;
+ struct conffile *tmp;
+
db_array[0] = PATH_SPAMD_CONF;
db_array[1] = NULL;
- if (cgetent(&buf, db_array, "all") != 0)
+ // Allocate at most 255 objects of type struct conffile
+ if ((tmp = reallocarray(NULL, MAX_OBJECTS, sizeof(struct conffile))) ==
NULL)
+ err(1, NULL);
+
+ if (cgetent(&buf, db_array, "all") != 0)
err(1, "Can't find \"all\" in spamd config");
name = strsep(&buf, ": \t"); /* skip "all" at start */
- blists = NULL;
- blc = bls = 0;
- while ((name = strsep(&buf, ": \t")) != NULL) {
+ tmpbuf = buf;
+ while ((name = strsep(&tmpbuf, ": \t")) != NULL) {
if (*name) {
- /* extract config in order specified in "all" tag */
- if (blc == bls) {
- struct blacklist *tmp;
-
- bls += 32;
- tmp = reallocarray(blists, bls,
- sizeof(struct blacklist));
- if (tmp == NULL)
- err(1, NULL);
- blists = tmp;
+ if (cgetent(&buf, db_array, name) != 0)
+ err(1, "Can't find \"%s\" in spamd config",
name);
+ buf = fix_quoted_colons(buf);
+ tmp[i].name = name;
+
+ if (cgetcap(buf, "black", ':') != NULL) {
+ tmp[i].capability = 1;
+ } else if (cgetcap(buf, "white", ':') != NULL) {
+ tmp[i].capability = 0;
+ } else
+ errx(1, "Must have \"black\" or \"white\" in
%s", name);
+
+ switch (cgetstr(buf, "msg", &message)) {
+ case -1:
+ if (tmp[i].capability == 1) {
+ errx(1, "No msg for blacklist \"%s\"",
name);
+ } else {
+ tmp[i].message = NULL;
+ break;
+ }
+ case -2:
+ err(1, NULL);
+ default:
+ tmp[i].message = message;
}
- if (blc == 0)
- black = white = 0;
- else {
- white = blc - 1;
- black = blc;
+
+ switch (cgetstr(buf, "method", &method)) {
+ case -1:
+ tmp[i].method = NULL;
+ break;
+ case -2:
+ err(1, NULL);
+ default:
+ tmp[i].method = method;
}
- memset(&blists[black], 0, sizeof(struct blacklist));
- black = getlist(db_array, name, &blists[white],
- &blists[black]);
- if (black && blc > 0) {
- /* collapse and free previous blacklist */
- send_blacklist(&blists[blc - 1], ent->s_port);
+
+ switch (cgetstr(buf, "file", &file)) {
+ case -1:
+ errx(1, "No file given for %slist %s",
+ tmp[i].capability ? "black" : "white",
name);
+ case -2:
+ err(1, NULL);
+ default:
+ tmp[i].file = file;
}
- blc += black;
+
+ ++i;
}
}
- /* collapse and free last blacklist */
- if (blc > 0)
- send_blacklist(&blists[blc - 1], ent->s_port);
- return (0);
+
+ free(buf);
+ free(tmpbuf);
+
+ if (i == 0)
+ errx(1, "Could not get any lists from file \"%s\"",
PATH_SPAMD_CONF);
+
+ return (tmp);
}
