From 8dc229500643b85310b275e697b69a7c100cc8a2 Mon Sep 17 00:00:00 2001 From: Maximilian Wilhelm Date: Fri, 25 Sep 2020 02:15:16 +0200 Subject: [PATCH 1/3] ifupdown: Don't configure errornous interfaces. Do not configure interfaces which have a configuration know to be broken. Currently this only happens when using "inherit" to an non-existing iface or to an iface and not a template, and having allow_any_iface_as_template set to 0. In those cases broken ifaces will not be configured unless ifup/ifdown is invoked with --force. Signed-off-by: Maximilian Wilhelm --- cmd/ifupdown.c | 7 +++++ libifupdown/interface-file.c | 61 ++++++++++++++++++++++++++++-------- libifupdown/interface.c | 13 ++------ libifupdown/interface.h | 5 ++- libifupdown/lifecycle.c | 14 +++++++++ 5 files changed, 76 insertions(+), 24 deletions(-) diff --git a/cmd/ifupdown.c b/cmd/ifupdown.c index 0a09e06..904eb5b 100644 --- a/cmd/ifupdown.c +++ b/cmd/ifupdown.c @@ -3,6 +3,7 @@ * Purpose: bring interfaces up or down * * Copyright (c) 2020 Ariadne Conill + * Copyright (c) 2020 Maximilian Wilhelm * * Permission to use, copy, modify, and/or distribute this software for any * purpose with or without fee is hereby granted, provided that the above @@ -105,6 +106,12 @@ skip_interface(struct lif_interface *iface, const char *ifname) if (exec_opts.force) return false; + if (iface->has_config_error) + { + fprintf(stderr, "%s: skipping interface %s due to config errors\n", argv0, ifname); + return true; + } + if (up && iface->refcount > 0) { if (exec_opts.verbose) diff --git a/libifupdown/interface-file.c b/libifupdown/interface-file.c index e6a4d09..dc2beb7 100644 --- a/libifupdown/interface-file.c +++ b/libifupdown/interface-file.c @@ -3,6 +3,7 @@ * Purpose: /etc/network/interfaces parser * * Copyright (c) 2020 Ariadne Conill + * Copyright (c) 2020 Maximilian Wilhelm * * Permission to use, copy, modify, and/or distribute this software for any * purpose with or without fee is hereby granted, provided that the above @@ -137,7 +138,8 @@ handle_address(struct lif_dict *collection, const char *filename, size_t lineno, if (cur_iface == NULL) { report_error(filename, lineno, "%s '%s' without interface", token, addr); - return false; + /* Ignore this address, but don't fail hard */ + return true; } lif_interface_address_add(cur_iface, addr); @@ -154,7 +156,10 @@ handle_auto(struct lif_dict *collection, const char *filename, size_t lineno, ch char *ifname = lif_next_token(&bufp); if (!*ifname && cur_iface == NULL) - return false; + { + report_error(filename, lineno, "auto without interface"); + return true; + } else { cur_iface = lif_interface_collection_find(collection, ifname); @@ -179,7 +184,8 @@ handle_gateway(struct lif_dict *collection, const char *filename, size_t lineno, if (cur_iface == NULL) { report_error(filename, lineno, "%s '%s' without interface", token, addr); - return false; + /* Ignore this gateway, but don't fail hard */ + return true; } lif_interface_use_executor(cur_iface, "static"); @@ -228,7 +234,8 @@ handle_iface(struct lif_dict *collection, const char *filename, size_t lineno, c if (!*ifname) { report_error(filename, lineno, "%s without any other tokens", token); - return false; + /* This is broken but not fatal */ + return true; } cur_iface = lif_interface_collection_find(collection, ifname); @@ -278,19 +285,44 @@ handle_inherit(struct lif_dict *collection, const char *filename, size_t lineno, if (cur_iface == NULL) { report_error(filename, lineno, "%s '%s' without interface", token, target); - return false; + /* This is broken but not fatal */ + return true; } if (!*target) { - report_error(filename, lineno, "%s: unspecified interface"); - return false; + report_error(filename, lineno, "iface %s: unspecified inherit target", cur_iface->ifname); + /* Mark this interface as errornous but carry on */ + cur_iface->has_config_error = true; + return true; } - if (!lif_interface_collection_inherit(cur_iface, collection, target)) + struct lif_interface *parent = lif_interface_collection_find(collection, target); + if (parent == NULL) { - report_error(filename, lineno, "could not inherit %s", target); - return false; + report_error(filename, lineno, "iface %s: could not inherit from %s: not found", + cur_iface->ifname, target); + /* Mark this interface as errornous but carry on */ + cur_iface->has_config_error = true; + return true; + + } + + if (!lif_config.allow_any_iface_as_template && !parent->is_template) + { + report_error(filename, lineno, "iface %s: could not inherit from %ss: inheritence from interface not allowed", + cur_iface->ifname, target); + /* Mark this interface as errornous but carry on */ + cur_iface->has_config_error = true; + return true; + } + + if (!lif_interface_collection_inherit(cur_iface, parent)) + { + report_error(filename, lineno, "iface %s: could not inherit from %s", cur_iface->ifname, target); + /* Mark this interface as errornous but carry on */ + cur_iface->has_config_error = true; + return true; } return true; @@ -305,14 +337,16 @@ handle_source(struct lif_dict *collection, const char *filename, size_t lineno, if (!*source_filename) { report_error(filename, lineno, "missing filename to source"); - return false; + /* Broken but not fatal */ + return true; } if (!strcmp(filename, source_filename)) { report_error(filename, lineno, "attempt to source %s would create infinite loop", source_filename); - return false; + /* Broken but not fatal */ + return true; } return lif_interface_file_parse(collection, source_filename); @@ -328,7 +362,8 @@ handle_use(struct lif_dict *collection, const char *filename, size_t lineno, cha if (cur_iface == NULL) { report_error(filename, lineno, "%s '%s' without interface", token, executor); - return false; + /* Broken but not fatal */ + return true; } lif_interface_use_executor(cur_iface, executor); diff --git a/libifupdown/interface.c b/libifupdown/interface.c index feb8b2f..175bae2 100644 --- a/libifupdown/interface.c +++ b/libifupdown/interface.c @@ -3,6 +3,7 @@ * Purpose: interface management * * Copyright (c) 2020 Ariadne Conill + * Copyright (c) 2020 Maximilian Wilhelm * * Permission to use, copy, modify, and/or distribute this software for any * purpose with or without fee is hereby granted, provided that the above @@ -233,21 +234,13 @@ lif_interface_collection_delete(struct lif_dict *collection, struct lif_interfac } bool -lif_interface_collection_inherit(struct lif_interface *interface, struct lif_dict *collection, const char *ifname) +lif_interface_collection_inherit(struct lif_interface *interface, struct lif_interface *parent) { - struct lif_interface *parent = lif_interface_collection_find(collection, ifname); - - if (parent == NULL) - return false; - - if (!lif_config.allow_any_iface_as_template && !parent->is_template) - return false; - /* maybe convert any interface we are inheriting from into a template */ if (lif_config.implicit_template_conversion) parent->is_template = true; - lif_dict_add(&interface->vars, "inherit", strdup(ifname)); + lif_dict_add(&interface->vars, "inherit", strdup(parent->ifname)); interface->is_bond = parent->is_bond; interface->is_bridge = parent->is_bridge; diff --git a/libifupdown/interface.h b/libifupdown/interface.h index c36690d..eeaa898 100644 --- a/libifupdown/interface.h +++ b/libifupdown/interface.h @@ -3,6 +3,7 @@ * Purpose: interface management * * Copyright (c) 2020 Ariadne Conill + * Copyright (c) 2020 Maximilian Wilhelm * * Permission to use, copy, modify, and/or distribute this software for any * purpose with or without fee is hereby granted, provided that the above @@ -52,6 +53,8 @@ struct lif_interface { bool is_bond; bool is_template; + bool has_config_error; /* error found in interface configuration */ + struct lif_dict vars; size_t refcount; /* > 0 if up, else 0 */ @@ -74,7 +77,7 @@ extern void lif_interface_collection_init(struct lif_dict *collection); extern void lif_interface_collection_fini(struct lif_dict *collection); extern struct lif_interface *lif_interface_collection_find(struct lif_dict *collection, const char *ifname); extern struct lif_interface *lif_interface_collection_upsert(struct lif_dict *collection, struct lif_interface *interface); -extern bool lif_interface_collection_inherit(struct lif_interface *interface, struct lif_dict *collection, const char *ifname); +extern bool lif_interface_collection_inherit(struct lif_interface *interface, struct lif_interface *parent); extern void lif_interface_collection_delete(struct lif_dict *collection, struct lif_interface *interface); #endif diff --git a/libifupdown/lifecycle.c b/libifupdown/lifecycle.c index bb59a02..2721642 100644 --- a/libifupdown/lifecycle.c +++ b/libifupdown/lifecycle.c @@ -3,6 +3,7 @@ * Purpose: management of interface lifecycle (bring up, takedown, reload) * * Copyright (c) 2020 Ariadne Conill + * Copyright (c) 2020 Maximilian Wilhelm * * Permission to use, copy, modify, and/or distribute this software for any * purpose with or without fee is hereby granted, provided that the above @@ -313,6 +314,19 @@ handle_dependents(const struct lif_execute_opts *opts, struct lif_interface *par { struct lif_interface *iface = lif_interface_collection_find(collection, tokenp); + if (iface->has_config_error) + { + if (opts->force) + fprintf (stderr, "ifupdown: (de)configuring dependent interface %s (of %s) despite config errors\n", + iface->ifname, parent->ifname); + else + { + fprintf (stderr, "ifupdown: skipping dependent interface %s (of %s) as it has config errors\n", + iface->ifname, parent->ifname); + continue; + } + } + /* if handle_refcounting returns true, it means we've already * configured the interface, or it is too soon to deconfigure * the interface. From a3d11ded4306808b7630dbee3d35197d575729f4 Mon Sep 17 00:00:00 2001 From: Maximilian Wilhelm Date: Fri, 25 Sep 2020 02:26:21 +0200 Subject: [PATCH 2/3] ifupdown: Be consistent in error messages. Signed-off-by: Maximilian Wilhelm --- cmd/ifupdown.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/cmd/ifupdown.c b/cmd/ifupdown.c index 904eb5b..8723310 100644 --- a/cmd/ifupdown.c +++ b/cmd/ifupdown.c @@ -103,15 +103,23 @@ skip_interface(struct lif_interface *iface, const char *ifname) return false; } - if (exec_opts.force) - return false; - if (iface->has_config_error) { - fprintf(stderr, "%s: skipping interface %s due to config errors\n", argv0, ifname); - return true; + if (exec_opts.force) + { + fprintf(stderr, "%s: (de)configuring interface %s despite config errors\n", argv0, ifname); + return false; + } + else + { + fprintf(stderr, "%s: skipping interface %s due to config errors\n", argv0, ifname); + return true; + } } + if (exec_opts.force) + return false; + if (up && iface->refcount > 0) { if (exec_opts.verbose) From 88f25e41c6b68583b3f571c6d440d0acd2a7c50a Mon Sep 17 00:00:00 2001 From: Maximilian Wilhelm Date: Fri, 25 Sep 2020 02:28:05 +0200 Subject: [PATCH 3/3] ifupdown: More specific error message. Signed-off-by: Maximilian Wilhelm --- libifupdown/interface-file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libifupdown/interface-file.c b/libifupdown/interface-file.c index dc2beb7..31971aa 100644 --- a/libifupdown/interface-file.c +++ b/libifupdown/interface-file.c @@ -310,7 +310,7 @@ handle_inherit(struct lif_dict *collection, const char *filename, size_t lineno, if (!lif_config.allow_any_iface_as_template && !parent->is_template) { - report_error(filename, lineno, "iface %s: could not inherit from %ss: inheritence from interface not allowed", + report_error(filename, lineno, "iface %s: could not inherit from %ss: inheritence from non-template interface not allowed", cur_iface->ifname, target); /* Mark this interface as errornous but carry on */ cur_iface->has_config_error = true;