[ticket/14448] Let user decide if remote upload certs should be checked

Also fixed some minor issues like coding style.

PHPBB3-14448

phpBB/config/default/container/services_files.yml

@@ -49,6 +49,7 @@ services:
         class: phpbb\files\types\remote
         scope: prototype
         arguments:
+            - '@config'
             - '@files.factory'
             - '@language'
             - '@php_ini'

phpBB/includes/acp/acp_board.php

@@ -415,6 +415,7 @@ class acp_board
 						'browser_check'			=> array('lang' => 'BROWSER_VALID',			'validate' => 'bool',	'type' => 'radio:yes_no', 'explain' => true),
 						'forwarded_for_check'	=> array('lang' => 'FORWARDED_FOR_VALID',	'validate' => 'bool',	'type' => 'radio:yes_no', 'explain' => true),
 						'referer_validation'	=> array('lang' => 'REFERRER_VALID',		'validate' => 'int:0:3','type' => 'custom', 'method' => 'select_ref_check', 'explain' => true),
+						'remote_upload_verify'	=> array('lang' => 'UPLOAD_CERT_VALID',	'validate' => 'bool',	'type' => 'radio:yes_no', 'explain' => true),
 						'check_dnsbl'			=> array('lang' => 'CHECK_DNSBL',			'validate' => 'bool',	'type' => 'radio:yes_no', 'explain' => true),
 						'email_check_mx'		=> array('lang' => 'EMAIL_CHECK_MX',		'validate' => 'bool',	'type' => 'radio:yes_no', 'explain' => true),
 						'max_pass_chars'		=> array('lang' => 'PASSWORD_LENGTH', 'validate' => 'int:8:255', 'type' => false, 'method' => false, 'explain' => false,),

phpBB/install/schemas/schema_data.sql

@@ -239,6 +239,7 @@ INSERT INTO phpbb_config (config_name, config_value) VALUES ('queue_interval', '
 INSERT INTO phpbb_config (config_name, config_value) VALUES ('ranks_path', 'images/ranks');
 INSERT INTO phpbb_config (config_name, config_value) VALUES ('read_notification_expire_days', '30');
 INSERT INTO phpbb_config (config_name, config_value) VALUES ('read_notification_gc', '86400');
+INSERT INTO phpbb_config (config_name, config_value) VALUES ('remote_upload_verify', '0');
 INSERT INTO phpbb_config (config_name, config_value) VALUES ('require_activation', '0');
 INSERT INTO phpbb_config (config_name, config_value) VALUES ('referer_validation', '1');
 INSERT INTO phpbb_config (config_name, config_value) VALUES ('script_path', '');

phpBB/language/en/acp/board.php

@@ -535,6 +535,8 @@ $lang = array_merge($lang, array(
 	'REFERRER_VALID_EXPLAIN'		=> 'If enabled, the referrer of POST requests will be checked against the host/script path settings. This may cause issues with boards using several domains and or external logins.',
 	'TPL_ALLOW_PHP'					=> 'Allow php in templates',
 	'TPL_ALLOW_PHP_EXPLAIN'			=> 'If this option is enabled, <code>PHP</code> and <code>INCLUDEPHP</code> statements will be recognised and parsed in templates.',
+	'UPLOAD_CERT_VALID'				=> 'Validate upload certificate',
+	'UPLOAD_CERT_VALID_EXPLAIN'		=> 'If enabled, certificates of remote uploads will be validated. This requires the CA bundle to be defined by the <samp>openssl.cafile</samp> or <samp>curl.cainfo</samp> setting in your php.ini.',
 ));
 
 // Email Settings

phpBB/phpbb/db/migration/data/v320/remote_upload_validation.php

@@ -0,0 +1,31 @@
+<?php
+/**
+*
+* This file is part of the phpBB Forum Software package.
+*
+* @copyright (c) phpBB Limited <https://www.phpbb.com>
+* @license GNU General Public License, version 2 (GPL-2.0)
+*
+* For full copyright and license information, please see
+* the docs/CREDITS.txt file.
+*
+*/
+
+namespace phpbb\db\migration\data\v320;
+
+class remote_upload_validation extends \phpbb\db\migration\migration
+{
+	static public function depends_on()
+	{
+		return array(
+			'\phpbb\db\migration\data\v320\v320a2',
+		);
+	}
+
+	public function update_data()
+	{
+		return array(
+			array('config.add', array('remote_upload_verify', '0')),
+		);
+	}
+}

phpBB/phpbb/files/types/remote.php

@@ -14,6 +14,7 @@
 namespace phpbb\files\types;
 
 use bantu\IniGetWrapper\IniGetWrapper;
+use phpbb\config\config;
 use phpbb\files\factory;
 use phpbb\files\filespec;
 use phpbb\language\language;
@@ -21,6 +22,9 @@ use phpbb\request\request_interface;
 
 class remote extends base
 {
+	/** @var config phpBB config */
+	protected $config;
+
 	/** @var factory Files factory */
 	protected $factory;
 
@@ -42,14 +46,16 @@ class remote extends base
 	/**
 	 * Construct a form upload type
 	 *
+	 * @param config $config phpBB config
 	 * @param factory $factory Files factory
 	 * @param language $language Language class
 	 * @param IniGetWrapper $php_ini ini_get() wrapper
 	 * @param request_interface $request Request object
 	 * @param string $phpbb_root_path phpBB root path
 	 */
-	public function __construct(factory $factory, language $language, IniGetWrapper $php_ini, request_interface $request, $phpbb_root_path)
+	public function __construct(config $config, factory $factory, language $language, IniGetWrapper $php_ini, request_interface $request, $phpbb_root_path)
 	{
+		$this->config = $config;
 		$this->factory = $factory;
 		$this->language = $language;
 		$this->php_ini = $php_ini;
@@ -97,8 +103,9 @@ class remote extends base
 		$remote_max_filesize = $this->get_max_file_size();
 
 		$guzzle_options = [
-			'timeout' => $this->upload->upload_timeout,
+			'timeout'			=> $this->upload->upload_timeout,
-			'connect_timeout' => $this->upload->upload_timeout,
+			'connect_timeout'	=> $this->upload->upload_timeout,
+			'verify'			=> !empty($this->config['remote_upload_verify']),
 		];
 		$client = new \GuzzleHttp\Client($guzzle_options);
 
@@ -118,24 +125,11 @@ class remote extends base
 			}
 			else
 			{
-				if (strpos($requestException->getMessage(), 'cURL error 60') !== false)
+				return $this->factory->get('filespec')->set_error($this->language->lang($this->upload->error_prefix . 'NOT_UPLOADED'));
-				{
-					// Work around non existent CA file
-					try
-					{
-						$response = $client->get($upload_url, array_merge($guzzle_options, ['verify' => false]));
-					}
-					catch (\GuzzleHttp\Exception\RequestException $requestException)
-					{
-						return $this->factory->get('filespec')->set_error($this->language->lang($this->upload->error_prefix . 'NOT_UPLOADED'));
-					}
-				}
-				else
-				{
-					return $this->factory->get('filespec')->set_error($this->language->lang($this->upload->error_prefix . 'NOT_UPLOADED'));
-				}
 			}
-		} catch (\Exception $e) {
+		}
+		catch (\Exception $e)
+		{
 			return $this->factory->get('filespec')->set_error($this->language->lang($this->upload->error_prefix . 'NOT_UPLOADED'));
 		}
 

tests/files/types_remote_test.php

@@ -20,6 +20,9 @@ class phpbb_files_types_remote_test extends phpbb_test_case
 
 	private $filesystem;
 
+	/** @var \phpbb\config\config */
+	protected $config;
+
 	/** @var \Symfony\Component\DependencyInjection\ContainerInterface */
 	protected $container;
 
@@ -43,6 +46,8 @@ class phpbb_files_types_remote_test extends phpbb_test_case
 		global $config, $phpbb_root_path, $phpEx;
 
 		$config = new \phpbb\config\config(array());
+		$this->config = $config;
+		$this->config->set('remote_upload_verify', 0);
 		$this->request = $this->getMock('\phpbb\request\request');
 
 		$this->filesystem = new \phpbb\filesystem\filesystem();
@@ -67,7 +72,7 @@ class phpbb_files_types_remote_test extends phpbb_test_case
 
 	public function test_upload_fsock_fail()
 	{
-		$type_remote = new \phpbb\files\types\remote($this->factory, $this->language, $this->php_ini, $this->request, $this->phpbb_root_path);
+		$type_remote = new \phpbb\files\types\remote($this->config, $this->factory, $this->language, $this->php_ini, $this->request, $this->phpbb_root_path);
 		$upload = new \phpbb\files\upload($this->filesystem, $this->factory, $this->language, $this->php_ini, $this->request, $this->phpbb_root_path);
 		$upload->set_allowed_extensions(array('png'));
 		$type_remote->set_upload($upload);
@@ -102,7 +107,7 @@ class phpbb_files_types_remote_test extends phpbb_test_case
 		$php_ini->expects($this->any())
 			->method('getString')
 			->willReturn($max_file_size);
-		$type_remote = new \phpbb\files\types\remote($this->factory, $this->language, $php_ini, $this->request, $this->phpbb_root_path);
+		$type_remote = new \phpbb\files\types\remote($this->config, $this->factory, $this->language, $php_ini, $this->request, $this->phpbb_root_path);
 		$upload = new \phpbb\files\upload($this->filesystem, $this->factory, $this->language, $this->php_ini, $this->request, $this->phpbb_root_path);
 		$upload->set_allowed_extensions(array('png'));
 		$type_remote->set_upload($upload);
@@ -114,7 +119,7 @@ class phpbb_files_types_remote_test extends phpbb_test_case
 
 	public function test_upload_wrong_path()
 	{
-		$type_remote = new \phpbb\files\types\foo($this->factory, $this->language, $this->php_ini, $this->request, $this->phpbb_root_path);
+		$type_remote = new \phpbb\files\types\foo($this->config, $this->factory, $this->language, $this->php_ini, $this->request, $this->phpbb_root_path);
 		$upload = new \phpbb\files\upload($this->filesystem, $this->factory, $this->language, $this->php_ini, $this->request, $this->phpbb_root_path);
 		$upload->set_allowed_extensions(array('png'));
 		$type_remote->set_upload($upload);

tests/functional/fileupload_remote_test.php

@@ -45,11 +45,12 @@ class phpbb_functional_fileupload_remote_test extends phpbb_functional_test_case
 
 		if (!is_array($config))
 		{
-			$config = array();
+			$config = new \phpbb\config\config(array());
 		}
 
 		$config['rand_seed'] = '';
 		$config['rand_seed_last_update'] = time() + 600;
+		$config['remote_upload_verify'] = 0;
 
 		$this->filesystem = new \phpbb\filesystem\filesystem();
 		$this->language = new \phpbb\language\language(new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx));
@@ -60,7 +61,7 @@ class phpbb_functional_fileupload_remote_test extends phpbb_functional_test_case
 		$container->set('files.filespec', new \phpbb\files\filespec($this->filesystem, $this->language, $this->php_ini, new \FastImageSize\FastImageSize(), $this->phpbb_root_path));
 		$this->factory = new \phpbb\files\factory($container);
 		$container->set('files.factory', $this->factory);
-		$container->set('files.types.remote', new \phpbb\files\types\remote($this->factory, $this->language, $this->php_ini, $this->request, $phpbb_root_path));
+		$container->set('files.types.remote', new \phpbb\files\types\remote($config, $this->factory, $this->language, $this->php_ini, $this->request, $phpbb_root_path));
 		$this->phpbb_root_path = $phpbb_root_path;
 	}