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

Cedar: DHCP server now assigns static IPv4 address, if present in user note #1218

Merged
merged 2 commits into from Oct 12, 2020
Merged

Cedar: DHCP server now assigns static IPv4 address, if present in user note #1218

merged 2 commits into from Oct 12, 2020

Conversation

PeTeeR-mpl
Copy link
Contributor

Feature proposed in this pull request:
assigning a static IP address to the user by the internal DHCP server.
It is independent of the client software.
How it works:
A) in the SessionMain() function:

  1. check incomming frames
  2. for DHCPDISCOVER and DHCPREQUEST frames, write down the static IP address
    ( which is readed form user's note ) in the SIADDR field of the DHCPHEADER
  3. process the modified frame like the others

B) in the VirtualDhcpServer() function:

  • for the DHCPDISCOVER and DHCPREQUEST frames, read the static IP address
    from the SIADDR field of the DHCPHEADER and treat it as the required IP address

@davidebeatrici
Copy link
Member

I would like to propose a different approach: MAC address <-> IP address mapping (commonly used in DHCP servers).

What are your thoughts about it?

@PeTeeR-mpl
Copy link
Contributor Author

I found the following problems with assigning a static IP address to a user based on the MAC<->IP binding:

  • a static MAC address can only be assigned to the L2TP over IPsec client, not the SEVPN client
  • I can not use the build-in DHCP server for MAC<->IP bindings, but I have to run external one.

All my users should receive:

  • IP address ( static or dynamic )
  • static routing table
  • no Default Gateway IP Address

An internal DHCP server will be sufficient for this, if it can perform a USER<->IP binding.

@davidebeatrici
Copy link
Member

The static MAC address should be applied client-side for L2 protocols; for L3 protocols it can be forced in the user notes field:

SoftEtherVPN/src/Cedar/Account.h

Line 11 in 684d17e

#define USER_MAC_STR_PREFIX L"MAC:"

SoftEtherVPN/src/Cedar/Account.c

Lines 1275 to 1320 in 2aeec32

// Get the MAC address from the user's note string
bool GetUserMacAddressFromUserNote(UCHAR *mac, wchar_t *note)
{
bool ret = false;
UINT i;
Zero(mac, 6);
if (mac == NULL || note == NULL)
{
return false;
}
i = UniSearchStrEx(note, USER_MAC_STR_PREFIX, 0, false);
if (i != INFINITE)
{
wchar_t *macstr_start = &note[i + UniStrLen(USER_MAC_STR_PREFIX)];
wchar_t macstr2[MAX_SIZE];
UNI_TOKEN_LIST *tokens;
UniStrCpy(macstr2, sizeof(macstr2), macstr_start);
UniTrim(macstr2);
tokens = UniParseToken(macstr2, L" ,/()[].");
if (tokens != NULL)
{
if (tokens->NumTokens >= 1)
{
wchar_t *macstr = tokens->Token[0];
if (UniIsEmptyStr(macstr) == false)
{
char macstr_a[MAX_SIZE];
UniToStr(macstr_a, sizeof(macstr_a), macstr);
ret = StrToMac(mac, macstr_a);
}
}
UniFreeToken(tokens);
}
}
return ret;
}

@PeTeeR-mpl
Copy link
Contributor Author

Davide Beatrice wrote:
"Static MAC address should be used on the client side for L2 protocols ..."

Yes, but I need to assign a static IP address to the user, not the device.
MAC <-> IP binding is not sufficient when several users can use the same device,
or one user can use multiple devices and should always get the same IP address.

@davidebeatrici
Copy link
Member

In that case we may want to implement a per-user setting that allows to set an allowed-IP list, as mentioned in #604.

@andrewfer000
Copy link

andrewfer000 commented Sep 25, 2020 •
edited

This is one feature that would be super helpful when setting up ACLs or using protocols that don't have a MAC address aasigned like L3 OpenVPN and PPP. Does this feature support both IPv4 and IPv6?

@PeTeeR-mpl
Copy link
Contributor Author

@davidebeatrici
OK, but that will be in the future. And, as andrewfer000 wrote:
"this would be a great feature for all the protocols", not only WG.
Now, the solution what I propose might help someone,
untill you implement a per-user setings for different protocols.

@PeTeeR-mpl
Copy link
Contributor Author

@andrewfer000
The solution I propose does not take into account the MAC address at all.
It works with the L2TP client, for which you had to force a MAC<->user bind
and then add static lease on an external DHCP server.
In addition, it works with the SoftEther Client, for which you cannot force a MAC<->user bind.

All you need to do if you want to apply my approach is to add an expression similar to:
IPv4: 10.1.2.3 in the user note and enable the built-in DHCP server. An external DHCP server is not needed.

Unfortunately, the built-in DHCP server only processes IPv4 frames,, so far.

@Neo-Desktop
Copy link

This PR would close #16 if merged - I think an implementation of this idea has been needed for a very long time

@PeTeeR-mpl @davidebeatrici
Cedar: DHCP server now assigns static IPv4 address, if present in use…
b890c7d 
…r note

This works for all VPN protocols.

In SessionMain(): for DHCPDISCOVER and DHCPREQUEST frames, write the static IP address (which is retrieved from the user notes) in the SIADDR field of DHCPHEADER.

In VirtualDhcpServer(): for DHCPDISCOVER and DHCPREQUEST frames, read the static IP address from the SIADDR field of DHCPHEADER and assign it to the client.
@PeTeeR-mpl @davidebeatrici
Add myself to AUTHORS
d29d1f5
@davidebeatrici davidebeatrici changed the title assign static IP addres Cedar: DHCP server now assigns static IPv4 address, if present in user note Oct 12, 2020
@davidebeatrici davidebeatrici merged commit 3c140dd into SoftEtherVPN:master Oct 12, 2020
@davidebeatrici
Copy link
Member

Thank you very much for your contribution!

@PeTeeR-mpl PeTeeR-mpl mentioned this pull request Jul 12, 2021
Closed
@domosekai domosekai mentioned this pull request Sep 19, 2021
Closed
@PeTeeR-mpl
Copy link
Contributor Author

PeTeeR-mpl commented Dec 29, 2021 via email

@domosekai domosekai mentioned this pull request May 22, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidebeatrici davidebeatrici davidebeatrici approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@PeTeeR-mpl @davidebeatrici @andrewfer000 @Neo-Desktop @Evengard @PizzaProgram