From: gilles <gilles@openbsd.org>
Date: Sun, 19 Apr 2015 20:29:12 +0000 (+0000)
Subject: Incorrect logic in smtpd(8) can lead to unexpected client disconnect, invalid
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=e7151b9671b30434b70c06a576763442c632ca98;p=openbsd

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 bb492d446a0..c2d8cbeac41 100644
--- 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 12b0f199097..0385bd4d1ed 100644
--- 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 49c379a4273..74fa20726ee 100644
--- 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;