Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gethostbyname() does not support IPv6 #141

Open
ppisar opened this issue Oct 23, 2024 · 13 comments
Open

gethostbyname() does not support IPv6 #141

ppisar opened this issue Oct 23, 2024 · 13 comments
Labels
enhancement New feature or request

Comments

@ppisar
Copy link
Contributor

ppisar commented Oct 23, 2024

downloadfile() calls gethostbyname(). gethostbyname() does not suport IPv6. I recommend using gettaddrinfo() instead and iterating over all returned socket address until one succeeds in connect().

@fcorbelli
Copy link
Owner

This is the auto-update function.

It is designed for Windows, to automatically update the executable by running

zpaqfranz upgrade -force

This command essentially downloads a file from my website (www.francocorbelli.it) that contains the current version of the program (32, 64 and 64bit-HW-accelerated), and, if necessary (-force), the updated EXE program itself.

Clearly, this function is potentially risky, since downloading executables is always a delicate activity.

That's why I have restricted it to my site (even with http, NO https!)

string	http_url="http://www.francocorbelli.it/zpaqfranz/win64/zpaqfranz.sha256";

For non-Windows systems (*nix), no executable or source code is downloaded, only the header file with the current version is fetched—again, from my website.

Essentially, this is NOT a sort of "universal" wget (that can be used from some rogue software), but simply a way to retrieve data from my site.

You'll see in the source code the function sanitizeline(), which is specifically designed—albeit a bit naive—to ensure that no "suspicious" files are downloaded, such as executables or even scripts (in case my website gets compromised, you never know). The idea is to be reasonably sure (of course, not 100%—nothing is entirely safe in life) that it can't be easily used as a sort of backdoor. On Windows, it's different, but we know that on Windows it's common to download .EXE files, and there's not much that can be done about that.

Therefore, to avoid potential problems on multi-platform systems that don't support IPv6 (yes, I could use #ifdef, but that would be a lot of work), I'd prefer to keep the current simplified version.

However, if this is an essential requirement, I can work on it. What do you think?

@fcorbelli fcorbelli added the enhancement New feature or request label Oct 23, 2024
@ppisar
Copy link
Contributor Author

ppisar commented Oct 24, 2024

It's not an essential feature, especially if your host is IPv4-only. (Though in some countries, mobile networks are becoming IPv6-only.)

I also noticed how insecure is the download process. You should think about signing the files and placing a public key into the executable.

@fcorbelli
Copy link
Owner

In the Windows world, it is entirely normal to download executable files.
The digital signature is essentially useless (again, in the Windows world), except for causing problems.
I could digitally sign with legal value in Italy (I have the necessary device), but it would only be valid for Italy.

The files downloaded from GitHub, SourceForge, or other sites are not digitally signed in any way.

The download procedure is similar to wget or curl.

Moreover, there is no self-update procedure (like Windows Update) in zpaqfranz.

The user must explicitly request it from the command line, even with a switch.
Regarding the exclusive use of IPv6 in certain places, I may improve this aspect in the future.

@fcorbelli
Copy link
Owner

PS If you know of some kind of IPv6-only site that can host pages for free (one is enough, of course, for testing) please let me know. So I can proceed in this direction

@fcorbelli
Copy link
Owner

Becaus IPv6 is not so widespread, at least in Italy

1

@fcorbelli
Copy link
Owner

2

ppisar added a commit to ppisar/zpaqfranz that referenced this issue Oct 25, 2024
gethostbyname() is deprecated by getaddrinfo().  The use of
gethostbyname() triggers warnings in quality checks in Fedora
distribution.

getaddrinfo() is defined in RFC 2553 and in POSIX.1-2001.

For compatibility, I guarded its use with a non-default IPV6 macro.
But since getaddrinfo() is already exclusively used in SERVER code,
I think it would make sense to make it default. If you indeed want to
enforce IPv4, you can change AF_UNSPEC to AF_INET.

getaddrinfo() has benefits even without IPv6: It supports multi-homed
servers when a single hostname has associcated multiple IP addresses.

fcorbelli#141
@ppisar
Copy link
Contributor Author

ppisar commented Oct 25, 2024

I posted a pull request #142 witch replaces the gethostbyname() call in a very conservative manner.

I noticed that you already uses getaddrinfo() in SERVER code, so maybe the proposed use of getaddrinfo() could be made default.

I don't have any IPv6-only server, but I have dual-stack server. I can place a file there if your want. For IPv6-only, I would have to setup another domain. But I don't how beneficial that would be for your testing. You can simply run a web server on localhost and add/remove domain names in /etc/hosts and block/unblock IPv4 or IPv6 connections with a firewall.

@fcorbelli
Copy link
Owner

I merge the change into the latest version, but I get quite a few warnings for using too advanced version of the compiler. I do some work on it

warning: C++ designated initializers only available with ‘-std=c++2a’ or ‘-std=gnu++2a’ [-Wpedantic]

@fcorbelli
Copy link
Owner

OK, changed to old-style version.
In a couple of hours I will re-certify the compilability across various platforms

@fcorbelli
Copy link
Owner

Does not compile on OpenBSD, NetBSD etc with -DIPV6

... work in progress ...

@fcorbelli
Copy link
Owner

OK fixed cutting

    #ifndef _POSIX_C_SOURCE
        #define _POSIX_C_SOURCE 200112L
    #endif

@fcorbelli
Copy link
Owner

1
I added a marker (4 or 6) to easily indentify the executables. The current loaded version should be OK

@fcorbelli
Copy link
Owner

I am trying to activate IPv6 for my domain, but I am not promising anything.

DNS seems to be working, but the network is not reachable

😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants