Re: [network] [PATCH] pppoe-server: Check for valid network

[Michael Tremer <michael.tremer@ipfire.org>]

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

Is there any update on this?

-Michael

On Thu, 2015-10-15 at 15:50 +0100, Michael Tremer wrote:
> Hi,
> 
> there are some issues with this patch.
> 
> On Thu, 2015-10-15 at 15:49 +0200, Stefan Schantl wrote:
> > The pppoe-server did not proper check if a valid
> > IPv4 or IPv6 network has been specified.
> > 
> > Signed-off-by: Stefan Schantl <stefan.schantl(a)ipfire.org>
> > ---
> >  src/hooks/configs/pppoe-server | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/src/hooks/configs/pppoe-server
> > b/src/hooks/configs/pppoe
> > -server
> > index 1ef3ba9..ac9b90a 100644
> > --- a/src/hooks/configs/pppoe-server
> > +++ b/src/hooks/configs/pppoe-server
> > @@ -42,6 +42,12 @@ hook_check_config_settings() {
> >  	assert isset SUBNET
> >  	assert isset MAX_SESSIONS
> >  
> > +	# Check input.
> > +	if ! ip_is_network "${SUBNET}"; then
> > +		log ERROR "Invalid subnet. Please provide a valid
> > IPv6 or IPv4 network."
> > +		return ${EXIT_ERROR}
> > +	fi
> > +
> 
> This will only validate the input when it is written to the
> configuration file or read from the configuration file. I think this
> should be as fast as possible and not clutter the log when something
> is
> going wrong.
> 
> The user should get an error when the input is received from the
> command line (that is a bit further down in the file).
> 
> The PPPoE server does NOT handle and IPv6 addresses here. It supports
> IPv6, but IP addresses are not handed out in the same style as on an
> IPv4 PPP link. You will have to check for IPv4 only. You should also
> check if the subnet is big enough.
> 
> Please consider sending more patches that validate the rest of the
> configuration.
> 
> >  	local server
> >  	for server in ${DNS_SERVERS}; do
> >  		assert ipv4_is_valid "${server}"
> 
> -Michael

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