DLZ $client% parameter segfault

[Vadim S. Goncharov]

On 04.04.2013 04:35, Evan Hunt wrote:
>> Thanks certainly blows up the possibility of doing native GeoDNS at the
>> moment? Any chance I am overlooking a method which I could effectively
>> get the clients address into a MySQL query with the current 9.9.2
>> release?
>
> It's not quite the same as %client%, but I suggest you take a look at
> contrib/dlz/example/dlz_example.c, note the use of the clientinfo
> structure in dlz_lookup(), and see if it's any use to you.  You'd have
> to hack the corresponding routine in the mysql DLZ driver.
>
> Incidentally, native GeoIP support has been added as a feature in the
> 9.10 release -- you'll be able to use it to specify ACLs, e.g.,
> "match-clients { geoip country US; };".  I expect it to be published
> in Q4 of this year.

Hmm. A positive step. I see that in 9.8 it was:

static isc_result_t
mysql_allnodes(const char *zone, void *driverarg, void *dbdata,
                dns_sdlzallnodes_t *allnodes)


and now it is:

static isc_result_t
mysql_lookup(const char *zone, const char *name, void *driverarg,
              void *dbdata, dns_sdlzlookup_t *lookup,
              dns_clientinfomethods_t *methods, dns_clientinfo_t *clientinfo)

Very glad to see that it is now available in the SDLZ API, in principle. 
But, it is still not used: for the %client% (actually $client$) to work, the 
dbi->client member must set to actual string ptr, to be used by 
build_querystring(), which processes all $escape$'s.

For this to happen, mysql_get_resultset() should be passed 'char *client' 
instead of NULL, from mysql_lookup() and others, but UNUSED(clientinfo); is 
there.

The block under 'if (strcmp(name, "source-addr") == 0)' in 
contrib/dlz/example/dlz_example.c does almost the needed thing, except that 
it also prints port (BTW, that better be handled by isc_sockaddr_format() 
imported, if port really needed). So probably isc_netaddr_totext() should be 
used (to eliminate code duplication).

Then, note that:

1) Still allnodes() method in SDLZ don't receive 'clientinfo', but this may
    be inconsistent for DLZ users: if one answers to client with his IP addr
    on lookup, probably the same RR should to AXFR to that client.

2) The other drivers utilizing build_querystring() need also be patched to
    set proper dbi->client, that is,

    * drivers/dlz_mysql_driver.c
    * drivers/dlz_ldap_driver.c
    * drivers/dlz_postgres_driver.c
    * drivers/dlz_odbc_driver.c
    * modules/ldap/dlz_ldap_dynamic.c
    * modules/wildcard/dlz_wildcard_dynamic.c

   Fortunately, they all call *_get_results*() function with 'client' arg,
   all the same as in mysql_lookup(), so the fix is similar; may be put in
   to helper func in common file? e.g. modules/common/dlz_dbi.c or
   drivers/sdlz_helper.c (they both already duplicate build_querystring() and
   other, though).

So, as fixing NULL dereferencing segfault by adding long-awaited feature is 
good, could you please do that ? :-) Processing client's IP address in DLZ 
queries is not limited to just GeoIP.

-- 
Vadim Goncharov     <vgoncharov at nic.ru>           RU-Center
NET Department                            http://www.nic.ru
NET-SYS Group             phone:+7(495)737-7646  (ext.4019)