From 36d2374ff9289416d2777b0895521b2662291d59 Mon Sep 17 00:00:00 2001 From: Martin Stein Date: Fri, 27 May 2022 10:50:30 +0200 Subject: [PATCH] wireguard: exit on invalid configurations With this commit, the WireGuard component exits with a descriptive uncaught exception on invalid configurations or when the user attempts to re-configure attributes that are not re-configurable (private_key, listen_port, interface). This is particularly important when it comes to the not re-configurable private key. If the component would just ignore the attempt to override the private key, the user may come to believe that his old (potentially compromised) private key is not in use anymore. The fact that the component now exits instead shouldn't be a problem, as the user would have to restart the component anyway in order to apply the new attribute values. The commit also extends the wg_reconfig run script to test that WireGuard exits on the attempt to re-configure the private key. Ref #4520 --- repos/dde_linux/run/wg_reconfig.run | 157 ++++++++++-------- .../src/app/wireguard/config_model.cc | 43 +++-- 2 files changed, 121 insertions(+), 79 deletions(-) diff --git a/repos/dde_linux/run/wg_reconfig.run b/repos/dde_linux/run/wg_reconfig.run index 9c3dcd306f..ee85ec4263 100644 --- a/repos/dde_linux/run/wg_reconfig.run +++ b/repos/dde_linux/run/wg_reconfig.run @@ -4,9 +4,10 @@ # WireGuard changes. Each peer has its own WireGuard instance and talks # to the other peers only through WireGuard. The server WireGuard (peer 2) # initially accepts only peer 1. After some time it gets re-configured to -# accept only peer 3. At the end, it gets re-configured to accept only peer 1 -# again. Note that the peer 1 WireGuard has to be reconfigured as well, in -# order to be forced to redo the initiation handshake for the last +# accept only peer 3. Then, it gets re-configured to accept only peer 1 again +# and, in the end, an attempt is made to re-configure the private key, which +# should fail. Note that the peer 1 WireGuard has to be reconfigured as well, +# in order to be forced to redo the initiation handshake for the third # configuration phase of the server WireGuard. # @@ -26,13 +27,13 @@ import_from_depot [depot_user]/src/libc \ [depot_user]/src/vfs_lwip \ [depot_user]/src/zlib -proc peer1_wg_config {variant} { +proc peer_1_wg_config {peers} { append result { } - if {$variant == "with_peer3"} { + if {$peers == "peer_3"} { append result { - } - if {$variant == "with_peer1"} { - append result { - + switch $private_key { + private_key_1 { + append result { + + append result { + listen_port="49002"> + } + switch $peers { + peer_1 { + append result { + + } + } + peer_3 { + append result { + + } } } append result { @@ -100,38 +114,44 @@ append config { - + - - } [peer2_wg_config with_peer1] { + + } [peer_2_wg_config peer_1 private_key_1] { - - } [peer2_wg_config with_peer3] { + + } [peer_2_wg_config peer_3 private_key_1] { - - } [peer2_wg_config with_peer1] { + + } [peer_2_wg_config peer_1 private_key_1] { + + + + + + } [peer_2_wg_config peer_1 private_key_2] { - + - } [peer1_wg_config with_peer3] { + } [peer_1_wg_config peer_3] { - } [peer1_wg_config without_peer] { + } [peer_1_wg_config no_peer] { - } [peer1_wg_config with_peer3] { + } [peer_1_wg_config peer_3] { @@ -158,7 +178,7 @@ append config { - + @@ -172,7 +192,7 @@ append config { - + @@ -205,12 +225,12 @@ append config { - + - + @@ -223,12 +243,12 @@ append config { - + - + @@ -241,7 +261,7 @@ append config { - + - - - + + + - + - + - - + + - + - + - - - + + + - + - - + + - + - - - + + + - + - + - - + + - + - + @@ -346,7 +366,7 @@ append config { - + @@ -486,11 +506,12 @@ build_boot_image $boot_modules append qemu_args "-nographic " -append output_pattern "peer1_ping. 64 bytes from 10.0.9.2.*\n" -append output_pattern ".*peer1_ping. 64 bytes from 10.0.9.2.*\n" -append output_pattern ".*child \"peer3_fetchurl\" exited with exit value 0.*\n" -append output_pattern ".*peer1_ping. 64 bytes from 10.0.9.2.*\n" -append output_pattern ".*peer1_ping. 64 bytes from 10.0.9.2.*\n" +append output_pattern "peer_1_ping. 64 bytes from 10.0.9.2.*\n" +append output_pattern ".*peer_1_ping. 64 bytes from 10.0.9.2.*\n" +append output_pattern ".*child \"peer_3_fetchurl\" exited with exit value 0.*\n" +append output_pattern ".*peer_1_ping. 64 bytes from 10.0.9.2.*\n" +append output_pattern ".*peer_1_ping. 64 bytes from 10.0.9.2.*\n" +append output_pattern ".*peer_2_wg. .*Error: Uncaught exception of type .*Invalid_reconfiguration_attempt.*\n" -run_genode_until $output_pattern 30 +run_genode_until $output_pattern 45 diff --git a/repos/dde_linux/src/app/wireguard/config_model.cc b/repos/dde_linux/src/app/wireguard/config_model.cc index da4251d673..c658096bb0 100644 --- a/repos/dde_linux/src/app/wireguard/config_model.cc +++ b/repos/dde_linux/src/app/wireguard/config_model.cc @@ -33,19 +33,35 @@ Config_model::Config_model(Genode::Allocator &alloc) void Config_model::update(genode_wg_config_callbacks &callbacks, Xml_node node) { - if (!_config.constructed()) { + Key_base64 const private_key_b64 { + node.attribute_value("private_key", Key_base64 { }) }; - _config.construct( - node.attribute_value("private_key", Key_base64 { }), - node.attribute_value("listen_port", (uint16_t)0U), - node.attribute_value("interface", Ipv4_address_prefix { })); + uint16_t const listen_port { + node.attribute_value("listen_port", (uint16_t)0U) }; + + Ipv4_address_prefix const interface { + node.attribute_value("interface", Ipv4_address_prefix { }) }; + + if (_config.constructed()) { + + if (_config->private_key_b64() != private_key_b64 || + _config->listen_port() != listen_port || + _config->interface() != interface) + { + class Invalid_reconfiguration_attempt { }; + throw Invalid_reconfiguration_attempt { }; + } + + } else { uint8_t private_key[WG_KEY_LEN]; - if (!_config->private_key_b64().valid() || - !key_from_base64(private_key, _config->private_key_b64().string())) { + if (!private_key_b64.valid() || + !key_from_base64(private_key, private_key_b64.string())) { - error("Invalid private key!"); + class Invalid_private_key { }; + throw Invalid_private_key { }; } + _config.construct(private_key_b64, listen_port, interface); callbacks.add_device(_config->listen_port(), private_key); } Peer_update_policy policy { _alloc, callbacks, _config->listen_port() }; @@ -91,7 +107,9 @@ void Config_model::Peer_update_policy::destroy_element(Element &peer) { uint8_t public_key[WG_KEY_LEN]; if (!key_from_base64(public_key, peer._public_key_b64.string())) { - error("Invalid public key!"); + + class Invalid_public_key { }; + throw Invalid_public_key { }; } _callbacks.remove_peer(public_key); @@ -111,10 +129,13 @@ Config_model::Peer_update_policy::create_element(Xml_node node) if (!public_key_b64.valid() || !key_from_base64(public_key, public_key_b64.string())) { - error("Invalid public key!"); + class Invalid_public_key { }; + throw Invalid_public_key { }; } if (!allowed_ip.valid()) { - error("Invalid allowed ip!"); + + class Invalid_allowed_ip { }; + throw Invalid_allowed_ip { }; } _callbacks.add_peer( _listen_port, endpoint_ip.addr, endpoint_port, public_key,