Incorrect logic in smtpd(8) can lead to unexpected client disconnect, invalid

certificate in SNI negotiation or server crash.

spotted by Edwin Torok

diff --git a/usr.sbin/smtpd/smtp_session.c b/usr.sbin/smtpd/smtp_session.c
index bb492d4..c2d8cbe 100644 (file)
--- a/usr.sbin/smtpd/smtp_session.c
+++ b/usr.sbin/smtpd/smtp_session.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: smtp_session.c,v 1.228 2015/04/06 13:47:00 gilles Exp $       */
+/*     $OpenBSD: smtp_session.c,v 1.229 2015/04/19 20:29:12 gilles Exp $       */
 
 /*
  * Copyright (c) 2008 Gilles Chehade <gilles@poolp.org>
@@ -191,6 +191,7 @@ static uint8_t dsn_notify_str_to_uint8(const char *);
 static void smtp_auth_failure_pause(struct smtp_session *);
 static void smtp_auth_failure_resume(int, short, void *);
 static int smtp_sni_callback(SSL *, int *, void *);
+static const char *smtp_sni_get_servername(struct smtp_session *);
 
 static void smtp_filter_connect(struct smtp_session *, struct sockaddr *);
 static void smtp_filter_rset(struct smtp_session *);
@@ -852,7 +853,7 @@ smtp_session_imsg(struct mproc *p, struct imsg *imsg)
                        pkiname = s->smtpname;
                ssl_ctx = dict_get(env->sc_ssl_dict, pkiname);
 
-               ssl = ssl_smtp_init(ssl_ctx, smtp_sni_callback, s);
+               ssl = ssl_smtp_init(ssl_ctx, smtp_sni_callback);
                io_set_read(&s->io);
                io_start_tls(&s->io, ssl);
 
@@ -1031,6 +1032,7 @@ smtp_io(struct io *io, int evt)
 {
        struct ca_cert_req_msg  req_ca_cert;
        struct smtp_session    *s = io->arg;
+       const char             *sn;
        char                   *line;
        size_t                  len, i;
        X509                   *x;
@@ -1048,6 +1050,14 @@ smtp_io(struct io *io, int evt)
                s->kickcount = 0;
                s->phase = PHASE_INIT;
 
+               sn = smtp_sni_get_servername(s);
+               if (sn) {
+                       if (strlcpy(s->sni, sn, sizeof s->sni) >= sizeof s->sni) {
+                               smtp_free(s, "client SNI exceeds max hostname length");
+                               return;
+                       }
+               }
+
                if (smtp_verify_certificate(s)) {
                        io_pause(&s->io, IO_PAUSE_IN);
                        break;
@@ -2139,26 +2149,24 @@ smtp_auth_failure_pause(struct smtp_session *s)
        evtimer_add(&s->pause, &tv);
 }
 
+static const char *
+smtp_sni_get_servername(struct smtp_session *s)
+{
+       return SSL_get_servername(s->io.ssl, TLSEXT_NAMETYPE_host_name);
+}
+
 static int
 smtp_sni_callback(SSL *ssl, int *ad, void *arg)
 {
        const char              *sn;
-       struct smtp_session     *s = arg;
        void                    *ssl_ctx;
 
        sn = SSL_get_servername(ssl, TLSEXT_NAMETYPE_host_name);
        if (sn == NULL)
                return SSL_TLSEXT_ERR_NOACK;
-       if (strlcpy(s->sni, sn, sizeof s->sni) >= sizeof s->sni) {
-               log_warnx("warn: client SNI exceeds max hostname length");
-               return SSL_TLSEXT_ERR_NOACK;
-       }
        ssl_ctx = dict_get(env->sc_ssl_dict, sn);
-       if (ssl_ctx == NULL) {
-               log_info("smtp-in: No PKI entry for requested SNI \"%s\""
-                   "on session %016"PRIx64, sn, s->id);
+       if (ssl_ctx == NULL)
                return SSL_TLSEXT_ERR_NOACK;
-       }
        SSL_set_SSL_CTX(ssl, ssl_ctx);
        return SSL_TLSEXT_ERR_OK;
 }

diff --git a/usr.sbin/smtpd/smtpd.h b/usr.sbin/smtpd/smtpd.h
index 12b0f19..0385bd4 100644 (file)
--- a/usr.sbin/smtpd/smtpd.h
+++ b/usr.sbin/smtpd/smtpd.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: smtpd.h,v 1.473 2015/01/20 17:37:54 deraadt Exp $     */
+/*     $OpenBSD: smtpd.h,v 1.474 2015/04/19 20:29:12 gilles Exp $      */
 
 /*
  * Copyright (c) 2008 Gilles Chehade <gilles@poolp.org>
@@ -1322,7 +1322,7 @@ int fork_proc_backend(const char *, const char *, const char *);
 
 /* ssl_smtpd.c */
 void   *ssl_mta_init(void *, char *, off_t);
-void   *ssl_smtp_init(void *, void *, void *);
+void   *ssl_smtp_init(void *, void *);
 
 
 /* stat_backend.c */

diff --git a/usr.sbin/smtpd/ssl_smtpd.c b/usr.sbin/smtpd/ssl_smtpd.c
index 49c379a..74fa207 100644 (file)
--- a/usr.sbin/smtpd/ssl_smtpd.c
+++ b/usr.sbin/smtpd/ssl_smtpd.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ssl_smtpd.c,v 1.8 2015/01/16 06:40:21 deraadt Exp $   */
+/*     $OpenBSD: ssl_smtpd.c,v 1.9 2015/04/19 20:29:12 gilles Exp $    */
 
 /*
  * Copyright (c) 2008 Pierre-Yves Ritschard <pyr@openbsd.org>
@@ -82,7 +82,7 @@ dummy_verify(int ok, X509_STORE_CTX *store)
 }
 
 void *
-ssl_smtp_init(void *ssl_ctx, void *sni, void *arg)
+ssl_smtp_init(void *ssl_ctx, void *sni)
 {
        SSL     *ssl = NULL;
        int     (*cb)(SSL *,int *,void *) = sni;
@@ -91,10 +91,8 @@ ssl_smtp_init(void *ssl_ctx, void *sni, void *arg)
 
        SSL_CTX_set_verify(ssl_ctx, SSL_VERIFY_PEER, dummy_verify);
 
-       if (cb) {
+       if (cb)
                SSL_CTX_set_tlsext_servername_callback(ssl_ctx, cb);
-               SSL_CTX_set_tlsext_servername_arg(ssl_ctx, arg);
-       }
 
        if ((ssl = SSL_new(ssl_ctx)) == NULL)
                goto err;