From: Wietse Venema <wietse@porcupine.org>
Date: Sun, 11 May 2008 05:00:00 +0000 (-0500)
Subject: postfix-2.6-20080511
X-Git-Tag: v2.6.0-RC1~27
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ed2e87fe29d865cc885d2da12d7b8cdc637dad15;p=thirdparty%2Fpostfix.git

postfix-2.6-20080511
---

diff --git a/postfix/HISTORY b/postfix/HISTORY
index 51750adaa..1cfc7c526 100644
--- a/postfix/HISTORY
+++ b/postfix/HISTORY
@@ -14457,8 +14457,18 @@ Apologies for any names omitted.
 
 20080510
 
-	Cleanup: when extracting common name and issuer name from
+	Cleanup: when extracting peer and issuer common name from
 	TLS certificates, convert the result into UTF-8, and use
 	RFC 2047 encoding when logging these as Received: header
 	comment fields. Based remotely on code by Victor Duchovni.
 	Files: smtpd/smtpd.c, tls/tls_verify.c.
+
+20080511
+
+	Cleanup: the RFC 2047 encoding of RFC*822 comments is too
+	problematic.  The text that explains the problems is as
+	long as the code itself.  That is usually a good indication
+	that code is not ready for use.  File: smtpd/smtpd.c.
+
+	Cleanup: block non-printable ASCII text in UTF8 encoded TLS
+	peer and issuer common names.  File: tls/tls_verify.c.
diff --git a/postfix/README_FILES/SMTPD_POLICY_README b/postfix/README_FILES/SMTPD_POLICY_README
index 80e2edbe9..7ba1838da 100644
--- a/postfix/README_FILES/SMTPD_POLICY_README
+++ b/postfix/README_FILES/SMTPD_POLICY_README
@@ -127,9 +127,8 @@ Notes:
     case of no certificate authentication. As of Postfix 2.2.11 these attribute
     values are encoded as xtext: some characters are represented by +XX, where
     XX is the two-digit hexadecimal representation of the character value. With
-    Postfix 2.5 and later, the decoded string may contain non-ASCII characters.
-    If so, this is a UTF-8 string; xtext encoding works with the bytes of the
-    UTF-8 string, not the characters.
+    Postfix 2.6 and later, the decoded string is an UTF-8 string without non-
+    printable ASCII characters.
 
   * The "encryption_*" attributes (Postfix 2.3 and later) specify information
     about how the connection is encrypted. With plaintext connections the
diff --git a/postfix/RELEASE_NOTES b/postfix/RELEASE_NOTES
index fb605fea6..ca72d68c3 100644
--- a/postfix/RELEASE_NOTES
+++ b/postfix/RELEASE_NOTES
@@ -22,7 +22,7 @@ Incompatibility with snapshot 20080510
 
 In the policy delegation protocol, certificate common name attributes
 are now xtext encoded UTF-8. The xtext decoded attributes may contain
-any UTF-8 value including control characters.
+any UTF-8 value except non-printable ASCII characters.
 
 Incompatibility with snapshot 20080428
 ======================================
diff --git a/postfix/html/SMTPD_POLICY_README.html b/postfix/html/SMTPD_POLICY_README.html
index 4471aaf16..fef97a1ac 100644
--- a/postfix/html/SMTPD_POLICY_README.html
+++ b/postfix/html/SMTPD_POLICY_README.html
@@ -174,10 +174,8 @@ stress=
     As of Postfix 2.2.11 these attribute values are encoded as
     xtext: some characters are represented by +XX, where XX is the
     two-digit hexadecimal representation of the character value. With
-    Postfix 2.5 and later, the decoded string may contain non-ASCII
-    characters. If so, this is a UTF-8 string; xtext encoding works
-    with the bytes of the UTF-8 string, not the characters.
-    </p>
+    Postfix 2.6 and later, the decoded string is an UTF-8 string
+    without non-printable ASCII characters.  </p>
 
     <li> <p> The "encryption_*" attributes (Postfix 2.3 and later)
     specify information about how the connection is encrypted. With
diff --git a/postfix/proto/SMTPD_POLICY_README.html b/postfix/proto/SMTPD_POLICY_README.html
index d3e6f005f..549a9d6e3 100644
--- a/postfix/proto/SMTPD_POLICY_README.html
+++ b/postfix/proto/SMTPD_POLICY_README.html
@@ -174,10 +174,8 @@ stress=
     As of Postfix 2.2.11 these attribute values are encoded as
     xtext: some characters are represented by +XX, where XX is the
     two-digit hexadecimal representation of the character value. With
-    Postfix 2.5 and later, the decoded string may contain non-ASCII
-    characters. If so, this is a UTF-8 string; xtext encoding works
-    with the bytes of the UTF-8 string, not the characters.
-    </p>
+    Postfix 2.6 and later, the decoded string is an UTF-8 string
+    without non-printable ASCII characters.  </p>
 
     <li> <p> The "encryption_*" attributes (Postfix 2.3 and later)
     specify information about how the connection is encrypted. With
diff --git a/postfix/src/global/mail_version.h b/postfix/src/global/mail_version.h
index c3fdb6d8d..9e80dff98 100644
--- a/postfix/src/global/mail_version.h
+++ b/postfix/src/global/mail_version.h
@@ -20,7 +20,7 @@
   * Patches change both the patchlevel and the release date. Snapshots have no
   * patchlevel; they change the release date only.
   */
-#define MAIL_RELEASE_DATE	"20080510"
+#define MAIL_RELEASE_DATE	"20080511"
 #define MAIL_VERSION_NUMBER	"2.6"
 
 #ifdef SNAPSHOT
diff --git a/postfix/src/smtpd/smtpd.c b/postfix/src/smtpd/smtpd.c
index c1c3b8710..dca5bd87e 100644
--- a/postfix/src/smtpd/smtpd.c
+++ b/postfix/src/smtpd/smtpd.c
@@ -2496,6 +2496,8 @@ static void rcpt_reset(SMTPD_STATE *state)
     state->rcpt_overshoot = 0;
 }
 
+#if 0
+
 /* rfc2047_comment_encode - encode comment string */
 
 static VSTRING *rfc2047_comment_encode(const char *str, const char *charset)
@@ -2505,22 +2507,29 @@ static VSTRING *rfc2047_comment_encode(const char *str, const char *charset)
     int     ch;
 
     /*
-     * XXX Most of the RFC 2047 "especials" are not special in RFC*822
-     * comments, but we encode them anyway to avoid complaints.
+     * XXX This is problematic code.
+     * 
+     * XXX Most of the RFC 2047 "especials" are not special in RFC*822 comments,
+     * but we encode them anyway to avoid complaints.
      * 
      * XXX In Received: header comments we enclose peer and issuer common names
-     * with "" quotes. This is the cause of several quirks.
+     * with "" quotes (inherited from the Lutz Jaenicke patch). This is the
+     * cause of several quirks.
      * 
      * 1) We encode text that contains the " character, even though that
-     * character is not special for RFC*822.
+     * character is not special for RFC*822 comments.
      * 
-     * 2) Long comments look ugly when folded in-between quotes, so we ignore
-     * the recommended limit of 75 characters per encoded word.
+     * 2) We ignore the recommended limit of 75 characters per encoded word,
+     * because long comments look ugly when folded in-between quotes.
      * 
-     * 3) We must encode the the enclosing quotes, to avoid producing invalid
-     * encoded words.
+     * 3) We encode the enclosing quotes, to avoid producing invalid encoded
+     * words. Microsoft abuses RFC 2047 encoding with attachment names, but
+     * we have no information on what decoders do with malformed encoding in
+     * comments. This means the comments are Jaenicke-compatible only after
+     * decoding.
      */
 #define ESPECIALS "()<>@,;:\"/[]?.="		/* Special in RFC 2047 */
+#define QSPECIALS "_" ESPECIALS			/* Special in RFC 2047 'Q' */
 #define CSPECIALS "\\\"()"			/* Special in our comments */
 
     /* Don't encode if not needed. */
@@ -2538,7 +2547,7 @@ static VSTRING *rfc2047_comment_encode(const char *str, const char *charset)
      */
     vstring_sprintf(buf, "=?%s?Q?=%02X", charset, '"');
     for (cp = (unsigned char *) str; (ch = *cp) != 0; ++cp) {
-	if (!ISPRINT(ch) || strchr(ESPECIALS CSPECIALS, ch)) {
+	if (!ISPRINT(ch) || strchr(QSPECIALS CSPECIALS, ch)) {
 	    vstring_sprintf_append(buf, "=%02X", ch);
 	} else if (ch == ' ') {
 	    VSTRING_ADDCH(buf, '_');
@@ -2550,6 +2559,8 @@ static VSTRING *rfc2047_comment_encode(const char *str, const char *charset)
     return (buf);
 }
 
+#endif
+
 /* comment_sanitize - clean up comment string */
 
 static void comment_sanitize(VSTRING *comment_string)
@@ -2709,10 +2720,6 @@ static int data_cmd(SMTPD_STATE *state, int argc, SMTPD_TOKEN *unused_argv)
 
 #define VSTRING_STRDUP(s) vstring_strcpy(vstring_alloc(strlen(s) + 1), (s))
 
-	/*
-	 * Certificate CN information is arbitrary content in the UTF-8
-	 * character set.
-	 */
 #ifdef USE_TLS
 	if (var_smtpd_tls_received_header && state->tls_context) {
 	    out_fprintf(out_stream, REC_TYPE_NORM,
@@ -2722,14 +2729,13 @@ static int data_cmd(SMTPD_STATE *state, int argc, SMTPD_TOKEN *unused_argv)
 			state->tls_context->cipher_usebits,
 			state->tls_context->cipher_algbits);
 	    if (TLS_CERT_IS_PRESENT(state->tls_context)) {
-		peer_CN =
-		    rfc2047_comment_encode(state->tls_context->peer_CN,
-					   "utf-8");
-		issuer_CN =
-		    rfc2047_comment_encode(state->tls_context->issuer_CN,
-					   "utf-8");
+		peer_CN = VSTRING_STRDUP(state->tls_context->peer_CN);
+		comment_sanitize(peer_CN);
+		issuer_CN = VSTRING_STRDUP(state->tls_context->issuer_CN ?
+					state->tls_context->issuer_CN : "");
+		comment_sanitize(issuer_CN);
 		out_fprintf(out_stream, REC_TYPE_NORM,
-			    "\t(Client CN %s, Issuer %s (%s))",
+			    "\t(Client CN \"%s\", Issuer \"%s\" (%s))",
 			    STR(peer_CN), STR(issuer_CN),
 			    TLS_CERT_IS_TRUSTED(state->tls_context) ?
 			    "verified OK" : "not verified");
diff --git a/postfix/src/tls/tls_verify.c b/postfix/src/tls/tls_verify.c
index 5a5e07668..d6da9b14e 100644
--- a/postfix/src/tls/tls_verify.c
+++ b/postfix/src/tls/tls_verify.c
@@ -30,12 +30,14 @@
 /*	tls_peer_CN() returns the text CommonName for the peer
 /*	certificate subject, or an empty string if no CommonName was
 /*	found. The result is allocated with mymalloc() and must be
-/*	freed by the caller; it is arbitrary UTF-8 content.
+/*	freed by the caller; it contains UTF-8 without non-printable
+/*	ASCII characters.
 /*
 /*	tls_issuer_CN() returns the text CommonName for the peer
 /*	certificate issuer, or an empty string if no CommonName was
 /*	found. The result is allocated with mymalloc() and must be
-/*	freed by the caller; it is arbitrary UTF-8 content.
+/*	freed by the caller; it contains UTF-8 without non-printable
+/*	ASCII characters.
 /*
 /*	tls_dns_name() returns the string value of a GENERAL_NAME
 /*	from a DNS subjectAltName extension. If non-printable characters
@@ -281,6 +283,8 @@ static char *tls_text_name(X509_NAME *name, int nid, const char *label,
     int     asn1_type;
     int     utf8_length;
     unsigned char *utf8_value;
+    int     ch;
+    unsigned char *cp;
 
     if (name == 0 || (pos = X509_NAME_get_index_by_NID(name, nid, -1)) < 0) {
 	if (gripe != DONT_GRIPE) {
@@ -347,16 +351,6 @@ static char *tls_text_name(X509_NAME *name, int nid, const char *label,
 	return (__tls_text_name_temp); \
     } while (0)
 
-#if 0
-    for (cp = utf8_value; (ch = *cp) != 0; cp++) {
-	if (ISASCII(ch) && !ISPRINT(ch)) {
-	    msg_warn("%s: %s: non-printable content in peer %s",
-		     myname, TLScontext->namaddr, label);
-	    TLS_TEXT_NAME_RETURN(0);
-	}
-    }
-#endif
-
     /*
      * Remove trailing null characters. They would give false alarms with the
      * length check and with the embedded null check.
@@ -376,14 +370,29 @@ static char *tls_text_name(X509_NAME *name, int nid, const char *label,
     }
 
     /*
-     * Don't allow embedded nulls in ASCII or UTF-8 names. OpenSSL is
-     * responsible for producing properly-formatted UTF-8.
+     * Reject embedded nulls in ASCII or UTF-8 names. OpenSSL is responsible
+     * for producing properly-formatted UTF-8.
      */
     if (utf8_length != strlen((char *) utf8_value)) {
 	msg_warn("%s: %s: NULL character in peer %s",
 		 myname, TLScontext->namaddr, label);
 	TLS_TEXT_NAME_RETURN(0);
     }
+
+    /*
+     * Reject non-printable ASCII characters from UTF-8 content.
+     * 
+     * Note: the code below does not find control characters in illegal UTF-8
+     * sequences. It's OpenSSL's job to produce valid UTF-8, and reportedly,
+     * it does validation.
+     */
+    for (cp = utf8_value; (ch = *cp) != 0; cp++) {
+	if (ISASCII(ch) && !ISPRINT(ch)) {
+	    msg_warn("%s: %s: non-printable content in peer %s",
+		     myname, TLScontext->namaddr, label);
+	    TLS_TEXT_NAME_RETURN(0);
+	}
+    }
     TLS_TEXT_NAME_RETURN(mystrdup((char *) utf8_value));
 }