From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Tremer To: development@lists.ipfire.org Subject: Re: DDNS: add new Provider key-sytems.net (domaindiscount24) Date: Thu, 28 Nov 2019 15:17:37 +0000 Message-ID: In-Reply-To: <97e435c3-36ba-0468-34a4-68081f4c4520@gmx.at> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============4080518464014894109==" List-Id: --===============4080518464014894109== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hi, > On 28 Nov 2019, at 14:58, Christof Weniger wrote: >=20 > Hi, >=20 > OK I removed the mx entry (I was not aware, that there can be a FQDN instea= d of an IP) >=20 > I did some more tests and it seems the server always responds with HTTP 200 That is horrible :) > In error cases (wrong Password, wrong url) I got the following responses: > =E2=80=A2 wrong url: > Error: Permanent error in command 'UpdateDynamicDNS': Authorization failed >=20 > =E2=80=A2 wrong password: > Error: Permanent error in command 'UpdateDynamicDNS': Authorization failed;= invalid password >=20 > =E2=80=A2 after too many tries: > Error: Permanent error in command 'UpdateDynamicDNS': Authorization failed;= abuse prevention triggered; please wait 1763 seconds to resubmit >=20 >=20 >=20 > the new patch: > diff --git a/README b/README > index c75c448..b6decb3 100644 > --- a/README > +++ b/README > @@ -72,6 +72,7 @@ SUPPORTED PROVIDERS: > inwx.com|de|at|ch|es > itsdns.de > joker.com > + key-systems.net > loopia.se > myonlineportal.net > namecheap.com > diff --git a/src/ddns/providers.py b/src/ddns/providers.py > index 661fbcc..f70f49a 100644 > --- a/src/ddns/providers.py > +++ b/src/ddns/providers.py > @@ -1204,6 +1204,52 @@ class DDNSProviderJoker(DDNSProtocolDynDNS2, DDNSPro= vider): > url =3D=20 > "https://svc.joker.com/nic/update" >=20 >=20 >=20 > +class DDNSProviderKEYSYSTEMS(DDNSProvider): > + handle =3D "key-systems.net" > + name =3D "dynamicdns.key-systems.net" > + website =3D=20 > "https://domaindiscount24.com/" >=20 > + protocols =3D ("ipv4",) > + > + # There are only information provided by the domaindiscount24 how to > + # perform an update with HTTP APIs > + #=20 > https://www.domaindiscount24.com/faq/dynamic-dns >=20 > + # examples:=20 > https://dynamicdns.key-systems.net/update.php?hostname=3Dhostname&password= =3Dpassword&ip=3Dauto >=20 > + # =20 > https://dynamicdns.key-systems.net/update.php?hostname=3Dhostname&password= =3Dpassword&ip=3D213.x.x.x&mx=3D213.x.x.x >=20 > + > + url =3D=20 > "https://dynamicdns.key-systems.net/update.php" >=20 > + can_remove_records =3D False > + > + def update_protocol(self, proto): > + address =3D self.get_address(proto) > + data =3D { > + "hostname" : self.hostname, > + "password" : self.password, > + "ip" : address, > + } > + > + # Send update to the server. > + response =3D self.send_request(self.url, data=3Ddata) > + > + # Get the full response message. > + output =3D response.read() > + > + # Handle success messages. > + if "code =3D 200" in output: > + return > + > + # Handle error messages. > + if "abuse prevention triggered" in output: > + raise DDNSRequestError(_("Too many failed requests")) We have DDNSAbuseError which should be raised here. There is no need for any = extra messages here. > + if "invalid password" in output: > + raise DDNSAuthenticationError This could be an elif as well. > + elif "Authorization failed" in output: > + raise DDNSRequestError(_("Invalid hostname specified")) Also it is very great that they are telling an attacker what they have guesse= d correctly. > + # If we got here, some other update error happened. > + raise DDNSUpdateError So this was an inline patch now. Good. It is very nice to be able to comment = on individual sections and lines. However, there should be more meta information (like who sent the patch and a= commit message). Did you ever use git send-email? https://wiki.ipfire.org/devel/submit-patches https://wiki.ipfire.org/devel/git/setup Best, -Michael > + > + > + > class DDNSProviderGoogle(DDNSProtocolDynDNS2, DDNSProvider): > handle =3D "domains.google.com" > name =3D "Google Domains" >=20 > best > Christof >=20 > On 28/11/2019 14:35, Michael Tremer wrote: >> Hi, >>=20 >> Thanks for testing this. Looks like I was very wrong here. >>=20 >>=20 >>> On 28 Nov 2019, at 13:03, Christof Weniger >>> wrote: >>>=20 >>> Hello Michael, >>>=20 >>> after change to DynDNS class I got the following "DDNSUpdateError" >>> Nov 28 13:42:17 minusrouter ddns[23192]: Dynamic DNS update for ovpn.XXXX= X.YYY (dynamicdns.key-systems.net) failed: >>> Nov 28 13:42:17 minusrouter ddns[23192]: DDNSUpdateError: The update co= uld not be performed >>> Nov 28 13:42:17 minusrouter ddns[23192]: Server response: [RESPONSE] co= de =3D 200 description =3D Command completed successfully queuetime =3D 0 run= time =3D 0.058 EOF >>>=20 >>> The DynDNS2 code (generating the ERROR): >>> class DDNSProviderKEYSYSTEMS(DDNSProtocolDynDNS2, DDNSProvider): >>> handle =3D "dynamicdns.key-systems.net" >>> name =3D "dynamicdns.key-systems.net" >>> website =3D >>>=20 >>> "https://domaindiscount24.com/" >>>=20 >>>=20 >>> #protocols =3D ("ipv4",) >>>=20 >>> # There are only information provided by the domaindiscount24 how= to >>> # perform an update with HTTP APIs >>> # >>>=20 >>> https://www.domaindiscount24.com/faq/dynamic-dns >>>=20 >>>=20 >>> # examples: >>>=20 >>> https://dynamicdns.key-systems.net/update.php?hostname=3Dhostname&passwor= d=3Dpassword&ip=3Dauto >>>=20 >>>=20 >>> # >>>=20 >>> https://dynamicdns.key-systems.net/update.php?hostname=3Dhostname&passwor= d=3Dpassword&ip=3D213.x.x.x&mx=3D213.x.x.x >>>=20 >>>=20 >>>=20 >>> url =3D >>>=20 >>> "https://dynamicdns.key-systems.net/update.php" >>>=20 >>>=20 >>>=20 >>> def prepare_request_data(self, proto): >>> address =3D self.get_address(proto) >>> data =3D { >>> "hostname" : self.hostname, >>> "password" : self.password, >>> "ip" : address, >>> "mx" : address, >>>=20 >> I wouldn=E2=80=99t set the MX record here. First of all it should not cont= ain an IP address and so it can be statically configured. I consider this bei= ng outside of the scope of ddns. >>=20 >>=20 >>> } >>>=20 >>> return data >>>=20 >>>=20 >>> The server response to a simple wget request was: >>> [RESPONSE] >>> code =3D 200 >>> description =3D Command completed successfully >>> queuetime =3D 0 >>> runtime =3D 0.053 >>> EOF >>>=20 >>>=20 >>> The working code: >>>=20 >>> class DDNSProviderKEYSYSTEMS(DDNSProvider): >>> handle =3D "dynamicdns.key-systems.net" >>> name =3D "dynamicdns.key-systems.net" >>> website =3D >>>=20 >>> "https://domaindiscount24.com/" >>>=20 >>>=20 >>> protocols =3D ("ipv4",) >>>=20 >>> # There are only information provided by the domaindiscount24 how= to >>> # perform an update with HTTP APIs >>> # >>>=20 >>> https://www.domaindiscount24.com/faq/dynamic-dns >>>=20 >>>=20 >>> # examples: >>>=20 >>> https://dynamicdns.key-systems.net/update.php?hostname=3Dhostname&passwor= d=3Dpassword&ip=3Dauto >>>=20 >>>=20 >>> # >>>=20 >>> https://dynamicdns.key-systems.net/update.php?hostname=3Dhostname&passwor= d=3Dpassword&ip=3D213.x.x.x&mx=3D213.x.x.x >>>=20 >>>=20 >>>=20 >>> url =3D >>>=20 >>> "https://dynamicdns.key-systems.net/update.php" >>>=20 >>>=20 >>> can_remove_records =3D False >>>=20 >>> #def prepare_request_data(self, proto): >>> def update_protocol(self, proto): >>> address =3D self.get_address(proto) >>> data =3D { >>> "hostname" : self.hostname, >>> "password" : self.password, >>> "ip" : address, >>> "mx" : address, >>>=20 >> See above. >>=20 >>=20 >>> } >>>=20 >>> # Send update to the server. >>> response =3D self.send_request(self.url, data=3Ddata) >>>=20 >>> # Handle success messages. >>> if response.code =3D=3D 200: >>> return >>>=20 >> This is just very vage. >>=20 >>=20 >>> # If we got here, some other update error happened. >>> raise DDNSUpdateError >>>=20 >> There must be other responses - one might hope. >>=20 >> Did you check sending an incorrect password and do you get 401 or 403 at l= east? >>=20 >>=20 >>> #return data >>>=20 >>>=20 >>>=20 >>>> Unfortunately this provider has no list of any possible responses - or m= aybe I have overlooked it. That can however be tested to check if those are s= till compatible with DynDNS. >>>>=20 >>> I couldn't find any more pointers on the providers page either, and for t= hat 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). >>>=20 >> Yes, so great to not have any documentation=E2=80=A6 >>=20 >> -Michael >>=20 >>=20 >>>=20 >>> best >>> Christof >>>=20 >>> On 28/11/2019 12:20, Michael Tremer wrote: >>>=20 >>>> Hello Christof, >>>>=20 >>>>=20 >>>>=20 >>>>> On 28 Nov 2019, at 10:45, Christof Weniger >>>>>=20 >>>>> wrote: >>>>>=20 >>>>> Hi, >>>>>=20 >>>>> I hope this is the correct way to submit this patch. >>>>>=20 >>>>>=20 >>>> Yes, you found the right place. >>>>=20 >>>> It would have been better to post the patch inline (and not as an attach= ment), so that I could have commented on it. >>>>=20 >>>>=20 >>>>=20 >>>>> https://www.domaindiscount24.com/ >>>>>=20 >>>>> has its own ddns service runnning, >>>>> which (for me) gets rid of the necessity of having to use an extra >>>>> service for that. >>>>>=20 >>>>> I tested the following patch on my system at home, and attached it to >>>>> this mail. >>>>>=20 >>>>> I started my quest at the community forum: >>>>>=20 >>>>>=20 >>>>> https://community.ipfire.org/t/adding-new-ddns-provider/428/2 >>>>>=20 >>>>>=20 >>>>>=20 >>>>> Christof >>>>>=20 >>>>>=20 >>>>>=20 >>>>>=20 >>>>>=20 >>>>> >>>>>=20 >>>>>=20 >>>> I will make an exception here now and still give you my thoughts :) >>>>=20 >>>> It looks like this is very close to the DynDNS protocol, so you can re-u= se that as some other providers do. >>>>=20 >>>> Unfortunately this provider has no list of any possible responses - or m= aybe I have overlooked it. That can however be tested to check if those are s= till compatible with DynDNS. >>>>=20 >>>> Finally, you are setting the IP addresses to =E2=80=9Cauto=E2=80=9D whic= h 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 addre= ss. >>>>=20 >>>> In the end I think this provider could look like =E2=80=9Cmyonlineportal= =E2=80=9D: >>>>=20 >>>> https://git.ipfire.org/?p=3Dddns.git;a=3Dblob;f=3Dsrc/ddns/providers.py;= h=3D661fbcc57a5aecba0d958ec05c26296b3cef0d70;hb=3DHEAD#l1274 >>>>=20 >>>>=20 >>>>=20 >>>> Best, >>>> -Michael >>>>=20 >>>>=20 >=20 > --===============4080518464014894109==--