Hey kn,
You got the order wrong on my diff :)
Before, the certs were loaded by root in memory and then set by _spamd, with my
diff they are still loaded by root but now also set, everything else
still has the same order so it should be:
tls_config_set_*_file()
fork()
setres*id()
pledge()
On 12:58 Tue 06 Jul , Klemens Nanni wrote:
> On Wed, Jun 30, 2021 at 01:11:38PM +0100, Ricardo Mestre wrote:
> > Hi,
> >
> > I may have seen it elsewhere, or probably not, but after checking on kn's
> > commit
> > to tls_load_file(3) it seems it's now possible to set the ca/cert/key
> > directly
> > without having to load them first from disk and set them afterwards from
> > memory.
>
> That's correct and the tls_config_set_*_{mem -> file}() changes are
> correct.
>
> > That being said the below applies this to spamd(8), no functional change
> > apart
> > from setting up TLS upfront, and now also before pledge(2). I tested it on
> > one
> > of my servers without issues, but also with only the cert or just with the
> > key
> > to ensure it would still error out.
>
> I am not a spamd user but this is what I see from code inspection:
> Currently, things happen in this order of
>
> tls_load_file()
> fork()
> setres*id()
> pledge()
> tls_config_set_*_mem()
>
> and your diff looks like changing that to
>
> fork()
> setres*id()
> tls_config_set_*_file()
> pledge()
>
> So it seems that currently file permissions on both TLS certificate and
> privat key files don't matter as they're read as root, while with your
> diff they'd be read as the unprivileged _spamd user?
>
> I didn't find details about required permissions for those files in our
> manual pages, but I'd expect them to be `root:wheel 0700' or so, i.e.
> not readable by anyone but root.
>
> Am I missing something here or are your TLS files readable by _spamd so
> that the diff works (for you)?
>
>
> > Index: spamd.c
> > ===================================================================
> > RCS file: /cvs/src/libexec/spamd/spamd.c,v
> > retrieving revision 1.156
> > diff -u -p -u -r1.156 spamd.c
> > --- spamd.c 6 Aug 2019 13:34:36 -0000 1.156
> > +++ spamd.c 30 Jun 2021 11:53:18 -0000
> > @@ -136,10 +136,6 @@ u_short cfg_port;
> > u_short sync_port;
> > struct tls_config *tlscfg;
> > struct tls *tlsctx;
> > -uint8_t *pubcert;
> > -size_t pubcertlen;
> > -uint8_t *privkey;
> > -size_t privkeylen;
> > char *tlskeyfile = NULL;
> > char *tlscertfile = NULL;
> >
> > @@ -464,9 +460,9 @@ spamd_tls_init()
> > if (tls_config_set_ciphers(tlscfg, "all") != 0)
> > errx(1, "failed to set tls ciphers");
> >
> > - if (tls_config_set_cert_mem(tlscfg, pubcert, pubcertlen) == -1)
> > + if (tls_config_set_cert_file(tlscfg, tlscertfile) == -1)
> > errx(1, "unable to set TLS certificate file %s", tlscertfile);
> > - if (tls_config_set_key_mem(tlscfg, privkey, privkeylen) == -1)
> > + if (tls_config_set_key_file(tlscfg, tlskeyfile) == -1)
> > errx(1, "unable to set TLS key file %s", tlskeyfile);
> > if (tls_configure(tlsctx, tlscfg) != 0)
> > errx(1, "failed to configure TLS - %s", tls_error(tlsctx));
> > @@ -1392,12 +1388,7 @@ main(int argc, char *argv[])
> > } else if (maxblack > maxcon)
> > usage();
> >
> > - if (tlscertfile &&
> > - (pubcert=tls_load_file(tlscertfile, &pubcertlen, NULL)) == NULL)
> > - errx(1, "unable to load TLS certificate file %s", tlscertfile);
> > - if (tlskeyfile &&
> > - (privkey=tls_load_file(tlskeyfile, &privkeylen, NULL)) == NULL)
> > - errx(1, "unable to load TLS key file %s", tlskeyfile);
> > + spamd_tls_init();
> >
> > rlp.rlim_cur = rlp.rlim_max = maxcon + 15;
> > if (setrlimit(RLIMIT_NOFILE, &rlp) == -1)
> > @@ -1546,8 +1537,6 @@ main(int argc, char *argv[])
> > jail:
> > if (pledge("stdio inet", NULL) == -1)
> > err(1, "pledge");
> > -
> > - spamd_tls_init();
> >
> > if (listen(smtplisten, 10) == -1)
> > err(1, "listen");
> >
>
