Download raw body.
smtpd: do not mutate shared dispatcher tls config
Hello,
This diff is, modulo one minor stylistic change, from Renaud Allard for
the -portable repository:
https://github.com/OpenSMTPD/OpenSMTPD/pull/1304
(Renaud: I've just tweaked the indentation, introduced a local variable
in mta_io, and added an error check in load_pki_tree to fail earlier.)
The underlying issue is that we currently have one shared tls config
structure and we might end up changing it when constructing a
tls_context for a relay that doesn't have an explicit TLS configuration.
The diff below tries to fix this by removing altogether the shared
dispatcher. Instead, we construct every time the tls_config on-demand.
We're also loading in memory the default CA file once to appease pledge
;-) and reduce the cost of loading it every time.
if this mail will find you well, it means that it works ;-)
jokes aside, any objections on the diff?
diffstat refs/remotes/origin/master refs/heads/op/wip
M usr.sbin/smtpd/mta.c | 11+ 24-
M usr.sbin/smtpd/mta_session.c | 13+ 8-
M usr.sbin/smtpd/smtpd.c | 6+ 0-
M usr.sbin/smtpd/smtpd.h | 3+ 1-
4 files changed, 33 insertions(+), 33 deletions(-)
diff refs/remotes/origin/master refs/heads/op/wip
commit - 551f60a76f060be70022727b3c9dd77b6ba8ce11
commit + 0624487cbff0bec0da7d8bc18d7cda7e6dc9892f
blob - 8022d23ecb02e4654968912bb112c2042701e89e
blob + 2415c421089fc6475d41964b908d0aa0e67a8d6d
--- usr.sbin/smtpd/mta.c
+++ usr.sbin/smtpd/mta.c
@@ -43,7 +43,6 @@
#define RELAY_ONHOLD 0x01
#define RELAY_HOLDQ 0x02
-static void mta_setup_dispatcher(struct dispatcher *);
static void mta_handle_envelope(struct envelope *, const char *);
static void mta_query_smarthost(struct envelope *);
static void mta_on_smarthost(struct envelope *, const char *);
@@ -456,21 +455,11 @@ mta_imsg(struct mproc *p, struct imsg *imsg)
void
mta_postfork(void)
{
- struct dispatcher *dispatcher;
- const char *key;
- void *iter;
-
- iter = NULL;
- while (dict_iter(env->sc_dispatchers, &iter, &key, (void **)&dispatcher)) {
- log_debug("%s: %s", __func__, key);
- mta_setup_dispatcher(dispatcher);
- }
}
-static void
-mta_setup_dispatcher(struct dispatcher *dispatcher)
+struct tls_config *
+mta_tls_config_create(struct dispatcher_remote *remote, int verify)
{
- struct dispatcher_remote *remote;
static const char *dheparams[] = { "none", "auto", "legacy" };
struct tls_config *config;
struct pki *pki;
@@ -478,11 +467,6 @@ mta_setup_dispatcher(struct dispatcher *dispatcher)
const char *ciphers;
uint32_t protos;
- if (dispatcher->type != DISPATCHER_REMOTE)
- return;
-
- remote = &dispatcher->u.remote;
-
if ((config = tls_config_new()) == NULL)
fatal("smtpd: tls_config_new");
@@ -520,13 +504,16 @@ mta_setup_dispatcher(struct dispatcher *dispatcher)
== -1)
fatalx("tls_config_set_ca_mem: %s",
tls_config_error(config));
+ } else {
+ if (env->sc_default_ca_cert == NULL)
+ fatalx("default CA certificate not available");
+ if (tls_config_set_ca_mem(config, env->sc_default_ca_cert,
+ env->sc_default_ca_cert_len) == -1)
+ fatalx("tls_config_set_ca_mem: %s",
+ tls_config_error(config));
}
- else if (tls_config_set_ca_file(config, tls_default_ca_cert_file())
- == -1)
- fatalx("tls_config_set_ca_file: %s",
- tls_config_error(config));
- if (remote->tls_verify) {
+ if (verify) {
tls_config_verify(config);
} else {
tls_config_insecure_noverifycert(config);
@@ -534,7 +521,7 @@ mta_setup_dispatcher(struct dispatcher *dispatcher)
tls_config_insecure_noverifytime(config);
}
- remote->tls_config = config;
+ return (config);
}
void
blob - 8aa23779f8f46fb4ec69517a724563eae2a0515f
blob + 94c4128ab72888936ffcf61192ed29925ef4b26e
--- usr.sbin/smtpd/mta_session.c
+++ usr.sbin/smtpd/mta_session.c
@@ -1198,6 +1198,7 @@ static void
mta_io(struct io *io, int evt, void *arg)
{
struct mta_session *s = arg;
+ struct dispatcher_remote *remote;
char *line, *msg, *p;
size_t len;
const char *error;
@@ -1227,7 +1228,10 @@ mta_io(struct io *io, int evt, void *arg)
log_info("%016"PRIx64" mta tls ciphers=%s",
s->id, tls_to_text(io_tls(s->io)));
s->flags |= MTA_TLS;
- if (s->relay->dispatcher->u.remote.tls_verify)
+
+ remote = &s->relay->dispatcher->u.remote;
+ if (remote->tls_verify ||
+ ((s->flags & MTA_WANT_SECURE) && !remote->tls_required))
s->flags |= MTA_TLS_VERIFIED;
mta_tls_started(s);
@@ -1568,6 +1572,7 @@ static void
mta_tls_init(struct mta_session *s)
{
struct dispatcher_remote *remote;
+ struct tls_config *config;
struct tls *tls;
if ((tls = tls_client()) == NULL) {
@@ -1577,19 +1582,19 @@ mta_tls_init(struct mta_session *s)
}
remote = &s->relay->dispatcher->u.remote;
- if ((s->flags & MTA_WANT_SECURE) && !remote->tls_required) {
- /* If TLS not explicitly configured, use implicit config. */
- remote->tls_required = 1;
- remote->tls_verify = 1;
- tls_config_verify(remote->tls_config);
- }
- if (tls_configure(tls, remote->tls_config) == -1) {
+ config = mta_tls_config_create(remote, remote->tls_verify ||
+ ((s->flags & MTA_WANT_SECURE) && !remote->tls_required));
+
+ if (tls_configure(tls, config) == -1) {
log_info("%016"PRIx64" mta closing reason=tls-failure", s->id);
tls_free(tls);
+ tls_config_free(config);
s->flags |= MTA_FREE;
return;
}
+ tls_config_free(config);
+
if (io_connect_tls(s->io, tls, s->mxname) == -1) {
log_info("%016"PRIx64" mta closing reason=tls-connect-failed", s->id);
tls_free(tls);
blob - 086bfa42a5b7ad6b5b1f30e7589948f2364a9462
blob + 12a0782b07d6a6b01e66001f245cb2eba60c9ac9
--- usr.sbin/smtpd/smtpd.c
+++ usr.sbin/smtpd/smtpd.c
@@ -1118,6 +1118,12 @@ load_pki_tree(void)
if (!ssl_load_cafile(sca, sca->ca_cert_file))
fatalx("load_pki_tree: failed to load CA file");
}
+
+ env->sc_default_ca_cert = tls_load_file(tls_default_ca_cert_file(),
+ &env->sc_default_ca_cert_len, NULL);
+ if (env->sc_default_ca_cert == NULL)
+ fatalx("load_pki_tree: failed to load default CA file: %s",
+ tls_default_ca_cert_file());
}
void
blob - 3574db4c03d611801672016b1d0d4314746802cf
blob + 4b13cfc0e71cfc8cebb6af5f64ba380c82516a12
--- usr.sbin/smtpd/smtpd.h
+++ usr.sbin/smtpd/smtpd.h
@@ -611,6 +611,8 @@ struct smtpd {
struct dispatcher *sc_dispatcher_bounce;
struct dict *sc_ca_dict;
+ uint8_t *sc_default_ca_cert;
+ size_t sc_default_ca_cert_len;
struct dict *sc_pki_dict;
struct dict *sc_ssl_dict;
@@ -1181,7 +1183,6 @@ struct dispatcher_remote {
char *source;
- struct tls_config *tls_config;
char *ca;
char *pki;
@@ -1492,6 +1493,7 @@ void m_clear_params(struct dict *);
/* mta.c */
+struct tls_config *mta_tls_config_create(struct dispatcher_remote *, int);
void mta_postfork(void);
void mta_postprivdrop(void);
void mta_imsg(struct mproc *, struct imsg *);
smtpd: do not mutate shared dispatcher tls config