johnmehr / gitup

A minimalist, dependency-free FreeBSD program to clone/pull Git repositories.
BSD 2-Clause "Simplified" License
51 stars 9 forks source link

add IPv6 support (AF_INET6) #54

Closed fiebrandt closed 3 years ago

fiebrandt commented 3 years ago

since git.freebsd.org and giblab.com also run on ipv6 - it would be great to add support for it.

johnmehr commented 3 years ago

I just merged your pull request (thank you!). How does it look now?

fiebrandt commented 3 years ago

You're welcome! Tested with pure ipv6 jail again and works perfectly. I haven't found any other section in the code that is specific to protocol versioning.

michael-o commented 3 years ago

I wouldn't say that this is complete. For the hypothetical case that someone works with IPv6 literals only the target URI and the host header would be invalid because it is not bracketed.

michael-o commented 3 years ago

Similar case: https://github.com/apache/httpcomponents-core/pull/279

johnmehr commented 3 years ago

I just added a fix to remove brackets encountered in the host and proxy_host values (thank you!) How does it look?

michael-o commented 3 years ago

Just checked. It the other way around pure IPv6 literal js unambiguous. It is only a problem when a host comes into play. So I don't expect the code to remove brackets, but only add them where required. Spots I have mentioned before.

johnmehr commented 3 years ago

Sorry about that. I just reversed it to add brackets to IPv6 host addresses if they're not present. How does it look now?

michael-o commented 3 years ago

@johnmehr I will pick this up next week and test. Wenn run HTTPd via IPv6 and let you know.

michael-o commented 3 years ago

I tried to test, but I get a segmentation fault. Bisecting leads me to:

7244a7dbaa5277d3de388ad6aee6fa42aa5a9f90 is the first bad commit
commit 7244a7dbaa5277d3de388ad6aee6fa42aa5a9f90
Author: johnmehr <jmehr@umn.edu>
Date:   Wed Apr 21 22:04:25 2021 -0500

    Updated fix for Issue #54

 gitup.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 49 insertions(+), 12 deletions(-)

From lldb:

(lldb) bt all
* thread #1, name = 'gitup', stop reason = signal SIGSEGV
  * frame #0: 0x00000008007f23b7 libc.so.7`strchr(p=0x0000000000000000, ch=47) at strchr.c:48:7
    frame #1: 0x000000000020cf38 gitup`extract_proxy_data(connection=0x00007fffffff8700, data=<unavailable>) at gitup.c:2266:14
    frame #2: 0x00000000002087a0 gitup`main [inlined] load_configuration(connection=0x00007fffffff8700, configuration_file="./gitup.conf", argv=0x00007fffffffe9a8, argc=<unavailable>) at gitup.c:2494:2
    frame #3: 0x0000000000208645 gitup`main(argc=4, argv=0x00007fffffffe9a8) at gitup.c:2615
    frame #4: 0x0000000000205900 gitup`_start(ap=<unavailable>, cleanup=<unavailable>) at crt1.c:76:7
michael-o commented 3 years ago

I think the issue is obvious:

    if ((temp = strchr(copy, '@')) != NULL) {

but before we only have:

    char *copy = NULL, *temp = NULL, *server = NULL;

I can continue by unsettting HTTP_PROXY and HTTPS_PROXY.

johnmehr commented 3 years ago

I believe I've fixed it plus also tightened up the code when IPv6 hosts are used in environment variables. How does it look now?

edinilsonjs commented 3 years ago

After this implementation if IPv6 isn´t compiled in kernel or is disabled, the following error is raised in FreeBSD 13.0 RELEASE: gitup: connect_server: socket failure: Address family not supported by protocol family

michael-o commented 3 years ago

After this implementation if IPv6 isn´t compiled in kernel or is disabled, the following error is raised in FreeBSD 13.0 RELEASE: gitup: connect_server: socket failure: Address family not supported by protocol family

I don't understand that. IPv6 is never requested specifically. AF_UNSPEC is used. Can you show your gitup.conf and your network config. Unless you provide an IPv6 literal it should work as expected.

edinilsonjs commented 3 years ago

After this implementation if IPv6 isn´t compiled in kernel or is disabled, the following error is raised in FreeBSD 13.0 RELEASE: gitup: connect_server: socket failure: Address family not supported by protocol family

I don't understand that. IPv6 is never requested specifically. AF_UNSPEC is used. Can you show your gitup.conf and your network config. Unless you provide an IPv6 literal it should work as expected.

My gitup.conf is very simple and default:

$FreeBSD$

#

Default configuration options for gitup.conf.

{ "defaults" : { "host" : "git.freebsd.org", "port" : 443, "verbosity" : 1, "work_directory" : "/var/db/gitup", },

    "ports" : {

"host" : "github.com",

"repository" : "/freebsd/freebsd-ports.git",

            "repository" : "/ports.git",
            "branch"     : "main",
            "target"     : "/usr/ports",
            "ignores"          : [
                    "distfiles",
                    "packages",
                    "INDEX-11",
                    "INDEX-12",
                    "INDEX-13",
                    "INDEX-14",
            ],
    },

    "release" : {
            "repository" : "/src.git",
            "branch"     : "releng/13.0",
            "target"     : "/usr/src",
            "ignores"    : [
                    "sys/amd64/conf",
                    "sys/arm64/conf",
                    "sys/i386/conf",
                    "sys/pc98/conf",
                    "sys/powerpc/conf",
                    "sys/riscv/conf",
                    "sys/sparc64/conf",
            ]
    }

}

michael-o commented 3 years ago

Please run gitup the following way:

truss -o out gitup ...

and upload the out file. Thanks

edinilsonjs commented 3 years ago

gitup-truss.log.gz

michael-o commented 3 years ago

I think I know where your problem comes from. I see this:

socket(PF_INET6,SOCK_STREAM,IPPROTO_TCP)     ERR#47 'Address family not supported by protocol family'

Since the source code does not mention IPv6 at all, it must come from DNS:

$ host git.freebsd.org
git.freebsd.org is an alias for gitmir.geo.freebsd.org.
gitmir.geo.freebsd.org has address 139.178.72.204
gitmir.geo.freebsd.org has IPv6 address 2604:1380:2000:9501::e6a:1
gitmir.geo.freebsd.org mail is handled by 0 .

and the source code asks the OS to resolve the hostname with:

    bzero(&hints, sizeof(struct addrinfo));
    hints.ai_family = AF_UNSPEC;
    hints.ai_socktype = SOCK_STREAM;

    if ((error = getaddrinfo(host, type, &hints, &start)))
        errx(EXIT_FAILURE, "%s", gai_strerror(error));

AF_UNSPEC requests both A and AAAA records. There are now two situations:

So the best would be, if possible, check at runtime whether IPv6 is enabled on this system, if not use AF_INET instead of AF_UNSPEC.

@johnmehr WDYT?

edinilsonjs commented 3 years ago

I think I know where your problem comes from. I see this:

socket(PF_INET6,SOCK_STREAM,IPPROTO_TCP)   ERR#47 'Address family not supported by protocol family'

Since the source code does not mention IPv6 at all, it must come from DNS:

$ host git.freebsd.org
git.freebsd.org is an alias for gitmir.geo.freebsd.org.
gitmir.geo.freebsd.org has address 139.178.72.204
gitmir.geo.freebsd.org has IPv6 address 2604:1380:2000:9501::e6a:1
gitmir.geo.freebsd.org mail is handled by 0 .

and the source code asks the OS to resolve the hostname with:

  bzero(&hints, sizeof(struct addrinfo));
  hints.ai_family = AF_UNSPEC;
  hints.ai_socktype = SOCK_STREAM;

  if ((error = getaddrinfo(host, type, &hints, &start)))
      errx(EXIT_FAILURE, "%s", gai_strerror(error));

AF_UNSPEC requests both A and AAAA records. There are now two situations:

  • IPv6 is disabled at compile time in the base OS and this should be solve with a define, BUT
  • IPv6 can be disabled with rc.conf and it will likely give you no route to host or similar.

So the best would be, if possible, check at runtime whether IPv6 is enabled on this system, if not use AF_INET instead of AF_UNSPEC.

@johnmehr WDYT?

I think that gitup could permit -4 or -6 as parameter, like (see the ssh example): https://stackoverflow.com/questions/21671706/configure-git-to-use-ipv4-instead-of-ipv6-by-default

michael-o commented 3 years ago

@edinilsonjs That's one option to the story.

fiebrandt commented 3 years ago

on a IPv4 only system, getaddrinfo should return AF_INET first:

% getaddrinfo -f unspec -p tcp git.freebsd.org stream inet tcp 139.178.72.204 0 stream inet6 tcp 2604:1380:2000:9501::e6a:1 0

looking at 13-RELEASE, dual stack system seems to return INET6, that is just done on a quick test. My guess is, that some defaults got changed with the major release jump. So we have now:

% getaddrinfo -f unspec -p tcp git.freebsd.org stream inet6 tcp 2604:1380:2000:9501::e6a:1 0 stream inet tcp 139.178.72.204 0

However, if I configure a IPv4 only enabled jail, I get ipv4 returned first properly:

% getaddrinfo -f unspec -p tcp git.freebsd.org stream inet tcp 139.178.72.204 0 stream inet6 tcp 2604:1380:2000:9501::e6a:1 0

As a sidenote, we should consider to enforce TCP as desired protocol:

hints.ai_protocol = IPPROTO_TCP;

So before we fiddle with the code (to e.g. interate thru the returned array until we get a connection), would you mind to test again with ipv4 explicitly set? e.g.:

ip6addrctl_policy="prefer_ipv4"

and execute getaddrinfo -f unspec -p tcp git.freebsd.org what order you get? While I dislike "forcing" things, we might need switches like -4 / -6. Alternate (or default) we could try "both" and see what comes together.

edinilsonjs commented 3 years ago

on a IPv4 only system, getaddrinfo should return AF_INET first:

% getaddrinfo -f unspec -p tcp git.freebsd.org stream inet tcp 139.178.72.204 0 stream inet6 tcp 2604:1380:2000:9501::e6a:1 0

looking at 13-RELEASE, dual stack system seems to return INET6, that is just done on a quick test. My guess is, that some defaults got changed with the major release jump. So we have now:

% getaddrinfo -f unspec -p tcp git.freebsd.org stream inet6 tcp 2604:1380:2000:9501::e6a:1 0 stream inet tcp 139.178.72.204 0

However, if I configure a IPv4 only enabled jail, I get ipv4 returned first properly:

% getaddrinfo -f unspec -p tcp git.freebsd.org stream inet tcp 139.178.72.204 0 stream inet6 tcp 2604:1380:2000:9501::e6a:1 0

As a sidenote, we should consider to enforce TCP as desired protocol:

hints.ai_protocol = IPPROTO_TCP;

So before we fiddle with the code (to e.g. interate thru the returned array until we get a connection), would you mind to test again with ipv4 explicitly set? e.g.:

ip6addrctl_policy="prefer_ipv4"

and execute getaddrinfo -f unspec -p tcp git.freebsd.org what order you get? While I dislike "forcing" things, we might need switches like -4 / -6. Alternate (or default) we could try "both" and see what comes together.

In my case: -IPv6 isn´t compile in kernel (nooptions INET6 ); -In rc.conf: ipv6_network_interfaces="none" # Default is auto ipv6_activate_all_interfaces="NO" # this is the default ip6addrctl_policy="ipv4_prefer"

Follow the getaddrinfo: getaddrinfo -f unspec -p tcp git.freebsd.org stream inet6 tcp 2610:1c1:1:606c::50:6d 0 stream inet tcp 96.47.72.109 0

Just a note here: Which one is the correct? ip6addrctl_policy="prefer_ipv4" OR ip6addrctl_policy="ipv4_prefer"

fiebrandt commented 3 years ago

Thanks for your quick update. I am wondering why disabled IPv6 host return INET6 address first at all. I can't verify it right now, but from defaults/rc.conf:

ip6addrctl_enable="YES" # Set to YES to enable default address selection ip6addrctl_verbose="NO" # Set to YES to enable verbose configuration messages ip6addrctl_policy="AUTO" # A pre-defined address selection policy

(ipv4_prefer, ipv6_prefer, or AUTO)

it should be ipv4_prefer then, but not sure if that is applicable at all if kernel is compiled without INET6. With that, I think we found now the root case why. The current code of gitup relies on "AUTO" to prefer the intended protocol version.

edinilsonjs commented 3 years ago

Thanks for your quick update. I am wondering why disabled IPv6 host return INET6 address first at all. I can't verify it right now, but from defaults/rc.conf:

ip6addrctl_enable="YES" # Set to YES to enable default address selection ip6addrctl_verbose="NO" # Set to YES to enable verbose configuration messages ip6addrctl_policy="AUTO" # A pre-defined address selection policy

(ipv4_prefer, ipv6_prefer, or AUTO)

it should be ipv4_prefer then, but not sure if that is applicable at all if kernel is compiled without INET6. With that, I think we found now the root case why. The current code of gitup relies on "AUTO" to prefer the intended protocol version.

Do you need some others tests or something else from me? Do you plan to release a new version?

fiebrandt commented 3 years ago

Do you need some others tests or something else from me? Do you plan to release a new version?

would you mind to try out my forked version (fiebrandt/gitup) if that solves the problem? Its a first implementation to iterate over returned list to raise chances to succeed connecting; only briefly tested but "should" work

edinilsonjs commented 3 years ago

Do you need some others tests or something else from me? Do you plan to release a new version?

would you mind to try out my forked version (fiebrandt/gitup) if that solves the problem? Its a first implementation to iterate over returned list to raise chances to succeed connecting; only briefly tested but "should" work

With this new version, 0.92, it´s working!!! Attached logfile from truss. gitup-0.92.log.gz

fiebrandt commented 3 years ago

Thanks for confirming. I will work with johnmehr to see how we commit that into the main stream. The same approach helps in cases where both protocols are present but only one actual works.

edinilsonjs commented 3 years ago

Thanks for confirming. I will work with johnmehr to see how we commit that into the main stream. The same approach helps in cases where both protocols are present but only one actual works.

@fiebrandt and @johnmehr thank you very much and congratulations for this great tool

Best regards

Edinilson

johnmehr commented 3 years ago

I just merged the pull request from @fiebrandt (thank you!) How is it working now?

edinilsonjs commented 3 years ago

I just merged the pull request from @fiebrandt (thank you!) How is it working now?

Version 0.92 isn´t in ports yet:

gitup ports

Scanning local repository...

Host: git.freebsd.org

Port: 443

Repository Path: /ports.git

Target Directory: /usr/ports

Have: 074d1888974ae64e1bdaf4e46a8e7f967f2c2618

Want: 074d1888974ae64e1bdaf4e46a8e7f967f2c2618

Branch: main

Done.

cat /usr/ports/net/gitup/distinfo TIMESTAMP = 1619071830 SHA256 (johnmehr-gitup-0.91_GH0.tar.gz) = 571adff7dda8b09c70954d7e468aeee087d85bb49c4cbb24d53ab737cb1a0169 SIZE (johnmehr-gitup-0.91_GH0.tar.gz) = 24116

Regards

michael-o commented 3 years ago

I don't know who the port maintainer is, but I would recommend to convert to .../gitup-devel because of the current speed gitup changes...

fiebrandt commented 3 years ago

I don't know who the port maintainer is, but I would recommend to convert to .../gitup-devel because of the current speed gitup changes...

agreed; perhaps after committing the latest fix for 13.0 kernels without IPv6 compiled in. Now that AF_INET6 is supported, moving the issue to closure.