[pve-devel] [PATCH common/manager/http-server/docs] v3] improve binding, docs and access-control for pveproxy/spiceproxy

[Stoiko Ivanov]

v2 -> v3:
* dropped the T-b tags
* Thanks to Thomas vigilant look and memory - added a patch to keep the behavior
  for pmgproxy as it currently is (listenaddress determined by the family
  returned by getaddrinfo on the nodename) - huge Thanks!!
** the patch is kept separate, as to be revertible cleanly once we can
   make the change in pmgproxy as well (and notify the users about the changed
   behavior in the appropriate places)
** quickly tested the version w/ and w/o this patch on my pmg to compare the
   behavior to previously - with it applied the results are consistent on
   my system.
* quickly tested the 'all' change to ALLOW_FROM/DENY_FROM on a system with
  ipv6 disabled - to verify it does not cause a 'Address family not supported'
  (or similar) error
* did not add the changed logging of IP-addresses when ipv6 is disabled on
  the kernel command-line - since it seems odd, to document a change that
  only happens if users are not following the recommendation

original cover-letter for v2:
v1 -> v2:
* incorporated Wolfgangs feedback regarding not checking for $@ but rather
  for definedness of the socket
* added Oguz Tested-By tags (Thanks for testing!) to the common/manager/
  http-server patches

original cover-letter for the v1:
this series is based on the RFC 'use appropriate wildcard address
for pveproxy/spiceproxy' I sent some time ago:
https://lists.proxmox.com/pipermail/pve-devel/2021-April/047988.html

changes from the RFC:
* incorporate Wolfgang's excellent feedback - huge Thanks!
  (or what I took away from it):
** instead of calling getaddrinfo a few additional times and sifting through
   the results simply doing in create_reusable_socket, what we want to do:
   * if not listen-address is provided try to bind to '::' and only if this
   fails (due to ipv6-disablement via kernel commandline), bind to '0.0.0.0'
** the PF_INET6 parameter added to the IO::Socket::IP->new call was unnecessary
   and misleading - I dropped it
* one of the original reporters of the bind-problems also created a thread in
  our community forum about the acls (ALLOW_FROM/DENY_FROM) not working anymore
  when set in /etc/default/pveproxy [0] - the patches for pve-http-server
  address the issue (at least in my tests)
* the 'all' ACL entry only matched IPv4 addresses, the second patch for
   pve-http-server changes this.
* added 3 documentation patches - mostly for the changed behavior, although
  the disabling ipv6 section in pve-networking.adoc is meant as an RFC
  (I just noticed that we have not official docs, and that too many HOWTOs
  suggest disabling it via kernel-cmdline, which I consider problematic)



[0] https://forum.proxmox.com/threads/my-pveproxy-file-doesnt-work.83228
original cover-letter for the RFC for reference:
The following patchset tries to address the small regression reported in our
forums [0,1], resulting from defaulting to '::' as listen-address in
pveproxy/spiceproxy.

The issue also affects proxmox-backup-proxy in PBS - and should this approach
be accepted I'll try to port it over to PBS as well.
(ftr: pmgproxy was not affected, since the patch for pmg-api was not applied)

In all cases the issue is only exhibited if ipv6 is diabled via kernel
commandline [2], not via sysctl [3].

* The patchset keeps the fix for pveproxy not starting if the /etc/hosts entry
  is not matching with a configured IP-address (I noticed and was pleasantly
  surprised while testing a v6only host and forgetting to set the entry)

I tested it in the following scenarios:
* ipv6 disabled via kernel commandline (listen on 0.0.0.0)
* ipv6 disabled via sysctl (listen on 0.0.0.0)
* no settings dual-stacked (listen on *)
* no settings v6 only (listen on *)

AFAICT listening on :: as long as possible is the best option, since it
makes the service available on all address-families (doing away, with
having a v4 only /etc/hosts entry, but a DNS AAAA record pointing to
the node for external access).

Took a quick look at how sshd [4,5] handles this (in the assumption that
they have to get it as right as possible), but it listens on multiple
sockets, something which I'd like to avoid for our proxy-daemons.

Sending as RFC, because whenever I come near getaddrinfo/getnameinfo I'm
certain to miss quite a few common cases.

[0] https://forum.proxmox.com/threads/connection-refused-595-nach-update-auf-pve-6-4.88347/#post-387034
[1] https://forum.proxmox.com/threads/ipv6-komplett-deaktivieren.88210/#post-387116
[2] https://www.kernel.org/doc/html/latest/networking/ipv6.html
[3] https://www.kernel.org/doc/html/latest/networking/ip-sysctl.html
[4] https://github.com/openssh/openssh-portable/blob/master/servconf.c
[5] https://github.com/openssh/openssh-portable/blob/master/sshd.c

pve-common:
Stoiko Ivanov (3):
  daemon: drop Domain parameter from create_reusable_socket
  daemon: explicitly bind to wildcard address.
  daemon: add compat code for pmgproxy 6.x

 src/PVE/Daemon.pm | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

pve-manager:
Stoiko Ivanov (1):
  proxy: fix wildcard address use

 PVE/Service/pveproxy.pm   | 2 +-
 PVE/Service/spiceproxy.pm | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

pve-http-server:
Stoiko Ivanov (2):
  access control: correctly match v4-mapped-v6 addresses
  access control: also include ipv6 in 'all'

 PVE/APIServer/AnyEvent.pm |  2 ++
 PVE/APIServer/Utils.pm    | 19 +++++++++++++++++--
 2 files changed, 19 insertions(+), 2 deletions(-)

pve-docs:
Stoiko Ivanov (3):
  pveproxy: add note about bindv6only sysctl
  pveproxy: update documentation on 'all' alias
  network: shortly document disabling ipv6 support

 pve-network.adoc | 19 +++++++++++++++++++
 pveproxy.adoc    | 12 +++++++++++-
 2 files changed, 30 insertions(+), 1 deletion(-)

-- 
2.20.1