Issue1446

Title hkps SRV lookup discards port from SRV
Priority bug Status resolved
Category gnupg Due Date
Version git current ExtLink  (go)
Superseder Nosy List dshaw, syscomet
Assigned To dshaw Topics  (help)

Created on 2012-10-07.02:31:09 by syscomet, last changed 2015-03-16.14:22:27 by werner.

Messages
msg6073 (view) Author: werner Date: 2015-03-16.14:22:27
Fixed with commit ba9e974f1fd85b3dbbfb5e26d7a14f71d07c7cf2 for 2.0
Fixed with commit f2f12f41efe5a476833295dc6c44fcd887d0abe6 for 1.4
msg4489 (view) Author: werner Date: 2012-12-17.21:51:06
Do we need to do something for 1.4?
msg4486 (view) Author: dshaw Date: 2012-12-15.15:00:43
Finishing things up now.
msg4481 (view) Author: werner Date: 2012-12-15.09:01:58
David, what is the status?
msg4472 (view) Author: dshaw Date: 2012-12-02.15:17:58
Taking
msg4440 (view) Author: syscomet Date: 2012-10-20.06:08:22
The behaviour matches that observed in released versions; I was debugging a 
problem observed in the released versions, not reviewing code looking for issues.

Whether or not it's used in the current development branch, this has caused an 
interoperability issue in the real world for the keyserver operators, causing a 
functionality deployment to be rolled back and resulting in filtered results, 
reducing the pool of available keyservers.

Since Issue1447 is a security impacting issue which will need a CVE and a security 
release to fix anyway, it would really be nice to try to get the fix for client 
behaviour into a version which is likely to be pushed out widely.  Not critical, 
security comes first, but if we can leverage the security release to improve 
interop, that would be helpful.

In practice, we (the keyserver operators and pool operators) are stuck not able to 
use SRV to point to non-default ports for at least a couple of years.  This is 
very unfortunate, given the efforts currently being made to make deployments more 
robust, with TLS more widely deployed.
msg4438 (view) Author: werner Date: 2012-10-19.09:45:49
Well, it might still exists, but it is not used anymore.  Remember that this is
the development branch.
msg4435 (view) Author: syscomet Date: 2012-10-11.18:23:51
% git remote -v
origin	git://git.gnupg.org/gnupg.git (fetch)
origin	git://git.gnupg.org/gnupg.git (push)
% git status
# On branch master
nothing to commit (working directory clean)
%

I did the pull on the day I filed the bug, and as of the commit stated, the 
directory exists. I just did a "git pull", no change.  I didn't write "git current" 
in this bug.

http://www.gnupg.org/download/cvs_access.en.html still points to the repo above, so 
that's what I pulled.  If that's no longer correct, I can pull another repo.

But still, if you check out the revision stated, you'll see the behaviour, which is 
reflected in current releases of GnuPG.
msg4433 (view) Author: werner Date: 2012-10-11.15:55:50
What do you mean by "git current"?  The current "git master" has no keyserver/
stuff.
msg4429 (view) Author: syscomet Date: 2012-10-07.02:31:08
GnuPG discards the port from SRV in constructing the URL for libcurl, as evidenced 
by:
 gpg2 --keyserver-options no-check-cert,debug,verbose --keyserver 
hkps://keys.kfwebs.net --recv-key 0x0B7F8B60E3EDFAE3

Analysis and line numbers of keyserver/gpgkeys_hkp.c as per git as of commit 
76055d49d1c8b8e4f6245e6729cae81b1eaecbf6.

if the scheme is hkps: then you set:
695   if(ascii_strcasecmp(opt->scheme,"hkps")==0)
696     {
697       proto="https";
698       port="443";
699     }

then we hit:
729   if(opt->port)
730     port=opt->port;
731   else if(try_srv)
732     {
733       char *srvtag;
734
735       if(ks_strcasecmp(opt->scheme,"hkp")==0)
736         srvtag="pgpkey-http";
737       else if(ks_strcasecmp(opt->scheme,"hkps")==0)
738         srvtag="pgpkey-https";
739       else
740         srvtag=NULL;
741
742 #ifdef HAVE_LIBCURL
743       /* We're using libcurl, so fake SRV support via our wrapper.
744          This isn't as good as true SRV support, as we do not try all
745          possible targets at one particular level and work our way
746          down the list, but it's better than nothing. */······
747       srv_replace(srvtag);

Now srv_replace will set opt->port:
531       if(newname && newport)
532         {
533           free(opt->host);
534           free(opt->port);
535           opt->host=newname;
536           snprintf(newport,MAX_PORT,"%u",srvlist->port);
537           opt->port=newport;
538         }

but then in get_key():
266   strcpy(request,proto);
267   strcat(request,"://");
268   strcat(request,opt->host);
269   strcat(request,":");
270   strcat(request,port);
271   strcat(request,opt->path);
[...]
294   curl_easy_setopt(curl,CURLOPT_URL,request);


So, there's a `port` and an `opt->port`; the SRV lookups set `opt->port`
but not `port`, while the URL given to curl uses `port`.

It seems like changing 537 to:
  port = opt->port = newport

should fix it as a stop-gap.
History
Date User Action Args
2015-03-16 14:22:27wernersetstatus: in-progress -> resolved
messages: + msg6073
2012-12-17 21:51:06wernersetmessages: + msg4489
2012-12-15 15:00:43dshawsetmessages: + msg4486
2012-12-15 09:01:58wernersetmessages: + msg4481
2012-12-02 15:17:58dshawsetstatus: chatting -> in-progress
assignedto: dshaw
messages: + msg4472
nosy: + dshaw
2012-10-20 06:08:22syscometsetmessages: + msg4440
2012-10-19 09:45:49wernersetmessages: + msg4438
2012-10-11 18:23:51syscometsetmessages: + msg4435
2012-10-11 15:55:50wernersetstatus: unread -> chatting
messages: + msg4433
2012-10-07 02:31:09syscometcreate