Re: [Dnssec-trigger] dnssec-trigger 0.16 released
I think you should release this fix straight away, because the infinite
loop is very likely to happen. My additional patch can wait a little longer.
Regards,
Martin
On 06/22/2018 11:52 AM, W.C.A. Wijngaards wrote:
> Hi Martin,
>
> Oops that was not supposed to happen. I'll import some part of your
> fixes, they are now mixed with your patch.
>
> This is what that looks like (move to next at front of loop and zone
> used to get the string contents):
>
> Is this something for release straight away, or something that can wait
> for your the additional patch?
>
> Index: riggerd/svr.c
> ===
> --- riggerd/svr.c (revision 790)
> +++ riggerd/svr.c (revision 792)
> @@ -909,29 +909,29 @@
> .string = iter->string,
> .length = iter->length
> };
> - if (nm_connection_list_contains_zone(connections, iter->string,
> iter->length)) {
> - verbose(VERB_DEBUG, "Iter over stored zones: %s is in
> connections",
> iter->string);
> + /* don't use FOR_EACH_STRING_IN_LIST because the stringlist
> + * edited in the loop. pick up the next pointer, then
> + * delete the item */
> + iter = iter->next;
> + if (nm_connection_list_contains_zone(connections, zone.string,
> zone.length)) {
> + verbose(VERB_DEBUG, "Iter over stored zones: %s is in
> connections",
> zone.string);
> continue;
> }
> - if (zone_in_reverse_zones(iter->string, iter->length)) {
> + if (zone_in_reverse_zones(zone.string, zone.length)) {
> if (global_svr->cfg->use_private_address_ranges) {
> - verbose(VERB_DEBUG, "Iter over stored zones: %s
> is in reverse
> zones", iter->string);
> + verbose(VERB_DEBUG, "Iter over stored zones: %s
> is in reverse
> zones", zone.string);
> continue;
> } else {
> - verbose(VERB_DEBUG, "Iter over stored zones: %s
> add to local zones
> using ubhook", iter->string);
> + verbose(VERB_DEBUG, "Iter over stored zones: %s
> add to local zones
> using ubhook", zone.string);
> hook_unbound_add_local_zone(zone, static_label);
> }
> }
> - if (nm_connection_list_contains_zone(_zones,
> iter->string,
> iter->length)) {
> - verbose(VERB_DEBUG, "Iter over stored zones: %s
> removing from
> forward zones", iter->string);
> - nm_connection_list_remove(_zones, iter->string,
> iter->length);
> + if (nm_connection_list_contains_zone(_zones,
> zone.string,
> zone.length)) {
> + verbose(VERB_DEBUG, "Iter over stored zones: %s
> removing from
> forward zones", zone.string);
> + nm_connection_list_remove(_zones, zone.string,
> zone.length);
> hook_unbound_remove_forward_zone(zone);
> }
> - verbose(VERB_DEBUG, "Iter over stored zones: %s removing from
> store",
> iter->string);
> - /* don't use FOR_EACH_STRING_IN_LIST because the stringlist
> - * edited in the loop. pick up the next pointer, then
> - * delete the item */
> - iter = iter->next;
> + verbose(VERB_DEBUG, "Iter over stored zones: %s removing from
> store",
> zone.string);
> store_remove(_zones, zone.string, zone.length);
> }
>
> Best regards, Wouter
>
> On 22/06/18 11:28, Martin Sehnoutka wrote:
>> Hi,
>>
>> be careful, there is an unfortunate bug in this loop:
>> https://github.com/NLnetLabs/dnssec-trigger/blob/master/riggerd/svr.c#L934
>> Since it was changed from the iterator macro, it needs to do the step
>> iter=iter->next regardless of the branch it takes. Right now it will
>> most likely end up in an infinite loop. I'm working on a patch:
>> https://github.com/InfrastructureServices/dnssec-trigger/commit/33177c0f27228719e969ee621358ec699fd9b3d4
>> I wanted to release this patch together with another one, which would
>> also fix a problem with installation of a forward zone with the same
>> name, but different name servers (this currently does not work), but I
>> haven't managed to finish it yet.
>>
>> Best regards,
>> Martin
>>
>>
>> On 06/21/2018 02:25 PM, W.C.A. Wijngaards wrote:
>>> Hi,
>>>
>>> dnssec-trigger 0.16 is available:
>>> https://nlnetlabs.nl/downloads/dnssec-trigger/dnssec-trigger-0.16.tar.gz
>>> sha256 e80aab8fd52074638f782a608bf433cbaa507cad087bcc5fb433353db9d057cb
>>> pgp
>>> https://nlnetlabs.nl/downloads/dnssec-trigger/dnssec-trigger-0.16.tar.gz.asc
>>>
>>> windows
>>> https://nlnetlabs.nl/downloads/dnssec-trigger/dnssec_trigger_setup_0.16.exe
>>> osx
>>>
Re: [Dnssec-trigger] dnssec-trigger 0.16 released
Hi Martin,
Oops that was not supposed to happen. I'll import some part of your
fixes, they are now mixed with your patch.
This is what that looks like (move to next at front of loop and zone
used to get the string contents):
Is this something for release straight away, or something that can wait
for your the additional patch?
Index: riggerd/svr.c
===
--- riggerd/svr.c (revision 790)
+++ riggerd/svr.c (revision 792)
@@ -909,29 +909,29 @@
.string = iter->string,
.length = iter->length
};
- if (nm_connection_list_contains_zone(connections, iter->string,
iter->length)) {
- verbose(VERB_DEBUG, "Iter over stored zones: %s is in
connections",
iter->string);
+ /* don't use FOR_EACH_STRING_IN_LIST because the stringlist
+* edited in the loop. pick up the next pointer, then
+* delete the item */
+ iter = iter->next;
+ if (nm_connection_list_contains_zone(connections, zone.string,
zone.length)) {
+ verbose(VERB_DEBUG, "Iter over stored zones: %s is in
connections",
zone.string);
continue;
}
- if (zone_in_reverse_zones(iter->string, iter->length)) {
+ if (zone_in_reverse_zones(zone.string, zone.length)) {
if (global_svr->cfg->use_private_address_ranges) {
- verbose(VERB_DEBUG, "Iter over stored zones: %s
is in reverse
zones", iter->string);
+ verbose(VERB_DEBUG, "Iter over stored zones: %s
is in reverse
zones", zone.string);
continue;
} else {
- verbose(VERB_DEBUG, "Iter over stored zones: %s
add to local zones
using ubhook", iter->string);
+ verbose(VERB_DEBUG, "Iter over stored zones: %s
add to local zones
using ubhook", zone.string);
hook_unbound_add_local_zone(zone, static_label);
}
}
- if (nm_connection_list_contains_zone(_zones,
iter->string,
iter->length)) {
- verbose(VERB_DEBUG, "Iter over stored zones: %s
removing from
forward zones", iter->string);
- nm_connection_list_remove(_zones, iter->string,
iter->length);
+ if (nm_connection_list_contains_zone(_zones,
zone.string,
zone.length)) {
+ verbose(VERB_DEBUG, "Iter over stored zones: %s
removing from
forward zones", zone.string);
+ nm_connection_list_remove(_zones, zone.string,
zone.length);
hook_unbound_remove_forward_zone(zone);
}
- verbose(VERB_DEBUG, "Iter over stored zones: %s removing from
store",
iter->string);
- /* don't use FOR_EACH_STRING_IN_LIST because the stringlist
-* edited in the loop. pick up the next pointer, then
-* delete the item */
- iter = iter->next;
+ verbose(VERB_DEBUG, "Iter over stored zones: %s removing from
store",
zone.string);
store_remove(_zones, zone.string, zone.length);
}
Best regards, Wouter
On 22/06/18 11:28, Martin Sehnoutka wrote:
> Hi,
>
> be careful, there is an unfortunate bug in this loop:
> https://github.com/NLnetLabs/dnssec-trigger/blob/master/riggerd/svr.c#L934
> Since it was changed from the iterator macro, it needs to do the step
> iter=iter->next regardless of the branch it takes. Right now it will
> most likely end up in an infinite loop. I'm working on a patch:
> https://github.com/InfrastructureServices/dnssec-trigger/commit/33177c0f27228719e969ee621358ec699fd9b3d4
> I wanted to release this patch together with another one, which would
> also fix a problem with installation of a forward zone with the same
> name, but different name servers (this currently does not work), but I
> haven't managed to finish it yet.
>
> Best regards,
> Martin
>
>
> On 06/21/2018 02:25 PM, W.C.A. Wijngaards wrote:
>> Hi,
>>
>> dnssec-trigger 0.16 is available:
>> https://nlnetlabs.nl/downloads/dnssec-trigger/dnssec-trigger-0.16.tar.gz
>> sha256 e80aab8fd52074638f782a608bf433cbaa507cad087bcc5fb433353db9d057cb
>> pgp
>> https://nlnetlabs.nl/downloads/dnssec-trigger/dnssec-trigger-0.16.tar.gz.asc
>>
>> windows
>> https://nlnetlabs.nl/downloads/dnssec-trigger/dnssec_trigger_setup_0.16.exe
>> osx
>> https://nlnetlabs.nl/downloads/dnssec-trigger/dnssectrigger-0.16.dmg
>>
>>
>> This release has a fix for the reports about .uk.uk. The patchset from
>> Martin Sehnoutka is integrated, it moves functionality from the linux
>> network change script into the dnssec-trigger process.
>>
>>
>> Features
>> - Patch set from Martin Sehnoutka,
>> It
Re: [Dnssec-trigger] dnssec-trigger 0.16 released
Hi, be careful, there is an unfortunate bug in this loop: https://github.com/NLnetLabs/dnssec-trigger/blob/master/riggerd/svr.c#L934 Since it was changed from the iterator macro, it needs to do the step iter=iter->next regardless of the branch it takes. Right now it will most likely end up in an infinite loop. I'm working on a patch: https://github.com/InfrastructureServices/dnssec-trigger/commit/33177c0f27228719e969ee621358ec699fd9b3d4 I wanted to release this patch together with another one, which would also fix a problem with installation of a forward zone with the same name, but different name servers (this currently does not work), but I haven't managed to finish it yet. Best regards, Martin On 06/21/2018 02:25 PM, W.C.A. Wijngaards wrote: > Hi, > > dnssec-trigger 0.16 is available: > https://nlnetlabs.nl/downloads/dnssec-trigger/dnssec-trigger-0.16.tar.gz > sha256 e80aab8fd52074638f782a608bf433cbaa507cad087bcc5fb433353db9d057cb > pgp > https://nlnetlabs.nl/downloads/dnssec-trigger/dnssec-trigger-0.16.tar.gz.asc > > windows > https://nlnetlabs.nl/downloads/dnssec-trigger/dnssec_trigger_setup_0.16.exe > osx > https://nlnetlabs.nl/downloads/dnssec-trigger/dnssectrigger-0.16.dmg > > > This release has a fix for the reports about .uk.uk. The patchset from > Martin Sehnoutka is integrated, it moves functionality from the linux > network change script into the dnssec-trigger process. > > > Features > - Patch set from Martin Sehnoutka, > It migrates the functionality currently provided by the script > into the daemon. the "update" command from the script is available > in the daemon as "update_all", so that they can live side by side. > > Bug Fixes > - Fix example.conf default printout text replacement. > - port of dnssec-trigger-script to libnm. > - Fix that NXDOMAIN for _probe.uk.uk is deemed allright. > - Modify the build system: > A new configure option 'with-forward-zones-support' was introduced, that > enables configuration of forward and local zones directly from the > daemon as opposed to the script. Without this option, there is almost no > change. > > The new functionality of the daemon can be triggered by the "update_all" > command, which is now used in NM dispatcher script and systemd service > file. Some configuration options were migrated from the script to the > daemon as well. > > Finally a testing suite was introduced using the cmocka library. > - Introduce string_buffer and string_list types: > String buffer is a fat pointer and list is a single linked list of fat > char pointers. > - Import JSON parsing library (BSD-MIT license). > - Connection list module: > A connection is a struct encapsulating the concept of "connection" as > known from NetworkManager. It is used to extract information about > global resolvers, DNS search zones and in the future about reverse zones > corresponding to the network address. > - Function to parse JSON into connection list > - Lock module, used to serialize execution in the script > This was introduced as a compatibility feature with the script, but once > the script is gone, this can be safely removed. > - Store module - persistent storage used by the script > The script uses few files stored on disk in order to create a persistent > cache of configured global forwarders and forward zones. This was > introduced as a compatibility module with the script. Again it can be > removed once the compatibility is not needed any more. > - Testing suite for previously introduced modules > It can be executed using 'make test' and it can be also used in CI. > - New configuration options, that were in the script > - Hook unbound control > It uses 'unbound-control' binary instead of the socket, so this should > probably be rewritten if possible. > - Reimplement update command from script in riggerd > - add testing file for global forwarders cache > - Fixes and modifications to the patch set. > - removed -vvv option from dnssec-triggerd daemon start script. > - removed unaligned memcpy > - More review fixes, store.c, error log and fixup of getline return, > and not variable use before declaration. Spelling, strdup, > bool removal for portability. Removed unsigned comparison warning. > - string_list, sprint with null termination and correct buffer check. > - Fix that update_connection_zones does not use item after free. > - Fix declare before code warnings. > - Use pclose for popen fds. > - Use snprintf instead of sprintf to fixed buffer. > - Fix gcc buffer size for snprintf warning (in dnssec-trigger update > code, not the patch set from 14may). > - Add check on shell commandline arguments, to make sure domain names > and IP addresses passed to it do not contain escape characters. > > Best regards, Wouter > > > > ___ > dnssec-trigger mailing list > dnssec-trigger@NLnetLabs.nl > https://open.nlnetlabs.nl/mailman/listinfo/dnssec-trigger > -- Martin Sehnoutka | Associate Software Engineer
[Dnssec-trigger] dnssec-trigger 0.16 released
Hi, dnssec-trigger 0.16 is available: https://nlnetlabs.nl/downloads/dnssec-trigger/dnssec-trigger-0.16.tar.gz sha256 e80aab8fd52074638f782a608bf433cbaa507cad087bcc5fb433353db9d057cb pgp https://nlnetlabs.nl/downloads/dnssec-trigger/dnssec-trigger-0.16.tar.gz.asc windows https://nlnetlabs.nl/downloads/dnssec-trigger/dnssec_trigger_setup_0.16.exe osx https://nlnetlabs.nl/downloads/dnssec-trigger/dnssectrigger-0.16.dmg This release has a fix for the reports about .uk.uk. The patchset from Martin Sehnoutka is integrated, it moves functionality from the linux network change script into the dnssec-trigger process. Features - Patch set from Martin Sehnoutka, It migrates the functionality currently provided by the script into the daemon. the "update" command from the script is available in the daemon as "update_all", so that they can live side by side. Bug Fixes - Fix example.conf default printout text replacement. - port of dnssec-trigger-script to libnm. - Fix that NXDOMAIN for _probe.uk.uk is deemed allright. - Modify the build system: A new configure option 'with-forward-zones-support' was introduced, that enables configuration of forward and local zones directly from the daemon as opposed to the script. Without this option, there is almost no change. The new functionality of the daemon can be triggered by the "update_all" command, which is now used in NM dispatcher script and systemd service file. Some configuration options were migrated from the script to the daemon as well. Finally a testing suite was introduced using the cmocka library. - Introduce string_buffer and string_list types: String buffer is a fat pointer and list is a single linked list of fat char pointers. - Import JSON parsing library (BSD-MIT license). - Connection list module: A connection is a struct encapsulating the concept of "connection" as known from NetworkManager. It is used to extract information about global resolvers, DNS search zones and in the future about reverse zones corresponding to the network address. - Function to parse JSON into connection list - Lock module, used to serialize execution in the script This was introduced as a compatibility feature with the script, but once the script is gone, this can be safely removed. - Store module - persistent storage used by the script The script uses few files stored on disk in order to create a persistent cache of configured global forwarders and forward zones. This was introduced as a compatibility module with the script. Again it can be removed once the compatibility is not needed any more. - Testing suite for previously introduced modules It can be executed using 'make test' and it can be also used in CI. - New configuration options, that were in the script - Hook unbound control It uses 'unbound-control' binary instead of the socket, so this should probably be rewritten if possible. - Reimplement update command from script in riggerd - add testing file for global forwarders cache - Fixes and modifications to the patch set. - removed -vvv option from dnssec-triggerd daemon start script. - removed unaligned memcpy - More review fixes, store.c, error log and fixup of getline return, and not variable use before declaration. Spelling, strdup, bool removal for portability. Removed unsigned comparison warning. - string_list, sprint with null termination and correct buffer check. - Fix that update_connection_zones does not use item after free. - Fix declare before code warnings. - Use pclose for popen fds. - Use snprintf instead of sprintf to fixed buffer. - Fix gcc buffer size for snprintf warning (in dnssec-trigger update code, not the patch set from 14may). - Add check on shell commandline arguments, to make sure domain names and IP addresses passed to it do not contain escape characters. Best regards, Wouter signature.asc Description: OpenPGP digital signature ___ dnssec-trigger mailing list dnssec-trigger@NLnetLabs.nl https://open.nlnetlabs.nl/mailman/listinfo/dnssec-trigger
