modified proxy.cgi to make it possible that all subnets declared in "network access control" will be translated from cidr to subnet notation in proxy.pac

Message ID 20201016080852.1222419-1-steffen.klammer@staderschulen.de
State Accepted
Commit badd010f86a816fa0c5eb951225bb9796b463ef2
Headers
Series modified proxy.cgi to make it possible that all subnets declared in "network access control" will be translated from cidr to subnet notation in proxy.pac |

Commit Message

steffen.klammer@staderschulen.de Oct. 16, 2020, 8:08 a.m. UTC
  From: Steffen Klammer <steffen.klammer@staderschulen.de>

---
 html/cgi-bin/proxy.cgi | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
  

Comments

Michael Tremer Oct. 17, 2020, 12:23 p.m. UTC | #1
Hello,

Thank you very much for submitting your patch.

This looks good to me. Has anyone tried this and can confirm that it works as intended?

What would be added if the net mask was invalid? Let’s say if someone writes 192.168.0.0/38?

Reviewed-by: Michael Tremer <michael.tremer@ipfire.org>

Best,
-Michael

> On 16 Oct 2020, at 09:08, steffen.klammer@staderschulen.de wrote:
> 
> From: Steffen Klammer <steffen.klammer@staderschulen.de>
> 
> ---
> html/cgi-bin/proxy.cgi | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/html/cgi-bin/proxy.cgi b/html/cgi-bin/proxy.cgi
> index fdf9bddaf..a63e1c2df 100644
> --- a/html/cgi-bin/proxy.cgi
> +++ b/html/cgi-bin/proxy.cgi
> @@ -3004,7 +3004,8 @@ END
> 					)
> 				{
> 					chomp $temp[1];
> -					print FILE " ||\n     (isInNet(myIpAddress(), \"$temp[0]\", \"$temp[1]\"))";
> +					my $tempmask = &Network::convert_prefix2netmask($temp[1]);
> +					print FILE " ||\n     (isInNet(myIpAddress(), \"$temp[0]\", \"$tempmask\"))";
> 				}
> 			}
> 
> -- 
> 2.25.1
>
  
Michael Tremer Oct. 19, 2020, 9:20 a.m. UTC | #2
Good Morning Steffen,

Thank you for checking this for me.

I will merge your patch into next shortly.

Best,
-Michael

P.S. Please make sure that you always keep the list copied in your replies :)

> On 19 Oct 2020, at 08:11, Steffen Klammer <steffen.klammer@staderschulen.de> wrote:
> 
> Hello Michael,
> 
> I just tested it with an invalid subnetmask and you get an error message within the WUI. So this exception seems to be caught in the script.
> 
> Greetings
> 
> Steffen
>
  

Patch

diff --git a/html/cgi-bin/proxy.cgi b/html/cgi-bin/proxy.cgi
index fdf9bddaf..a63e1c2df 100644
--- a/html/cgi-bin/proxy.cgi
+++ b/html/cgi-bin/proxy.cgi
@@ -3004,7 +3004,8 @@  END
 					)
 				{
 					chomp $temp[1];
-					print FILE " ||\n     (isInNet(myIpAddress(), \"$temp[0]\", \"$temp[1]\"))";
+					my $tempmask = &Network::convert_prefix2netmask($temp[1]);
+					print FILE " ||\n     (isInNet(myIpAddress(), \"$temp[0]\", \"$tempmask\"))";
 				}
 			}