pppoe-server: Check for valid network
Message ID | 1444916992-7458-1-git-send-email-stefan.schantl@ipfire.org |
---|---|
State | New |
Headers |
Return-Path: <network-bounces@lists.ipfire.org> Received: from mail01.ipfire.org (mail01.tremer.info [172.28.1.200]) by septima.ipfire.org (Postfix) with ESMTP id DF92262040 for <patchwork@ipfire.org>; Thu, 15 Oct 2015 15:50:11 +0200 (CEST) Received: from hedwig.ipfire.org (localhost [IPv6:::1]) by mail01.ipfire.org (Postfix) with ESMTP id E7BFC2DB; Thu, 15 Oct 2015 15:50:10 +0200 (CEST) Received: from tuxedo.stevee (host228-133-28-81.hiway.at [81.28.133.228]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail01.ipfire.org (Postfix) with ESMTPSA id 3FB391E5; Thu, 15 Oct 2015 15:50:08 +0200 (CEST) From: Stefan Schantl <stefan.schantl@ipfire.org> To: network@lists.ipfire.org Date: Thu, 15 Oct 2015 15:49:52 +0200 Message-Id: <1444916992-7458-1-git-send-email-stefan.schantl@ipfire.org> X-Mailer: git-send-email 2.4.3 Subject: [network] [PATCH] pppoe-server: Check for valid network X-BeenThere: network@lists.ipfire.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: <network.lists.ipfire.org> List-Unsubscribe: <http://lists.ipfire.org/mailman/options/network>, <mailto:network-request@lists.ipfire.org?subject=unsubscribe> List-Archive: <http://lists.ipfire.org/pipermail/network/> List-Post: <mailto:network@lists.ipfire.org> List-Help: <mailto:network-request@lists.ipfire.org?subject=help> List-Subscribe: <http://lists.ipfire.org/mailman/listinfo/network>, <mailto:network-request@lists.ipfire.org?subject=subscribe> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: network-bounces@lists.ipfire.org Sender: "network" <network-bounces@lists.ipfire.org> |
Message
Stefan Schantl
Oct. 16, 2015, 12:49 a.m. UTC
The pppoe-server did not proper check if a valid
IPv4 or IPv6 network has been specified.
Signed-off-by: Stefan Schantl <stefan.schantl@ipfire.org>
---
src/hooks/configs/pppoe-server | 6 ++++++
1 file changed, 6 insertions(+)
Comments
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@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
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@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