Re: smtpd: mta cert-check result="unverified" on smarthost
On Tue, Feb 15, 2022 at 08:40:11AM -0700, Todd C. Miller wrote: > On Mon, 14 Feb 2022 20:05:30 +0100, Sebastien Marie wrote: > > > One aspect that I haven't verified for now is the difference between > > using "tls" (early initialisation) and not using it (late > > initialisation). I will try to look at it to ensure that the > > connection is always used with tls_config_verify(). > > TLS configuration for urls is a little tricky. We could parse the > url type for simple things like: > > action "relay-free" relay host "smtps://f...@smtp.free.fr" auth > > but if there is a table involved that will not work, which is why > I deferred the TLS setup. > I looked at it a bit: it isn't still a 100% review (it would implied to me to fully read and understand the code path), but I have good confidence in the diff :) If you have a TLS connection, you had to use tls_configure(), and very few places calls it: - mta_session.c : for outgoing connection - smtp.c : for incoming connection only outgoing connection are implied here. So, as soon you have MTA_WANT_SECURE and no tls_required, tls_config_verify() will be called (and the connection will be properly verified). tls_required is directly modified only in parse.y (with "tls" or "tls no-verify" options). I didn't check for indirect modification (bzero or memset... but the 0 value would be RELAY_TLS_OPPORTUNISTIC, and it is this value since 2018, so I don't expect behaviour change here). MTA_WANT_SECURE is sets when the remote url starts with "smtps://" (RELAY_TLS_SMTPS) or "smtp+tls://" (RELAY_TLS_STARTTLS). So I assume there is no gap in the initialisation, and if you request a remote with "smtps://" or "smtp+tls://" you will always get a verified connection (assuming no existent hole in previous code). The diff is still ok semarie@ :) Thanks. -- Sebastien Marie
Re: smtpd: mta cert-check result="unverified" on smarthost
On Mon, 14 Feb 2022 20:05:30 +0100, Sebastien Marie wrote: > One aspect that I haven't verified for now is the difference between > using "tls" (early initialisation) and not using it (late > initialisation). I will try to look at it to ensure that the > connection is always used with tls_config_verify(). TLS configuration for urls is a little tricky. We could parse the url type for simple things like: action "relay-free" relay host "smtps://f...@smtp.free.fr" auth but if there is a table involved that will not work, which is why I deferred the TLS setup. Maybe eric@ can comment on this. - todd
Re: smtpd: mta cert-check result="unverified" on smarthost
On Mon, Feb 14, 2022 at 11:10:01AM -0700, Todd C. Miller wrote:
> On Mon, 14 Feb 2022 17:43:47 +0100, Sebastien Marie wrote:
>
> > It seems I need to explicitly add "tls" on the action line to enforce
> > the tls verification now.
> >
> > - action "relay-free" relay host "smtps://f...@smtp.free.fr" auth
> > > s>
> > + action "relay-free" relay host "smtps://f...@smtp.free.fr" auth
> > > s> tls
> >
> > I am unsure if the behaviour change was intented or not. If it
> > persists it might need some documentation (a current.html entry) for
> > users to update their configuration (as TLS session might not be
> > checked anymore whereas it was previously).
>
> The change is not intentional. As you found, if there is no explicit
> tls config for the dispatcher then the default is not to verify.
>
> One way to fix this is to update the TLS config in mta_tls_init()
> before calling tls_configure() for smtps:// and smtp+tls:// relays.
> Can you try the following diff against -current?
With the diff, the connection is verified.
One aspect that I haven't verified for now is the difference between
using "tls" (early initialisation) and not using it (late
initialisation). I will try to look at it to ensure that the
connection is always used with tls_config_verify().
But for now, the diff is good.
OK semarie@
Thanks.
>
> - todd
>
> Index: usr.sbin/smtpd/mta_session.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/mta_session.c,v
> retrieving revision 1.145
> diff -u -p -u -r1.145 mta_session.c
> --- usr.sbin/smtpd/mta_session.c 10 Feb 2022 14:59:35 - 1.145
> +++ usr.sbin/smtpd/mta_session.c 14 Feb 2022 18:05:02 -
> @@ -1563,7 +1563,7 @@ mta_error(struct mta_session *s, const c
> static void
> mta_tls_init(struct mta_session *s)
> {
> - struct tls_config *tls_config;
> + struct dispatcher_remote *remote;
> struct tls *tls;
>
> if ((tls = tls_client()) == NULL) {
> @@ -1572,8 +1572,14 @@ mta_tls_init(struct mta_session *s)
> return;
> }
>
> - tls_config = s->relay->dispatcher->u.remote.tls_config;
> - if (tls_configure(tls, tls_config) == -1) {
> + 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) {
> log_info("%016"PRIx64" mta closing reason=tls-failure", s->id);
> tls_free(tls);
> mta_free(s);
--
Sebastien Marie
Re: smtpd: mta cert-check result="unverified" on smarthost
On Mon, 14 Feb 2022 17:43:47 +0100, Sebastien Marie wrote:
> It seems I need to explicitly add "tls" on the action line to enforce
> the tls verification now.
>
> - action "relay-free" relay host "smtps://f...@smtp.free.fr" auth s>
> + action "relay-free" relay host "smtps://f...@smtp.free.fr" auth s> tls
>
> I am unsure if the behaviour change was intented or not. If it
> persists it might need some documentation (a current.html entry) for
> users to update their configuration (as TLS session might not be
> checked anymore whereas it was previously).
The change is not intentional. As you found, if there is no explicit
tls config for the dispatcher then the default is not to verify.
One way to fix this is to update the TLS config in mta_tls_init()
before calling tls_configure() for smtps:// and smtp+tls:// relays.
Can you try the following diff against -current?
- todd
Index: usr.sbin/smtpd/mta_session.c
===
RCS file: /cvs/src/usr.sbin/smtpd/mta_session.c,v
retrieving revision 1.145
diff -u -p -u -r1.145 mta_session.c
--- usr.sbin/smtpd/mta_session.c10 Feb 2022 14:59:35 - 1.145
+++ usr.sbin/smtpd/mta_session.c14 Feb 2022 18:05:02 -
@@ -1563,7 +1563,7 @@ mta_error(struct mta_session *s, const c
static void
mta_tls_init(struct mta_session *s)
{
- struct tls_config *tls_config;
+ struct dispatcher_remote *remote;
struct tls *tls;
if ((tls = tls_client()) == NULL) {
@@ -1572,8 +1572,14 @@ mta_tls_init(struct mta_session *s)
return;
}
- tls_config = s->relay->dispatcher->u.remote.tls_config;
- if (tls_configure(tls, tls_config) == -1) {
+ 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) {
log_info("%016"PRIx64" mta closing reason=tls-failure", s->id);
tls_free(tls);
mta_free(s);
Re: smtpd: mta cert-check result="unverified" on smarthost
Hi, It seems I need to explicitly add "tls" on the action line to enforce the tls verification now. - action "relay-free" relay host "smtps://f...@smtp.free.fr" auth + action "relay-free" relay host "smtps://f...@smtp.free.fr" auth tls I am unsure if the behaviour change was intented or not. If it persists it might need some documentation (a current.html entry) for users to update their configuration (as TLS session might not be checked anymore whereas it was previously). Thanks. -- Sebastien Marie On Mon, Feb 14, 2022 at 05:30:41PM +0100, Sebastien Marie wrote: > Hi, > > After upgrading from > OpenBSD 7.0-current (GENERIC.MP) #133: Sat Feb 5 12:11:10 CET 2022" > to OpenBSD 7.0-current (GENERIC.MP) #335: Sun Feb 13 16:41:43 MST 2022 > > I am seeing smtpd to report smarthost connection (when my local user is > sending a mail) with: > Feb 14 16:58:42 quade smtpd[14803]: 48abc0eafe1f6d7d mta cert-check > result="unverified" fingerprint="SHA256:abcxyz" > previously, it was: > Feb 14 10:31:16 quade smtpd[84045]: 2a0974f82839e80c mta cert-check > result="valid" fingerprint="SHA256:abcxyz" > > As it is a smarthost connection (connection with smtps:// to send a > mail), I am expecting the connection to be verified before sending > my credentials on the wire. > > In the timeframe, there is two commits: > - 2022-02-12 3abbdc76 eric use new libtls signer api > - 2022-02-10 89818320 millert Do not verify the cert or CA for a relay using > opportunistic TLS. > > if I backout 3abbdc76, I still have result="unverified", and if I > backout the 2 commits (there are conflicts with only 89818320 backout), > I get back verified connection. > > commit 89818320f51ce9b89c144087357e3182ba7f3dda > from: millert > date: Thu Feb 10 14:59:35 2022 UTC > > Do not verify the cert or CA for a relay using opportunistic TLS. > If a relay is not explicitly configured to use TLS but the remote > side supports STARTTLS, we will try to use it. However, in this > case we should not verify the cert or CA (which may be self-signed). > This restores the relay behavior before the switch to libtls was made. > There is no change if the relay is explicitly configured to use TLS. > OK eric@ > > > The smtpd daemon doesn't accept mail from internet, but only on > localhost. the sender mail is used to choose the the smarthost used. > > My config is: > > > table aliases file:/etc/mail/aliases > table secrets file:/etc/mail/secrets > table senders-free file:/etc/mail/senders-free > table senders-o2switch file:/etc/mail/senders-o2switch > > bounce warn-interval 1d > > # listens > listen on lo0 > > # actions > action "local" mbox alias > action "relay-o2switch" relay host "smtps://o2swi...@flexo.o2switch.net" auth > > action "relay-free" relay host "smtps://f...@smtp.free.fr" auth > > # matches > match from local for local action "local" > match from local mail-from for any action "relay-free" > match from local mail-from for any action "relay-o2switch" > match for domain "xyz.fr" action "relay-o2switch" > > Thanks. > -- > Sebastien Marie
