nic_router: don't call DHCP client on requests

Until now, the DHCP client was called also for DHCP requests when an interface
had a domain but yet no IP config. Now, an interface distinguishes between DHCP
requests and replies first and then accordingly calls the DHCP server or the
DHCP client if they're available. This also prevents that the DHCP client has
to handle packet headers other than that of DHCP.

Fixes #3681

repos/os/src/server/nic_router/dhcp_client.cc

@@ -117,79 +117,55 @@ void Dhcp_client::_handle_timeout(Duration)
 }
 
 
-void Dhcp_client::handle_ip(Ethernet_frame &eth,
-                            Size_guard     &size_guard)
+void Dhcp_client::handle_dhcp_reply(Dhcp_packet &dhcp)
 {
-	if (eth.dst() != _interface.router_mac() &&
-	    eth.dst() != Mac_address(0xff))
-	{
-		throw Drop_packet("DHCP client expects Ethernet targeting the router"); }
+	try {
+		Message_type const msg_type =
+			dhcp.option<Dhcp_packet::Message_type_option>().value();
 
-	Ipv4_packet &ip = eth.data<Ipv4_packet>(size_guard);
-	if (ip.protocol() != Ipv4_packet::Protocol::UDP) {
-		throw Drop_packet("DHCP client expects UDP packet"); }
+		switch (_state) {
+		case State::SELECT:
 
-	Udp_packet &udp = ip.data<Udp_packet>(size_guard);
-	if (!Dhcp_packet::is_dhcp(&udp)) {
-		throw Drop_packet("DHCP client expects DHCP packet"); }
+			if (msg_type != Message_type::OFFER) {
+				throw Drop_packet("DHCP client expects an offer");
+			}
+			_set_state(State::REQUEST, _config().dhcp_request_timeout());
+			_send(Message_type::REQUEST, Ipv4_address(),
+			      dhcp.option<Dhcp_packet::Server_ipv4>().value(),
+			      dhcp.yiaddr());
+			break;
 
-	Dhcp_packet &dhcp = udp.data<Dhcp_packet>(size_guard);
-	if (dhcp.op() != Dhcp_packet::REPLY) {
-		throw Drop_packet("DHCP client expects DHCP reply"); }
+		case State::REQUEST:
+			{
+				if (msg_type != Message_type::ACK) {
+					throw Drop_packet("DHCP client expects an acknowledgement");
+				}
+				_lease_time_sec = dhcp.option<Dhcp_packet::Ip_lease_time>().value();
+				_set_state(State::BOUND, _rerequest_timeout(1));
+				Ipv4_address dns_server;
+				try { dns_server = dhcp.option<Dhcp_packet::Dns_server_ipv4>().value(); }
+				catch (Dhcp_packet::Option_not_found) { }
+				_domain().ip_config(dhcp.yiaddr(),
+				                    dhcp.option<Dhcp_packet::Subnet_mask>().value(),
+				                    dhcp.option<Dhcp_packet::Router_ipv4>().value(),
+				                    dns_server);
+				break;
+			}
+		case State::RENEW:
+		case State::REBIND:
 
-	if (dhcp.client_mac() != _interface.router_mac()) {
-		throw Drop_packet("DHCP client expects DHCP targeting the router"); }
-
-	try { _handle_dhcp_reply(dhcp); }
-	catch (Dhcp_packet::Option_not_found) {
-		throw Drop_packet("DHCP client misses DHCP option"); }
-}
-
-
-void Dhcp_client::_handle_dhcp_reply(Dhcp_packet &dhcp)
-{
-	Message_type const msg_type =
-		dhcp.option<Dhcp_packet::Message_type_option>().value();
-
-	switch (_state) {
-	case State::SELECT:
-
-		if (msg_type != Message_type::OFFER) {
-			throw Drop_packet("DHCP client expects an offer");
-		}
-		_set_state(State::REQUEST, _config().dhcp_request_timeout());
-		_send(Message_type::REQUEST, Ipv4_address(),
-		      dhcp.option<Dhcp_packet::Server_ipv4>().value(),
-		      dhcp.yiaddr());
-		break;
-
-	case State::REQUEST:
-		{
 			if (msg_type != Message_type::ACK) {
 				throw Drop_packet("DHCP client expects an acknowledgement");
 			}
-			_lease_time_sec = dhcp.option<Dhcp_packet::Ip_lease_time>().value();
 			_set_state(State::BOUND, _rerequest_timeout(1));
-			Ipv4_address dns_server;
-			try { dns_server = dhcp.option<Dhcp_packet::Dns_server_ipv4>().value(); }
-			catch (Dhcp_packet::Option_not_found) { }
-			_domain().ip_config(dhcp.yiaddr(),
-			                    dhcp.option<Dhcp_packet::Subnet_mask>().value(),
-			                    dhcp.option<Dhcp_packet::Router_ipv4>().value(),
-			                    dns_server);
+			_lease_time_sec = dhcp.option<Dhcp_packet::Ip_lease_time>().value();
 			break;
-		}
-	case State::RENEW:
-	case State::REBIND:
 
-		if (msg_type != Message_type::ACK) {
-			throw Drop_packet("DHCP client expects an acknowledgement");
+		default: throw Drop_packet("DHCP client doesn't expect a packet");
 		}
-		_set_state(State::BOUND, _rerequest_timeout(1));
-		_lease_time_sec = dhcp.option<Dhcp_packet::Ip_lease_time>().value();
-		break;
-
-	default: throw Drop_packet("DHCP client doesn't expect a packet");
+	}
+	catch (Dhcp_packet::Option_not_found) {
+		throw Drop_packet("DHCP reply misses required option");
 	}
 }
 

repos/os/src/server/nic_router/dhcp_client.h

@@ -43,8 +43,6 @@ class Net::Dhcp_client
 		Timer::One_shot_timeout<Dhcp_client>  _timeout;
 		Genode::uint64_t                      _lease_time_sec = 0;
 
-		void _handle_dhcp_reply(Dhcp_packet &dhcp);
-
 		void _handle_timeout(Genode::Duration);
 
 		void _rerequest(State next_state);
@@ -68,8 +66,7 @@ class Net::Dhcp_client
 		            Timer::Connection &timer,
 		            Interface         &interface);
 
-		void handle_ip(Ethernet_frame &eth,
-		               Size_guard     &size_guard);
+		void handle_dhcp_reply(Dhcp_packet &dhcp);
 
 		void discover();
 };

repos/os/src/server/nic_router/interface.cc

@@ -1100,18 +1100,34 @@ void Interface::_handle_ip(Ethernet_frame          &eth,
 
 				/* get DHCP packet */
 				Dhcp_packet &dhcp = udp.data<Dhcp_packet>(size_guard);
-				if (dhcp.op() == Dhcp_packet::REQUEST) {
-					try {
-						_handle_dhcp_request(eth, dhcp, local_domain);
-						return;
+				switch (dhcp.op()) {
+				case Dhcp_packet::REQUEST:
+
+					try { _handle_dhcp_request(eth, dhcp, local_domain); }
+					catch (Pointer<Dhcp_server>::Invalid) {
+						throw Drop_packet("DHCP request while DHCP server inactive");
 					}
-					catch (Pointer<Dhcp_server>::Invalid) { }
-				} else {
-					if (!_dhcp_client.constructed()) {
-						throw Drop_packet("DHCP client not active");
-					}
-					_dhcp_client->handle_ip(eth, size_guard);
 					return;
+
+				case Dhcp_packet::REPLY:
+
+					if (eth.dst() != router_mac() &&
+					    eth.dst() != Mac_address(0xff))
+					{
+						throw Drop_packet("Ethernet of DHCP reply doesn't target router"); }
+
+					if (dhcp.client_mac() != router_mac()) {
+						throw Drop_packet("DHCP reply doesn't target router"); }
+
+					if (!_dhcp_client.constructed()) {
+						throw Drop_packet("DHCP reply while DHCP client inactive"); }
+
+					_dhcp_client->handle_dhcp_reply(dhcp);
+					return;
+
+				default:
+
+					throw Drop_packet("Bad DHCP opcode");
 				}
 			}
 		}
@@ -1475,13 +1491,44 @@ void Interface::_handle_eth(Ethernet_frame           &eth,
 	} else {
 
 		switch (eth.type()) {
-		case Ethernet_frame::Type::IPV4:
-			if (!_dhcp_client.constructed()) {
-				throw Drop_packet("DHCP client not active");
+		case Ethernet_frame::Type::IPV4: {
+
+			if (eth.dst() != router_mac() &&
+			    eth.dst() != Mac_address(0xff))
+			{
+				throw Drop_packet("Expecting Ethernet targeting the router"); }
+
+			Ipv4_packet &ip = eth.data<Ipv4_packet>(size_guard);
+			if (ip.protocol() != Ipv4_packet::Protocol::UDP) {
+				throw Drop_packet("Expecting UDP packet"); }
+
+			Udp_packet &udp = ip.data<Udp_packet>(size_guard);
+			if (!Dhcp_packet::is_dhcp(&udp)) {
+				throw Drop_packet("Expecting DHCP packet"); }
+
+			Dhcp_packet &dhcp = udp.data<Dhcp_packet>(size_guard);
+			switch (dhcp.op()) {
+			case Dhcp_packet::REPLY:
+
+				if (dhcp.client_mac() != router_mac()) {
+					throw Drop_packet("Expecting DHCP targeting the router"); }
+
+				if (!_dhcp_client.constructed()) {
+					throw Drop_packet("Expecting DHCP client to be active"); }
+
+				_dhcp_client->handle_dhcp_reply(dhcp);
+				break;
+
+			default:
+
+				throw Drop_packet("Expecting DHCP reply");
 			}
-			_dhcp_client->handle_ip(eth, size_guard);
 			break;
-		default: throw Bad_network_protocol(); }
+		}
+		default:
+
+			throw Bad_network_protocol();
+		}
 	}
 }