Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Dhcpcd generating INIT-REBOOT messages during RENEWING/REBINDING when --noconfigure is on #355

Open
acst1223 opened this issue Aug 26, 2024 · 6 comments
Open

Dhcpcd generating INIT-REBOOT messages during RENEWING/REBINDING when --noconfigure is on #355

acst1223 opened this issue Aug 26, 2024 · 6 comments

Comments

@acst1223
Copy link
Contributor

Hi,

I would like to report that when using dhcpcd with --noconfigure, it generates INIT-REBOOT messages during RENEWING/REBINDING.

According to RFC 2131 4.3.6, when the DHCP client renews or rebinds the lease, the requested-ip must not be set and ciaddr should be the IP address. However I've observed that in the actual dhcpcd packets, the requested-ip is set and ciaddr is zero. This is exactly the behavior of INIT-REBOOT.

After some investigation in the code, I've found that the reason lies in the state->addr property. When sending a DHCPREQUEST, if state->addr is NULL, ciaddr will be zero and the requested-ip will be set. Otherwise if state->addr is not NULL, ciaddr will be set and the requested-ip will not be set. When dhcpcd with --configure binds a lease, ipv4_applyaddr() is called and state->addr is set, so the RENEWING/REBINDING later for the lease works fine. However, when --noconfigure is used, ipv4_applyaddr() is not called in dhcp_bind(), for dhcp_bind() has returned earlier. This leads to state->addr keeping NULL and dhcpcd sending wrong messages during RENEWING/REBINDING.

I believe that although dhcpcd does not configure the system with --noconfigure, since it is still responsible for the DHCP negotiation, it still needs to maintain the correct state for each interface and send correct DHCP messages to the server.

Roy, could you take a look at this? Thanks!

Zikai
acst1223 added a commit to acst1223/dhcpcd that referenced this issue Aug 31, 2024
@acst1223
Fix dhcpcd sending wrong message when RENEWING/REBINDING
bb968ea 
When RENEWING/REBINDING, the DHCP client should send messages with
ciaddr set and requested-ip unset. However when dhcpcd 10 is configured
with --noconfigure, the messages sent are the opposite: ciaddr unset
and requested-ip set - these messages are for INIT-REBOOT, not RENEWING
or REBINDING.

The reason for it is that when --noconfigure is enabled, dhcpcd won't
setup the state->addr property of the interface when the lease is bound.
When dhcpcd renews or rebinds the lease later, it needs state->addr to
be set to correctly configure ciaddr and requested-ip. Since state->addr
is NULL, dhcpcd sends wrong messages (for the detail see
github.com/NetworkConfiguration/issues/355).

This patch fixes this by also setting up state->addr with
ipv4_applyaddr() when --noconfigure is enabled. Note that this function
also configure the IPv4 address and build routes in the system, so we
need to disable these in the function.

Change-Id: I94bfffd95b62066995bd436d3b8bf185eafda398
@acst1223 acst1223 mentioned this issue Aug 31, 2024
Closed
acst1223 added a commit to acst1223/dhcpcd that referenced this issue Aug 31, 2024
@acst1223
Fix dhcpcd sending wrong message when RENEWING/REBINDING
dfc0587 
When RENEWING/REBINDING, the DHCP client should send messages with
ciaddr set and requested-ip unset. However when dhcpcd 10 is configured
with --noconfigure, the messages sent are the opposite: ciaddr unset
and requested-ip set - these messages are for INIT-REBOOT, not RENEWING
or REBINDING.

The reason for it is that when --noconfigure is enabled, dhcpcd won't
setup the state->addr property of the interface when the lease is bound.
When dhcpcd renews or rebinds the lease later, it needs state->addr to
be set to correctly configure ciaddr and requested-ip. Since state->addr
is NULL, dhcpcd sends wrong messages (for the detail see
github.com/NetworkConfiguration/issues/355).

This patch fixes this by also setting up state->addr with
ipv4_applyaddr() when --noconfigure is enabled. Note that this function
also configure the IPv4 address and build routes in the system, so we
need to disable these in the function.

Change-Id: I94bfffd95b62066995bd436d3b8bf185eafda398
@acst1223 acst1223 mentioned this issue Aug 31, 2024
Closed
acst1223 added a commit to acst1223/dhcpcd that referenced this issue Sep 3, 2024
@acst1223
Fix dhcpcd sending wrong message when RENEWING/REBINDING
ddb65ae 
When RENEWING/REBINDING, the DHCP client should send messages with
ciaddr set and requested-ip unset. However when dhcpcd 10 is configured
with --noconfigure, the messages sent are the opposite: ciaddr unset
and requested-ip set - these messages are for INIT-REBOOT, not RENEWING
or REBINDING.

The reason for it is that when --noconfigure is enabled, dhcpcd won't
setup the state->addr property of the interface when the lease is bound.
When dhcpcd renews or rebinds the lease later, it needs state->addr to
be set to correctly configure ciaddr and requested-ip. Since state->addr
is NULL, dhcpcd sends wrong messages (for the detail see
github.com/NetworkConfiguration/issues/355).

This patch fixes this by also setting up state->addr with
ipv4_applyaddr() when --noconfigure is enabled. Note that this function
also configure the IPv4 address and build routes in the system, so we
need to disable these in the function.
@rsmarples
Copy link
Member

I'm in two minds about this.
Even though dhcpcd isn't configuring the interface, it can detect if something has.
I would much rather we detect if something has and then adjust accordingly.
If at renew time and nothing has configured the interface then technically we cannot renew as there is no source address to renew from so INIT-REBOOT makes more sense than RENEW anyway.

BTW, just --force your push to your PR rather than creating new ones.

@acst1223
Copy link
Contributor Author

Thanks for the reminder! I now understand that this detection is in dhcp_handleifa(). However, it only sets the address to state if PRIVSEP is used. What's the purpose for this? If we are using --noconfigure without PRIVSEP then we can never get the address set in the state.

I'll close the PR since dhcp_handleifa() now seems to be the issue.

@acst1223
Copy link
Contributor Author

Hi @rsmarples , is there any idea on this issue? Do you think we can remove the PRIVSEP here?

rsmarples added a commit that referenced this issue Oct 16, 2024
@rsmarples @acst1223
DHCP: when ifa sees the bound address, always add it to state
68d7c8c 
This should fix dhcpcd sending the correct DHCP message type
when entering the RENEW or REBIND state.

Fixes #355.

Co-authored-by: Zikai Chen <chenzikai@google.com>
@rsmarples
Copy link
Member

@acst1223 Ah sorry, time got away and I forgot about this.
It was a bit more complicated, but I think the above patch covers it.
Needs some testing though. If it fixes your use case or not please let me know!

@acst1223
Copy link
Contributor Author

Thanks for your patch! I've tried it and it fixes this issue perfectly!

@rsmarples
Copy link
Member

Need to do some testing myself before I commit, but the feedback is appreciated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@rsmarples @acst1223 and others