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

* [pve-devel] [PATCH common/manager/http-server/docs] improve binding, docs and access-control for pveproxy/spiceproxy
@ 2021-05-04 10:12 Stoiko Ivanov
  2021-05-04 10:12 ` [pve-devel] [PATCH common 1/2] daemon: drop Domain parameter from create_reusable_socket Stoiko Ivanov
                   ` (9 more replies)
  0 siblings, 10 replies; 14+ messages in thread
From: Stoiko Ivanov @ 2021-05-04 10:12 UTC (permalink / raw)
  To: pve-devel

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 (2):
  daemon: drop Domain parameter from create_reusable_socket
  daemon: explicitly bind to wildcard address.

 src/PVE/Daemon.pm | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 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

^ permalink raw reply	[flat|nested] 14+ messages in thread