On Thu, Jan 15, 2015 at 03:20:27AM +0000, Viktor Dukhovni wrote:
> And of course with "may", we need to avoid any attempt at cleartext
> fallback if we're doing wrapper-mode SMTP.
With the previous posttls-finger patch, it was still possible to
attempt both wrapper-mode (-w) and TLS disabled (-l none). This
ended up attempting to send "QUIT" as the sole cleartext command
to an SSL server.
I am attaching a slightly more comprehensive patch, which also
cleans up a bit of kludge in how "dane-only" was implemented. Now
it no longer automagically morphs to "dane" after successful TLSA
lookup. Instead the level-related in tls.h are extended to add
TLS_DANE_BASED - either "dane" or "dane-only"
TLS_OPPORTUNISTIC - either "may" or "dane"
which can be used when appropriate. The resulting code is cleaner
I think (wrapper-mode aside). Just 3 extra lines of code, outside
the changes to posttls-finger to support wrapper-mode.
This patch works with either the latest nonprod or regular 2.12
snapshot and does not conflict with the "sslbitrot" patch to address
future upstream changes in the OpenSSL development branch. So please
apply both when you get a chance.
As for wrapper mode, I think it should insist on the destinations
security level being neither disabled nor opportunitic as in the
revised posttls-finger. Something along the lines of:
+ if (state->wrapper_mode
+ && (state->level <= TLS_LEV_NONE
+ || TLS_OPPORTUNISTIC(state->level))) {
+ msg_info("Failed to establish session to %s via %s: %s",
+ dest, HNAME(addr),
+ "SSL wrapper-mode requires mandatory TLS");
+ continue;
+ }
The trouble with allowing "may" in wrapper-mode is that we'd
otherwise need to change the logic that implements cleartext
fallback, future audit logs, ...
The trouble with "dane" is more of a judgement call, "dane" may
work when originally tested, provided TLSA records are published
for port 465, and later revert to "may" and break if TLSA records
are removed by the domain owner. This triggers potentially misleading
error messages. Since "dane" is effectively mandatory in wrapper-mode,
why not say so by specifying dane-only"?
--
Viktor.
>From 0934b6658bbe4a8c3bc5c359b4bc0ae49262a900 Mon Sep 17 00:00:00 2001
From: Viktor Dukhovni <postfix-us...@dukhovni.org>
Date: Wed, 14 Jan 2015 23:30:38 -0500
Subject: [PATCH 1/1] Wrapper mode for posttls-finger and related DANE cleanup
---
src/posttls-finger/posttls-finger.c | 156 ++++++++++++++++++++++--------------
src/smtp/smtp_tls_policy.c | 5 +-
src/tls/tls.h | 2 +
src/tls/tls_client.c | 10 +--
src/tls/tls_fprint.c | 2 +-
5 files changed, 109 insertions(+), 66 deletions(-)
diff --git a/src/posttls-finger/posttls-finger.c
b/src/posttls-finger/posttls-finger.c
index 8be46e4..10c5831 100644
--- a/src/posttls-finger/posttls-finger.c
+++ b/src/posttls-finger/posttls-finger.c
@@ -228,6 +228,12 @@
/* .IP "\fB-v\fR"
/* Enable verose Postfix logging. Specify more than once to increase
/* the level of verbose logging.
+/* .IP "\fB-w\fR"
+/* Enable outgoing TLS wrapper mode, or SMTPS support. This is typically
+/* provided on port 465 by servers that are compatible with the ad-hoc
+/* SMTP in SSL protocol, rather than the standard STARTTLS protocol.
+/* The destination \fIdomain\fR:\fIport\fR should of course provide such
+/* a service.
/* .IP "[\fBinet:\fR]\fIdomain\fR[:\fIport\fR]"
/* Connect via TCP to domain \fIdomain\fR, port \fIport\fR. The default
/* port is \fBsmtp\fR (or 24 with LMTP). With SMTP an MX lookup is
@@ -421,6 +427,7 @@ typedef struct STATE {
VSTRING *buffer; /* Response buffer */
VSTREAM *stream; /* Open connection */
int level; /* TLS security level */
+ int wrapper_mode; /* SMTPS support */
#ifdef USE_TLS
char *mdalg; /* fingerprint digest algorithm */
char *CAfile; /* Trusted public CAs */
@@ -547,6 +554,34 @@ static char *exception_text(int except)
}
}
+/* greeting - read server's 220 greeting */
+
+static int greeting(STATE *state)
+{
+ VSTREAM *stream = state->stream;
+ int except;
+ RESPONSE *resp;
+
+ /*
+ * Prepare for disaster.
+ */
+ smtp_stream_setup(stream, conn_tmout, 1);
+ if ((except = vstream_setjmp(stream)) != 0) {
+ msg_info("%s while reading server greeting", exception_text(except));
+ return (1);
+ }
+
+ /*
+ * Read and parse the server's SMTP greeting banner.
+ */
+ if (((resp = response(state, 1))->code / 100) != 2) {
+ msg_info("SMTP service not available: %d %s", resp->code, resp->str);
+ return (1);
+ }
+
+ return (0);
+}
+
/* ehlo - send EHLO/LHLO */
static RESPONSE *ehlo(STATE *state)
@@ -647,25 +682,27 @@ static int starttls(STATE *state)
VSTREAM *stream = state->stream;
TLS_CLIENT_START_PROPS tls_props;
- /* SMTP stream with deadline timeouts */
- smtp_stream_setup(stream, smtp_tmout, 1);
- if ((except = vstream_setjmp(stream)) != 0) {
- msg_fatal("%s while sending STARTTLS", exception_text(except));
- return (1);
- }
- command(state, state->pass == 1, "STARTTLS");
+ if (state->wrapper_mode == 0) {
+ /* SMTP stream with deadline timeouts */
+ smtp_stream_setup(stream, smtp_tmout, 1);
+ if ((except = vstream_setjmp(stream)) != 0) {
+ msg_fatal("%s while sending STARTTLS", exception_text(except));
+ return (1);
+ }
+ command(state, state->pass == 1, "STARTTLS");
- resp = response(state, state->pass == 1);
- if (resp->code / 100 != 2) {
- msg_info("STARTTLS rejected: %d %s", resp->code, resp->str);
- return (1);
- }
+ resp = response(state, state->pass == 1);
+ if (resp->code / 100 != 2) {
+ msg_info("STARTTLS rejected: %d %s", resp->code, resp->str);
+ return (1);
+ }
- /*
- * Discard any plain-text data that may be piggybacked after the server's
- * 220 STARTTLS reply. Should we abort the session instead?
- */
- vstream_fpurge(stream, VSTREAM_PURGE_READ);
+ /*
+ * Discard any plain-text data that may be piggybacked after the
server's
+ * 220 STARTTLS reply. Should we abort the session instead?
+ */
+ vstream_fpurge(stream, VSTREAM_PURGE_READ);
+ }
#define ADD_EXCLUDE(vstr, str) \
do { \
@@ -718,6 +755,10 @@ static int starttls(STATE *state)
state->stream = 0;
return (1);
}
+
+ if (state->wrapper_mode && greeting(state) != 0)
+ return (1);
+
if (state->pass == 1) {
ehlo(state);
if (!TLS_CERT_IS_PRESENT(state->tls_context))
@@ -743,42 +784,28 @@ static int doproto(STATE *state)
int except;
int n;
char *lines;
- char *words;
+ char *words = 0;
char *word;
- /*
- * Prepare for disaster.
- */
- smtp_stream_setup(stream, conn_tmout, 1);
- if ((except = vstream_setjmp(stream)) != 0)
- msg_fatal("%s while reading server greeting", exception_text(except));
-
- /*
- * Read and parse the server's SMTP greeting banner.
- */
- if (((resp = response(state, 1))->code / 100) != 2) {
- msg_info("SMTP service not available: %d %s", resp->code, resp->str);
- return (1);
- }
-
- /*
- * Send the standard greeting with our hostname
- */
- if ((resp = ehlo(state)) == 0)
- return (1);
+ if (! state->wrapper_mode) {
+ if (greeting(state) != 0)
+ return (1);
+ if ((resp = ehlo(state)) == 0)
+ return (1);
- lines = resp->str;
- for (n = 0; (words = mystrtok(&lines, "\n")) != 0; ++n) {
- if ((word = mystrtok(&words, " \t=")) != 0) {
- if (n == 0)
- state->helo = mystrdup(word);
- if (strcasecmp(word, "STARTTLS") == 0)
- break;
+ lines = resp->str;
+ for (n = 0; (words = mystrtok(&lines, "\n")) != 0; ++n) {
+ if ((word = mystrtok(&words, " \t=")) != 0) {
+ if (n == 0)
+ state->helo = mystrdup(word);
+ if (strcasecmp(word, "STARTTLS") == 0)
+ break;
+ }
}
}
#ifdef USE_TLS
- if (words && state->tls_ctx)
+ if ((state->wrapper_mode || words) && state->tls_ctx)
if (starttls(state))
return (1);
#endif
@@ -791,9 +818,9 @@ static int doproto(STATE *state)
msg_warn("%s while sending QUIT command", exception_text(except));
return (0);
}
+
command(state, 1, "QUIT");
(void) response(state, 1);
-
return (0);
}
@@ -1191,7 +1218,7 @@ static int dane_host_level(STATE *state, DNS_RR *addr)
int level = state->level;
#ifdef USE_TLS
- if (level == TLS_LEV_DANE) {
+ if (TLS_DANE_BASED(level)) {
if (state->mx == 0 || state->mx->dnssec_valid) {
if (state->log_mask & (TLS_LOG_CERTMATCH | TLS_LOG_VERBOSE))
tls_dane_verbose(1);
@@ -1214,7 +1241,8 @@ static int dane_host_level(STATE *state, DNS_RR *addr)
HNAME(addr), ntohs(state->port));
level = TLS_LEV_INVALID;
} else if (tls_dane_notfound(state->ddane)
- || tls_dane_unusable(state->ddane)) {
+ || tls_dane_unusable(state->ddane)
+ || level == TLS_LEV_DANE_ONLY) {
if (msg_verbose)
msg_info("no %sTLSA records found, "
"resorting to \"secure\"",
@@ -1223,7 +1251,7 @@ static int dane_host_level(STATE *state, DNS_RR *addr)
level = TLS_LEV_SECURE;
} else if (!TLS_DANE_HASTA(state->ddane)
&& !TLS_DANE_HASEE(state->ddane)) {
- msg_panic("empty DANE match list");
+ msg_panic("DANE activated with no TLSA records to match");
} else {
if (state->match)
argv_free(state->match);
@@ -1313,8 +1341,18 @@ static void connect_remote(STATE *state, char *dest)
}
}
for (addr = state->addr; addr; addr = addr->next) {
- int level = dane_host_level(state, addr);
+ int level;
+
+ if (state->wrapper_mode
+ && (state->level <= TLS_LEV_NONE
+ || TLS_OPPORTUNISTIC(state->level))) {
+ msg_info("Failed to establish session to %s via %s: %s",
+ dest, HNAME(addr),
+ "SSL wrapper-mode requires mandatory TLS");
+ continue;
+ }
+ level = dane_host_level(state, addr);
if (level == TLS_LEV_INVALID
|| (state->stream = connect_addr(state, addr)) == 0) {
msg_info("Failed to establish session to %s via %s: %s",
@@ -1531,7 +1569,7 @@ static void usage(void)
#ifdef USE_TLS
fprintf(stderr, "usage: %s %s \\\n\t%s \\\n\t%s \\\n\t%s"
" destination [match ...]\n", var_procname,
- "[-acCfSv] [-t conn_tmout] [-T cmd_tmout] [-L logopts]",
+ "[-acCfSvw] [-t conn_tmout] [-T cmd_tmout] [-L logopts]",
"[-h host_lookup] [-l level] [-d mdalg] [-g grade] [-p protocols]",
"[-A tafile] [-F CAfile.pem] [-P CApath/] [-m count] [-r delay]",
"[-o name=value]");
@@ -1594,6 +1632,7 @@ static void parse_options(STATE *state, int argc, char
*argv[])
state->pass = 1;
state->reconnect = -1;
state->max_reconnect = 5;
+ state->wrapper_mode = 0;
#ifdef USE_TLS
state->protocols = mystrdup("!SSLv2");
state->grade = mystrdup("medium");
@@ -1603,7 +1642,7 @@ static void parse_options(STATE *state, int argc, char
*argv[])
#define OPTS "a:ch:o:St:T:v"
#ifdef USE_TLS
-#define TLSOPTS "A:Cd:fF:g:l:L:m:p:P:r:"
+#define TLSOPTS "A:Cd:fF:g:l:L:m:p:P:r:w"
state->mdalg = mystrdup("sha1");
state->CApath = mystrdup("");
@@ -1692,6 +1731,9 @@ static void parse_options(STATE *state, int argc, char
*argv[])
case 'r':
state->reconnect = atoi(optarg);
break;
+ case 'w':
+ state->wrapper_mode = 1;
+ break;
#endif
}
}
@@ -1725,9 +1767,6 @@ static void parse_options(STATE *state, int argc, char
*argv[])
state->level = tls_level_lookup(state->options.level);
switch (state->level) {
- case TLS_LEV_DANE_ONLY:
- state->level = TLS_LEV_DANE;
- break;
case TLS_LEV_NONE:
return;
case TLS_LEV_INVALID:
@@ -1741,8 +1780,8 @@ static void parse_options(STATE *state, int argc, char
*argv[])
* required for DANE support.
*/
tls_init(state);
- if (state->level == TLS_LEV_DANE && !tls_dane_avail()) {
- msg_warn("The \"dane\" TLS security level is not available");
+ if (TLS_DANE_BASED(state->level) && !tls_dane_avail()) {
+ msg_warn("DANE TLS support is not available, resorting to \"secure\"");
state->level = TLS_LEV_SECURE;
}
state->tls_bio = 0;
@@ -1780,6 +1819,7 @@ static void parse_match(STATE *state, int argc, char
*argv[])
state->mdalg, *argv++, "");
break;
case TLS_LEV_DANE:
+ case TLS_LEV_DANE_ONLY:
state->match = argv_alloc(2);
argv_add(state->match, "nexthop", "hostname", ARGV_END);
break;
diff --git a/src/smtp/smtp_tls_policy.c b/src/smtp/smtp_tls_policy.c
index 22b76ab..deb7e04 100644
--- a/src/smtp/smtp_tls_policy.c
+++ b/src/smtp/smtp_tls_policy.c
@@ -437,6 +437,7 @@ static void set_cipher_grade(SMTP_TLS_POLICY *tls)
break;
case TLS_LEV_DANE:
+ case TLS_LEV_DANE_ONLY:
case TLS_LEV_FPRINT:
case TLS_LEV_VERIFY:
case TLS_LEV_SECURE:
@@ -534,7 +535,7 @@ static void *policy_create(const char *unused_key, void
*context)
* "dane-only" changes to "dane" once we obtain the requisite TLSA
* records.
*/
- if (tls->level == TLS_LEV_DANE || tls->level == TLS_LEV_DANE_ONLY)
+ if (TLS_DANE_BASED(tls->level))
dane_init(tls, iter);
if (tls->level == TLS_LEV_INVALID)
return ((void *) tls);
@@ -563,6 +564,7 @@ static void *policy_create(const char *unused_key, void
*context)
case TLS_LEV_MAY:
case TLS_LEV_ENCRYPT:
case TLS_LEV_DANE:
+ case TLS_LEV_DANE_ONLY:
break;
case TLS_LEV_FPRINT:
if (tls->dane == 0)
@@ -844,7 +846,6 @@ static void dane_init(SMTP_TLS_POLICY *tls, SMTP_ITERATOR
*iter)
} else if (!TLS_DANE_HASEE(dane))
msg_panic("empty DANE match list");
tls->dane = dane;
- tls->level = TLS_LEV_DANE;
return;
}
diff --git a/src/tls/tls.h b/src/tls/tls.h
index 9e5f3dc..16e49c5 100644
--- a/src/tls/tls.h
+++ b/src/tls/tls.h
@@ -53,6 +53,8 @@
#define TLS_MUST_MATCH(l) ((l) > TLS_LEV_ENCRYPT)
#define TLS_MUST_TRUST(l) ((l) >= TLS_LEV_DANE)
#define TLS_MUST_PKIX(l) ((l) >= TLS_LEV_VERIFY)
+#define TLS_OPPORTUNISTIC(l) ((l) == TLS_LEV_MAY || (l) == TLS_LEV_DANE)
+#define TLS_DANE_BASED(l) ((l) == TLS_LEV_DANE || (l) ==
TLS_LEV_DANE_ONLY)
extern const NAME_CODE tls_level_table[];
diff --git a/src/tls/tls_client.c b/src/tls/tls_client.c
index 8211e26..1ca244f 100644
--- a/src/tls/tls_client.c
+++ b/src/tls/tls_client.c
@@ -827,10 +827,10 @@ TLS_SESS_STATE *tls_client_start(const
TLS_CLIENT_START_PROPS *props)
/*
* When certificate verification is required, log trust chain validation
* errors even when disabled by default for opportunistic sessions. For
- * "dane" this only applies when using trust-anchor associations.
+ * DANE this only applies when using trust-anchor associations.
*/
if (TLS_MUST_TRUST(props->tls_level)
- && (props->tls_level != TLS_LEV_DANE || TLS_DANE_HASTA(props->dane)))
+ && (! TLS_DANE_BASED(props->tls_level) || TLS_DANE_HASTA(props->dane)))
log_mask |= TLS_LOG_UNTRUSTED;
if (log_mask & TLS_LOG_VERBOSE)
@@ -849,8 +849,8 @@ TLS_SESS_STATE *tls_client_start(const
TLS_CLIENT_START_PROPS *props)
props->namaddr, props->protocols);
return (0);
}
- /* The DANE level requires SSLv3 or later, not SSLv2. */
- if (props->tls_level == TLS_LEV_DANE)
+ /* DANE requires SSLv3 or later, not SSLv2. */
+ if (TLS_DANE_BASED(props->tls_level))
protomask |= TLS_PROTOCOL_SSLv2;
/*
@@ -945,7 +945,7 @@ TLS_SESS_STATE *tls_client_start(const
TLS_CLIENT_START_PROPS *props)
}
}
#ifdef TLSEXT_MAXLEN_host_name
- if (props->tls_level == TLS_LEV_DANE
+ if (TLS_DANE_BASED(props->tls_level)
&& strlen(props->host) <= TLSEXT_MAXLEN_host_name) {
/*
diff --git a/src/tls/tls_fprint.c b/src/tls/tls_fprint.c
index abd5ff7..a03e3cc 100644
--- a/src/tls/tls_fprint.c
+++ b/src/tls/tls_fprint.c
@@ -227,7 +227,7 @@ char *tls_serverid_digest(const TLS_CLIENT_START_PROPS
*props, long protomask,
#if 0
digest_dane(props->dane, ee); /* See above */
#endif
- digest_string(props->tls_level == TLS_LEV_DANE ? props->host : "");
+ digest_string(TLS_DANE_BASED(props->tls_level) ? props->host : "");
}
checkok(EVP_DigestFinal_ex(mdctx, md_buf, &md_len));
EVP_MD_CTX_destroy(mdctx);
--
1.9.3 (Apple Git-50)
