While keeping the fixed addresses out of the DHCP pool range is ideal, it is not always practicable or convenient. In the current implementation, if you specify a fixed lease that overlaps with the DHCP dynamic address pool, DHCP will issue conflicting addresses under certain circumstances. This is because the range definition is not modified in dhcp.conf to exclude the fixed lease. Here are a few links that discuss this topic: https://lists.isc.org/pipermail/dhcp-users/2013-August/017054.html http://serverfault.com/questions/289831/exclude-ip-address-from-dhcp-pool Example: Subnet 10.3.0.0/24 DHCP Start Address: 10.3.0.10 DHCP End Address: 10.3.0.50 Fixed Lease for host foo: 10.3.0.15 Fixed Least for host bar: 10.3.0.34 In this scenario, IPFire writes its dhcp.conf (in part) like this: subnet 10.3.0.0 netmask 255.255.255.0 #BLUE { range 10.3.0.10 10.3.0.50; option subnet-mask 255.255.255.0; option domain-name "domain.dom"; option routers 10.3.0.1; option domain-name-servers 10.3.0.1; default-lease-time 86400; max-lease-time 172800; } #BLUE host fix0 # foo { hardware ethernet aa:bb:cc:dd:ee:ff; fixed-address 10.3.0.15; } host fix1 # bar { hardware ethernet ff:ee:dd:cc:bb:aa; fixed-address 10.3.0.34; } Instead, it should be writing it like this: subnet 10.3.0.0 netmask 255.255.255.0 #BLUE { range 10.3.0.10 10.3.0.14; range 10.3.0.16 10.3.0.33; range 10.3.0.35 10.3.0.50; option subnet-mask 255.255.255.0; <snip> By modifying the range entries to exclude the fixed leases, we can eliminate the possibility that the server will provide the fixed address to a different host.
I'm working a solution according to my remark in https://bugzilla.ipfire.org/show_bug.cgi?id=10598
As I have posted on the forum, I would like to chime in here and express my opinion that modifying the WUI to prevent users from defining fixed addresses that overlap the dynamic pool is NOT the proper way to deal with this problem. I understand that it is preferable to keep the fixed and dynamic spaces separate (this is how I set up my networks. However, sometimes that is not always easily achieved, and there are any number of situations where choosing not to do that might make sense. For that reason, among others, I very strongly feel that using multiple range statements is a far better solution.
Because of the IP allocation process in DHCP server it is necessary, that the two sets are disjunct. Therefore in first place we need a check in the WUI to assure this condition. The discussion about the contents of these two sets is another topic. The check function is ready now. I'm working on the incorporation into the DHCP page at the moment.
This was in the wrong category. Besides: Is it still valid?
Don't know. Didn't work on this since. I also got no more informations. There two questions remaining: - Is the violation of the assumption checked? - Is it worth the effort to construct the range parts suggested in the desciption or is it possible to avoid that sectioning?
This is still valid. As I see it, there are three options: 1.) Rewrite the dhcp.conf file to break the range up into multiple segments, excluding the fixed leases that overlap. 2.) Prohibit the creation of fixed leases that overlap with the dynamic range. 3.) Leave it as-is and tell users that they should have known better. #1 is the most complicated, and #2 is easier, but will need to take into account when someone migrates a dynamic lease to DHCP and re-assign that device a new address somehow. Presumably other open source implementations have already dealt with this problem, and perhaps we can implement their solution? The problem with #3 is that Windows' DHCP server doesn't do this, so many are not expecting this to be a problem. Secondly, the WUI, when you click the "Add" button, will create an overlapping lease by default, as it simply re-uses the existing IP Address, so the WUI is doing it wrong, too.
Without reading the cited discussions once more ( some time gone since 2014 ;) ), I think we should do it as follows: - sets of dynamic IPs and static IPs MUST be disjunct - condition SHOULD be checked in the WUI - adding a dynamic lease MAC to the fixed leases set SHOULD request an editing of the definition ( behaviour similiar to 'add new' operation in current Ipfire version ( 130 ) ) During the remainder of the lease time of the dynamic there is double definition, with warnings in the logs. We should investigate to delete this unwanted ( and somehow 'illegal' ) dynamic lease definition.
One more thing to consider is that adding a fixed lease from an existing dynamic lease does not force the user to choose an address outside of the dynamic pool. I see two possible solutions: 1.) Always force segregation of dynamic and fixed leases. 2.) Write the file properly to exclude all of the fixed leases from the pool, even if the numbers overlap. I think #2 would be easiest. It still allows any user to implement #1 if they really want to, but will require less work to check everywhere to ensure that no entries overlap. My $0.02, after inflation.
I am willing to pick up this bug and have a go at it if I am not treading on any toes by doing that.
No problem from my side. I started some work, but didn't really produce a solution. The dhcp.cgi file is bit messy, thus I think it is necessary to clean up the code. But looking at the problem, which is solved, that was too much work in generating separate patches for clean up/cosmetics. With a bit admin discipline, there is no problem. ;) You can contact me for information on my former steps or for testing your changes. - Bernhard
Adolf: I think you will find nothing but thanks for giving it a go.
I have read through the links and comments in this bug and carried out some searching on the topic. Splitting the dynamic pool is usually carried out to have a separate contiguous range of IP's for different groups of clients with different options defined. So for instance one pool could be for clients on a subnet that are given leases with a long lease time and the other pool gives leases in the same subnet but with shorter lease times. What is being described here is splitting the range up into multiple pools because an existing IP is being moved from dynamic to fixed, not to structurally have different pools for clients groups with different settings. The WUI would need to be modified to show these multiple ranges otherwise you would end up with multiple ranges in the conf file but not visible in the wui. In fact you would need to keep the ranges separated for all IP's listed in the fixed table, even if they are not enabled. The other option is to check if an IP is being specified for a fixed address that is from the range and prevent its use with an error message. This then forces the selection of an IP from outside the defined range. This is how pfSense and OPNsense do it. They prevent the selection of an IP from the dynamic range. I think it makes sense for us to follow the same approach. Having the ability to have an additional dynamic pool where different dhcp-options can be set could be an extension for the future. pfSense has this Additional Pools option. Keeping a dynamic pool with one set of dhcp-options contiguous is also talked about in the isc form as "best practice" rather than splitting the pool up. So based on what I have read about my proposal is to keep a defined contiguous dynamic range intact, check for fixed IP's being specified from this dynamic range and giving an error. For the situation of moving a client from a dynamic address to a fixed address then this would require the IP address to be modified to an IP outside from the dynamic range. The only downside of this option is that the client will maintain its previous IP until the client network is restarted or the 50% leasetime recorded in the client is reached. This will apply in all situations where an IP address is changed for a client in the server. If moving to the new IP is important then the network admin needs to ensure that the client network is restarted.
I have made some changes to dhcp.cgi, created a sub-routine in general-functions.pl to detect if an ip is in an ip-range and added an error message into en.pl. I have tested the changes and they gave me an error message if the Fixed IP Address selected was in the dynamic range. I have tried this with fixed addresses in both Green and Blue and also for the Next Server IP Address. Also tested for adding an existing dynamic lease to the fixed lease list. I would appreciate it if someone could test this and confirm that it works as expected or highlight issues that I have missed. The patches are based on the next git repository.
Created attachment 851 [details] patch file for evaluation of dhcp.cgi changes
Created attachment 852 [details] patch file for evaluation of general-functions.pl changes
Created attachment 853 [details] patch file for evaluation of en.pl language file changes
No feedback so I will create a patch for the change and send it to the development list.
I was thinking about the patch for this bug while I was walking to the supermarket today and I suddenly realised that I need to do some more work on it. The patch works to prevent any fixed ip being specified that overlaps with the dynamic range. However if the dynamic range is modified to overlap already existing fixed ip's there is nothing to stop that. Just confirmed that. So need to extend the patch to also check any change to the dynamic range does not overlap the existing fixed ip's (enabled or not).
Modified patch created and tested and works both for creating fixed lease IP's in the existing dynamic range and for modifying the dynamic range to overlap with existing fixed lease definitions enabled or disabled. Patch submitted to development list for review and additional testing. https://patchwork.ipfire.org/patch/3894/
Since the patch proposed by Adolf did not made it into the ipfire-2.x repository (yet), I am resetting this bug back to ASSIGNED.
Created an updated patch set. The first patch updates the bgcolor entries to css background-color approach as used by @Leo in the Zone Configuration cgi page. bgcolor was deprecated in HTML4.01 and is no longer used in HTML5 The orange colouring for cells with IP's that are outside of the IPFire green and blue subnets is not showing up on my browsers. The conversion to using css for bgcolor works for IUP's outside of the subnets. The second patch then uses the new css approach to highlight in red any IP's in the Fixed Leases table that overlap with the dynamic assigned IP range. The IP's are still usable but being highlighted in red makes it clear that it is an incorrect assignment. https://lists.ipfire.org/pipermail/development/2023-February/015397.html https://lists.ipfire.org/pipermail/development/2023-February/015398.html https://patchwork.ipfire.org/project/ipfire/list/?series=3375
https://git.ipfire.org/?p=ipfire-2.x.git;a=commit;h=de4dea96aa9a45785e7bb64aadf14f97cfc1a359 https://git.ipfire.org/?p=ipfire-2.x.git;a=commit;h=b52a84ddc77f9db7e4ad7b129f21fbf528c6f990
https://blog.ipfire.org/post/ipfire-2-27-core-update-174-is-available-for-testing
An update done for the bgcolor fix had an error in it. The css statements were moved to header.pl as they will be used in many of the cgi files so it made sense to move to thye central header.pl file. However forgot to add the variable definition in header.pl to reference colors.txt which has all the color= statements such as for the shading differentiation for each row in a table. So this bug has the red shading on any fixed leaswe IP's that are in the defined dynamic range and also the orange shading works again but the row shading for the table does not work due to the above error. Patch submitted to dev mailing list to fix this error. https://lists.ipfire.org/pipermail/development/2023-March/015662.html https://patchwork.ipfire.org/project/ipfire/patch/20230316103403.3761-1-adolf.belka@ipfire.org/
Thank you, just pushed this to "next". I will ping Michael to have the patch merged into "master" in due course. https://git.ipfire.org/?p=ipfire-2.x.git;a=commit;h=a84b9ed2feb926681ad94273d8c2efc5d7b71b4f
Patch is in the latest nightly master build. Have tested Core Update 174 build b7c95899 and can confirm that it now works correctly.
https://blog.ipfire.org/post/ipfire-2-27-core-update-174-released