[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

[Stoiko Ivanov]

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

[pve-devel] [PATCH common 1/2] daemon: drop Domain parameter from create_reusable_socket

[Stoiko Ivanov]

The Domain parameter for IO::Socket::IP is not used/needed.
It is needed to create a IP Socket when calling IO::Socket->new,
but here we call IO::Socket::IP-new directly (see [0]).

[0] https://perldoc.perl.org/IO::Socket::IP
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PVE/Daemon.pm | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/PVE/Daemon.pm b/src/PVE/Daemon.pm
index 905635a..79b90ad 100644
--- a/src/PVE/Daemon.pm
+++ b/src/PVE/Daemon.pm
@@ -820,7 +820,6 @@ sub create_reusable_socket {
     } else {
 
 	$socket = IO::Socket::IP->new(
-	    Domain => PF_INET6,
 	    LocalHost => $host,
 	    LocalPort => $port,
 	    Listen => SOMAXCONN,
-- 
2.20.1

[pve-devel] [PATCH common 2/2] daemon: explicitly bind to wildcard address.

[Stoiko Ivanov]

with the recent change in pve-manager pveproxy (and spiceproxy)
try binding to '::' per default. This fails for hosts having disabled
ipv6 via kernel commandline.

Our desired behavior of binding on '::' and only falling back to
'0.0.0.0' in case this is not supported is not directly possible with
IO::Socket::IP->new (or rather by Socket::GetAddrInfo, which at least
on my system always returns the v4 wildcard-address first).

the code now binds to:
* the provided $host if not undef
* '::' if $host is not set
* '0.0.0.0' if $host is not set and binding on '::' yields an error

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PVE/Daemon.pm | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/src/PVE/Daemon.pm b/src/PVE/Daemon.pm
index 79b90ad..f4786b8 100644
--- a/src/PVE/Daemon.pm
+++ b/src/PVE/Daemon.pm
@@ -819,14 +819,24 @@ sub create_reusable_socket {
 	$socket->fcntl(Fcntl::F_SETFD(), Fcntl::FD_CLOEXEC);
     } else {
 
-	$socket = IO::Socket::IP->new(
-	    LocalHost => $host,
+	my %sockargs = (
 	    LocalPort => $port,
 	    Listen => SOMAXCONN,
 	    Proto  => 'tcp',
 	    GetAddrInfoFlags => 0,
-	    ReuseAddr => 1) ||
-	    die "unable to create socket - $@\n";
+	    ReuseAddr => 1,
+	);
+	if (defined($host)) {
+	    $socket = IO::Socket::IP->new( LocalHost => $host, %sockargs) ||
+		die "unable to create socket - $@\n";
+	} else {
+	    # disabling AF_INET6 (by adding ipv6.disable=1 to the kernel cmdline)
+	    # causes bind on :: to fail, try 0.0.0.0 in that case
+	    $socket = IO::Socket::IP->new( LocalHost => '::', %sockargs);
+
+	    $socket = IO::Socket::IP->new( LocalHost => '0.0.0.0', %sockargs) if $@;
+	    die "unable to create socket - $@\n" if !$socket;
+	}
 
 	# we often observe delays when using Nagle algorithm,
 	# so we disable that to maximize performance
-- 
2.20.1

[pve-devel] [PATCH manager 1/1] proxy: fix wildcard address use

[Stoiko Ivanov]

This patch fixes a regression for hosts disabling ipv6 via kernel
commandline ('ipv6.disable=1')introduced in commit
fc087ec2b924dc9c72d3bf80face8a1731c15405
(disabling IPv6 via sysctl did not exhibit these problems)

by hardcoding the address to '::', pveproxy and spiceproxy failed to
start with:
'unable to create socket - Address family not supported by protocol'

This patch depends on the commit in pve-common, which tries first
binding to '::' and then falling back to '0.0.0.0', and needs a
versioned dependency bump on libpve-common-perl.

With this patch the listening addresses are (`ss -tlnp |grep 8006` output)
* ipv6 disabled via kernel cmdline: '0.0.0.0:8006'
* sysctl net.ipv6.conf.all.disable_ipv6=1: '*:8006'
* sysctl net.ipv6.bindv6only=1: '[::]:8006'
* else: '*:8006'

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 PVE/Service/pveproxy.pm   | 2 +-
 PVE/Service/spiceproxy.pm | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/PVE/Service/pveproxy.pm b/PVE/Service/pveproxy.pm
index 4ecd442a..d10c4fe9 100755
--- a/PVE/Service/pveproxy.pm
+++ b/PVE/Service/pveproxy.pm
@@ -69,7 +69,7 @@ sub init {
     my $lockfh = IO::File->new(">>${accept_lock_fn}") ||
 	die "unable to open lock file '${accept_lock_fn}' - $!\n";
 
-    my $listen_ip = $proxyconf->{LISTEN_IP} // "::0";
+    my $listen_ip = $proxyconf->{LISTEN_IP};
     my $socket = $self->create_reusable_socket(8006, $listen_ip);
 
     my $dirs = {};
diff --git a/PVE/Service/spiceproxy.pm b/PVE/Service/spiceproxy.pm
index 24be0ed7..50b81c18 100755
--- a/PVE/Service/spiceproxy.pm
+++ b/PVE/Service/spiceproxy.pm
@@ -39,7 +39,7 @@ sub init {
     my $lockfh = IO::File->new(">>${accept_lock_fn}") ||
 	die "unable to open lock file '${accept_lock_fn}' - $!\n";
 
-    my $listen_ip = $proxyconf->{LISTEN_IP} // "::0";
+    my $listen_ip = $proxyconf->{LISTEN_IP};
     my $socket = $self->create_reusable_socket(3128, $listen_ip);
 
     $self->{server_config} = {
-- 
2.20.1

[pve-devel] [PATCH http-server 1/2] access control: correctly match v4-mapped-v6 addresses

[Stoiko Ivanov]

With recent changes to the listening socket code in pve-manager
the proxy daemons now usually bind to '::' and ipv4 clients are
read as v4-mapped-v6 addresses [0] from ::ffff:0:0/96.

This caused the allow_from/deny_from matching to break.

This patch addresses the issue by normalizing addresses from
::ffff:0:0/96 using Net::IP::ip_get_embedded_ipv4
(which roughly splits on ':' and checks if the last part looks like an
ipv4 address).

Issue was originally reported in our community forum [1]

[0] https://en.wikipedia.org/wiki/IPv6_address
[1] https://forum.proxmox.com/threads/my-pveproxy-file-doesnt-work.83228/
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 PVE/APIServer/AnyEvent.pm |  2 ++
 PVE/APIServer/Utils.pm    | 13 ++++++++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/PVE/APIServer/AnyEvent.pm b/PVE/APIServer/AnyEvent.pm
index 0654bd4..f0e2e68 100644
--- a/PVE/APIServer/AnyEvent.pm
+++ b/PVE/APIServer/AnyEvent.pm
@@ -34,6 +34,7 @@ use PVE::SafeSyslog;
 use PVE::INotify;
 use PVE::Tools;
 use PVE::APIServer::Formatter;
+use PVE::APIServer::Utils;
 
 use Net::IP;
 use URI;
@@ -1662,6 +1663,7 @@ sub wait_end_loop {
 sub check_host_access {
     my ($self, $clientip) = @_;
 
+    $clientip = PVE::APIServer::Utils::normalize_v4_in_v6($clientip);
     my $cip = Net::IP->new($clientip);
 
     if (!$cip) {
diff --git a/PVE/APIServer/Utils.pm b/PVE/APIServer/Utils.pm
index 36e3ae6..8470f80 100644
--- a/PVE/APIServer/Utils.pm
+++ b/PVE/APIServer/Utils.pm
@@ -34,7 +34,7 @@ sub read_proxy_config {
 	    my $ips = [];
 	    foreach my $ip (split(/,/, $value)) {
 		$ip = "0/0" if $ip eq 'all';
-		push @$ips, Net::IP->new($ip) || die Net::IP::Error() . "\n";
+		push @$ips, Net::IP->new(normalize_v4_in_v6($ip)) || die Net::IP::Error() . "\n";
 	    }
 	    $res->{$key} = $ips;
 	} elsif ($key eq 'LISTEN_IP') {
@@ -57,4 +57,15 @@ sub read_proxy_config {
     return $res;
 }
 
+sub normalize_v4_in_v6 {
+    my ($ip_text) = @_;
+
+    my $ip = Net::IP->new($ip_text) || die Net::IP::Error() . "\n";
+    my $v4_mapped_v6_prefix = Net::IP->new('::ffff:0:0/96');
+    if ($v4_mapped_v6_prefix->overlaps($ip)) {
+	return Net::IP::ip_get_embedded_ipv4($ip_text);
+    }
+    return $ip_text;
+}
+
 1;
-- 
2.20.1

[pve-devel] [PATCH http-server 2/2] access control: also include ipv6 in 'all'

[Stoiko Ivanov]

Net::IP objects are bound to a version - 0/0 is treated as ipv4 only.
If 'all' is present in the allow_from/deny_from list we should also
add ::/0 for matching all ipv6 addresses.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 PVE/APIServer/Utils.pm | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/PVE/APIServer/Utils.pm b/PVE/APIServer/Utils.pm
index 8470f80..449d764 100644
--- a/PVE/APIServer/Utils.pm
+++ b/PVE/APIServer/Utils.pm
@@ -33,7 +33,11 @@ sub read_proxy_config {
 	if ($key eq 'ALLOW_FROM' || $key eq 'DENY_FROM') {
 	    my $ips = [];
 	    foreach my $ip (split(/,/, $value)) {
-		$ip = "0/0" if $ip eq 'all';
+		if ($ip eq 'all') {
+		    push @$ips, Net::IP->new('0/0') || die Net::IP::Error() . "\n";
+		    push @$ips, Net::IP->new('::/0') || die Net::IP::Error() . "\n";
+		    next;
+		}
 		push @$ips, Net::IP->new(normalize_v4_in_v6($ip)) || die Net::IP::Error() . "\n";
 	    }
 	    $res->{$key} = $ips;
-- 
2.20.1

[pve-devel] [PATCH docs 1/3] pveproxy: add note about bindv6only sysctl

[Stoiko Ivanov]

Seems certain hosting environments (e.g. OVH) set net.ipv6.bindv6only
to 1, which caused problems for those users after the 6.4 upgrade.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 pveproxy.adoc | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/pveproxy.adoc b/pveproxy.adoc
index 8d02418..665e575 100644
--- a/pveproxy.adoc
+++ b/pveproxy.adoc
@@ -62,6 +62,9 @@ The default policy is `allow`.
 Listening IP
 ------------
 
+By default the `pveproxy` and `spiceproxy` daemons listen on the wildcard
+address and accept connections from both IPv4 and IPv6 clients.
+
 By setting `LISTEN_IP` in `/etc/default/pveproxy` you can control to which IP
 address the `pveproxy` and `spiceproxy` daemons bind. The IP-address needs to
 be configured on the system.
@@ -102,6 +105,12 @@ long-running worker processes, for example a running console or shell from a
 virtual guest. So, please use a maintenance window to bring this change in
 effect.
 
+NOTE: setting the `sysctl` `net.ipv6.bindv6only` to `1` will cause the daemons
+  to only accept connection from IPv6 clients. This non-default setting usually
+  also causes other issues. Either remove the `sysctl` setting, or set the
+  `LISTEN_IP` to `0.0.0.0` (which will only allow IPv4 clients).
+
+
 SSL Cipher Suite
 ----------------
 
-- 
2.20.1

[pve-devel] [PATCH docs 2/3] pveproxy: update documentation on 'all' alias

[Stoiko Ivanov]

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 pveproxy.adoc | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/pveproxy.adoc b/pveproxy.adoc
index 665e575..09ac5cf 100644
--- a/pveproxy.adoc
+++ b/pveproxy.adoc
@@ -45,7 +45,8 @@ POLICY="allow"
 ----
 
 IP addresses can be specified using any syntax understood by `Net::IP`. The
-name `all` is an alias for `0/0`.
+name `all` is an alias for `0/0` and `::/0` (meaning all IPv4 and IPv6
+addresses).
 
 The default policy is `allow`.
 
-- 
2.20.1

[pve-devel] [PATCH docs 3/3] network: shortly document disabling ipv6 support

[Stoiko Ivanov]

Given that quite a few HOWTOs on the internet suggest disabling ipv6
support via kernel commandline, which can cause quite many undesired
side-effects (e.g. ip6tables as used in pve-firewall errors out)
this patch adds a short section documenting, that disabling ipv6 is
not necessary usually and if needed better done via sysctl.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 pve-network.adoc | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/pve-network.adoc b/pve-network.adoc
index add220e..37667b8 100644
--- a/pve-network.adoc
+++ b/pve-network.adoc
@@ -548,6 +548,25 @@ iface vmbr0 inet manual
 
 ----
 
+Disabling IPv6 on the Node
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+{pve} works correctly in all environments, irrespective of whether IPv6 is
+deployed or not. We recommend leaving all settings at the provided defaults.
+
+Should you still need to disable support for IPv6 on your node, do so by
+creating an appropriate `sysctl.conf (5)` snippet file and setting the proper
+https://www.kernel.org/doc/Documentation/networking/ip-sysctl.txt[sysctls],
+for example adding `/etc/sysctl.d/disable-ipv6.conf` with content:
+
+----
+net.ipv6.conf.all.disable_ipv6 = 1
+net.ipv6.conf.default.disable_ipv6 = 1
+----
+
+This method is preferred to disabling the loading of the IPv6 module on the
+https://www.kernel.org/doc/Documentation/networking/ipv6.rst[kernel commandline].
+
 ////
 TODO: explain IPv6 support?
 TODO: explain OVS
-- 
2.20.1

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

[Oguz Bektas]

hi,

thank you for the fixes :)


tested the following to verify:
> 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 *)
>
and tested some scenarios also with ALLOW_FROM and LISTEN_IP.

it's also worth noting that disabling ipv6 in the commandline will
change the access.log format to show the standard IPv4 address instead
of the mapped v6 address.

Tested-by: Oguz Bektas <o.bektas@proxmox.com>

On Tue, May 04, 2021 at 12:12:14PM +0200, Stoiko Ivanov wrote:
> 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
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 

Re: [pve-devel] [PATCH common 2/2] daemon: explicitly bind to wildcard address.

[Wolfgang Bumiller]

On Tue, May 04, 2021 at 12:12:16PM +0200, Stoiko Ivanov wrote:
> with the recent change in pve-manager pveproxy (and spiceproxy)
> try binding to '::' per default. This fails for hosts having disabled
> ipv6 via kernel commandline.
> 
> Our desired behavior of binding on '::' and only falling back to
> '0.0.0.0' in case this is not supported is not directly possible with
> IO::Socket::IP->new (or rather by Socket::GetAddrInfo, which at least
> on my system always returns the v4 wildcard-address first).
> 
> the code now binds to:
> * the provided $host if not undef
> * '::' if $host is not set
> * '0.0.0.0' if $host is not set and binding on '::' yields an error
> 
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
>  src/PVE/Daemon.pm | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/src/PVE/Daemon.pm b/src/PVE/Daemon.pm
> index 79b90ad..f4786b8 100644
> --- a/src/PVE/Daemon.pm
> +++ b/src/PVE/Daemon.pm
> @@ -819,14 +819,24 @@ sub create_reusable_socket {
>  	$socket->fcntl(Fcntl::F_SETFD(), Fcntl::FD_CLOEXEC);
>      } else {
>  
> -	$socket = IO::Socket::IP->new(
> -	    LocalHost => $host,
> +	my %sockargs = (
>  	    LocalPort => $port,
>  	    Listen => SOMAXCONN,
>  	    Proto  => 'tcp',
>  	    GetAddrInfoFlags => 0,
> -	    ReuseAddr => 1) ||
> -	    die "unable to create socket - $@\n";
> +	    ReuseAddr => 1,
> +	);
> +	if (defined($host)) {
> +	    $socket = IO::Socket::IP->new( LocalHost => $host, %sockargs) ||
> +		die "unable to create socket - $@\n";
> +	} else {
> +	    # disabling AF_INET6 (by adding ipv6.disable=1 to the kernel cmdline)
> +	    # causes bind on :: to fail, try 0.0.0.0 in that case
> +	    $socket = IO::Socket::IP->new( LocalHost => '::', %sockargs);
> +
> +	    $socket = IO::Socket::IP->new( LocalHost => '0.0.0.0', %sockargs) if $@;

So apparently this doesn't *die* but just returns `undef` on error. But
we also should not rely on internal implementation details, so we cannot
assume that $@ will be cleared on success.
So we should probably bind the two calls together via `//` (or clear $@
before the first call?)

    $socket = IO::Socket::IP->new( LocalHost => '::', %sockargs)
        // IO::Socket::IP->new( LocalHost => '0.0.0.0', %sockargs);

> +	    die "unable to create socket - $@\n" if !$socket;
> +	}
>  
>  	# we often observe delays when using Nagle algorithm,
>  	# so we disable that to maximize performance
> -- 
> 2.20.1

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

[Wolfgang Bumiller]

On Tue, May 04, 2021 at 12:12:14PM +0200, Stoiko Ivanov wrote:
> 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

Other than my note on the common 2/2 patch the series LGTM.

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

[Thomas Lamprecht]

On 04.05.21 13:25, Oguz Bektas wrote:
> hi,
> 
> thank you for the fixes :)
> 
> 
> tested the following to verify:
>> 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 *)
>>
> and tested some scenarios also with ALLOW_FROM and LISTEN_IP.

Please list what scenarios you actually tested, else a T-b tag is not really
telling... I mean, you said you tested the patches you send too, but obv. not in
IPv6 disable setups, so having the actual list of things here can really help.

If unsure, check out how Dominic reports such things, those are always good,
concise but not leaving out interesting (test scenario/setup) details.

For example,
https://lists.proxmox.com/pipermail/pve-devel/2021-March/047375.html
https://lists.proxmox.com/pipermail/pve-devel/2021-April/047827.html

> 
> it's also worth noting that disabling ipv6 in the commandline will
> change the access.log format to show the standard IPv4 address instead
> of the mapped v6 address.

good note, could have been used in the new "Disabling IPv6 on the Node" docs
section Stoiko adds.

Updating https://pve.proxmox.com/wiki/Fail2ban could help too, or did you
already check if mapped notation works there too just fine with the config
proposal from the wiki?

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

[Oguz Bektas]

hi,

> > tested the following to verify:
> >> 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 *)
> >>
> > and tested some scenarios also with ALLOW_FROM and LISTEN_IP.
> 
> Please list what scenarios you actually tested, else a T-b tag is not really
> telling... I mean, you said you tested the patches you send too, but obv. not in
> IPv6 disable setups, so having the actual list of things here can really help.
> 
> If unsure, check out how Dominic reports such things, those are always good,
> concise but not leaving out interesting (test scenario/setup) details.
> 
> For example,
> https://lists.proxmox.com/pipermail/pve-devel/2021-March/047375.html
> https://lists.proxmox.com/pipermail/pve-devel/2021-April/047827.html
> 


i tested the /etc/default/pveproxy combined with the previously
mentioned scenarios and the following settings:


----
ALLOW_FROM="127.0.0.1"
DENY_FROM="all"
POLICY="allow"
----
----
LISTEN_IP="pve-dev-machine.proxmox.com"
ALLOW_FROM="127.0.0.1"
DENY_FROM="all"
POLICY="allow"
----

1.2.3.4 here is my workstation IP
----
LISTEN_IP="pve-dev-machine.proxmox.com"
ALLOW_FROM="1.2.3.4"
DENY_FROM="all"
POLICY="allow"
----

----
ALLOW_FROM="1.2.3.4"
DENY_FROM="all"
POLICY="allow"
----

to check i used:
$ systemctl restart pvedaemon pveproxy spiceproxy
$ ss -antlp | grep -E '(8006|3128)'

and the result match for the scenarios that stoiko mentioned.

to test ACLs from my workstation i used curl.

> > 
> > it's also worth noting that disabling ipv6 in the commandline will
> > change the access.log format to show the standard IPv4 address instead
> > of the mapped v6 address.
> 
> good note, could have been used in the new "Disabling IPv6 on the Node" docs
> section Stoiko adds.
> 
> Updating https://pve.proxmox.com/wiki/Fail2ban could help too, or did you
> already check if mapped notation works there too just fine with the config
> proposal from the wiki?


for fail2ban i followed the wiki steps, the configuration works also for
the v4 to v6 mapped addresses:

$ grep 'authentication failure' /var/log/daemon.log
May  5 11:17:08 pve-dev-machine pvedaemon[4120]: authentication failure;
rhost=1.2.3.4 user=root@pam msg=Authentication failure
May  5 11:19:08 pve-dev-machine pvedaemon[1831]: authentication failure;
rhost=::ffff:1.2.3.4 user=root@pam msg=Authentication failure

$ fail2ban-regex /var/log/daemon.log /etc/fail2ban/filter.d/proxmox.conf
...
Results
=======

Failregex: 2 total
|-  #) [# of hits] regular expression
|   1) [2] pvedaemon\[.*authentication failure; rhost=<HOST> user=.*
msg=.*
`-


all seems to work with both (dual stack) ipv6 and v4-only setups
(disabled via kernel cmdline and/or sysctl), i will update the fail2ban
page to mention the last test was with 6.4

cheers,
oguz