This is mostly equivalent to dhcp_packet_verify_headers(), but
- it optionally returns the UDP payload as iovec, and
- supports IP header with options,
- check packet length more strictly.
Then make dhcp_packet_append_ip_headers() just a wrapper of the new
function. Currently, the wrapper is inefficient, but will be removed in
a later commit.
discover_attempts should be reset only when
- the client is stopped, to make the counter starts from zero on
the next invocation.
- we acquire a bound lease, to make the counter starts from zero
when the lease is expired.
request_attempts should be reset only when the client enter a new state
that sends DHCPREQUEST, that is, when enter one of the REBOOTING,
REQUESTING, RENEWING, and REBINDING state.
This moves resetting counter to client_set_state() as it should happen
only when the state transition.
Otherwise, if we emit the notification without a valid bound lease, networkd
may be confused (of course should not, but for safety).
Also, increment the delay before calling client_start_delayed().
Otherwise, the first reboot is done instantaneously.
If we fail to setup timer event sources about the lease lifetime or
T1/T2, then the lease will be never updated, and the user (networkd)
will not receive any notification about the expire. The situation is
terrible. Let's stop the client with error code earlier, and notify the
failure to networkd.
This drop delay after ACK, as it has many problems. See comment in
sd_dhcp_client_is_waiting_for_ipv6_connectivity() for more details.
This way, the logic becomes much much simpler.
Also, do not restart the client if we lost IPv6 connectivity in
sd_dhcp_client side, but restart the client by networkd. As,
sd_dhcp_client does not know if we can start the client or not,
e.g., the interface may be currently down.
It is not necessary to enable another timer event source to send
DHCPREQUEST from the T1/T2 timer event source. Just call the callback
function for sending message.
Also, T1 hits only we have a bound lease. Drop spurious conditions.
When we start the client, any previous state/configuration should be cleaned.
Let's effectively do the same thing as client_initialize() in that
function.
This also several assertions in client_start_delayed() to
sd_dhcp_client_start(). These kind of checks should be done earlier.
Similarly, enter the REBOOTING state before sending DHCPREQUEST on reboot.
Also, this makes DHCPREQUEST message is sent several times also in REBOOTING
state. Previously, we wait about 4 seconds after DHCPREQUEST on reboot, and
entered the init state if no response. Now we wait 1 second after the first
DHCPREQUEST, resend another DHCPREQUEST, wait 2 seconds, then enter the
init state if no response. So, even in the worst case, we have slight
speed up.
To make gracefully ignore unexpected messages from outside at unexpected timing.
This potentially reduces work load to handle such messages, and slightly
reduces attack surface by malicious DHCP messages.
This also makes the socket fd is owned by the relevant IO event source.
Except for the performance optimization and security hardening, this
should not change any behaviors. So, just refactoring.
The commit 6d7cb9a6b8 fixes the assignment
of the these options when specified through SendOption=. However, it
breaks when specified through UserClass= or SendVendorOption=.
When UserClass= or SendVendorOption= is specified, the option length is
calculated from the sd_dhcp_client.user_class or .vendor_options. Hence,
we can use 0 for the length in that case.
Follow-up for 6d7cb9a6b8.
Call client_stop() on error and return 0 only on callback.
Normal non-callback functions should propagate errors.
This also makes client_initialize_time_events() use
event_reset_time_relative().
FORCERENEW message support has been disabled so long time for security
concern. Most other implementations of DHCP server/client neither
support FORCERENEW. Let's completely drop relevant code.
Typically, the top-level JSON object has to be an object, in any json
document we parse, hence let's add a simple way to enforce that.
Make use of this in various places.
(Note, various other JSON parsers insist on this logic right from the
beginning, but I actually thinking making this insisting optional like
this patch does it is the cleaner approach)
- Extract common logic to client_send_release_or_decline().
- Do not send DECLINE message on BOOTP protocol.
- Drop redundant assignment of chaddr, as it is already set by
client_message_init() -> dhcp_message_init().
- Do not assign acquired address in ciaddr field of DECLINE message,
but use Requested IP Address option.
- Broadcast DECLINE message, rather than unicast.
- Set server identifier in both cases.
Fixes#39299.
When a DHCP client uses SendOption=82:string:..., option_append() calls
the SD_DHCP_OPTION_RELAY_AGENT_INFORMATION case which was written for
the server relay path. It casts optval to sd_dhcp_server* and calls
strlen() on its members, but optval is actually raw binary data from
the client, causing SIGSEGV. The same is applicable when option 43 and
option 77 are passed to SendOption.
Fix by checking optlen > 0 and appending the option as a plain TLV,
skipping the server-specific relay agent logic.
Suppress warnings like the following from clang tidy:
```
../src/boot/addon.c:11:19: error: function 'efi_main' can be made static to enforce internal linkage [misc-use-internal-linkage,-warnings-as-errors]
11 | EFIAPI EFI_STATUS efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *system_table);
| ^
```
Some warnings are suppressed simply by setting comments to ignore the warning,
some are by making global variables static, or include a suitable header.
Yu Watanabe
Zbigniew Jędrzejewski-Szmek
Lennart Poettering
Michael Vogt
Daan De Meyer
Nandakumar Raghavan
Frantisek Sumsal
Skye Soss
Mike Yuan