[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#818611: netcfg: Misleading error message when parsing line with trailing blank



Package: netcfg
Version: 1.137
Severity: normal
Tags: d-i

Dear maintainer(s),

when specifying IP addresses with leading or trailing blanks, netcfg does not
correctly identify the IP address and fails.  For example,

	Configure a network using static addressing  
	-------------------------------------------  
	  
	The IP address is unique to your computer and may be:  
	  
	 * four numbers separated by periods (IPv4);  
	 * blocks of hexadecimal characters separated by colons (IPv6).  
	  
	You can also optionally append a CIDR netmask (such as "/24").  
	  
	If you don't know what to use here, consult your network administrator.  
	IP address:  
	Prompt: '?' for help> 9.152.162.103
	9.152.162.103     
	  
	!! ERROR: Malformed IP address  
	  
	The IP address you provided is malformed. It should be in the form x.x.x.x   
	where each 'x' is no larger than 255 (an IPv4 address), or a sequence of blocks 
	 
	of hexadecimal digits separated by colons (an IPv6 address). Please try again.  
	Press enter to continue

This can easily happen, especially, when entering IP addresses within
command line console, such as the z/VM console on z Systems (s390x).

The expected behavior is that leading and trailing blanks should be ignored.
Note that the root cause of the error condition above is triggered by a
failing inet_pton() call which does not expect blanks for an IP address
string.

To correct this behavior, I have attached two patches for discussion:

The first patch removes trailing blanks for the case above. Note that there
is already an rtrim() function that has been reused.  Also an additional
function, strtrim(), is introduced to remove any leading blanks.
The "make test" has been enhanced to verify this behavior.

The second patch removes leading and trailing blanks on IP addresses entered
for other network configurations, for example, point-to-point.

Feedback is welcome.

Thanks and kind regards,
  Hendrik

-- 
Hendrik Brueckner
brueckner@linux.vnet.ibm.com      | IBM Deutschland Research & Development GmbH
Linux on z Systems Development    | Schoenaicher Str. 220, 71032 Boeblingen
>From 944acf2a089e5eb60a76c58d896f8c3713aad0a4 Mon Sep 17 00:00:00 2001
From: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>
Date: Fri, 5 Feb 2016 16:32:42 +0100
Subject: [PATCH 1/2] common/ipaddr: remove leading and trailing whitespaces

Leading or trailing whitespaces are not removed from the IP address,
for example, when specified by the user.  If the IP address contains
whitespaces, parsing the address fails and a malformed address is
reported even if the IP address would be valid.

To remove trailing whitespaces, extend the rtrim() function to remove
multiple spaces.  Introduce a new function, strtrim() to remove any
leading spaces.

Also add a test case to verify the changed behavior.

Signed-off-by: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>
---
 netcfg-common.c                       | 24 +++++++++++++++++++++---
 netcfg.h                              |  1 +
 test/test_netcfg_parse_cidr_address.c | 26 ++++++++++++++++++++++++++
 3 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/netcfg-common.c b/netcfg-common.c
index 376e6ca..c6d1d8d 100644
--- a/netcfg-common.c
+++ b/netcfg-common.c
@@ -1577,10 +1577,11 @@ int netcfg_parse_cidr_address(const char *address, struct netcfg_interface *inte
     struct in_addr addr;
     struct in6_addr addr6;
     int ok;
-    char *maskptr, addrstr[NETCFG_ADDRSTRLEN];
+    char *maskptr, *addrstr, addrbuf[NETCFG_ADDRSTRLEN];
     int i;
     
-    strncpy(addrstr, address, NETCFG_ADDRSTRLEN);
+    strncpy(addrbuf, address, NETCFG_ADDRSTRLEN);
+    addrstr = strtrim(addrbuf);
     
     if ((maskptr = strchr(addrstr, '/'))) {
         /* Houston, we have a netmask; split it into bits */
@@ -1730,7 +1731,24 @@ void rtrim(char *s)
 	
 	n = strlen(s) - 1;
 	
-	while (isspace(s[n])) {
+	while (n >= 0 && isspace(s[n])) {
 		s[n] = '\0';
+		n--;
 	}
 }
+
+char *strtrim(char *s)
+{
+	size_t len;
+
+	len = strlen(s);
+	if (!len)
+		return s;
+
+	rtrim(s);
+
+	while (*s && isspace(*s))
+		s++;
+
+	return s;
+}
diff --git a/netcfg.h b/netcfg.h
index 771fed3..00a2cea 100644
--- a/netcfg.h
+++ b/netcfg.h
@@ -249,6 +249,7 @@ extern void preseed_hostname_from_fqdn(struct debconfclient *client, char *fqdn)
 extern int netcfg_dhcp(struct debconfclient *client, struct netcfg_interface *interface);
 
 extern void rtrim(char *);
+extern char *strtrim(char *s);
 
 /* ipv6.c */
 extern void nc_v6_wait_for_complete_configuration(const struct netcfg_interface *interface);
diff --git a/test/test_netcfg_parse_cidr_address.c b/test/test_netcfg_parse_cidr_address.c
index 8f0f919..2b1cdca 100644
--- a/test/test_netcfg_parse_cidr_address.c
+++ b/test/test_netcfg_parse_cidr_address.c
@@ -101,6 +101,31 @@ START_TEST(test_parse_cidr_v6_address)
 }
 END_TEST
 
+START_TEST(test_parse_cidr_ignore_leading_trailing_spaces)
+{
+	struct netcfg_interface interface;
+	netcfg_interface_init(&interface);
+	int rv;
+
+	interface.masklen = 7;
+	rv = netcfg_parse_cidr_address("   192.0.2.12   ", &interface);
+
+	fail_unless (rv,
+	             "parsing failed, rv = %i", rv);
+
+	fail_unless (interface.masklen == 0,
+	             "masklen was %i, should have been 0",
+	             interface.masklen);
+
+	fail_unless (strcmp("192.0.2.12", interface.ipaddress) == 0,
+	             "IP address was %s, should have been 192.10.2.12",
+	             interface.ipaddress);
+
+	fail_unless (interface.address_family == AF_INET,
+	             "Address family should have been AF_INET");
+}
+END_TEST
+
 Suite *test_netcfg_parse_cidr_address_suite (void)
 {
 	Suite *s = suite_create ("netcfg_parse_cidr_address");
@@ -110,6 +135,7 @@ Suite *test_netcfg_parse_cidr_address_suite (void)
 	tcase_add_test (tc, test_parse_cidr_v4_address);
 	tcase_add_test (tc, test_parse_standalone_v6_address);
 	tcase_add_test (tc, test_parse_cidr_v6_address);
+	tcase_add_test (tc, test_parse_cidr_ignore_leading_trailing_spaces);
 	
 	suite_add_tcase (s, tc);
 	
-- 
2.7.0

>From f14ecde4b8e70cddc1b55c0f2b942f0ce3c28e3e Mon Sep 17 00:00:00 2001
From: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>
Date: Fri, 5 Feb 2016 17:04:39 +0100
Subject: [PATCH 2/2] static: trim user-specified values for IP and other
 addresses

Remove leading and trailing spaces for IP addresses specified by
the user.  The inet_pton() function fails to parse addresses if
spaces are present.

Signed-off-by: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>
---
 static.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/static.c b/static.c
index 608c11d..ea12fba 100644
--- a/static.c
+++ b/static.c
@@ -39,6 +39,7 @@ static int netcfg_get_pointopoint(struct debconfclient *client, struct netcfg_in
 {
     int ret, ok = 0;
     union inX_addr addr;
+    char *ptr;
     
     while (!ok) {
         debconf_input(client, "critical", "netcfg/get_pointopoint");
@@ -48,13 +49,14 @@ static int netcfg_get_pointopoint(struct debconfclient *client, struct netcfg_in
             return ret;
 
         debconf_get(client, "netcfg/get_pointopoint");
+        ptr = strtrim(client->value);
 
-        if (empty_str(client->value)) {           /* No P-P is ok */
+        if (empty_str(ptr)) {           /* No P-P is ok */
             interface->pointopoint[0] = '\0';
             return 0;
         }
 
-        ok = inet_pton (interface->address_family, client->value, &addr);
+        ok = inet_pton (interface->address_family, ptr, &addr);
 
         if (!ok) {
             debconf_capb(client);
@@ -72,12 +74,14 @@ static int netcfg_get_netmask(struct debconfclient *client, struct netcfg_interf
 {
     int ret, ok = 0;
     union inX_addr addr;
+    char *ptr;
 
     /* Preseed a vaguely sensible looking default netmask if one wasn't
      * provided.
      */
     debconf_get (client, "netcfg/get_netmask");
-    if (empty_str(client->value)) {
+    ptr = strtrim(client->value);
+    if (empty_str(ptr)) {
         if (interface->address_family == AF_INET) {
             debconf_set(client, "netcfg/get_netmask", "255.255.255.0");
         } else if (interface->address_family == AF_INET6) {
@@ -94,7 +98,8 @@ static int netcfg_get_netmask(struct debconfclient *client, struct netcfg_interf
 
         debconf_get (client, "netcfg/get_netmask");
 
-        ok = inet_pton (interface->address_family, client->value, &addr);
+        ptr = strtrim(client->value);
+        ok = inet_pton (interface->address_family, ptr, &addr);
 
         if (!ok) {
             debconf_capb(client);
@@ -104,7 +109,7 @@ static int netcfg_get_netmask(struct debconfclient *client, struct netcfg_interf
         }
     }
 
-    inet_ptom(interface->address_family, client->value, &(interface->masklen));
+    inet_ptom(interface->address_family, ptr, &(interface->masklen));
     return 0;
 }
 
@@ -163,7 +168,7 @@ static int netcfg_get_gateway(struct debconfclient *client, struct netcfg_interf
             return ret;
 
         debconf_get(client, "netcfg/get_gateway");
-        ptr = client->value;
+        ptr = strtrim(client->value);
 
         if (empty_str(ptr) || /* No gateway, that's fine */
             (strcmp(ptr, "none") == 0)) /* special case for preseeding */ {
-- 
2.7.0


Reply to: