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.

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

Fix Virtual DHCP Server: Correct DHCP Sequence #1989

Merged
merged 3 commits into from
May 4, 2024

Conversation

hiura2023
Copy link
Contributor

@hiura2023 hiura2023 commented Apr 26, 2024

Changes proposed in this pull request:

  • Virtual DHCP sequence is not correct.
    Why does DHCP NAK respond to DHCP DISCOVER ?

  • Change DHCP sequence like below.

  • Actual action
    ①DHCP DISCOVER
    ②DHCP NAK
    ③DHCP DISCOVER
    ④DHCP NAK
    ⑤Sequence ③,④ repeated several times.
    ⑥DHCP DISCOVER
    ⑦DHCP OFFER
    ⑧DHCP REQUEST
    ⑨DHCP ACK
    Attached A and B

  • Expected action
    ①DHCP DISCOVER first
    ②DHCP DISCOVER second
    ③DHCP DISCOVER repeated several times
    ④DHCP OFFER
    ⑤DHCP REQUEST
    ⑥DHCP ACK
    DHCP client resend ②,③ if VPN server is not ready to reply ④ immediately
    Refer to RFC2131

  • Ideal action
    ①DHCP DISCOVER
    ②DHCP OFFER
    ③DHCP REQUEST
    ④DHCP ACK

Refer to RFC2131:
https://www.rfc-editor.org/rfc/rfc2131#section-4.4
[Page 34]Figure 5: State-transition diagram for DHCP clients

Timeout example:
https://www.cente.jp/files/20100222_TCPIPv4_001-0016.pdf

Attached A:
The time-out period is 20 seconds on "security policy of user" screen.
DHCP_ACTUAL_20SEC2024-04-29

Attached B:
SEVPN_DHCP_TOO_LATE_REMOTE.txt

@chipitsine
Copy link
Member

chipitsine commented Apr 26, 2024 via email

@hiura2023
Copy link
Contributor Author

hiura2023 commented Apr 29, 2024

I updated first comment.

I think that DHCP_NAK responds to DHCP_REQUEST, not DHCP_DISCOVER.
Refer to RFC2131.
Who added virtual DHCP server ? Ask him.

@hiura2023
Copy link
Contributor Author

How do you find all those ? Just curious

I added the test environment and test result later.
find what ?

@chipitsine
Copy link
Member

How do you find all those ? Just curious

I added the test environment and test result later. find what ?

I've some ideas on automated testing.

in theory we can try ISC dhclient, which is flexible and allows dhclient-script to handle response instead of applying to system live. but those are ideas for future improvements

@hiura2023
Copy link
Contributor Author

hiura2023 commented May 3, 2024

Description of second pull request I added

  1. The second pull request applies to fixed IP address assignment using the user's "note" field.
  2. Reassigning static IP address does not work.
  3. I changed the way of assigning static IP address to work.
  4. As to the issue, refer to
    [Bug] SecureNAT DHCP does not give any IP address, if client is re-connecting, and if set to Fixed IPv4: #1947

Ideal action

①DHCP DISCOVER
②DHCP OFFER
③DHCP REQUEST
④DHCP ACK
⑤DHCP REQUEST FOR RENEWAL LEASE TIME 1
⑥DHCP ACK
⑦DHCP REQUEST FOR RENEWAL LEASE TIME 2
⑧DHCP ACK
⑨DHCP REQUEST FOR RENEWAL LEASE TIME 3
⑩DHCP ACK
CONTINUED

Attached A:

Lease Limit:40 seconds
DisableSessionReconnect: true
Time-out Period: 20 seconds
DHCP_REQ_RENEWAL_20SEC2024-05-03

@PizzaProgram
Copy link

PizzaProgram commented May 4, 2024

@hiura2023 Excellent work!

I've checked the code changes and as much as I understood it's perfect!
(I'm not a C expert, only Delphi but your coding is very nice and understandable. Special thanks for the inserted comments! )

If this Commit will be applied,

  • FIXed IPs MUST be outside of the normal DHCP range. Right ?

@chipitsine chipitsine merged commit 9378c34 into SoftEtherVPN:master May 4, 2024
11 checks passed
@chipitsine
Copy link
Member

thank you for contribution!

@chipitsine
Copy link
Member

I'm going to make a release today

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants