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

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

On 04.05.21 19:00, Stoiko Ivanov wrote:
> 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

but at least the changed common patch was not tested?!

Please do not do that for patches you alter, makes no sense...

> 
> 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 (2):
>   daemon: drop Domain parameter from create_reusable_socket
>   daemon: explicitly bind to wildcard address.
> 
>  src/PVE/Daemon.pm | 18 +++++++++++++-----
>  1 file changed, 13 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(-)
>