From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 4CCC98D44 for ; Tue, 7 Mar 2023 11:20:55 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 32A54A907 for ; Tue, 7 Mar 2023 11:20:55 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Tue, 7 Mar 2023 11:20:51 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 3D649416E0 for ; Tue, 7 Mar 2023 11:20:51 +0100 (CET) Date: Tue, 07 Mar 2023 11:20:43 +0100 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox VE development discussion References: <20230303172951.197711-1-m.carrara@proxmox.com> In-Reply-To: <20230303172951.197711-1-m.carrara@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.16.0 (https://github.com/astroidmail/astroid) Message-Id: <1678184417.d27uxjdwgm.astroid@yuna.none> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL -0.277 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_ASCII_DIVIDERS 0.8 Email that uses ascii formatting dividers and possible spam tricks KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [httpwg.org, mozilla.org, anyevent.pm, proxmox.com] Subject: [pve-devel] applied series: [PATCH v2 http-server 0/4] refactor HTTP request processing X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 07 Mar 2023 10:20:55 -0000 with some style follow-ups and slight rewording of the commit titles. On March 3, 2023 6:29 pm, Max Carrara wrote: > This patch series refactors the request processing algorithm and > also provides a patch for redirecting HTTP requests to HTTPS. >=20 > The refactor provided in the first two patches is much simpler than > the one made in v1[0], as it only moves the header processing, > authentication handling, and further request handling into two > separate subroutines, as suggested[1]. Contrary to v1[0], the > behaviour remains *actually* identical. >=20 > The third patch builds upon the refactor of the first two patches > and implements the enhancement proposed in #4494[2], adding another > subroutine which ensures that clients are connecting via TLS/SSL > (if the server itself uses it). >=20 > The fourth patch fixes some whitespace formatting issues scattered > across the file and may be dropped if not desired. >=20 >=20 > Testing > ------- >=20 > Each patch was tested separately on a VM which can not only be > accessed via its IP, but also via a FQDN that a DNS in my virtual > network is able to resolve. Using PVE (logging in, uploading ISOs, > accessing the VNC console, creating VMs, etc.) worked as it did before > each patch, regardless of whether the VM is accessed via its IP or > its FQDN. >=20 > The redirection from HTTP to HTTPS works consistently, irrespective > of which actions are performed. >=20 >=20 > Notes Regarding Security > ------------------------ >=20 > If a server uses TLS/SSL, such as pveproxy, every single request from > clients which have not initiated and successfully completed a TLS > handshake is answered with either a '301 Moved Permanently' or a > '308 Permanent Redirect'. >=20 > However: >> The user agent MAY use the Location field value for automatic >> redirection. The server's response content usually contains a >> short hypertext note with a hyperlink to the new URI(s). > - RFC9110, 15.4.2. [3] and 15.4.9. [4] >=20 > The client doesn't have to follow the `Location` field that's > provided by the server. They may therefore still send HTTP requests, > which in this case will just be answered with more 301s or 308s. > If such an HTTP request contains something like an authentication > cookie, said cookie could be stolen (e.g. a man-in-the-middle attack). > However, our authentication cookie[5] uses the Secure Attribute specified > in RFC 6265[6], so any compliant browser will include that cookie > only over HTTPS connections. >=20 > This can be verified e.g. via Firefox: >=20 > 1. Open the PVE web UI after applying patches 1-3 > 2. Open the Firefox Developer Tools using CTRL+SHIFT+I > 3. Navigate to the "Network" tab > 4. Inspect any secure request's header - you will find that the > `Cookie` header field includes the `PVEAuthCookie` > 5. Change `https://` of the domain of your PVE web UI to `http://` > and hit Enter > 6. Check the insecure request that you just forced Firefox to make > in the network tab - the `Cookie` header might not even be included > anymore, because the `PVEAuthCookie` isn't being used. >=20 > Therefore, as long as the user's browser is compliant, there shouldn't > be any security concerns in our case. >=20 >=20 > Further Considerations > ---------------------- >=20 > Despite all the above, we do not support HSTS[7] and SameSite Cookies[8] > yet, which we should implement eventually, since Firefox e.g. even > emits a warning regarding SameSite Cookies in the developer console: >=20 >> Cookie =E2=80=9CPVEAuthCookie=E2=80=9D does not have a proper =E2=80=9CS= ameSite=E2=80=9D attribute=20 >> value. Soon, cookies without the =E2=80=9CSameSite=E2=80=9D attribute or= with an >> invalid value will be treated as =E2=80=9CLax=E2=80=9D. This means that = the cookie >> will no longer be sent in third-party contexts. If your application >> depends on this cookie being available in such contexts, please add >> the =E2=80=9CSameSite=3DNone=E2=80=9C attribute to it. To know more abou= t the >> =E2=80=9CSameSite=E2=80=9C attribute, read https://developer.mozilla.org= /docs/Web/HTTP/Headers/Set-Cookie/SameSite >=20 >=20 > If required, I can implement both HSTS and SameSite cookies in a > follow-up series (if this one happens to get applied) or in a v3. >=20 >=20 >=20 > [0] https://lists.proxmox.com/pipermail/pve-devel/2023-February/055809.ht= ml > [1] https://lists.proxmox.com/pipermail/pve-devel/2023-February/055950.ht= ml > [2] https://bugzilla.proxmox.com/show_bug.cgi?id=3D4494 > [3] https://httpwg.org/specs/rfc9110.html#status.301 > [4] https://httpwg.org/specs/rfc9110.html#status.308 > [5] https://git.proxmox.com/?p=3Dpve-http-server.git;a=3Dblob;f=3Dsrc/PVE= /APIServer/Formatter.pm;h=3D20455a02704e579c18f0455abb66b88eb04098f7;hb=3DH= EAD#l95 > [6] https://httpwg.org/specs/rfc6265.html#secure-attribute > [7] https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Strict-Tran= sport-Security > [8] https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/= SameSite >=20 >=20 > Max Carrara (4): > anyevent: move header processing into separate subroutine > anyevent: move auth and request handling into separate subroutine > fix #4494: anyevent: redirect HTTP to HTTPS > anyevent: fix whitespace >=20 > src/PVE/APIServer/AnyEvent.pm | 451 ++++++++++++++++++++-------------- > 1 file changed, 271 insertions(+), 180 deletions(-) >=20 > --=20 > 2.30.2 >=20 >=20 >=20 > _______________________________________________ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel >=20