On Sat, May 30, 2020 at 05:40:43PM +0200, Sebastien Marie wrote:
> Hi,
>
> I am looking to make smtpd to set SNI (SSL_set_tlsext_host_name) when
> connecting
> to smarthost when relaying mail.
>
> After digging a bit in libtls (to stole the right code) and smtpd (to see
> where
> to put the stolen code), I have the following diff:
>
>
> diff 73b535ef4537e8454483912fc3420bc304759e96 /home/semarie/repos/openbsd/src
> blob - d384692a0e43de47d645142a6b99e72b7d83b687
> file + usr.sbin/smtpd/mta_session.c
> --- usr.sbin/smtpd/mta_session.c
> +++ usr.sbin/smtpd/mta_session.c
> @@ -26,6 +26,7 @@
> #include <sys/stat.h>
> #include <sys/uio.h>
>
> +#include <arpa/inet.h>
> #include <ctype.h>
> #include <err.h>
> #include <errno.h>
> @@ -1604,6 +1605,8 @@ mta_cert_init_cb(void *arg, int status, const char *na
> struct mta_session *s = arg;
> void *ssl;
> char *xname = NULL, *xcert = NULL;
> + struct in_addr addrbuf4;
> + struct in6_addr addrbuf6;
>
> if (s->flags & MTA_WAIT)
> mta_tree_pop(&wait_tls_init, s->id);
> @@ -1623,6 +1626,24 @@ mta_cert_init_cb(void *arg, int status, const char *na
> free(xcert);
> if (ssl == NULL)
> fatal("mta: ssl_mta_init");
> +
> + /*
> + * RFC4366 (SNI): Literal IPv4 and IPv6 addresses are not
> + * permitted in "HostName".
> + */
> + if (s->relay->domain->as_host == 1) {
> + xname = xstrdup(s->relay->domain->name);
This allocation appears to be unnecessary. I believe you should be
able to simply check with inet_pton, and then use
s->relay->domain->name
On a strictly smtpd front, it seems odd to have somthing called
domain->name possibly being an ip address in text form. It would seem
prudent to keep these things separate as we resolve things. (domain->ip, or
domain->mxip if we have resolved down to that might be better) but
that's a separate issue and larger change.
> + if (inet_pton(AF_INET, xname, &addrbuf4) != 1 &&
> + inet_pton(AF_INET6, xname, &addrbuf6) != 1) {
> + log_info("%016"PRIx64" mta setting SNI name=%s",
> + s->id, xname);
> + if (SSL_set_tlsext_host_name(ssl, xname) == 0)
> + log_warnx("%016"PRIx64" mta setting SNI failed",
> + s->id);
> + }
> + free(xname);
> + }
> +
> io_start_tls(s->io, ssl);
> }
>
>
>
> For what I understood:
>
> mta_cert_init_cb() function is responsable to prepare a connection. the SSL
> initialization (SSL_new() call) occured in ssl_mta_init() which was just
> called,
> so it seems it is the right place to call SSL_set_tlsext_host_name().
>
> We just need the hostname to configure it.
>
> Regarding mta_session structure, relay->domain->as_host is set to 1 when the
> domain is linked to smarthost configuration (or when the mx is ip address I
> think). And in smarthost case, the domain->name is the hostname. For SNI, we
> are
> excluding ip, so I assume it should copte with domain->name as ip.
>
> Does someone with better understanding of smtpd code source could confirm the
> approch is right and comment ?
>
> Please note I have only tested it on simple configuration.
>
> Thanks.
> --
> Sebastien Marie
>
