[cvsnt] ipv6 handling in cvsnt is broken

Arkadiusz Miskiewicz arekm at maven.pl
Tue Nov 11 21:10:40 GMT 2008


Community technical support mailing list was retired 2010 and replaced with a professional technical support team. For assistance please contact: Pre-sales Technical support via email to sales@march-hare.com.


On Tuesday 11 of November 2008, Arthur Barrett wrote:
> Arkadiusz,
>
> I appreciate your time and effort on this - however if you just slow down a
> little and explain things a little clearer we'll be able to make much more
> use of the information being supplied.

Ok, I'll try to put everything in this single email. Note that there are 
several mixing issues here:

a) my problem with cvslockd listening only on one address (::1)
b) no support for [address]:port notation
c) CSocketIO::bind() ignores some errors from bind()
d) CSocketIO::bind() totally ignores listen() errors

a) solution is SO_REUSEADDR (read below)
b) solution is to add parsing for [address]:port
c) solution is to use IPV6_V6ONLY and check if all bind() call succeeded
d) is to actually test return code for listen

c) solution would also catch a) problem instead of silently ignoring it


> > > Again.  You're trying to claim something is broken that
> > > I use every day.
> >
> > You don't use it in a way that triggers brokeness.
>
> You need to explain more clearly what that is.

My problem was that cvslockd listened only on IPv6 address ::1 and not on IPv4 
address 127.0.0.1. That was because:

"Anyway I found out what was cause of my initial the problem. IPv4 socket 
hangt in TIME_WAIT state so bind() failed with "address already in use" but 
IPv6 socket succeeded. That's why it was listening only on ::1 and not on 
127.0.0.1. "

This problem can be solved by using SO_REUSEADDR setsockopt(2).

> Are you running *only* IPv6, which we've already stated is not currently
> supported?

It's Linux with kernel 2.6.16.53, dual stack - IPv4 and IPv6.

> Now if you want to go through and get the whole stack to work on IPv6
> (without breaking the current behaviour - particularly on HPUX 11iv1) then
> that'd be really really appreciated.  We don't support a pure IPv6 stack at
> the moment simply because we've not had the time and resources or
> motivation to do it.
>
> > Show me how to connect to it using cvsnt client by
> > specifing IPv6 address (+ port) and not hostname please.
>
> Err, no - you show me?  I wouldn't know an IPv6 address if I fell over it.

::1 - this is IPv6 address (and means the same as 127.0.0.1 in IPv4 world) - a 
loopback address.

Currently cvsntcode does strchr(..., ':') to find out where port part begins. 
This breaks with IPv6 addresses since : is part of normal IPv6 address.

The code should do something like (pseudocode):

x = "address:port" or "[address]:port"
if x begins with "[" and contains "]" then this is ipv6 address {
  y = strchr(x, ']')
  port_begins = strchr(y, ':')+1
  address = x[1, y-1]
} else
 port_begins y=strchr(x, ':')

(strip [] and find : as port separator after ])

>
> If anything I think you've found a documentation error - that parameter is
> for a hostname and port, addresses are partly supported for our own
> internal purposes but we do not intend to ever support addresses in that
> parameter.

Ok, "LockServer" config directive docs indeed say "hostname" not "host".


Too bad that CVSROOTs like:

:pserver:cvs at 217.73.31.16:/cvsroot
:pserver:cvs@[2a01:390:1:0:a800:ffff:fede:ad05]:/cvsroot

are unsupported (according to docs of course). Looks like cvsnt is a first 
program I see that uses TCP and supports hostnames only and oficially doesn't 
support IP addresses where "hostname" is used :)

> Again if you want to amend the code and provide a patch...

Understood.

>
> > > Please read http://www.cvsnt.org/wiki/BugReporting and
> > > provide all the information requested.

Originality - don't see any discussion about that. There is related 
discussion - 
http://www.cvsnt.org/pipermail/cvsnt-dev/2005-February/000134.html but with 
false information included (that "second bind fails" - it depends on Linux 
configuration)

Reproducibility - when cvslockd binds it tries to bind to ::1 and 127.0.0.1 on 
dual stack. You need to cause one socket to stay in TIME_WAIT thus preventing 
bind to it. I have no known way how to do that reliably. Probably SIGKILL 
already started cvslockd could do something like that. After that and killing 
cvslockd try to start cvslockd again - bind()ing to a socket in TIME_WAIT 
state will fail with "address already in use", there will be no message about 
failure from cvsnt and started cvslockd will listen only on single socket 
while correct condition is to listen on both socket.

Specificity - Linux 2.6.16.53, glibc 2.8, cvsnt 2.5.04.3236, IPv4 and IPv6 
configured, LockServerLocal=1

Examples - see Reproducibility

> >
> > cvslockd binding issue - forget about it (works for me
> > now with that little patch)
>
> Which you will need to keep re-applying - why not just answer the question?

I hope above answer is complete now.

>  We can't apply a patch which is going to break every platform except
> yours.  

You don't even know that the behaviour my patch _explictly_ sets is *default* 
behaviour on *BSD platform for example. So you *already use* it even without 
knowning.

This patch is not going to break any platforms (unless they have totally 
broken IPV6_V6ONLY support).

See, there are two types of IPv6 stacks:

1) socket bound to "::" address, port 1234 receives IPv6 traffic to port 1234 
AND IPv4 traffic directed to 1234 port. In such stack bind to "::" address 
succeeds and second bind to "0.0.0.0" fails with "address already in use". 
Fails because IPv6 socket gets IPv4 traffic, too. 

2) socket bound to "::" address, port 1234 receives ONLY IPv6 traffic and no 
IPv4 traffic. In such stack bind to "::" succeeds and second bind 
to "0.0.0.0" will also succeed. In this type of stack two sockets are 
required if you want to handle both, IPv6 and IPv4 traffic.

1) type of stack is seen on Linux.
2) type of stack is seen on *BSD.

Linux has ability to switch to 2) way of working via net.ipv6.bindv6only 
sysctl (it's default to zero == off). Setting it to 1 makes Linux stack 
behave as 2).

Now RFC3493 introduces IPV6_V6ONLY setsockopt() option which allows to choose 
between 1) and 2) on per application basis.

My patch explictly sets 2) way of working. Why it's preferable? It allows 
CSocketIO::bind() to be changed to succeed only if all bind()s succeed, so my 
initial issue would be "noticed" and error would be returned.

I guess someone tried to support 1) and 2) type of stack at once and easies 
(but broken) way was to make CSocketIO::bind() ignore valid errors.

It has another advantage. On 1) type of stack when IPv4 traffic goes to IPv6 
socket you will see only IPv6 addresses! IPv4 addressess will be represented 
as "IPv4-mapped IPv6 address" which for 1.2.3.4 is "::ffff:1.2.3.4" for 
example. Thus if you have any rule that depends on IP addresses you have to 
be very careful. 

For example if you have some kind of acl that denies traffic from 1.2.3.4 then 
if that IPv4 traffic will come via IPv6 socket you will.... allow it - since 
it will come from ::ffff:1.2.3.4. Handling IPv4 and IPv6 traffic via separate 
sockets makes this problem go away. Not sure if this "advantage" is useful 
for cvsnt but still...

> If I knew what platform you were running and if the native compiler 
> for that platform has a standard platform #define then at least I could
> apply the patch with a #if defined (some-obscure-platform) around your
> patch so that you don't need to keep re-applying.

Per platform hacks are worse than hell. What I propose is well defined in 
RFCs.

>
> > [ipv6]:port issue - what do you need more beside
> > what's in this thread?
>
> Have you read the page Tony sent you?  Originality, Reproducibility,
> Specificity and Examples.


Originality - don't see any discussion about that. 

Reproducibility - always.

Example:
$ cvs -z3 -d:pserver:[2a01:390:1:0:a800:ffff:fede:ad05]:/cvsroot ls
cvs ls: CVSROOT (":pserver:[2a01:390:1:0:a800:ffff:fede:ad05]:/cvsroot")
cvs ls: may only specify a positive, non-zero, integer port 
(not "390:1:0:a800:ffff:fede:ad05]:").
cvs ls: perhaps you entered a relative pathname?
cvs [ls aborted]: Bad CVSROOT.

Specificity - Linux 2.6.16.53, glibc 2.8, cvsnt 2.5.04.3236, IPv4 and IPv6 
configured

But as I wrote docs say "hostname" not "host" so you could tell me that IP 
addresses are not supported.

>
> In particular what operating system are you running and how is it
> configured so these problems occur?

Linux 2.6.16.53, glibc 2.8, cvsnt 2.5.04.3236, IPv4 and IPv6 configured

>
> > This itself is not a fix but a change that makes stable
> > behaviour - IPv6 separate from IPv4 (ipv6 is handled
> > via one socket, ipv4 is handled via second socet)
>
> Is this what your fix *does* or is this a fix your are proposing to provide
> a patch for?

Proposed patch (here -  
http://www.cvsnt.org/pipermail/cvsnt/2008-November/031499.html) does that.

>
> > CSocketIO::bind() also does weird thing - it assumes that
> > single succeeded bind is "enough". It's not. There are
> > various errors (other that IPv6 with IPV6_V6ONLY being
> > default off type of tcp stack) where bind can fail and
> > these errors are just siletnly ignored.
>
> Care to submit a patch?

That would be something like (that's just to show the concept coded in few 
minutes)
http://pld.pastebin.com/f49e5058

What it does? If  IPV6_V6ONLY "works" then CSocketIO::bind() should fail if 
any bind() call fails. 

If IPV6_V6ONLY doesn't work (os doesn't support or setsockopt() fails) then do 
what's done currently (well, there is room for improvement - bind() errors 
other than EADDRINUSE could be promoted to fatal ones)

> Arthur

-- 
Arkadiusz Miśkiewicz        PLD/Linux Team
arekm / maven.pl            http://ftp.pld-linux.org/


More information about the cvsnt mailing list
Download the latest CVSNT, TortosieCVS, WinCVS etc. for Windows 8 etc.
@CVSNT on Twitter   CVSNT on Facebook