Project

General

Profile

Actions

Bug #12007

open

Dynamic DNS cache expiration time check calculation method may cause update to happen on the wrong day

Added by Jaakko Kantojärvi 5 months ago. Updated 2 months ago.

Status:
Feedback
Priority:
Normal
Category:
Dynamic DNS
Target version:
Start date:
06/07/2021
Due date:
% Done:

0%

Estimated time:
Plus Target Version:
21.09
Release Notes:
Default
Affected Version:
Affected Architecture:

Description

Dynamic DNS update is executed if a) no update has been done for the provider yet, b) the IP address has changed after the last update or c) the cache expires. First two (a-b) are very straight forward, but the last (c) is a bit problematic, at least, when expect to work "correctly".

Currently, the dynamic DNS update code is executed once per day by the cron (or when provider is edited). In addition, the program code evaluates in this order:

  1. Check if cache is expired using the contents of the cache file and current time (method "_detectChange()")
  2. Make a web request using curl to do the update (method "_update()")
  3. Check the update status (method "_checkStatus()")
  4. Write the wan ip and current time (get new time) to the cache file (the same method).

Side note, it seems that the wan ip is resolved multiple times during this process (i.e., method "_checkIP()" is called more than once).

It's important to notice that the time between step 1 and step 4 can be easily multiple seconds, as the process includes at least one web requests, which as suspect to failures and delays. Hence, if the update is started at 01:01:00 each day, it could easily end at 01:01:10. In the first step, we will execute the following code to evaluate if the cache is expired and the update should be done.

$maxCacheAgeSecs = $this->_dnsMaxCacheAgeDays * 24 * 60 * 60;
expired = ($currentTime - $cacheTime) > $maxCacheAgeSecs;

It reads, "if current time is more than cache time plus max cache age, then update the provider". If this happens at 01:01:00 a day later, then the cache has been active for 23 hours 59 minutes and 50 seconds. If the "_dnsMaxCacheAgeDays" is 1, then the cache would be consider to be still valid (i.e., exactly one day has not yet passed). As a result, the update would happen on the second day.

As of writing, the "_dnsMaxCacheAgeDays" is hard coded to 25 days. Additionally, I expect that none of the current providers care if the actual update happens 25 or 26 days apart. However, I'm working on adding provider dy.fi, which expects the update to occur from 5 to 7 days. Also, I'm working on making the "_dnsMaxCacheAgeDays" configurable for the custom provider.

For example, in the case of the dy.fi, the DNS record is dropped after 7 days from the last update. Hence, if the latter update is 7 days and 5 minutes from the previous, then the DNS name doesn't work for 5 minutes. However, if the update happens more often than every 5 days, then the account might get banned due to abuse. As a result, the provider requires quite exact update schedule, when calculated in days. In conclusion, the most suitable update schedule for dy.fi would be every 6 calendar days exactly.

I propose that we edit the cache expiration code to take in account a bit of leeway. I have a PR with one hour of leeway, but I think it could be up to 12 hours. The former should be sufficient to count all the slowness and should not break anything existing. The latter would be more in line of typical algorithm for rounding using integers. If the calculation would be done in days, we also would need to replace "greater than" with "greater or equal than". Here is a few different versions of how this check could be implemented.

// basic rounding with integers, i.e., round to full days
expired = int(currentTimeDays - cacheTimeDays + 0.5) >= maxCacheAgeDays ;
// multiply everything with 24 and drop the int() as the code is executed once per day
expired = currentTimeHours - cacheTimeHours + 12 >= maxCacheAgeHours ;
// move the 12 hours to right side, so it matches the PR
expired = currentTimeHours - cacheTimeHours >= maxCacheAgeHours - 12 ;

My PR uses the last version.

Fix https://github.com/pfsense/pfsense/pull/4524

Actions #1

Updated by Renato Botelho 4 months ago

  • Status changed from New to Feedback
  • Assignee set to Renato Botelho
  • Target version set to 2.6.0
  • Plus Target Version set to 21.09

PR has been merged. Thanks!

Actions #2

Updated by Jim Pingle 2 months ago

  • Subject changed from Dynamic DNS update can happen on a wrong day due to cache expiration time check calculation to Dynamic DNS cache expiration time check calculation method may cause update to happen on the wrong day

Updating subject for release notes.

Actions

Also available in: Atom PDF