From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stefan Schantl <stefan.schantl@ipfire.org>
To: development@lists.ipfire.org
Subject: Re: Question regarding upstream proxy validation in ids-functions.pl
Date: Mon, 20 May 2019 19:27:45 +0200
Message-ID: <a75270ad5ebfcc62387e2e48daf1ca9ecf89a2ab.camel@ipfire.org>
In-Reply-To: <7433eaad-7c6d-dfd2-84c8-2409d71b4b3b@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============2182598538270673889=="
List-Id: <development.lists.ipfire.org>

--===============2182598538270673889==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit

Hello Peter,

thanks for testing and your provided patch.

The messy code for proxy configuration was part of the former ids.cgi
which simply has been copied to the download function.

The new code is much cleaner and better to understand and maintain -
thanks for that.

@Michael or Arne please merge this patch so it can be shipped by the
next core update.

Acked-by: Stefan Schantl <stefan.schantl(a)ipfire.org>
> Hello Stefan,
> 
> while upgrading to Core Update 131, I stumbled across the
> "downloadruleset()" function in ids-functions.pl . It seems
> to contain a validation for read proxy information which is
> faulty and will not return any information.
> 
> By removing the validation as shown in the diff below, the
> CGI works correct behind an upstream proxy:
> 
> diff --git a/config/cfgroot/ids-functions.pl b/config/cfgroot/ids-
> functions.pl
> index deb287bb7..5530da11e 100644
> --- a/config/cfgroot/ids-functions.pl
> +++ b/config/cfgroot/ids-functions.pl
> @@ -174,28 +174,18 @@ sub downloadruleset {
>  
>  	# Check if an upstream proxy is configured.
>  	if ($proxysettings{'UPSTREAM_PROXY'}) {
> -		my ($peer, $peerport) = (/^(?:[a-zA-Z ]+\:\/\/)?(?:[A-
> Za-z0-9\_\.\-]*?(?:\:[A-Za-z0-9\_\.\-]*?)?\@)?([a-zA-Z0-9\.\_\-
> ]*?)(?:\:([0-9]{1,5}))?(?:\/.*?)?$/);
>  		my $proxy_url;
>  
> -		# Check if we got a peer.
> -		if ($peer) {
> -			$proxy_url = "http://";
> +		$proxy_url = "http://";
>  
> -			# Check if the proxy requires authentication.
> -			if (($proxysettings{'UPSTREAM_USER'}) &&
> ($proxysettings{'UPSTREAM_PASSWORD'})) {
> -				$proxy_url .=
> "$proxysettings{'UPSTREAM_USER'}\:$proxysettings{'UPSTREAM_PASSWORD'}
> \@";
> -			}
> -
> -			# Add proxy server address and port.
> -			$proxy_url .= "$peer\:$peerport";
> -		} else {
> -			# Log error message and break.
> -			&_log_to_syslog("Could not proper configure the
> proxy server access.");
> -
> -			# Return "1" - false.
> -			return 1;
> +		# Check if the proxy requires authentication.
> +		if (($proxysettings{'UPSTREAM_USER'}) &&
> ($proxysettings{'UPSTREAM_PASSWORD'})) {
> +			$proxy_url .=
> "$proxysettings{'UPSTREAM_USER'}\:$proxysettings{'UPSTREAM_PASSWORD'}
> \@";
>  		}
>  
> +		# Add proxy server address and port.
> +		$proxy_url .= $proxysettings{'UPSTREAM_PROXY'};
> +
>  		# Setup proxy settings.
>  		$downloader->proxy(['http', 'https'], $proxy_url);
>  	}
> 
> Since I guess the validation was intentional, could you please
> explain to me what it was supposed to do? I am not sure if I got
> the regex right...
> 
> Either was, the CGI is currently not working behind an upstream
> proxy. To be honest, I accidentally have not tested this (firewall
> talked directly to the internet :-/ ), sorry.
> 
> Thanks, and best regards,
> Peter Müller

--===============2182598538270673889==
Content-Type: application/pgp-signature
Content-Transfer-Encoding: base64
Content-Disposition: attachment; filename="signature.asc"
MIME-Version: 1.0

LS0tLS1CRUdJTiBQR1AgU0lHTkFUVVJFLS0tLS0KCmlRSXpCQUFCQ2dBZEZpRUVXTzBOWHRTcnZo
YXN5dERuVHRkT0ZZK1RzdDRGQWx6aTQ1RUFDZ2tRVHRkT0ZZK1QKc3Q3TlpBLzlHU0hBejJHMkw4
OXd4R0pPNDhQQVIrUTNmeDJaZDBMUkNiTmgvek9BY3BWR0VMejZjdG9tbTZnNgo5ZEoxYlBtemdY
b0VPa2NZcnB3NklkVWJNRnMxOURyQ1pTRGRIa1JrSTg4YWwxaHg2aGUzMC9QbWRCT2FrOTFNCjdi
ZDhVVVRPdStzQVM2Q1FKaHhlOVZBdzFVdzJMUmlqeXhCYWlLTTk0dWpoaFZudmFndm9aUzZVRTVO
dUVSUjYKZ05LcW02TlZxK2lOV09FaldKMXVVRTNaMlkyWGZMSHQyM01ndXU3eFlmd1RkaFovVXNM
enhKMk9EWk1EZnErTAoxTFdXUVRuQytjMldQYkNMSmlOSXIyWVFvRm5rZ0lGM0x1QWdDQTVvTGp5
L3M4ckhGTElVZmN0ZFk5VjdoRWVYCkhnMSsvWmp3L2V3UFRCeGpMNG4vTVZrRnpKZ0MvZ0hkT2o4
Nk5HVDJlV0pZYTVweU9HTyszb1dZbW5TODdsZCsKMDVxYm9uNXNxV0VKa1FYbWY2RmxhOFV3SVhD
NjM2Sk5jd0FuSys1V1JJcFFFOUlrSmlBSGZQSCtDK2txV2M1Qwp0OXBBS3ZTSmVteCtrVk5uZVdI
MjU1cGJxY1ZkbjdmVWpTY21mNmRwTEhRV010OEhvYTl6N09HR0lTclVqa2RXCk0xRlFmVGdETmN1
N0JxQVVzRFNBSy9XTVloandHR3JqMkNoWlNvN0d5czRWQk5pU1diYXQ1MlZDTGZ2c3RlYjYKWFF6
YVc1MlhhYkE1WjArMVhxd0tPTlhHMVhtL1JIVnlRVTNmckxJa0NvQnBoTjZPKzJiR3J6RGY3Z3E1
elNxagpOeTZLTEFVaXJJYUJLRUtVQjJFV2dhSk5sMTZCNXBVb0tBNXc1WWF4NWNEWVExajNxVTA9
Cj12N2tzCi0tLS0tRU5EIFBHUCBTSUdOQVRVUkUtLS0tLQo=

--===============2182598538270673889==--