public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH common/manager/http-server/docs] v2] improve binding, docs and access-control for pveproxy/spiceproxy
@ 2021-05-04 17:00 Stoiko Ivanov
  2021-05-04 17:00 ` [pve-devel] [PATCH common v2 1/2] daemon: drop Domain parameter from create_reusable_socket Stoiko Ivanov
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Stoiko Ivanov @ 2021-05-04 17:00 UTC (permalink / raw)
  To: pve-devel

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 (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(-)

-- 
2.20.1





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

* [pve-devel] [PATCH common v2 1/2] daemon: drop Domain parameter from create_reusable_socket
  2021-05-04 17:00 [pve-devel] [PATCH common/manager/http-server/docs] v2] improve binding, docs and access-control for pveproxy/spiceproxy Stoiko Ivanov
@ 2021-05-04 17:00 ` Stoiko Ivanov
  2021-05-04 17:00 ` [pve-devel] [PATCH common v2 2/2] daemon: explicitly bind to wildcard address Stoiko Ivanov
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Stoiko Ivanov @ 2021-05-04 17:00 UTC (permalink / raw)
  To: pve-devel

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

Tested-by: Oguz Bektas <o.bektas@proxmox.com>
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





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

* [pve-devel] [PATCH common v2 2/2] daemon: explicitly bind to wildcard address.
  2021-05-04 17:00 [pve-devel] [PATCH common/manager/http-server/docs] v2] improve binding, docs and access-control for pveproxy/spiceproxy Stoiko Ivanov
  2021-05-04 17:00 ` [pve-devel] [PATCH common v2 1/2] daemon: drop Domain parameter from create_reusable_socket Stoiko Ivanov
@ 2021-05-04 17:00 ` Stoiko Ivanov
  2021-05-04 17:00 ` [pve-devel] [PATCH manager v2 1/1] proxy: fix wildcard address use Stoiko Ivanov
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Stoiko Ivanov @ 2021-05-04 17:00 UTC (permalink / raw)
  To: pve-devel

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

Tested-by: Oguz Bektas <o.bektas@proxmox.com>
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PVE/Daemon.pm | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/src/PVE/Daemon.pm b/src/PVE/Daemon.pm
index 79b90ad..2ab4f35 100644
--- a/src/PVE/Daemon.pm
+++ b/src/PVE/Daemon.pm
@@ -819,14 +819,23 @@ 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) //
+		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





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

* [pve-devel] [PATCH manager v2 1/1] proxy: fix wildcard address use
  2021-05-04 17:00 [pve-devel] [PATCH common/manager/http-server/docs] v2] improve binding, docs and access-control for pveproxy/spiceproxy Stoiko Ivanov
  2021-05-04 17:00 ` [pve-devel] [PATCH common v2 1/2] daemon: drop Domain parameter from create_reusable_socket Stoiko Ivanov
  2021-05-04 17:00 ` [pve-devel] [PATCH common v2 2/2] daemon: explicitly bind to wildcard address Stoiko Ivanov
@ 2021-05-04 17:00 ` Stoiko Ivanov
  2021-05-04 17:00 ` [pve-devel] [PATCH http-server v2 1/2] access control: correctly match v4-mapped-v6 addresses Stoiko Ivanov
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Stoiko Ivanov @ 2021-05-04 17:00 UTC (permalink / raw)
  To: pve-devel

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'


Tested-by: Oguz Bektas <o.bektas@proxmox.com>
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





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

* [pve-devel] [PATCH http-server v2 1/2] access control: correctly match v4-mapped-v6 addresses
  2021-05-04 17:00 [pve-devel] [PATCH common/manager/http-server/docs] v2] improve binding, docs and access-control for pveproxy/spiceproxy Stoiko Ivanov
                   ` (2 preceding siblings ...)
  2021-05-04 17:00 ` [pve-devel] [PATCH manager v2 1/1] proxy: fix wildcard address use Stoiko Ivanov
@ 2021-05-04 17:00 ` Stoiko Ivanov
  2021-05-04 17:00 ` [pve-devel] [PATCH http-server v2 2/2] access control: also include ipv6 in 'all' Stoiko Ivanov
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Stoiko Ivanov @ 2021-05-04 17:00 UTC (permalink / raw)
  To: pve-devel

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/

Tested-by: Oguz Bektas <o.bektas@proxmox.com>
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





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

* [pve-devel] [PATCH http-server v2 2/2] access control: also include ipv6 in 'all'
  2021-05-04 17:00 [pve-devel] [PATCH common/manager/http-server/docs] v2] improve binding, docs and access-control for pveproxy/spiceproxy Stoiko Ivanov
                   ` (3 preceding siblings ...)
  2021-05-04 17:00 ` [pve-devel] [PATCH http-server v2 1/2] access control: correctly match v4-mapped-v6 addresses Stoiko Ivanov
@ 2021-05-04 17:00 ` Stoiko Ivanov
  2021-05-04 17:00 ` [pve-devel] [PATCH docs v2 1/3] pveproxy: add note about bindv6only sysctl Stoiko Ivanov
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Stoiko Ivanov @ 2021-05-04 17:00 UTC (permalink / raw)
  To: pve-devel

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.


Tested-by: Oguz Bektas <o.bektas@proxmox.com>
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





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

* [pve-devel] [PATCH docs v2 1/3] pveproxy: add note about bindv6only sysctl
  2021-05-04 17:00 [pve-devel] [PATCH common/manager/http-server/docs] v2] improve binding, docs and access-control for pveproxy/spiceproxy Stoiko Ivanov
                   ` (4 preceding siblings ...)
  2021-05-04 17:00 ` [pve-devel] [PATCH http-server v2 2/2] access control: also include ipv6 in 'all' Stoiko Ivanov
@ 2021-05-04 17:00 ` Stoiko Ivanov
  2021-05-04 17:00 ` [pve-devel] [PATCH docs v2 2/3] pveproxy: update documentation on 'all' alias Stoiko Ivanov
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Stoiko Ivanov @ 2021-05-04 17:00 UTC (permalink / raw)
  To: pve-devel

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





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

* [pve-devel] [PATCH docs v2 2/3] pveproxy: update documentation on 'all' alias
  2021-05-04 17:00 [pve-devel] [PATCH common/manager/http-server/docs] v2] improve binding, docs and access-control for pveproxy/spiceproxy Stoiko Ivanov
                   ` (5 preceding siblings ...)
  2021-05-04 17:00 ` [pve-devel] [PATCH docs v2 1/3] pveproxy: add note about bindv6only sysctl Stoiko Ivanov
@ 2021-05-04 17:00 ` Stoiko Ivanov
  2021-05-04 17:00 ` [pve-devel] [PATCH docs v2 3/3] network: shortly document disabling ipv6 support Stoiko Ivanov
  2021-05-05  5:25 ` [pve-devel] [PATCH common/manager/http-server/docs] v2] improve binding, docs and access-control for pveproxy/spiceproxy Thomas Lamprecht
  8 siblings, 0 replies; 10+ messages in thread
From: Stoiko Ivanov @ 2021-05-04 17:00 UTC (permalink / raw)
  To: pve-devel

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





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

* [pve-devel] [PATCH docs v2 3/3] network: shortly document disabling ipv6 support
  2021-05-04 17:00 [pve-devel] [PATCH common/manager/http-server/docs] v2] improve binding, docs and access-control for pveproxy/spiceproxy Stoiko Ivanov
                   ` (6 preceding siblings ...)
  2021-05-04 17:00 ` [pve-devel] [PATCH docs v2 2/3] pveproxy: update documentation on 'all' alias Stoiko Ivanov
@ 2021-05-04 17:00 ` Stoiko Ivanov
  2021-05-05  5:25 ` [pve-devel] [PATCH common/manager/http-server/docs] v2] improve binding, docs and access-control for pveproxy/spiceproxy Thomas Lamprecht
  8 siblings, 0 replies; 10+ messages in thread
From: Stoiko Ivanov @ 2021-05-04 17:00 UTC (permalink / raw)
  To: pve-devel

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





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

* Re: [pve-devel] [PATCH common/manager/http-server/docs] v2] improve binding, docs and access-control for pveproxy/spiceproxy
  2021-05-04 17:00 [pve-devel] [PATCH common/manager/http-server/docs] v2] improve binding, docs and access-control for pveproxy/spiceproxy Stoiko Ivanov
                   ` (7 preceding siblings ...)
  2021-05-04 17:00 ` [pve-devel] [PATCH docs v2 3/3] network: shortly document disabling ipv6 support Stoiko Ivanov
@ 2021-05-05  5:25 ` Thomas Lamprecht
  8 siblings, 0 replies; 10+ messages in thread
From: Thomas Lamprecht @ 2021-05-05  5:25 UTC (permalink / raw)
  To: Proxmox VE development discussion, Stoiko Ivanov

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(-)
> 





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

end of thread, other threads:[~2021-05-05  5:25 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-04 17:00 [pve-devel] [PATCH common/manager/http-server/docs] v2] improve binding, docs and access-control for pveproxy/spiceproxy Stoiko Ivanov
2021-05-04 17:00 ` [pve-devel] [PATCH common v2 1/2] daemon: drop Domain parameter from create_reusable_socket Stoiko Ivanov
2021-05-04 17:00 ` [pve-devel] [PATCH common v2 2/2] daemon: explicitly bind to wildcard address Stoiko Ivanov
2021-05-04 17:00 ` [pve-devel] [PATCH manager v2 1/1] proxy: fix wildcard address use Stoiko Ivanov
2021-05-04 17:00 ` [pve-devel] [PATCH http-server v2 1/2] access control: correctly match v4-mapped-v6 addresses Stoiko Ivanov
2021-05-04 17:00 ` [pve-devel] [PATCH http-server v2 2/2] access control: also include ipv6 in 'all' Stoiko Ivanov
2021-05-04 17:00 ` [pve-devel] [PATCH docs v2 1/3] pveproxy: add note about bindv6only sysctl Stoiko Ivanov
2021-05-04 17:00 ` [pve-devel] [PATCH docs v2 2/3] pveproxy: update documentation on 'all' alias Stoiko Ivanov
2021-05-04 17:00 ` [pve-devel] [PATCH docs v2 3/3] network: shortly document disabling ipv6 support Stoiko Ivanov
2021-05-05  5:25 ` [pve-devel] [PATCH common/manager/http-server/docs] v2] improve binding, docs and access-control for pveproxy/spiceproxy Thomas Lamprecht

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal