Opened at 2013-11-22T17:35:00Z
Closed at 2014-09-27T14:27:35Z
#1098 closed defect (fixed)
patch: srv_lookup on cygwin
Reported by: | Owned by: | ||
---|---|---|---|
Priority: | major | Milestone: | |
Component: | BitlBee | Version: | 3.2 |
Keywords: | patch | Cc: | leva@… |
IRC client+version: | Client-independent | Operating System: | Public server |
OS version/distro: |
Description
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.
- It changes the configure script to check for
res_qeruy
anddn_expandname
when looking forlibresolv
. It also has a new check forns_initparse
andns_initparserr
to see iflibresolv
orlibc
provides them.
- It changes the
lib/misc.c
srv_lookup
routine to usedn_expandname
instead of the custom code. This allowssrv_lookup
to process compressed names correctly. Cygwinres_query
returns a compressed name and since bitlbee already requireslibresolv
this seems like the more correct solution.
- It adds
lib/ns_parse.c
to providens_initparse
andns_parserr
on platforms where those were not detected at./configure
time. This is a copy of a file fromlibbind-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#ifdefd
s 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.
Attachments (3)
Change History (15)
Changed at 2013-11-22T17:35:34Z by
Attachment: | srv_lookup_fix.patch added |
---|
Changed at 2014-01-09T13:48:43Z by
Attachment: | srv_lookup_fix_fixed.patch added |
---|
Two minor fixes to the patch above for situations when ns_parse.c isn't needed
comment:1 Changed at 2014-02-04T03:42:53Z by
Keywords: | patch added |
---|
comment:2 Changed at 2014-02-04T18:30:03Z by
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.
comment:3 Changed at 2014-02-04T20:10:07Z by
comment:5 Changed at 2014-02-05T00:24:50Z by
Cc: | leva@… added |
---|
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.
comment:6 Changed at 2014-02-05T12:52:02Z by
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/
comment:7 Changed at 2014-02-05T16:36:39Z by
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.
comment:8 Changed at 2014-02-05T18:34:02Z by
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
comment:9 Changed at 2014-02-05T20:24:24Z by
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.
comment:10 Changed at 2014-02-11T14:09:13Z by
Priority: | normal → major |
---|
Changed at 2014-06-25T04:06:19Z by
Attachment: | srv_lookup_fix_v3.patch added |
---|
proposal for cygwin and openbsd. added detection of missing types and custom definitions if not there.
comment:11 Changed at 2014-06-25T04:07:39Z by
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.
comment:12 Changed at 2014-09-27T14:27:35Z by
Resolution: | → fixed |
---|---|
Status: | new → closed |
Merged in revision 1039
fix srv_lookup routine on Cygwin