Address joining pitfall

[Menno Smits]

... and remember to fix for 1.25 and master....

On 19 November 2015 at 15:40, Menno Smits <menno.smits at canonical.com> wrote:

> Hi all,
>
> While testing something completely unrelated today I noticed that the
> rsyslog worker was continually failing to start for one of the machines in
> the environment I was using. When I investigated, I found that the root
> cause was that the worker was  trying to connect to the state servers with
> an invalid IPv6 address and port combination (I recently enabled IPv6 on my
> notebook and home network). As you probably know, IPv6 addresses look
> something like "fe80::fcb3:4ff:fe6c:ecd3" but what you might not have
> realised is that the correct way to add a port number to a IPv6 address is
> to use square brackets like this: "[fe80::fcb3:4ff:fe6c:ecd3]:80"
> (otherwise - due to IPv6 address contraction it is unclear whether the port
> is the port number or the end of the address). Various Go "net" package
> functions will return errors if IPv6 address:port pairs are given without
> the square brackets.
>
> The rsyslog worker was joining the address to the port like this:
> fmt.Sprintf("%s:%d", host, port) which works fine for hostnames and IPv4
> addresses but does the wrong thing for IPv6 addresses. The right tool for
> the job is net.JoinHostPort which will do the right thing with all address
> types.
>
> I've fixed the rsyslog worker but have found a number of other places in
> the code that look like they could suffer from the same problem. Given that
> IPv6 adoption is increasing[1] we need to fix these areas before Juju's
> users encounter problems when using Juju with IPv6 networks. Here's what I
> found:
>
> provider/local/config.go:       return fmt.Sprintf("%s:%d",
> c.bootstrapIPAddress(), c.storagePort())
> provider/manual/config.go:      return fmt.Sprintf("%s:%d",
> c.bootstrapHost(), c.storagePort())
> provider/manual/config.go:      return fmt.Sprintf("%s:%d",
> c.storageListenIPAddress(), c.storagePort())
> state/address.go:               newAddrs[i] = fmt.Sprintf("%s:%d", addr,
> port)
> state/backups/backups_linux.go: memberHostPort := fmt.Sprintf("%s:%d",
> args.PrivateAddress, ssi.StatePort)
> state/backups/restore_test.go:  c.Assert(dialInfo.Addrs, gc.DeepEquals,
> []string{fmt.Sprintf("%s:%d", privateAddress, statePort)})
> state/backups/restore.go:       Addrs: []string{fmt.Sprintf("%s:%d",
> privateAddr, ssi.StatePort)},
> utils/ssh/ssh_gocrypto.go:      addr: fmt.Sprintf("%s:%d", host, port),
>
> Can people who are familiar with each of these areas please take a look,
> fixing and testing when appropriate.
>
> And for future work, please remember: just use net.JoinHostPort..
>
> - Menno
>
> [1] - on the public internet at least:
> https://www.google.com/intl/en/ipv6/statistics.html
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/juju-dev/attachments/20151119/e7ce7ca2/attachment.html>