Re: smtpd: make smarthost to use SNI when relaying
looks good to me
ok beck@
On Sun, May 31, 2020 at 03:38:00PM +0200, Sebastien Marie wrote:
> Hi,
>
> updated diff after millert@ and beck@ remarks:
> - use union to collapse in_addr + in6_addr
> - doesn't allocate buffer and directly use s->relay->domain->name
>
> Thanks.
> --
> Sebastien Marie
>
>
> 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
> #include
>
> +#include
> #include
> #include
> #include
> @@ -1604,6 +1605,10 @@ mta_cert_init_cb(void *arg, int status, const char *na
> struct mta_session *s = arg;
> void *ssl;
> char *xname = NULL, *xcert = NULL;
> + union {
> + struct in_addr in4;
> + struct in6_addr in6;
> + } addrbuf;
>
> if (s->flags & MTA_WAIT)
> mta_tree_pop(_tls_init, s->id);
> @@ -1623,6 +1628,22 @@ 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) {
> + if (inet_pton(AF_INET, s->relay->domain->name, ) != 1 &&
> + inet_pton(AF_INET6, s->relay->domain->name, ) != 1)
> {
> + log_debug("%016"PRIx64" mta tls setting SNI name=%s",
> + s->id, s->relay->domain->name);
> + if (SSL_set_tlsext_host_name(ssl,
> s->relay->domain->name) == 0)
> + log_warnx("%016"PRIx64" mta tls setting SNI
> failed",
> +s->id);
> + }
> + }
> +
> io_start_tls(s->io, ssl);
> }
>
>
Re: smtpd: make smarthost to use SNI when relaying
Hi,
updated diff after millert@ and beck@ remarks:
- use union to collapse in_addr + in6_addr
- doesn't allocate buffer and directly use s->relay->domain->name
Thanks.
--
Sebastien Marie
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
#include
+#include
#include
#include
#include
@@ -1604,6 +1605,10 @@ mta_cert_init_cb(void *arg, int status, const char *na
struct mta_session *s = arg;
void *ssl;
char *xname = NULL, *xcert = NULL;
+ union {
+ struct in_addr in4;
+ struct in6_addr in6;
+ } addrbuf;
if (s->flags & MTA_WAIT)
mta_tree_pop(_tls_init, s->id);
@@ -1623,6 +1628,22 @@ 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) {
+ if (inet_pton(AF_INET, s->relay->domain->name, ) != 1 &&
+ inet_pton(AF_INET6, s->relay->domain->name, ) != 1)
{
+ log_debug("%016"PRIx64" mta tls setting SNI name=%s",
+ s->id, s->relay->domain->name);
+ if (SSL_set_tlsext_host_name(ssl,
s->relay->domain->name) == 0)
+ log_warnx("%016"PRIx64" mta tls setting SNI
failed",
+ s->id);
+ }
+ }
+
io_start_tls(s->io, ssl);
}
Re: smtpd: make smarthost to use SNI when relaying
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
> #include
>
> +#include
> #include
> #include
> #include
> @@ -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(_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, ) != 1 &&
> + inet_pton(AF_INET6, xname, ) != 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
>
Re: smtpd: make smarthost to use SNI when relaying
On Sat, 30 May 2020 17:40:43 +0200, Sebastien Marie wrote:
> 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:
Consider using a union for addrbuf, e.g.
union {
struct in_addr addr4;
struct in6_addr addr6;
} addrbuf;
There's no need to have both on the stack. Otherwise looks reasonable
to me.
- todd
