[PATCH] DYNDNS: Add a new option dyndns_server

[Jakub Hrozek]

Hi,

one of our users ran into an interesting problem -- her AD
infrastructure was different from the DNS server. Because by default, we
perform update against the server we're connected to, the DNS update
didn't work.

Per Simo's suggestion, I've implemented a new option that allows the
administrator to override the DNS server used for DNS updates.

[Simo Sorce]

On Wed, 2014-07-09 at 23:06 +0200, Jakub Hrozek wrote:
> Hi,
>
> one of our users ran into an interesting problem -- her AD
> infrastructure was different from the DNS server. Because by default, we
> perform update against the server we're connected to, the DNS update
> didn't work.
>
> Per Simo's suggestion, I've implemented a new option that allows the
> administrator to override the DNS server used for DNS updates.

I am not sure I like the premises here.

The problem is that we are trying to set a DNS server at all in the
normal case.
The normal case should be that nsupdate is not passed any server name
and it discovers the correct server via normal DNS lookup queries.

If someone is using a messed up DNS setup then, yeah I would offer a
setting for specifying exactly what DNS server to contact, but I am not
sure it makes sense to fallback to a random Domain Controller by
default.

IIRC the cldap ping reply does return you information on whether the
domain controller is also a DNS server, I think we need to extract that
info and keep it around, and fallback to directly contact the domain
controller only if the cldap ping indicated DNS support.

Comments on code follow:

I think I would change the following:
+                        <para>
+                            Setting this option makes sense for environments
+                            where the DNS server is different from the identity
+                            server.
+                        </para>

I think you should say something like this:
"Set this option if your DNS environment is broken and the correct
server cannot be discovered via regular DNS lookups."

On the code side I think you are missing to remove the code in
ad_dyndns.c:ad_dyndns_update_send() that unconditionally sets
stat->servername, by default no server name should be set.

Exactly the same issue is present in
ipa_dyndns.c:ipa_dyndns_update_send(), once again the current freeipa
server name is taken as a DNS server. And once again not all IPA servers
are DNS servers. Here too we need to remove all the code that sets the
server name and let nsupdate find the server through the normal dns
lookup queries or use the dyndns_server fallback if set.

Using unconditioanlly a domain controller you are randomly bound to is
normally more wrong than right and we should stop doing that completely.

So NACK, for now.

Simo.

[Pavel Reichl]

On 07/10/2014 10:30 AM, Simo Sorce wrote:
> On Wed, 2014-07-09 at 23:06 +0200, Jakub Hrozek wrote:
> > Hi,
> >
> > one of our users ran into an interesting problem -- her AD
> > infrastructure was different from the DNS server. Because by default, we
> > perform update against the server we're connected to, the DNS update
> > didn't work.
> >
> > Per Simo's suggestion, I've implemented a new option that allows the
> > administrator to override the DNS server used for DNS updates.
>
> I am not sure I like the premises here.
>
> The problem is that we are trying to set a DNS server at all in the
> normal case.
> The normal case should be that nsupdate is not passed any server name
> and it discovers the correct server via normal DNS lookup queries.
>
> If someone is using a messed up DNS setup then, yeah I would offer a
> setting for specifying exactly what DNS server to contact, but I am not
> sure it makes sense to fallback to a random Domain Controller by
> default.
>
> IIRC the cldap ping reply does return you information on whether the
> domain controller is also a DNS server, I think we need to extract that
> info and keep it around, and fallback to directly contact the domain
> controller only if the cldap ping indicated DNS support.
>
> Comments on code follow:
>
> I think I would change the following:
>                    <para>
>                        Setting this option makes sense for environments
>                        where the DNS server is different from the identity
>                        server.
>                    </para>
>
> I think you should say something like this:
> "Set this option if your DNS environment is broken and the correct
> server cannot be discovered via regular DNS lookups."
>
> On the code side I think you are missing to remove the code in
> ad_dyndns.c:ad_dyndns_update_send() that unconditionally sets
> stat->servername, by default no server name should be set.
>
> Exactly the same issue is present in
> ipa_dyndns.c:ipa_dyndns_update_send(), once again the current freeipa
> server name is taken as a DNS server. And once again not all IPA servers
> are DNS servers. Here too we need to remove all the code that sets the
> server name and let nsupdate find the server through the normal dns
> lookup queries or use the dyndns_server fallback if set.
>
> Using unconditioanlly a domain controller you are randomly bound to is
> normally more wrong than right and we should stop doing that completely.
>
> So NACK, for now.
>
> Simo.

Hello,

I've discussed this ticket and Simo's comments off list with Jakub and 
Petr Spacek. Please see linked design page which is result of the 
conversation. Please comment here or directly edit the page.

Thanks!

https://fedorahosted.org/sssd/wiki/DesignDocs/DDNSMessagesUpdate

/////////////////////////////////////////////////////////
/////////////////////////////////////////////////////////
/////////////////////////////////////////////////////////
= DDNS - update quality of input for nsupdate =

Related ticket(s):
  * https://fedorahosted.org/sssd/ticket/2495

=== Problem statement ===
SSSD provides input to nsupdate that's redundant and might actually 
impair proper resolution using DNS. Further, format of messages pasted 
to nsupdate is grouped by command type - deleting and adding addresses 
are two separate batches which is not optimal as update of one of 
address families might be prohibited to be changed on server.

=== Use cases ===
If DNS system is broken using dyndns_server option might be an workaround

=== Overview of the solution ===
* Allow SSSD to add a new option that would specify which server to 
update DNS with ('dyndns_server')
* In first attempt to perform DDNS '''do not specify 
zone/server/realm''' commands in input for nsupdate (nsupdate will then 
utilize DNS)
* As fallback (if first attempt fails) specify realm command and if 
'dyndns_server' option is specified then also add server command with 
value of 'dyndns_server'
* Bulk deleting and adding addresses from the same address family into 
one batch rather then grouping by command type

=== Implementation details ===
* Remove servername and dns_zone parameters from 
sdap_dyndns_update_send() as they are no longer used. Remove code from 
AD and IPA provider which was passing this data to 
sdap_dyndns_update_send().
* Remove dns_zone field from struct sdap_dyndns_update_state and remove 
all code relating to it.
* In sdap_dyndns_update_done() make setting command realm conditional 
same way as command server is.
* Update nsupdate_msg_add_fwd() to group commands by address family 
processed IP belongs to.

=== Configuration changes ===
New option dyndns_server

=== How To Test ===
Check that IP addresses get changed in IPA and on AD. Break DNS 
resolving to force first attempt of DDNS to fail. Check that messages 
generated as input for nsupdate in domain logs are matching the expectation.

==== Example of expected format of messages ====
'''First attempt'''
{{{
update delete husker.human.bs. in A
update add husker.human.bs. 1200 in A 192.168.122.180
send
update delete husker.human.bs. in AAAA
update add husker.human.bs. 1200 in AAAA 2001:cdba::666
send
}}}
'''Fallback attempt'''
{{{
;sever is present only if option dyndns_server is set in sssd.conf
server 192.168.122.20
;realm is used always in fallback message
realm IPA.WORK
update delete husker.human.bs. in A
update add husker.human.bs. 1200 in A 192.168.122.180
send
update delete husker.human.bs. in AAAA
update add husker.human.bs. 1200 in AAAA 2001:cdba::666
send
}}}
=== Authors ===
[e-mail]

[Petr Spacek]

On 22.7.2015 14:09, Pavel Reichl wrote:
> On 07/10/2014 10:30 AM, Simo Sorce wrote:
> > On Wed, 2014-07-09 at 23:06 +0200, Jakub Hrozek wrote:
> > > Hi,
> > >
> > > one of our users ran into an interesting problem -- her AD
> > > infrastructure was different from the DNS server. Because by default, we
> > > perform update against the server we're connected to, the DNS update
> > > didn't work.
> > >
> > > Per Simo's suggestion, I've implemented a new option that allows the
> > > administrator to override the DNS server used for DNS updates.
> >
> > I am not sure I like the premises here.
> >
> > The problem is that we are trying to set a DNS server at all in the
> > normal case.
> > The normal case should be that nsupdate is not passed any server name
> > and it discovers the correct server via normal DNS lookup queries.
> >
> > If someone is using a messed up DNS setup then, yeah I would offer a
> > setting for specifying exactly what DNS server to contact, but I am not
> > sure it makes sense to fallback to a random Domain Controller by
> > default.
> >
> > IIRC the cldap ping reply does return you information on whether the
> > domain controller is also a DNS server, I think we need to extract that
> > info and keep it around, and fallback to directly contact the domain
> > controller only if the cldap ping indicated DNS support.
> >
> > Comments on code follow:
> >
> > I think I would change the following:
> >                    <para>
> >                        Setting this option makes sense for environments
> >                        where the DNS server is different from the
> >
> > identity
> >                        server.
> >                    </para>
> >
> > I think you should say something like this:
> > "Set this option if your DNS environment is broken and the correct
> > server cannot be discovered via regular DNS lookups."
> >
> > On the code side I think you are missing to remove the code in
> > ad_dyndns.c:ad_dyndns_update_send() that unconditionally sets
> > stat->servername, by default no server name should be set.
> >
> > Exactly the same issue is present in
> > ipa_dyndns.c:ipa_dyndns_update_send(), once again the current freeipa
> > server name is taken as a DNS server. And once again not all IPA servers
> > are DNS servers. Here too we need to remove all the code that sets the
> > server name and let nsupdate find the server through the normal dns
> > lookup queries or use the dyndns_server fallback if set.
> >
> > Using unconditioanlly a domain controller you are randomly bound to is
> > normally more wrong than right and we should stop doing that completely.
> >
> > So NACK, for now.
> >
> > Simo.
>
> Hello,
>
> I've discussed this ticket and Simo's comments off list with Jakub and Petr
> Spacek. Please see linked design page which is result of the conversation.
> Please comment here or directly edit the page.
>
> Thanks!
>
> https://fedorahosted.org/sssd/wiki/DesignDocs/DDNSMessagesUpdate
>
> /////////////////////////////////////////////////////////
> /////////////////////////////////////////////////////////
> /////////////////////////////////////////////////////////
> = DDNS - update quality of input for nsupdate =
>
> Related ticket(s):
> https://fedorahosted.org/sssd/ticket/2495
>
> === Problem statement ===
> SSSD provides input to nsupdate that's redundant and might actually impair
> proper resolution using DNS. Further, format of messages pasted to nsupdate is
> grouped by command type - deleting and adding addresses are two separate
> batches which is not optimal as update of one of address families might be
> prohibited to be changed on server.
>
> === Use cases ===
> If DNS system is broken using dyndns_server option might be an workaround
>
> === Overview of the solution ===
> Allow SSSD to add a new option that would specify which server to update DNS
>
> with ('dyndns_server')
> In first attempt to perform DDNS '''do not specify zone/server/realm'''
>
> commands in input for nsupdate (nsupdate will then utilize DNS)
> As fallback (if first attempt fails) specify realm command and if
>
> 'dyndns_server' option is specified then also add server command with value of
> 'dyndns_server'
> Bulk deleting and adding addresses from the same address family into one
>
> batch rather then grouping by command type
>
> === Implementation details ===
> Remove servername and dns_zone parameters from sdap_dyndns_update_send() as
>
> they are no longer used. Remove code from AD and IPA provider which was
> passing this data to sdap_dyndns_update_send().
> Remove dns_zone field from struct sdap_dyndns_update_state and remove all
>
> code relating to it.
> In sdap_dyndns_update_done() make setting command realm conditional same way
>
> as command server is.
> Update nsupdate_msg_add_fwd() to group commands by address family processed
>
> IP belongs to.
>
> === Configuration changes ===
> New option dyndns_server
>
> === How To Test ===
> Check that IP addresses get changed in IPA and on AD. Break DNS resolving to
> force first attempt of DDNS to fail. Check that messages generated as input
> for nsupdate in domain logs are matching the expectation.
>
> ==== Example of expected format of messages ====
> '''First attempt'''
> {{{
> update delete husker.human.bs. in A
> update add husker.human.bs. 1200 in A 192.168.122.180
> send
> update delete husker.human.bs. in AAAA
> update add husker.human.bs. 1200 in AAAA 2001:cdba::666
> send
> }}}
> '''Fallback attempt'''
> {{{
> ;sever is present only if option dyndns_server is set in sssd.conf
> server 192.168.122.20
> ;realm is used always in fallback message
> realm IPA.WORK
> update delete husker.human.bs. in A
> update add husker.human.bs. 1200 in A 192.168.122.180
> send
> update delete husker.human.bs. in AAAA
> update add husker.human.bs. 1200 in AAAA 2001:cdba::666
> send
> }}}
> === Authors ===
> [e-mail]

LGTM

[Jakub Hrozek]

On Wed, Jul 22, 2015 at 02:09:18PM +0200, Pavel Reichl wrote:
> On 07/10/2014 10:30 AM, Simo Sorce wrote:
> > On Wed, 2014-07-09 at 23:06 +0200, Jakub Hrozek wrote:
> > > Hi,
> > >
> > > one of our users ran into an interesting problem -- her AD
> > > infrastructure was different from the DNS server. Because by default, we
> > > perform update against the server we're connected to, the DNS update
> > > didn't work.
> > >
> > > Per Simo's suggestion, I've implemented a new option that allows the
> > > administrator to override the DNS server used for DNS updates.
> >
> > I am not sure I like the premises here.
> >
> > The problem is that we are trying to set a DNS server at all in the
> > normal case.
> > The normal case should be that nsupdate is not passed any server name
> > and it discovers the correct server via normal DNS lookup queries.
> >
> > If someone is using a messed up DNS setup then, yeah I would offer a
> > setting for specifying exactly what DNS server to contact, but I am not
> > sure it makes sense to fallback to a random Domain Controller by
> > default.
> >
> > IIRC the cldap ping reply does return you information on whether the
> > domain controller is also a DNS server, I think we need to extract that
> > info and keep it around, and fallback to directly contact the domain
> > controller only if the cldap ping indicated DNS support.
> >
> > Comments on code follow:
> >
> > I think I would change the following:
> >                    <para>
> >                        Setting this option makes sense for environments
> >                        where the DNS server is different from the identity
> >                        server.
> >                    </para>
> >
> > I think you should say something like this:
> > "Set this option if your DNS environment is broken and the correct
> > server cannot be discovered via regular DNS lookups."
> >
> > On the code side I think you are missing to remove the code in
> > ad_dyndns.c:ad_dyndns_update_send() that unconditionally sets
> > stat->servername, by default no server name should be set.
> >
> > Exactly the same issue is present in
> > ipa_dyndns.c:ipa_dyndns_update_send(), once again the current freeipa
> > server name is taken as a DNS server. And once again not all IPA servers
> > are DNS servers. Here too we need to remove all the code that sets the
> > server name and let nsupdate find the server through the normal dns
> > lookup queries or use the dyndns_server fallback if set.
> >
> > Using unconditioanlly a domain controller you are randomly bound to is
> > normally more wrong than right and we should stop doing that completely.
> >
> > So NACK, for now.
> >
> > Simo.
>
> Hello,
>
> I've discussed this ticket and Simo's comments off list with Jakub and Petr
> Spacek. Please see linked design page which is result of the conversation.
> Please comment here or directly edit the page.
>
> Thanks!
>
> https://fedorahosted.org/sssd/wiki/DesignDocs/DDNSMessagesUpdate
>
> /////////////////////////////////////////////////////////
> /////////////////////////////////////////////////////////
> /////////////////////////////////////////////////////////
> = DDNS - update quality of input for nsupdate =
>
> Related ticket(s):
> https://fedorahosted.org/sssd/ticket/2495
>
> === Problem statement ===
> SSSD provides input to nsupdate that's redundant and might actually impair
> proper resolution using DNS. Further, format of messages pasted to nsupdate
> is grouped by command type - deleting and adding addresses are two separate
> batches which is not optimal as update of one of address families might be
> prohibited to be changed on server.
>
> === Use cases ===
> If DNS system is broken using dyndns_server option might be an workaround
>
> === Overview of the solution ===
> Allow SSSD to add a new option that would specify which server to update
>
> DNS with ('dyndns_server')
> In first attempt to perform DDNS '''do not specify zone/server/realm'''
>
> commands in input for nsupdate (nsupdate will then utilize DNS)
> As fallback (if first attempt fails) specify realm command and if
>
> 'dyndns_server' option is specified then also add server command with value
> of 'dyndns_server'
> Bulk deleting and adding addresses from the same address family into one
>
> batch rather then grouping by command type
>
> === Implementation details ===
> Remove servername and dns_zone parameters from sdap_dyndns_update_send()
>
> as they are no longer used. Remove code from AD and IPA provider which was
> passing this data to sdap_dyndns_update_send().
> Remove dns_zone field from struct sdap_dyndns_update_state and remove all
>
> code relating to it.
> In sdap_dyndns_update_done() make setting command realm conditional same
>
> way as command server is.
> Update nsupdate_msg_add_fwd() to group commands by address family
>
> processed IP belongs to.
>
> === Configuration changes ===
> New option dyndns_server
>
> === How To Test ===
> Check that IP addresses get changed in IPA and on AD. Break DNS resolving to
> force first attempt of DDNS to fail. Check that messages generated as input
> for nsupdate in domain logs are matching the expectation.
>
> ==== Example of expected format of messages ====
> '''First attempt'''
> {{{
> update delete husker.human.bs. in A
> update add husker.human.bs. 1200 in A 192.168.122.180
> send
> update delete husker.human.bs. in AAAA
> update add husker.human.bs. 1200 in AAAA 2001:cdba::666
> send
> }}}
> '''Fallback attempt'''
> {{{
> ;sever is present only if option dyndns_server is set in sssd.conf
> server 192.168.122.20
> ;realm is used always in fallback message
> realm IPA.WORK
> update delete husker.human.bs. in A
> update add husker.human.bs. 1200 in A 192.168.122.180
> send
> update delete husker.human.bs. in AAAA
> update add husker.human.bs. 1200 in AAAA 2001:cdba::666
> send
> }}}
> === Authors ===
> [e-mail]

The page matches what we discussed yesterday in a private conversation.

Thank you for bringing it to the list!

[Pavel Reichl]

On 07/22/2015 02:54 PM, Jakub Hrozek wrote:
> On Wed, Jul 22, 2015 at 02:09:18PM +0200, Pavel Reichl wrote:
> > On 07/10/2014 10:30 AM, Simo Sorce wrote:
> > > On Wed, 2014-07-09 at 23:06 +0200, Jakub Hrozek wrote:
> > > > Hi,
> > > >
> > > > one of our users ran into an interesting problem -- her AD
> > > > infrastructure was different from the DNS server. Because by default, we
> > > > perform update against the server we're connected to, the DNS update
> > > > didn't work.
> > > >
> > > > Per Simo's suggestion, I've implemented a new option that allows the
> > > > administrator to override the DNS server used for DNS updates.
> > >
> > > I am not sure I like the premises here.
> > >
> > > The problem is that we are trying to set a DNS server at all in the
> > > normal case.
> > > The normal case should be that nsupdate is not passed any server name
> > > and it discovers the correct server via normal DNS lookup queries.
> > >
> > > If someone is using a messed up DNS setup then, yeah I would offer a
> > > setting for specifying exactly what DNS server to contact, but I am not
> > > sure it makes sense to fallback to a random Domain Controller by
> > > default.
> > >
> > > IIRC the cldap ping reply does return you information on whether the
> > > domain controller is also a DNS server, I think we need to extract that
> > > info and keep it around, and fallback to directly contact the domain
> > > controller only if the cldap ping indicated DNS support.
> > >
> > > Comments on code follow:
> > >
> > > I think I would change the following:
> > >                    <para>
> > >                        Setting this option makes sense for environments
> > >                        where the DNS server is different from the identity
> > >                        server.
> > >                    </para>
> > >
> > > I think you should say something like this:
> > > "Set this option if your DNS environment is broken and the correct
> > > server cannot be discovered via regular DNS lookups."
> > >
> > > On the code side I think you are missing to remove the code in
> > > ad_dyndns.c:ad_dyndns_update_send() that unconditionally sets
> > > stat->servername, by default no server name should be set.
> > >
> > > Exactly the same issue is present in
> > > ipa_dyndns.c:ipa_dyndns_update_send(), once again the current freeipa
> > > server name is taken as a DNS server. And once again not all IPA servers
> > > are DNS servers. Here too we need to remove all the code that sets the
> > > server name and let nsupdate find the server through the normal dns
> > > lookup queries or use the dyndns_server fallback if set.
> > >
> > > Using unconditioanlly a domain controller you are randomly bound to is
> > > normally more wrong than right and we should stop doing that completely.
> > >
> > > So NACK, for now.
> > >
> > > Simo.
> >
> > Hello,
> >
> > I've discussed this ticket and Simo's comments off list with Jakub and Petr
> > Spacek. Please see linked design page which is result of the conversation.
> > Please comment here or directly edit the page.
> >
> > Thanks!
> >
> > https://fedorahosted.org/sssd/wiki/DesignDocs/DDNSMessagesUpdate
> >
> > /////////////////////////////////////////////////////////
> > /////////////////////////////////////////////////////////
> > /////////////////////////////////////////////////////////
> > = DDNS - update quality of input for nsupdate =
> >
> > Related ticket(s):
> > https://fedorahosted.org/sssd/ticket/2495
> >
> > === Problem statement ===
> > SSSD provides input to nsupdate that's redundant and might actually impair
> > proper resolution using DNS. Further, format of messages pasted to nsupdate
> > is grouped by command type - deleting and adding addresses are two separate
> > batches which is not optimal as update of one of address families might be
> > prohibited to be changed on server.
> >
> > === Use cases ===
> > If DNS system is broken using dyndns_server option might be an workaround
> >
> > === Overview of the solution ===
> > Allow SSSD to add a new option that would specify which server to update
> >
> > DNS with ('dyndns_server')
> > In first attempt to perform DDNS '''do not specify zone/server/realm'''
> >
> > commands in input for nsupdate (nsupdate will then utilize DNS)
> > As fallback (if first attempt fails) specify realm command and if
> >
> > 'dyndns_server' option is specified then also add server command with value
> > of 'dyndns_server'
> > Bulk deleting and adding addresses from the same address family into one
> >
> > batch rather then grouping by command type
> >
> > === Implementation details ===
> > Remove servername and dns_zone parameters from sdap_dyndns_update_send()
> >
> > as they are no longer used. Remove code from AD and IPA provider which was
> > passing this data to sdap_dyndns_update_send().
> > Remove dns_zone field from struct sdap_dyndns_update_state and remove all
> >
> > code relating to it.
> > In sdap_dyndns_update_done() make setting command realm conditional same
> >
> > way as command server is.
> > Update nsupdate_msg_add_fwd() to group commands by address family
> >
> > processed IP belongs to.
> >
> > === Configuration changes ===
> > New option dyndns_server
> >
> > === How To Test ===
> > Check that IP addresses get changed in IPA and on AD. Break DNS resolving to
> > force first attempt of DDNS to fail. Check that messages generated as input
> > for nsupdate in domain logs are matching the expectation.
> >
> > ==== Example of expected format of messages ====
> > '''First attempt'''
> > {{{
> > update delete husker.human.bs. in A
> > update add husker.human.bs. 1200 in A 192.168.122.180
> > send
> > update delete husker.human.bs. in AAAA
> > update add husker.human.bs. 1200 in AAAA 2001:cdba::666
> > send
> > }}}
> > '''Fallback attempt'''
> > {{{
> > ;sever is present only if option dyndns_server is set in sssd.conf
> > server 192.168.122.20
> > ;realm is used always in fallback message
> > realm IPA.WORK
> > update delete husker.human.bs. in A
> > update add husker.human.bs. 1200 in A 192.168.122.180
> > send
> > update delete husker.human.bs. in AAAA
> > update add husker.human.bs. 1200 in AAAA 2001:cdba::666
> > send
> > }}}
> > === Authors ===
> > [e-mail]
>
> The page matches what we discussed yesterday in a private conversation.
>
> Thank you for bringing it to the list!
> _______________________________________________
> sssd-devel mailing list
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Hello,

patch-set implementing this design proposal was sent in thread "DYDNDS: 
update quality of input for nsupdate". Thanks!