DANE: avoid info leak by checking TLSA dnssec before connecting to MX

3 files changed, 64 insertions, 50 deletions

diff --git a/src/src/functions.h b/src/src/functions.h
index 7bbc23aea..963f48ed4 100644
--- a/src/src/functions.h
+++ b/src/src/functions.h
@@ -395,7 +395,7 @@ extern int     sieve_interpret(uschar *, int, uschar *, uschar *, uschar *,
 extern void    sigalrm_handler(int);
 extern BOOL    smtp_buffered(void);
 extern void    smtp_closedown(uschar *);
-extern int     smtp_connect(host_item *, int, int, uschar *, int,
+extern int     smtp_connect(host_item *, int, uschar *, int,
 	       	 transport_instance *);
 extern int     smtp_sock_connect(host_item *, int, int, uschar *,
 		 transport_instance * tb, int);
@@ -415,6 +415,7 @@ extern void    smtp_proxy_tls(uschar *, size_t, int, int);
 extern BOOL    smtp_read_response(smtp_inblock *, uschar *, int, int, int);
 extern void    smtp_respond(uschar *, int, BOOL, uschar *);
 extern void    smtp_notquit_exit(uschar *, uschar *, uschar *, ...);
+extern void    smtp_port_for_connect(host_item *, int);
 extern void    smtp_send_prohibition_message(int, uschar *);
 extern int     smtp_setup_msg(void);
 extern BOOL    smtp_start_session(void);
diff --git a/src/src/smtp_out.c b/src/src/smtp_out.c
index c4f32a0c2..4328cb342 100644
--- a/src/src/smtp_out.c
+++ b/src/src/smtp_out.c
@@ -243,6 +243,24 @@ else
   }
 }
 
+
+
+
+
+void
+smtp_port_for_connect(host_item * host, int port)
+{
+if (host->port != PORT_NONE)
+  {
+  HDEBUG(D_transport|D_acl|D_v)
+    debug_printf_indent("Transport port=%d replaced by host-specific port=%d\n", port,
+      host->port);
+  port = host->port;
+  }
+else host->port = port;    /* Set the port actually used */
+}
+
+
 /*************************************************
 *           Connect to remote host               *
 *************************************************/
@@ -252,15 +270,9 @@ detected by checking for a colon in the address. AF_INET6 is defined even on
 non-IPv6 systems, to enable the code to be less messy. However, on such systems
 host->address will always be an IPv4 address.
 
-The port field in the host item is used if it is set (usually router from SRV
-records or elsewhere). In other cases, the default passed as an argument is
-used, and the host item is updated with its value.
-
 Arguments:
-  host        host item containing name and address (and sometimes port)
+  host        host item containing name and address and port
   host_af     AF_INET or AF_INET6
-  port        default remote port to connect to, in host byte order, for those
-                hosts whose port setting is PORT_NONE
   interface   outgoing interface address or NULL
   timeout     timeout value or 0
   tb          transport
@@ -269,23 +281,15 @@ Returns:      connected socket number, or -1 with errno set
 */
 
 int
-smtp_connect(host_item *host, int host_af, int port, uschar *interface,
+smtp_connect(host_item *host, int host_af, uschar *interface,
   int timeout, transport_instance * tb)
 {
+int port = host->port;
 #ifdef SUPPORT_SOCKS
 smtp_transport_options_block * ob =
   (smtp_transport_options_block *)tb->options_block;
 #endif
 
-if (host->port != PORT_NONE)
-  {
-  HDEBUG(D_transport|D_acl|D_v)
-    debug_printf_indent("Transport port=%d replaced by host-specific port=%d\n", port,
-      host->port);
-  port = host->port;
-  }
-else host->port = port;    /* Set the port actually used */
-
 callout_address = string_sprintf("[%s]:%d", host->address, port);
 
 HDEBUG(D_transport|D_acl|D_v)
diff --git a/src/src/transports/smtp.c b/src/src/transports/smtp.c
index 758f1143a..8f1e0bff8 100644
--- a/src/src/transports/smtp.c
+++ b/src/src/transports/smtp.c
@@ -1186,20 +1186,24 @@ tlsa_lookup(const host_item * host, dns_answer * dnsa, BOOL dane_required)
 /* move this out to host.c given the similarity to dns_lookup() ? */
 uschar buffer[300];
 const uschar * fullname = buffer;
+int rc;
+BOOL sec;
 
 /* TLSA lookup string */
 (void)sprintf(CS buffer, "_%d._tcp.%.256s", host->port, host->name);
 
-switch (dns_lookup(dnsa, buffer, T_TLSA, &fullname))
+rc = dns_lookup(dnsa, buffer, T_TLSA, &fullname);
+sec = dns_is_secure(dnsa);
+DEBUG(D_transport)
+  debug_printf("TLSA lookup ret %d %sDNSSEC\n", rc, sec ? "" : "not ");
+
+switch (rc)
   {
   case DNS_SUCCEED:
-    if (!dns_is_secure(dnsa))
-      {
-      log_write(0, LOG_MAIN, "DANE error: TLSA lookup not DNSSEC");
-      return DEFER;
-      }
-    return OK;
+    if (sec) return OK;
 
+    log_write(0, LOG_MAIN, "DANE error: TLSA lookup not DNSSEC");
+    /*FALLTHROUGH*/
   case DNS_AGAIN:
     return DEFER; /* just defer this TLS'd conn */
 
@@ -1553,34 +1557,17 @@ if (sx->smtps)
 the initial interaction and HELO/EHLO/LHLO. Connect timeout errors are handled
 specially so they can be identified for retries. */
 
-if (continue_hostname == NULL)
+if (!continue_hostname)
   {
   if (sx->verify)
     HDEBUG(D_verify) debug_printf("interface=%s port=%d\n", sx->interface, sx->port);
 
-  /* This puts port into host->port */
-  sx->inblock.sock = sx->outblock.sock =
-    smtp_connect(sx->host, sx->host_af, sx->port, sx->interface,
-		  sx->ob->connect_timeout, sx->tblock);
+  /* Get the actual port the connection will use, into sx->host */
 
-  if (sx->inblock.sock < 0)
-    {
-    uschar * msg = NULL;
-    if (sx->verify)
-      {
-      msg = US strerror(errno);
-      HDEBUG(D_verify) debug_printf("connect: %s\n", msg);
-      }
-    set_errno_nohost(sx->addrlist,
-      errno == ETIMEDOUT ? ERRNO_CONNECTTIMEOUT : errno,
-      sx->verify ? string_sprintf("could not connect: %s", msg)
-	     : NULL,
-      DEFER, FALSE);
-    sx->send_quit = FALSE;
-    return DEFER;
-    }
+  smtp_port_for_connect(sx->host, sx->port);
 
 #if defined(SUPPORT_TLS) && defined(EXPERIMENTAL_DANE)
+    /* Do TLSA lookup for DANE */
     {
     tls_out.dane_verified = FALSE;
     tls_out.tlsa_usage = 0;
@@ -1592,7 +1579,9 @@ if (continue_hostname == NULL)
 	)
 	switch (rc = tlsa_lookup(sx->host, &tlsa_dnsa, sx->dane_required))
 	  {
-	  case OK:		sx->dane = TRUE; break;
+	  case OK:		sx->dane = TRUE;
+				sx->ob->tls_tempfail_tryclear = FALSE;
+				break;
 	  case FAIL_FORCED:	break;
 	  default:		set_errno_nohost(sx->addrlist, ERRNO_DNSDEFER,
 				  string_sprintf("DANE error: tlsa lookup %s",
@@ -1608,12 +1597,32 @@ if (continue_hostname == NULL)
 	FAIL, FALSE);
       return FAIL;
       }
-
-    if (sx->dane)
-      sx->ob->tls_tempfail_tryclear = FALSE;
     }
 #endif	/*DANE*/
 
+  /* Make the TCP connection */
+
+  sx->inblock.sock = sx->outblock.sock =
+    smtp_connect(sx->host, sx->host_af, sx->interface,
+		  sx->ob->connect_timeout, sx->tblock);
+
+  if (sx->inblock.sock < 0)
+    {
+    uschar * msg = NULL;
+    if (sx->verify)
+      {
+      msg = US strerror(errno);
+      HDEBUG(D_verify) debug_printf("connect: %s\n", msg);
+      }
+    set_errno_nohost(sx->addrlist,
+      errno == ETIMEDOUT ? ERRNO_CONNECTTIMEOUT : errno,
+      sx->verify ? string_sprintf("could not connect: %s", msg)
+	     : NULL,
+      DEFER, FALSE);
+    sx->send_quit = FALSE;
+    return DEFER;
+    }
+
   /* Expand the greeting message while waiting for the initial response. (Makes
   sense if helo_data contains ${lookup dnsdb ...} stuff). The expansion is
   delayed till here so that $sending_interface and $sending_port are set. */