sendmail: don't talk until 220 code is seen. Closes 3487

function                                             old     new   delta
sendmail_main                                        934     939      +5
smtp_checkp                                          167     165      -2
packed_usage                                       28634   28621     -13

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>

2 files changed, 58 insertions, 43 deletions

diff --git a/mailutils/mail.c b/mailutils/mail.c
index 66c7947..f5260d9 100644
--- a/mailutils/mail.c
+++ b/mailutils/mail.c
@@ -57,10 +57,13 @@ void FAST_FUNC launch_helper(const char **argv)
 	G.helper_pid = xvfork();
 
 	i = (!G.helper_pid) * 2; // for parent:0, for child:2
-	close(pipes[i + 1]); // 1 or 3 - closing one write end
-	close(pipes[2 - i]); // 2 or 0 - closing one read end
-	xmove_fd(pipes[i], STDIN_FILENO); // 0 or 2 - using other read end
-	xmove_fd(pipes[3 - i], STDOUT_FILENO); // 3 or 1 - other write end
+	close(pipes[i + 1]);     // 1 or 3 - closing one write end
+	close(pipes[2 - i]);     // 2 or 0 - closing one read end
+	xmove_fd(pipes[i], STDIN_FILENO);      // 0 or 2 - using other read end
+	xmove_fd(pipes[3 - i], STDOUT_FILENO); // 3 or 1 - using other write end
+	// End result:
+	// parent stdout [3] -> child stdin [2]
+	// child stdout [1] -> parent stdin [0]
 
 	if (!G.helper_pid) {
 		// child: try to execute connection helper
diff --git a/mailutils/sendmail.c b/mailutils/sendmail.c
index e0aff20..dbd4910 100644
--- a/mailutils/sendmail.c
+++ b/mailutils/sendmail.c
@@ -26,18 +26,18 @@
 //usage:     "\n			Examples:"
 //usage:     "\n			-H 'exec openssl s_client -quiet -tls1 -starttls smtp"
 //usage:     "\n				-connect smtp.gmail.com:25' <email.txt"
-//usage:     "\n				[4<username_and_passwd.txt | -au<username> -ap<password>]"
+//usage:     "\n				[4<username_and_passwd.txt | -auUSER -apPASS]"
 //usage:     "\n			-H 'exec openssl s_client -quiet -tls1"
 //usage:     "\n				-connect smtp.gmail.com:465' <email.txt"
-//usage:     "\n				[4<username_and_passwd.txt | -au<username> -ap<password>]"
+//usage:     "\n				[4<username_and_passwd.txt | -auUSER -apPASS]"
 //usage:     "\n	-S HOST[:PORT]	Server"
-//usage:     "\n	-au<username>	Username for AUTH LOGIN"
-//usage:     "\n	-ap<password>	Password for AUTH LOGIN"
-//usage:     "\n	-am<method>	Authentication method. Ignored. LOGIN is implied"
+//usage:     "\n	-auUSER		Username for AUTH LOGIN"
+//usage:     "\n	-apPASS 	Password for AUTH LOGIN"
+////usage:     "\n	-amMETHOD	Authentication method. Ignored. LOGIN is implied"
 //usage:     "\n"
 //usage:     "\nOther options are silently ignored; -oi -t is implied"
 //usage:	IF_MAKEMIME(
-//usage:     "\nUse makemime applet to create message with attachments"
+//usage:     "\nUse makemime to create emails with attachments"
 //usage:	)
 
 #include "libbb.h"
@@ -66,7 +66,7 @@ static int smtp_checkp(const char *fmt, const char *param, int code)
 	// if not equal -> die saying msg
 	while ((answer = xmalloc_fgetline(stdin)) != NULL) {
 		if (verbose)
-			bb_error_msg("recv:'%.*s' %d", (int)(strchrnul(answer, '\r') - answer), answer, verbose);
+			bb_error_msg("recv:'%.*s'", (int)(strchrnul(answer, '\r') - answer), answer);
 		if (strlen(answer) <= 3 || '-' != answer[3])
 			break;
 		free(answer);
@@ -75,10 +75,11 @@ static int smtp_checkp(const char *fmt, const char *param, int code)
 		int n = atoi(answer);
 		if (timeout)
 			alarm(0);
-		free(msg);
 		free(answer);
-		if (-1 == code || n == code)
+		if (-1 == code || n == code) {
+			free(msg);
 			return n;
+		}
 	}
 	bb_error_msg_and_die("%s failed", msg);
 }
@@ -176,11 +177,35 @@ int sendmail_main(int argc UNUSED_PARAM, char **argv)
 		const char *args[] = { "sh", "-c", opt_connect, NULL };
 		// plug it in
 		launch_helper(args);
-	// vanilla connection
+		// Now:
+		// our stdout will go to helper's stdin,
+		// helper's stdout will be available on our stdin.
+
+		// Wait for initial server message.
+		// If helper (such as openssl) invokes STARTTLS, the initial 220
+		// is swallowed by helper (and not repeated after TLS is initiated).
+		// We will send NOOP cmd to server and check the response.
+		// We should get 220+250 on plain connection, 250 on STARTTLSed session.
+		//
+		// The problem here is some servers delay initial 220 message,
+		// and consider client to be a spammer if it starts sending cmds
+		// before 220 reached it. The code below is unsafe in this regard:
+		// in non-STARTTLSed case, we potentially send NOOP before 220
+		// is sent by server.
+		// Ideas? (--delay SECS opt? --assume-starttls-helper opt?)
+		code = smtp_check("NOOP", -1);
+		if (code == 220)
+			// we got 220 - this is not STARTTLSed connection,
+			// eat 250 response to our NOOP
+			smtp_check(NULL, 250);
+		else
+		if (code != 250)
+			bb_error_msg_and_die("SMTP init failed");
 	} else {
+		// vanilla connection
 		int fd;
 		// host[:port] not explicitly specified? -> use $SMTPHOST
-		// no $SMTPHOST ? -> use localhost
+		// no $SMTPHOST? -> use localhost
 		if (!(opts & OPT_S)) {
 			opt_connect = getenv("SMTPHOST");
 			if (!opt_connect)
@@ -191,25 +216,14 @@ int sendmail_main(int argc UNUSED_PARAM, char **argv)
 		// and make ourselves a simple IO filter
 		xmove_fd(fd, STDIN_FILENO);
 		xdup2(STDIN_FILENO, STDOUT_FILENO);
-	}
-	// N.B. from now we know nothing about network :)
 
-	// wait for initial server OK
-	// N.B. if we used openssl the initial 220 answer is already swallowed during openssl TLS init procedure
-	// so we need to kick the server to see whether we are ok
-	code = smtp_check("NOOP", -1);
-	// 220 on plain connection, 250 on openssl-helped TLS session
-	if (220 == code)
-		smtp_check(NULL, 250); // reread the code to stay in sync
-	else if (250 != code)
-		bb_error_msg_and_die("INIT failed");
+		// Wait for initial server 220 message
+		smtp_check(NULL, 220);
+	}
 
 	// we should start with modern EHLO
-	if (250 != smtp_checkp("EHLO %s", domain, -1)) {
+	if (250 != smtp_checkp("EHLO %s", domain, -1))
 		smtp_checkp("HELO %s", domain, 250);
-	}
-	if (ENABLE_FEATURE_CLEAN_UP)
-		free(domain);
 
 	// perform authentication
 	if (opts & OPT_a) {
@@ -224,7 +238,7 @@ int sendmail_main(int argc UNUSED_PARAM, char **argv)
 	}
 
 	// set sender
-	// N.B. we have here a very loosely defined algotythm
+	// N.B. we have here a very loosely defined algorythm
 	// since sendmail historically offers no means to specify secrets on cmdline.
 	// 1) server can require no authentication ->
 	//	we must just provide a (possibly fake) reply address.
@@ -241,8 +255,6 @@ int sendmail_main(int argc UNUSED_PARAM, char **argv)
 	//	G.user = xuid2uname(getuid());
 	//	opt_from = xasprintf("%s@%s", G.user, domain);
 	//}
-	//if (ENABLE_FEATURE_CLEAN_UP)
-	//	free(domain);
 	smtp_checkp("MAIL FROM:<%s>", opt_from, 250);
 
 	// process message
@@ -272,26 +284,26 @@ int sendmail_main(int argc UNUSED_PARAM, char **argv)
 		if (0 == strncasecmp("To:", s, 3) || 0 == strncasecmp("Bcc:" + 1, s, 3)) {
 			rcptto(sane_address(s+3));
 			goto addheader;
+		}
 		// Bcc: header adds blind copy (hidden) recipient
-		} else if (0 == strncasecmp("Bcc:", s, 4)) {
+		if (0 == strncasecmp("Bcc:", s, 4)) {
 			rcptto(sane_address(s+4));
 			free(s);
 			// N.B. Bcc: vanishes from headers!
-
-		// other headers go verbatim
-
-		// N.B. RFC2822 2.2.3 "Long Header Fields" allows for headers to occupy several lines.
-		// Continuation is denoted by prefixing additional lines with whitespace(s).
-		// Thanks (stefan.seyfried at googlemail.com) for pointing this out.
-		} else if (strchr(s, ':') || (list && skip_whitespace(s) != s)) {
+		} else
+		if (strchr(s, ':') || (list && skip_whitespace(s) != s)) {
+			// other headers go verbatim
+			// N.B. RFC2822 2.2.3 "Long Header Fields" allows for headers to occupy several lines.
+			// Continuation is denoted by prefixing additional lines with whitespace(s).
+			// Thanks (stefan.seyfried at googlemail.com) for pointing this out.
  addheader:
 			// N.B. we allow MAX_HEADERS generic headers at most to prevent attacks
 			if (MAX_HEADERS && ++nheaders >= MAX_HEADERS)
 				goto bail;
 			llist_add_to_end(&list, s);
-		// a line without ":" (an empty line too, by definition) doesn't look like a valid header
-		// so stop "analyze headers" mode
 		} else {
+			// a line without ":" (an empty line too, by definition) doesn't look like a valid header
+			// so stop "analyze headers" mode
  reenter:
 			// put recipients specified on cmdline
 			while (*argv) {