On Thu, May 31, 2018 at 10:25:54PM +0200, Eric Faurot wrote:
>
> Hello.
>
> This makes sense, indeed.
>
> Here is a slightly updated diff for your proposal. It makes the
> documentatino more accurate: the server certificate is always
> verified, the flag is only meant to accept invalid certificates. It
> also fixes build (apparently the mta.c chunk was incorrect).
the initial diff was to set F_TLS_VERIFY by default, and only remove it
on request. with the new diff, the logic is inversed. it makes no
functional changes, but it means the code logic could be more fragile as
if code path changes (in future developpement or refactoring) invalidate
certificate could be accepted by default.
but it is fine with me (and invalid certificates are logged).
another comment inline.
Thanks
--
Sebastien Marie
> Index: mta.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/mta.c,v
> retrieving revision 1.212
> diff -u -p -r1.212 mta.c
> --- mta.c 31 May 2018 11:56:10 -0000 1.212
> +++ mta.c 31 May 2018 19:56:03 -0000
> @@ -677,6 +677,9 @@ mta_handle_envelope(struct envelope *evp
> return;
> }
>
> + if (dispatcher->u.remote.tls_noverify == 0)
> + evp->agent.mta.relay.flags |= F_TLS_VERIFY;
> +
I am unsure if the condition should be:
if (smarthost && dispatcher->u.remote.tls_noverify == 0)
it ensures the flag will be set only if smarthost is used.
here, the flag will be added everytime (and removed only with "tls
no-verify" which is usable only with "host example.com").
> relay = mta_relay(evp);
> /* ignore if we don't know the limits yet */
> if (relay->limits &&
> Index: parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/parse.y,v
> retrieving revision 1.206
> diff -u -p -r1.206 parse.y
> --- parse.y 30 May 2018 19:01:58 -0000 1.206
> +++ parse.y 31 May 2018 19:56:04 -0000
> @@ -182,7 +182,7 @@ typedef struct {
> %token KEY
> %token LIMIT LISTEN LMTP LOCAL
> %token MAIL_FROM MAILDIR MASK_SRC MASQUERADE MATCH MAX_MESSAGE_SIZE
> MAX_DEFERRED MBOX MDA MTA MX
> -%token NODSN
> +%token NODSN NOVERIFY
> %token ON
> %token PKI PORT
> %token QUEUE
> @@ -541,6 +541,19 @@ HELO STRING {
>
> dispatcher->u.remote.smarthost = strdup(t->t_name);
> }
> +| TLS NOVERIFY {
> + if (dispatcher->u.remote.smarthost == NULL) {
> + yyerror("tls no-verify may not be specified without host on a
> dispatcher");
> + YYERROR;
> + }
> +
> + if (dispatcher->u.remote.tls_noverify == 1) {
> + yyerror("tls no-verify already specified for this dispatcher");
> + YYERROR;
> + }
> +
> + dispatcher->u.remote.tls_noverify = 1;
> +}
> | AUTH tables {
> struct table *t = $2;
>
> @@ -1571,6 +1584,7 @@ lookup(char *s)
> { "mta", MTA },
> { "mx", MX },
> { "no-dsn", NODSN },
> + { "no-verify", NOVERIFY },
> { "on", ON },
> { "pki", PKI },
> { "port", PORT },
> Index: smtpd.conf.5
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/smtpd.conf.5,v
> retrieving revision 1.183
> diff -u -p -r1.183 smtpd.conf.5
> --- smtpd.conf.5 31 May 2018 13:36:35 -0000 1.183
> +++ smtpd.conf.5 31 May 2018 19:56:04 -0000
> @@ -205,6 +205,9 @@ to advertise during the HELO phase.
> .It Cm host Ar relay-url
> Do not perform MX lookups but relay messages to the relay host described by
> .Ar relay-url .
> +If the url uses tls, the certificate will be verified by default.
> +.It Cm tls Ar no-verify
> +Do not require valid certificate for the specified host.
> .It Cm auth Pf < Ar table Ns >
> Use the mapping
> .Ar table
> Index: smtpd.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/smtpd.h,v
> retrieving revision 1.545
> diff -u -p -r1.545 smtpd.h
> --- smtpd.h 29 May 2018 21:05:52 -0000 1.545
> +++ smtpd.h 31 May 2018 19:56:05 -0000
> @@ -1068,6 +1068,7 @@ struct dispatcher_remote {
>
> char *smarthost;
> char *auth;
> + int tls_noverify;
>
> int backup;
> char *backupmx;
