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");
>
