Question regarding upstream proxy validation in ids-functions.pl

Question regarding upstream proxy validation in ids-functions.pl

[Peter Müller]

[-- Attachment #1: Type: text/plain, Size: 2333 bytes --]

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
-- 
The road to Hades is easy to travel.
	-- Bion of Borysthenes

Re: Question regarding upstream proxy validation in ids-functions.pl

[Stefan Schantl]

[-- Attachment #1: Type: text/plain, Size: 2766 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]