patch: srv_lookup on cygwin

[jcopenha@…]

The srv_lookup routine in lib/misc.c always returns zero responses on Cygwin. It is configured to do this because the ns_initparse and ns_parserr routines are not provided on Cygwin. Because srv_lookup always returns zero results Jabber accounts where the server does not match the @server.com portion of the username have to explicitly set a server name. This means that you can not do tls_verify on those accounts. For instance, username@gmail.com with a srv_lookup fail must set server name to talk.google.com. But Google returns a certificate for gmaill.com which the verify code thinks is the wrong server because it does not match the server name.

This patch does 3 things.

1. It changes the configure script to check for res_qeruy and dn_expandname when looking for libresolv. It also has a new check for ns_initparse and ns_initparserr to see if libresolv or libc provides them.

2. It changes the lib/misc.c srv_lookup routine to use dn_expandname instead of the custom code. This allows srv_lookup to process compressed names correctly. Cygwin res_query returns a compressed name and since bitlbee already requires libresolv this seems like the more correct solution.

3. It adds lib/ns_parse.c to provide ns_initparse and ns_parserr on platforms where those were not detected at ./configure time. This is a copy of a file from libbind-6.0 source with the copyright still in place and just minor modification. Removal of two functions that bitlbee does not use and added some #ifdefds and #include “bittlbee.h”. Reading the copyright/license notice I believe this is acceptable.

The end goal of this patch is to allow me to set up a Jabber account with username@gmail.com and tls_verify to true.

I know bitlbee doesn’t officially support Cygwin which is why I’m providing a patch instead of just reporting a bug. If the addition of ns_parse.c is more than bitlbee wants to take on I think the first two parts of the patch are still valuable and will help me in maintaining my own patch.

[jcopenha@…]

fix srv_lookup routine on Cygwin

[dx <dx@…>]

Two minor fixes to the patch above for situations when ns_parse.c isn't needed

[dx]

Related: #1003 for openbsd fixes

jcopenha: Would you mind reviewing how to integrate this with your version of the patch? I don't really understand what's going on.

[anonymous]

The #1003 patch looks like it implements custom parsing of the res_query response on OpenBSD because OpenBSD doesn't provide the ns_parserrr function either. I'd say the patch for #1098, which provides ns_parserr, would make #1003 unnecessary.

[anonymous]

last comment is from jcopenha ..

[dx]

Adding Daniel Levai to CC - Could you test this patch with openbsd?

I prefer it over custom parsing, and it seems to solve both the lack of ns_parserr (not quite sure if that's what's missing in openbsd like jcopenha said, but the #ifdef block of openbsd in ticket:1003:openbsd-ns_parse.diff​ seems to avoid that function in particular) AND the dn_expand usage, plus really nice configure tests.

Anyway yeah, we already wanted to merge this particular patch (read: i've told wilmer to merge it, he said it looks okay, but didn't actually merge it :D), so a confirmation that it fixes openbsd too would be great.

[Daniel]

This patch doesn't seem to fix the SRV lookups on OpenBSD.

The patch that is included in the OpenBSD ports tree has a new RESOLVE_TESTCODE in configure that takes into account that OpenBSD *doesn't have* ns_*() functions. I mean, this was the fundamental "problem" with the code, and that is what the patches fixed in the ports tree. Correct me if I'm wrong, but you now test this the same way as before, and of course, on OpenBSD this doesn't happen, and config.h misses the HAVE_RESOLV_A define. Please see the patches in ​http://www.openbsd.org/cgi-bin/cvsweb/ports/net/bitlbee/patches/

[dx]

Well, seems like that's exactly what this patch fixes! :D

Cygwin doesn't have ns_ either, so this patch just includes the ns functions we need in lib/ns_parse.c (see point 3 in the ticket description).

It also changes RESOLV_TESTCODE to be very very similar to the openbsd patch (it tests for "dn_skipname" too), and adds RESOLVE_NS_TESTCODE which looks like the previous tests and defines HAVE_RESOLV_A_WITH_NS to avoid using the included lib/ns_parse.c since the system already provides that.

Also, thanks for the confirmation that ns_*() were the main issue. So yeah, I'd appreciate if you could take the time to test this patch. You will probably want to add a OpenBSD case after the part that says # In Cygwin res_*/dn_* routines are present in libc.so in the patched configure.

[Daniel]

What I meant with "doesn't seem to fix" is that it does not. Perhaps that was not clear, sorry, english is my second language. Probably it should've been more like "it doesn't seem to work".

So yeah, I've tested it, and HAVE_RESOLV_A doesn't make it into config.h (well, at least HAVE_RESOLV_A_WITH_NS doesn't either :) ).

And what you say about adding a specific case for OpenBSD to configure is exactly what we did in the ports tree. You did not attach that to #1003, only the patch for misc.c, perhaps that's why it got overlooked.

Again, please see the patches in ​​http://www.openbsd.org/cgi-bin/cvsweb/ports/net/bitlbee/patches/

The reason I suggest you guys look at those patches, is because it shows what can, and what can not be done currently on OpenBSD.

Yes, you factored out and included some of the ns_*() functions in a separate file, but you failed to deal with eg. ns_rr and ns_msg (and the likes). Even if I patch configure to define HAVE_RESOLV_A in config.h on OpenBSD, with your patch they get referenced now under the #ifdef HAVE_RESOLV_A in misc.c. Those are #ifdef'd out in the OpenBSD ports tree patches, and that is because we don't have those in our arpa/nameser.h (unlike eg. Linux). Or NS_MAXDNAME for that matter. Also, see how we initialized eg.: ns_c_in or ns_t_srv.

Please please please see those simple patches for OpenBSD, and you'll see what we did to workaround this issue.

I'll be glad to test any future patches, of course.

Thanks, Daniel

[dx]

Gotcha, will investigate further.

I did notice the changes to the configure file, I was only comparing the testcode of each:

OpenBSD patch:

int main()
{
       res_query( NULL, 0, 0, NULL, 0 );
       dn_expand( NULL, NULL, NULL, NULL, 0 );
}

vs Cygwin patch:

int main()
{

	res_query( NULL, 0, 0, NULL, 0);
	dn_expand( NULL, NULL, NULL, NULL, 0);
	dn_skipname( NULL, NULL);
}

I must be missing something at the moment, will report back when I get more details.

Thanks for your time, btw.

[jcopenha]

proposal for cygwin and openbsd. added detection of missing types and custom definitions if not there.

[jcopenha]

I uploaded a new patch, srv_lookup_fix_v3.patch. This is the same core cygwin fix from the original patch along with the missing NS types to make it work on OpenBSD as well.

[dx]

Merged in revision 1039