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

Author: nunoplopes

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). 

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

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)

ext/ftp/tests/server.inc

@@ -59,15 +59,21 @@ $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") {
 		fputs($s, "500 Syntax error, command unrecognized.\r\n");
 		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");