Add default feature for using public-ip to look up external IP address

[ssokolow]

I'm not familiar with your procedures, policies, and testing regime, so this is a proof-of-concept draft in those respects, but I thought I'd come with a discussable MVP when proposing to integrate one of the more convenient features of the "miniserve, but it's an image gallery" learning project I wrote ages ago, which has been sitting in a "dogfoodable, but not quite publicly releasable" state for ages.

This complements the default behaviour from double-clicking miniserve with using the public-ip crate to also give the externally visible IPv4 and IPv6 addresses.

(The screenshot only shows a redacted public IPv4 because I found that IPv6 support is flaky with my ISP and/or DSL modem and turned it off to solve "after a few hours/days, things like yt-dlp without Happy Eyeballs start failing.")

The draft PR adds a default public-ip feature flag which can be unset at build time, and a --no-public-ip command-line argument and MINISERVE_NO_PUBLIC_IP environment variable to disable it at runtime, but one thing I know I want feedback on is what set of feature flags for the public-ip crate you'd prefer turned off.

On a related note, there are three other complementary features (two implemented in my "useful learning project" and one planned) if you're interested:

First, when using --auth, my experimental project (but not this PR) will include user:pass@ in the displayed copy-pastable URLs feature. (The first set of credentials in the list would probably be a decent "don't overthink it"/YAGNI solution for implementing it for miniserve.)

Some chat systems with rich preview explictly say that returning 401 Unauthorized on a credential-less request is how you opt out of rich preview while, for others, letting them strip out the credentials and slam into it is just the least "one custom opt-out for each vendor" way to prevent side-channel leakage.

Second, for that use-case, my experimental project has an -r/--random-auth which will generate a random username and password which then gets picked up by the copy-pastable URL generation.

(The underline is a Konsole/Yakuake/KonsolePart on-hover thing for detected URLs. I wasn't paying attention to where my cursor was when taking that screenshot.)

The randomly generated passwords are constructed based on this rationale:

// Specify a human-friendly set of characters
// - Exclude i/I/l/1 and 0/O/o as too prone to misreads
// - Use only lowercase to make it easier and quicker to read over a voice-only channel
// - Omit digits to stay within a single mode of an iPhone's on-screen keyboard
//
// According to a quick Python `math.log2(choices**len)` check, assuming the RNG
// source is properly random, this will have slightly more entropy
// (50.43... vs. 47.63...) than the old "8 digits of alphanumeric".

The third complementary feature that I haven't written it yet (mainly because I have UPnP disabled in my router, so testing is more of a hassle), is UPnP integration to automatically forward and un-forward the claimed port so that my tool (and, if you're up for it, miniserve) can potentially be a "viable for non-techies" alternative to cloud services like Dropbox, MEGA, etc. for quickly sharing a file or two.

(Yeah, I'm one of those people who yearns for the days when the end-to-end principle was still respected.)

TL;DR: ...so yeah, four questions:

1. Do you want a feature like this and, if so, what do I need to revise to get it in?
2. Would you be up for presenting one of the --auth credentials in the copy-pastable URL output?
3. Assuming "yes" for 2, would you be receptive to that -r/--random-auth code for use-cases where the credentials don't matter as long as they're there to HTTP 401 rich preview bots?
4. Assuming I can make time to write it, would you be receptive to the UPnP port forwarding integration?

[ssokolow]

Oh, I just noticed that validate_printed_urls is failing.

I didn't realize those tests were there because, on the cargo test run I did on a fresh checkout, cargo test died early in bind_ipv4_ipv6::case_2 with "timeout waiting for port 33147" and I only just realized now that it seems to progress further each time I run it.

The second time, my PR failed in test auth_multiple_accounts_pass::case_1 with Failed to bind server to [::]:44629 caused by Address already in use (os error 98) and it took until the third attempt to reach validate_printed_urls... though it appears to have flipped back to bind_ipv4_ipv6::case_2 failing now.

Oh well. They're failing with ConnectionRefused on the 127.0.0.1 bindings and my Firefox has no problem loading http://127.0.0.1:8080 after cargo run -- .-ing my PR so, whatever is broken, I didn't do it.

[ssokolow]

OK, having looked at it a little more closely and filed #1514, I think I "did it" in one specific sense: By making program startup take slightly longer for validate_printed_urls, I made a pre-existing race condition trigger much more reliably.

[svenstaro]

So, I think this is cool but I'm definitely skeptical about this. We definitely need to talk about the implementation of the NAT hole punching part before we get more into it.

I think this implementation only really makes sense with NAT hole punching in it. I don't think UPnP should be the target here. Let's use STUN servers instead.

I also am not sure about the feature. It's not like people use this as a library. I have the tls feature because I need to disable tls on some architecture that have no support by ring. Just a CLI flag (off by default) might be the way to go here?

[svenstaro]

I think pulling this out makes sense even this PR doesn't make it in.

[ssokolow]

I think this implementation only really makes sense with NAT hole punching in it.

I use it frequently on my UPnP-less LAN as part of my "quickgallery" HTTP daemon and I honestly find it quite annoying that, whenever I want to use miniserve to share something with a friend that is excluded by quickgallery's "supported image files" filter (i.e. not an image), I have to manually -a gibberish:gibberish, then pop over to https://whatismyipaddress.com/ and then composite them together with the port to get a URL I can actually share as opposed to just feeding it -p 8001 -r (--port=8001 --random-auth) and copy-pasting a ready-made URL.

(On my system, I have persistent firewall rules and NAT forwards for "port 8001 is public, port 8002 is LAN, and anything else can be used for localhost-only".)

Given that I've always turned off UPnP support in my routers as a security thing, I see the automatic hole-punching as being more about novice-friendliness and less as a necessity.

I think this implementation only really makes sense with NAT hole punching in it. I don't think UPnP should be the target here. Let's use STUN servers instead.

I'll admit I missed that STUN had been extended to be for more than just UDP, but
You're going to need UPnP as a backup option either way.

Even the original "Simple Traversal of User Datagram Protocol (UDP) through Network Address Translators" meaning of STUN depends on the good graces of the NAT router.

Ages ago when I was using an old Pentium 3 450MHz running OpenBSD as a router, STUN wouldn't have worked because I set it up with "strict" connection tracking, where the connection tracking table would only have allowed the STUN server to reply to the outbound UDP packet, not arbitrary machines out on the net with which it shares the associated ephemeral port.

Beyond that, I remember confirming that a crate for UPnP requesting exists, but I haven't yet determined whether a STUN client for Rust exists which supports STUN for TCP, rather than just UDP-only STUN and I'm not currently willing to commit to taking on that level of an infrastructural project.

Just a CLI flag (off by default) might be the way to go here?

The reason I made it on by default with a build flag is because the people who need features like this the most are the people who just double-click the EXE and have possibly never used a command line, but I can certainly see many people who see on-by-default as a privacy or security smell.