this major release still needs to have an incompatible client, the next
one can drop setting a protocol client-side, and the one after that can
remove the protocol handling on the server side.
Stoiko Ivanov [Wed, 5 May 2021 14:36:27 +0000 (16:36 +0200)]
access control: also include ipv6 in 'all'
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.
Stoiko Ivanov [Wed, 5 May 2021 14:36:26 +0000 (16:36 +0200)]
access control: correctly match v4-mapped-v6 addresses
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]
Stefan Reiter [Thu, 22 Apr 2021 15:34:53 +0000 (17:34 +0200)]
allow stream download from path and over pvedaemon-proxy
Allow specifying a filepath for stream=1 instead of either a path or fh
with stream=1.
With this in place, we can also just return the path to the proxy in
case we want to stream a response back, and let it read from the file
itself. This way, the pvedaemon is cut out of the transfer pipe.
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
Stefan Reiter [Thu, 22 Apr 2021 15:34:52 +0000 (17:34 +0200)]
support streaming data form fh to client
Use an explicit AnyEvent::Handle similar to websocket proxying.
Needs some special care to make sure we apply backpressure correctly to
avoid caching too much data. Note that because of AnyEvent restrictions,
specifying a "fh" to point to a file or a packet-based socket may result
in unwanted behaviour[0].
Stefan Reiter [Wed, 21 Apr 2021 11:15:35 +0000 (13:15 +0200)]
allow 'download' to be passed from API handler
PVE::HTTPServer in pve-manager wraps the API return value in a 'data'
element, look for a 'download' element there too to allow an API call to
instruct the HTTP server to return a file via path or filehandle.
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
Stoiko Ivanov [Thu, 10 Dec 2020 14:02:50 +0000 (15:02 +0100)]
accept-phase: shutdown socket on early error
if an error happens before AnyEvent::Handle registers the cleanup
callback, we should shutdown/close the socket, when handling it.
Using close, instead of shutdown($sock, SHUT_WR) here, since we are in
an error-state, and would not read from the socket anyways.
(Additionally close sends just on packet (RST,ACK), vs shutdown
(FIN,ACK+RST,ACK) in its use here).
Stoiko Ivanov [Thu, 10 Dec 2020 14:02:49 +0000 (15:02 +0100)]
accept-phase: fix conn_count "leak"
When handling new connections in 'accept_connections' the number of
active connections (conn_count) got increased before the callback, which
would eventually decrease it got registered in AnyEvent::Handle->new.
Any error/die before registering the callback would skip the
decrement, and leave the process in an endless loop upon exiting in
wait_end_loop.
This can happen e.g. when the call to getpeername fails, or if the
connection is denied by the ALLOW_FROM/DENY_FROM settings in
'/etc/default/pveproxy' (which is also a simple reproducer for that).
Additionally it can cause a denial of service, by attempting to
connect from a denied ip until the connection count exeeds the maximum
connections of all child-processes.
This patch addresses the issue by incrementing the connection count
before attempting to create the handle, and decrementing it again, if
handle creation fails.
A warning is logged if 'conn_count' turns negative when decrementing
during cleanup on error/eof. In case creating a new handle during
initial accept_connection fails, a warning is logged as well, but
'conn_count' is not decremented.
Reported via our community-forum:
https://forum.proxmox.com/threads/pveproxy-eats-available-ram.79617/
increase max headers to 64 to cope with modern browsers + proxy combinations
This is mostly a "do not allow infinity headers" limit in the sense
of "it's good to have limits". With modern browsers and users behind
proxies we may actually get over 30 headers, so increase it for now
to 64 - hopefully enough for another decade ;)
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com> Reported-by: Victor Hooi <victorhooi@yahoo.com>
> The Ping frame contains an opcode of 0x9.
> [...]
> The Pong frame contains an opcode of 0xA.
-- Section 5.5.2 cf. https://tools.ietf.org/html/rfc6455#section-5.5.2
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Tim Marx [Tue, 21 Jan 2020 12:54:19 +0000 (13:54 +0100)]
allow ticket in auth header as fallback
based on idea & RFC by Tim Marx, incorporating feedback by Thomas
Lamprecht. this will be extended to support API tokens in the
Authorization header as well, so make it generic.
Thomas Lamprecht [Fri, 25 Oct 2019 15:34:27 +0000 (17:34 +0200)]
Revert "tls: make dh to openssl 1.1 compatible"
The libanyevent-perl version 7.140-3 included a fix for this.
It migrated to the then still testing (buster was not yet released)
on 07.04.2019, and so we can safely revert this workaround again
here.
Albeit this was fixed since Buster was officially released, still
bump the version dependency to libanyevent-perl in debian/control.
A future libanyevent-perl will use "ffdhe3072" for DH; another good
reason to revert this, to not keep hardcoded parameters with possible
(future) security implications here.
Dominik Csapak [Thu, 10 Oct 2019 09:21:51 +0000 (11:21 +0200)]
AnyEvent: extend send_file with content-type
for pmg, we need to send temporary files (for the attachment quarantine),
but we cannot know beforehand what content-type it is, so we
optionally give it to send_file_start
we give a hash with the open filehandle and the content-type in that case
this also removes the unnecessary open on the filename, since we open
it in send_file_start anyway...
Avoids syslog/journal warning like:
> Use of uninitialized value $v in substitution (s///) at
> /usr/share/perl5/PVE/APIServer/AnyEvent.pm line 648.
If one passes a "value-less" GET argument to a request, e.g.,
GET /?debug
Besides the fact that this allows us to even use such arguments it
also is a general improvement against a slight "syslog DOS attack",
because anybody can pass such parameters to the '/' page, and all
proxmox daemons providing a API/UI using libpve-http-server-perl
allow to do such requests unauthenticated (which itself is OK, as
else one could not show the login window at all). As each of such
request produces two log lines in the syslog/journal it's far from
ideal.
A simple reproducer of the possible outcome can be seen with the
following shell script using curl:
> PVEURL='127.0.0.1'
> ARGS='?a'; # send multiple args at once to amplify the per-connection cost
> for c in {a..z}; do for i in {0..9}; do ARGS="$ARGS&$c$i"; done; done
> while true; do curl --insecure --silent --output /dev/null "https://$PVEURL:8006$ARGS"; done
Not really bad, but not nice either, as logging is not too cheap this
has some resource usage cost and noise in the syslog is never nice.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Wed, 22 May 2019 06:07:17 +0000 (08:07 +0200)]
anyevent: rpcenv is optional and from our child instance
This fixes the simple-demo, which was regressed with commit 8782148642e4a09c5852781ec057017cc1145f17 were we falsely assumed that
we always have an rpcenv instance here, but actually that's just
optional as it comes from our child class instance
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Tue, 21 May 2019 19:37:12 +0000 (21:37 +0200)]
buildsys: fix dependencies for bootstrap files
avoid removing and unzipping the bootstrap source archive as many
times as ${BTDATA} has file entries, add an intermediate target for
the directory, which is the producer for all those BTDATA files, and
that directory then depends on the zip archive.
I mean, it would be even better to just use the libjs-bootstrap
package (and jQuery for that matter) but that a little bigger change
for now
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Fri, 22 Mar 2019 09:32:46 +0000 (10:32 +0100)]
tls: make dh to openssl 1.1 compatible
Effective the same approach used in libanyevent-perl 7.140-3[0]
Stretch is also compatible with this, and we can remove it for
buster/PVE 6 once the libanyevent-perl package transitioned to
from unstable to buster, until then do it ourself to have a
functioning api/proxy...
Stoiko Ivanov [Fri, 22 Feb 2019 18:51:59 +0000 (19:51 +0100)]
move read_proxy_conf from PVE::API2Tools to new Utils module
move the read_proxy_conf method into a new perl module
'PVE::APIServer::Utils'.
It now takes the proxy_name (e.g. pveproxy, pmgproxy) as variable to be used
for the configfile location (/etc/default/$proxy_name)
This serves as preparation to make pmgproxy configurable in the same way as
pveproxy.
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Stoiko Ivanov [Thu, 21 Feb 2019 09:35:11 +0000 (10:35 +0100)]
Don't override explicit $nocomp with default
By making compression configurable the $nocomp flag in response got set to
the configured (or default) setting, irrespective of the explicitly passed
value to response.
Stoiko Ivanov [Fri, 15 Feb 2019 11:36:00 +0000 (12:36 +0100)]
Add configurable 'compression'
Rationale for disabling compression is the potential for being affected by
the BREACH (CVE-2013-3587) attack and it's considered good practice for https
configuration (see e.g. [0]).
The default remains: to have compression enabled for compressible file-types.
commit a4d8bbafbe400be78bebeab169963025dc46e29b
introduced an additional empty line after '200 OK'
for remote-viewer 7 to work, but we also have to read this line
in our own proxy reader else the connection to a remote node does
not work
Dominik Csapak [Thu, 16 Aug 2018 12:48:12 +0000 (14:48 +0200)]
fix #1869: send correct http response in spice proxy
the glib implementation of the http proxy correctly checks the
http response (response code, followed by an empty line)
so we need to answer with the correct status
Dietmar Maurer [Wed, 6 Jun 2018 15:30:40 +0000 (17:30 +0200)]
websocket: set $max_payload_size = 128*1024; (131072)
AnyEvent checks rbuf_max after calling the callback (too late), so
we can receive larger data, because AnyEvent uses MAX_READ_SIZE=131072
to fill the buffer.
So a more elegant solution is to set $max_payload_size=128*1024. At least
I am not able to receive rbuf larger than 128*1024 now. But I keep the
protection from the previous patch - just to be sure.
René Jochum [Fri, 25 May 2018 16:15:22 +0000 (18:15 +0200)]
Fix #1684 WebSocket proxy behind a buffered proxy.
The given patch fixes incoming WebSocket traffic behind buffered Proxies
like NGINX.
NGINX buffers multiple requests from the Browser into one frame and sends that to pveproxy,
before this patch we then processed the first message of the frame and cleared the buffer which
may contained more messages.
With this patch we process each message and clear the buffer right.
This fixes the "NoVNC blank screen" problem users reported on the forums.
auth_handler: respond with passed error if we get a PVE::Exception
Allows to fix a problem where a logged in connected client was logged
out because we could not verify him for this call as the cluster
filesystem was unavailable.
If we get such a exception then use it for responding.
THis is save as no logged out client can get ever do anything where
login privileges are required and a logged in client cannot to
anything during the problematic period, but does not gets logged out.
Partail fix for #1589
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
projects / pve-http-server.git / log