Cleanup ephemeral IP routes on exception and handle two different routes for the same ip

This commit is contained in:
sxt1001 2023-05-19 23:47:20 +08:00
parent 753859600d
commit e179549335
3 changed files with 237 additions and 1 deletions

View File

@ -0,0 +1,160 @@
From 0273712c90d6facfc0fbf8d6def352f9810902a3 Mon Sep 17 00:00:00 2001
From: sxt1001 <shixuantong1@huawei.com>
Date: Mon, 3 Apr 2023 23:52:15 +0800
Subject: [PATCH] Cleanup ephemeral IP routes on exception (#2100)
If an exception occurs during EphemeralIPv4Network setup, any routes
that were setup need to be torn down. This wasn't happening, and this
commit adds the teardown.
---
cloudinit/net/__init__.py | 43 +++++++++++--------
cloudinit/net/tests/test_init.py | 72 ++++++++++++++++++++++++++++++++
2 files changed, 98 insertions(+), 17 deletions(-)
diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py
index f81f3a7..b007c9a 100644
--- a/cloudinit/net/__init__.py
+++ b/cloudinit/net/__init__.py
@@ -1090,23 +1090,32 @@ class EphemeralIPv4Network(object):
' to %s', self.connectivity_url_data['url'])
return
- self._bringup_device()
-
- # rfc3442 requires us to ignore the router config *if* classless static
- # routes are provided.
- #
- # https://tools.ietf.org/html/rfc3442
- #
- # If the DHCP server returns both a Classless Static Routes option and
- # a Router option, the DHCP client MUST ignore the Router option.
- #
- # Similarly, if the DHCP server returns both a Classless Static Routes
- # option and a Static Routes option, the DHCP client MUST ignore the
- # Static Routes option.
- if self.static_routes:
- self._bringup_static_routes()
- elif self.router:
- self._bringup_router()
+ try:
+ self._bringup_device()
+
+ # rfc3442 requires us to ignore the router config *if*
+ # classless static routes are provided.
+ #
+ # https://tools.ietf.org/html/rfc3442
+ #
+ # If the DHCP server returns both a Classless Static Routes
+ # option and a Router option, the DHCP client MUST ignore
+ # the Router option.
+ #
+ # Similarly, if the DHCP server returns both a Classless
+ # Static Routes option and a Static Routes option, the DHCP
+ # client MUST ignore the Static Routes option.
+ if self.static_routes:
+ self._bringup_static_routes()
+ elif self.router:
+ self._bringup_router()
+ except subp.ProcessExecutionError:
+ LOG.error(
+ "Error bringing up EphemeralIPv4Network. "
+ "Datasource setup cannot continue"
+ )
+ self.__exit__(None, None, None)
+ raise
def __exit__(self, excp_type, excp_value, excp_traceback):
"""Teardown anything we set up."""
diff --git a/cloudinit/net/tests/test_init.py b/cloudinit/net/tests/test_init.py
index c6a74c0..1ad6fce 100644
--- a/cloudinit/net/tests/test_init.py
+++ b/cloudinit/net/tests/test_init.py
@@ -12,6 +12,7 @@ import pytest
import requests
import cloudinit.net as net
+from cloudinit import subp
from cloudinit import safeyaml as yaml
from cloudinit.tests.helpers import CiTestCase, HttprettyTestCase
from cloudinit.subp import ProcessExecutionError
@@ -614,6 +615,77 @@ class TestEphemeralIPV4Network(CiTestCase):
self.assertEqual(expected_setup_calls, m_subp.call_args_list)
m_subp.assert_has_calls(expected_teardown_calls)
+ def test_teardown_on_enter_exception(self, m_subp):
+ """Ensure ephemeral teardown happens.
+ Even though we're using a context manager, we need to handle any
+ exceptions raised in __enter__ manually and do the appropriate
+ teardown.
+ """
+
+ def side_effect(args, **kwargs):
+ if args[3] == "append" and args[4] == "3.3.3.3/32":
+ raise subp.ProcessExecutionError("oh no!")
+
+ m_subp.side_effect = side_effect
+
+ with pytest.raises(subp.ProcessExecutionError):
+ with net.EphemeralIPv4Network(
+ interface="eth0",
+ ip="1.1.1.1",
+ prefix_or_mask="255.255.255.0",
+ broadcast="1.1.1.255",
+ static_routes=[
+ ("2.2.2.2/32", "9.9.9.9"),
+ ("3.3.3.3/32", "8.8.8.8"),
+ ],
+ ):
+ pass
+
+ expected_teardown_calls = [
+ mock.call(
+ [
+ "ip",
+ "-4",
+ "route",
+ "del",
+ "2.2.2.2/32",
+ "via",
+ "9.9.9.9",
+ "dev",
+ "eth0",
+ ],
+ capture=True,
+ ),
+ mock.call(
+ [
+ "ip",
+ "-family",
+ "inet",
+ "link",
+ "set",
+ "dev",
+ "eth0",
+ "down",
+ ],
+ capture=True,
+ ),
+ mock.call(
+ [
+ "ip",
+ "-family",
+ "inet",
+ "addr",
+ "del",
+ "1.1.1.1/24",
+ "dev",
+ "eth0",
+ ],
+ capture=True,
+ ),
+ ]
+ for teardown in expected_teardown_calls:
+ assert teardown in m_subp.call_args_list
+
@mock.patch('cloudinit.net.readurl')
def test_ephemeral_ipv4_no_network_if_url_connectivity(
self, m_readurl, m_subp):
--
2.33.0

View File

@ -0,0 +1,71 @@
From 0e25076b34fa995161b83996e866c0974cee431f Mon Sep 17 00:00:00 2001
From: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Date: Mon, 6 Dec 2021 18:34:26 +0100
Subject: [PATCH] cloudinit/net: handle two different routes for the same ip
(#1124)
If we set a dhcp server side like this:
$ cat /var/tmp/cloud-init/cloud-init-dhcp-f0rie5tm/dhcp.leases
lease {
...
option classless-static-routes 31.169.254.169.254 0.0.0.0,31.169.254.169.254
10.112.143.127,22.10.112.140 0.0.0.0,0 10.112.140.1;
...
}
cloud-init fails to configure the routes via 'ip route add' because to there are
two different routes for 169.254.169.254:
$ ip -4 route add 192.168.1.1/32 via 0.0.0.0 dev eth0
$ ip -4 route add 192.168.1.1/32 via 10.112.140.248 dev eth0
But NetworkManager can handle such scenario successfully as it uses "ip route append".
So change cloud-init to also use "ip route append" to fix the issue:
$ ip -4 route append 192.168.1.1/32 via 0.0.0.0 dev eth0
$ ip -4 route append 192.168.1.1/32 via 10.112.140.248 dev eth0
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
RHBZ: #2003231
---
cloudinit/net/__init__.py | 2 +-
cloudinit/net/tests/test_init.py | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py
index 7558745..f81f3a7 100644
--- a/cloudinit/net/__init__.py
+++ b/cloudinit/net/__init__.py
@@ -1157,7 +1157,7 @@ class EphemeralIPv4Network(object):
if gateway != "0.0.0.0":
via_arg = ['via', gateway]
subp.subp(
- ['ip', '-4', 'route', 'add', net_address] + via_arg +
+ ['ip', '-4', 'route', 'append', net_address] + via_arg +
['dev', self.interface], capture=True)
self.cleanup_cmds.insert(
0, ['ip', '-4', 'route', 'del', net_address] + via_arg +
diff --git a/cloudinit/net/tests/test_init.py b/cloudinit/net/tests/test_init.py
index f9102f7..c6a74c0 100644
--- a/cloudinit/net/tests/test_init.py
+++ b/cloudinit/net/tests/test_init.py
@@ -723,13 +723,13 @@ class TestEphemeralIPV4Network(CiTestCase):
['ip', '-family', 'inet', 'link', 'set', 'dev', 'eth0', 'up'],
capture=True),
mock.call(
- ['ip', '-4', 'route', 'add', '192.168.2.1/32',
+ ['ip', '-4', 'route', 'append', '192.168.2.1/32',
'dev', 'eth0'], capture=True),
mock.call(
- ['ip', '-4', 'route', 'add', '169.254.169.254/32',
+ ['ip', '-4', 'route', 'append', '169.254.169.254/32',
'via', '192.168.2.1', 'dev', 'eth0'], capture=True),
mock.call(
- ['ip', '-4', 'route', 'add', '0.0.0.0/0',
+ ['ip', '-4', 'route', 'append', '0.0.0.0/0',
'via', '192.168.2.1', 'dev', 'eth0'], capture=True)]
expected_teardown_calls = [
mock.call(
--
2.33.0

View File

@ -1,6 +1,6 @@
Name: cloud-init
Version: 21.4
Release: 14
Release: 15
Summary: the defacto multi-distribution package that handles early initialization of a cloud instance.
License: ASL 2.0 or GPLv3
URL: http://launchpad.net/cloud-init
@ -24,6 +24,8 @@ Patch12: backport-Do-not-change-permissions-of-netrules-target.patch
Patch13: fix-a-small-unitest-error.patch
Patch14: backport-CVE-2022-2084.patch
Patch15: remove-schema-errors-from-log-for-cloudinit-config-cc_.patch
Patch16: backport-cloudinit-net-handle-two-different-routes-for-the-sa.patch
Patch17: backport-Cleanup-ephemeral-IP-routes-on-exception-2100.patch
Patch9000: Fix-the-error-level-logs-displayed-for-the-cloud-init-local-service.patch
@ -135,6 +137,9 @@ fi
%exclude /usr/share/doc/*
%changelog
* Fri May 19 2023 shixuantong <shixuantong1@huawei.com> - 21.4-15
- Cleanup ephemeral IP routes on exception and handle two different routes for the same ip
* Sun May 14 2023 shixuantong <shixuantong1@huawei.com> - 21.4-14
- fix CVE-2022-2084