Re: DDNS: add new Provider key-sytems.net (domaindiscount24)

[Michael Tremer <michael.tremer@ipfire.org>]

[-- Attachment #1: Type: text/plain, Size: 6398 bytes --]

Hi,

Thanks for testing this. Looks like I was very wrong here.

> On 28 Nov 2019, at 13:03, Christof Weniger <ChristofWeniger(a)gmx.at> wrote:
> 
> Hello Michael,
> 
> after change to DynDNS class I got the following "DDNSUpdateError"
> Nov 28 13:42:17 minusrouter ddns[23192]: Dynamic DNS update for ovpn.XXXXX.YYY (dynamicdns.key-systems.net) failed:
> Nov 28 13:42:17 minusrouter ddns[23192]:   DDNSUpdateError: The update could not be performed
> Nov 28 13:42:17 minusrouter ddns[23192]:   Server response: [RESPONSE] code = 200 description = Command completed successfully queuetime = 0 runtime = 0.058 EOF 
> 
> The DynDNS2 code (generating the ERROR):
> class DDNSProviderKEYSYSTEMS(DDNSProtocolDynDNS2, DDNSProvider):
>         handle    = "dynamicdns.key-systems.net"
>         name      = "dynamicdns.key-systems.net"
>         website   = 
> "https://domaindiscount24.com/"
> 
>         #protocols = ("ipv4",)
> 
>         # There are only information provided by the domaindiscount24 how to
>         # perform an update with HTTP APIs
>         # 
> https://www.domaindiscount24.com/faq/dynamic-dns
> 
>         # examples: 
> https://dynamicdns.key-systems.net/update.php?hostname=hostname&password=password&ip=auto
> 
>         #           
> https://dynamicdns.key-systems.net/update.php?hostname=hostname&password=password&ip=213.x.x.x&mx=213.x.x.x
> 
> 
>         url = 
> "https://dynamicdns.key-systems.net/update.php"
> 
> 
>         def prepare_request_data(self, proto):
>                 address = self.get_address(proto)
>                 data = {
>                         "hostname"      : self.hostname,
>                         "password"      : self.password,
>                         "ip"            : address,
>                         "mx"            : address,

I wouldn’t set the MX record here. First of all it should not contain an IP address and so it can be statically configured. I consider this being outside of the scope of ddns.

>                 }
> 
>                 return data
> 
> 
> The server response to a simple wget request was:
> [RESPONSE]
> code = 200
> description = Command completed successfully
> queuetime = 0
> runtime = 0.053
> EOF
> 
> 
> The working code:
> 
> class DDNSProviderKEYSYSTEMS(DDNSProvider):
>         handle    = "dynamicdns.key-systems.net"
>         name      = "dynamicdns.key-systems.net"
>         website   = 
> "https://domaindiscount24.com/"
> 
>         protocols = ("ipv4",)
> 
>         # There are only information provided by the domaindiscount24 how to
>         # perform an update with HTTP APIs
>         # 
> https://www.domaindiscount24.com/faq/dynamic-dns
> 
>         # examples: 
> https://dynamicdns.key-systems.net/update.php?hostname=hostname&password=password&ip=auto
> 
>         #           
> https://dynamicdns.key-systems.net/update.php?hostname=hostname&password=password&ip=213.x.x.x&mx=213.x.x.x
> 
> 
>         url = 
> "https://dynamicdns.key-systems.net/update.php"
> 
>         can_remove_records = False
> 
>         #def prepare_request_data(self, proto):
>         def update_protocol(self, proto):
>                 address = self.get_address(proto)
>                 data = {
>                         "hostname"      : self.hostname,
>                         "password"      : self.password,
>                         "ip"            : address,
>                         "mx"            : address,

See above.

>                 }
> 
>                 # Send update to the server.
>                 response = self.send_request(self.url, data=data)
> 
>                 # Handle success messages.
>                 if response.code == 200:
>                         return

This is just very vage.

> 
>                 # If we got here, some other update error happened.
>                 raise DDNSUpdateError

There must be other responses - one might hope.

Did you check sending an incorrect password and do you get 401 or 403 at least?

> 
>                 #return data
> 
> 
>> Unfortunately this provider has no list of any possible responses - or maybe I have overlooked it. That can however be tested to check if those are still compatible with DynDNS.
> I couldn't find any more pointers on the providers page either, and for that testing I would have to dig in too deep (into the code and the protocols) for me to handle at the moment. (maybe next week/month).

Yes, so great to not have any documentation… 

-Michael

> 
> 
> best
> Christof
> 
> On 28/11/2019 12:20, Michael Tremer wrote:
>> Hello Christof,
>> 
>> 
>>> On 28 Nov 2019, at 10:45, Christof Weniger <ChristofWeniger(a)gmx.at>
>>>  wrote:
>>> 
>>> Hi,
>>> 
>>> I hope this is the correct way to submit this patch.
>>> 
>> Yes, you found the right place.
>> 
>> It would have been better to post the patch inline (and not as an attachment), so that I could have commented on it.
>> 
>> 
>>> https://www.domaindiscount24.com/
>>>  has its own ddns service runnning,
>>> which (for me) gets rid of the necessity of having to use an extra
>>> service for that.
>>> 
>>> I tested the following patch on my system at home, and attached it to
>>> this mail.
>>> 
>>> I started my quest at the community forum:
>>> 
>>> https://community.ipfire.org/t/adding-new-ddns-provider/428/2
>>> 
>>> 
>>> Christof
>>> 
>>> 
>>> 
>>> 
>>> 
>>> <add_key_systems_ddns.patch>
>>> 
>> I will make an exception here now and still give you my thoughts :)
>> 
>> It looks like this is very close to the DynDNS protocol, so you can re-use that as some other providers do.
>> 
>> Unfortunately this provider has no list of any possible responses - or maybe I have overlooked it. That can however be tested to check if those are still compatible with DynDNS.
>> 
>> Finally, you are setting the IP addresses to “auto” which probably is not a good idea. I would prefer that it is explicitly being set. There is a function for it that finds out which one is the correct IP address.
>> 
>> In the end I think this provider could look like “myonlineportal”: 
>> https://git.ipfire.org/?p=ddns.git;a=blob;f=src/ddns/providers.py;h=661fbcc57a5aecba0d958ec05c26296b3cef0d70;hb=HEAD#l1274
>> 
>> 
>> Best,
>> -Michael
>> 
>