Fixed bug #37799 (ftp_ssl_connect() falls back to non-ssl connection)

4 files changed, 67 insertions, 42 deletions

diff --git a/NEWS b/NEWS
index d84df83fb1..aeb5c44645 100644
--- a/NEWS
+++ b/NEWS
@@ -16,6 +16,7 @@ PHP                                                                        NEWS
 - Fixed bug #40410 (ext/posix does not compile on MacOS 10.3.9). (Tony)
 - Fixed bug #40109 (iptcembed fails on non-jfif jpegs). (Tony)
 - Fixed bug #39836 (SplObjectStorage empty after unserialize). (Marcus)
+- Fixed bug #37799 (ftp_ssl_connect() falls back to non-ssl connection). (Nuno)
 
 08 Feb 2007, PHP 5.2.1
 - Added read-timeout context option "timeout" for HTTP streams. (Hannes, Ilia). 
diff --git a/ext/ftp/ftp.c b/ext/ftp/ftp.c
index 988380eef8..400e017a6f 100644
--- a/ext/ftp/ftp.c
+++ b/ext/ftp/ftp.c
@@ -266,60 +266,57 @@ ftp_login(ftpbuf_t *ftp, const char *user, const char *pass TSRMLS_DC)
 			}
 				
 			if (ftp->resp != 334) {
-				ftp->use_ssl = 0;
+				return 0;
 			} else {
 				ftp->old_ssl = 1;
 				ftp->use_ssl_for_data = 1;
 			}
 		}
 		
-		/* now enable ssl if we still need to */
-		if (ftp->use_ssl) {
-			ctx = SSL_CTX_new(SSLv23_client_method());
-			if (ctx == NULL) {
-				php_error_docref(NULL TSRMLS_CC, E_WARNING, "failed to create the SSL context");
+		ctx = SSL_CTX_new(SSLv23_client_method());
+		if (ctx == NULL) {
+			php_error_docref(NULL TSRMLS_CC, E_WARNING, "failed to create the SSL context");
+			return 0;
+		}
+
+		SSL_CTX_set_options(ctx, SSL_OP_ALL);
+
+		ftp->ssl_handle = SSL_new(ctx);
+		if (ftp->ssl_handle == NULL) {
+			php_error_docref(NULL TSRMLS_CC, E_WARNING, "failed to create the SSL handle");
+			SSL_CTX_free(ctx);
+			return 0;
+		}
+
+		SSL_set_fd(ftp->ssl_handle, ftp->fd);
+
+		if (SSL_connect(ftp->ssl_handle) <= 0) {
+			php_error_docref(NULL TSRMLS_CC, E_WARNING, "SSL/TLS handshake failed");
+			SSL_shutdown(ftp->ssl_handle);
+			return 0;
+		}
+
+		ftp->ssl_active = 1;
+
+		if (!ftp->old_ssl) {
+
+			/* set protection buffersize to zero */
+			if (!ftp_putcmd(ftp, "PBSZ", "0")) {
+				return 0;
+			}
+			if (!ftp_getresp(ftp)) {
 				return 0;
 			}
 
-			SSL_CTX_set_options(ctx, SSL_OP_ALL);
-
-			ftp->ssl_handle = SSL_new(ctx);
-			if (ftp->ssl_handle == NULL) {
-				php_error_docref(NULL TSRMLS_CC, E_WARNING, "failed to create the SSL handle");
-				SSL_CTX_free(ctx);
+			/* enable data conn encryption */
+			if (!ftp_putcmd(ftp, "PROT", "P")) {
 				return 0;
 			}
-			
-			SSL_set_fd(ftp->ssl_handle, ftp->fd);
-			
-			if (SSL_connect(ftp->ssl_handle) <= 0) {
-				php_error_docref(NULL TSRMLS_CC, E_WARNING, "SSL/TLS handshake failed");
-				SSL_shutdown(ftp->ssl_handle);
+			if (!ftp_getresp(ftp)) {
 				return 0;
 			}
 			
-			ftp->ssl_active = 1;
-			
-			if (!ftp->old_ssl) {
-				
-				/* set protection buffersize to zero */
-				if (!ftp_putcmd(ftp, "PBSZ", "0")) {
-					return 0;
-				}
-				if (!ftp_getresp(ftp)) {
-					return 0;
-				}
-					
-				/* enable data conn encryption */
-				if (!ftp_putcmd(ftp, "PROT", "P")) {
-					return 0;
-				}
-				if (!ftp_getresp(ftp)) {
-					return 0;
-				}
-				
-				ftp->use_ssl_for_data = (ftp->resp >= 200 && ftp->resp <=299);		
-			}
+			ftp->use_ssl_for_data = (ftp->resp >= 200 && ftp->resp <=299);		
 		}
 	}
 #endif
diff --git a/ext/ftp/tests/bug37799.phpt b/ext/ftp/tests/bug37799.phpt
new file mode 100644
index 0000000000..73f191a7a8
--- /dev/null
+++ b/ext/ftp/tests/bug37799.phpt
@@ -0,0 +1,21 @@
+--TEST--
+Bug #37799: ftp_ssl_connect() falls back to non-ssl connection
+--SKIPIF--
+<?php
+require 'skipif.inc';
+?>
+--FILE--
+<?php
+$bug37799=$ssl=1;
+require 'server.inc';
+
+$ftp = ftp_ssl_connect('127.0.0.1', $port);
+if (!$ftp) die("Couldn't connect to the server");
+
+var_dump(ftp_login($ftp, 'user', 'pass'));
+
+ftp_close($ftp);
+?>
+--EXPECTF--
+Warning: ftp_login(): bogus msg in %sbug37799.php on line 8
+bool(false)
diff --git a/ext/ftp/tests/server.inc b/ext/ftp/tests/server.inc
index e08eeb438a..c101c7c70f 100644
--- a/ext/ftp/tests/server.inc
+++ b/ext/ftp/tests/server.inc
@@ -59,7 +59,7 @@ $buf = fread($s, 2048);
 
 
 function user_auth($buf) {
-	global $user, $s, $ssl;
+	global $user, $s, $ssl, $bug37799;
 
 if (!empty($ssl)) {
 	if ($buf !== "AUTH TLS\r\n") {
@@ -67,7 +67,13 @@ if (!empty($ssl)) {
 		dump_and_exit($buf);
 	}
 
-	fputs($s, "234 auth type accepted\r\n");
+	if (empty($bug37799)) {
+		fputs($s, "234 auth type accepted\r\n");
+	} else {
+		fputs($s, "666 dummy\r\n");
+		fputs($s, "666 bogus msg\r\n");
+		exit;
+	}
 
 	if (!stream_socket_enable_crypto($s, true, STREAM_CRYPTO_METHOD_SSLv23_SERVER)) {
 		die("SSLv23 handshake failed.\n");