From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 9120A1FF173 for ; Mon, 25 Nov 2024 10:05:45 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 80AB0D1C2; Mon, 25 Nov 2024 10:05:55 +0100 (CET) Date: Mon, 25 Nov 2024 10:05:22 +0100 (CET) From: =?UTF-8?Q?Fabian_Gr=C3=BCnbichler?= To: Thomas Lamprecht , Proxmox VE development discussion , Thomas Skinner Message-ID: <599388882.9673.1732525522446@webmail.proxmox.com> In-Reply-To: <348c5253-0595-42af-809e-82a86bff243e@proxmox.com> References: <1731496954.sd63rzj5wa.astroid@yuna.none> <20241124234938.2668814-1-thomas@atskinner.net> <348c5253-0595-42af-809e-82a86bff243e@proxmox.com> MIME-Version: 1.0 X-Priority: 3 Importance: Normal X-Mailer: Open-Xchange Mailer v7.10.6-Rev69 X-Originating-Client: open-xchange-appsuite X-SPAM-LEVEL: Spam detection results: 0 AWL 0.049 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH http-server 1/1] fix #5699: pveproxy: add library methods for real IP support 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: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" > Thomas Lamprecht hat am 25.11.2024 09:31 CET geschrieben: > > > Am 25.11.24 um 00:49 schrieb Thomas Skinner: > >> On September 10, 2024 2:30 am, Thomas Skinner wrote: > >>> --- > >>> src/PVE/APIServer/AnyEvent.pm | 43 ++++++++++++++++++++++++++++++++--- > >>> src/PVE/APIServer/Utils.pm | 15 ++++++++++++ > >>> 2 files changed, 55 insertions(+), 3 deletions(-) > >>> > >>> diff --git a/src/PVE/APIServer/AnyEvent.pm b/src/PVE/APIServer/AnyEvent.pm > >>> index a8d60c1..c2afb4d 100644 > >>> --- a/src/PVE/APIServer/AnyEvent.pm > >>> +++ b/src/PVE/APIServer/AnyEvent.pm > >>> @@ -85,20 +85,21 @@ sub log_request { > >>> > >>> my $loginfo = $reqstate->{log}; > >>> > >>> - # like apache2 common log format > >>> - # LogFormat "%h %l %u %t \"%r\" %>s %b \"%{Referer}i\" \"%{User-agent}i\"" > >>> + # like apache2 common log format + client IP address > >>> + # LogFormat "%a %h %l %u %t \"%r\" %>s %b \"%{Referer}i\" \"%{User-agent}i\"" > >> > >> this has the potential to break a lot of things analysing this log > >> file.. would it make sense to only do it conditionally (if a "real IP" is set)? > >> > >> or *just* log the "real IP" instead of the peerip (which would be > >> backwards-compatible as well)? > > > > I agree that it would break existing log analyzers. I also believe it's important > > to have the option to log the peer/proxy IP as well (in the event there are multiple > > proxies or that the proxy itself is the source). I'll change the formatting back to > > what was there previously and add an option to log the peer IP as well. > > > > Note that people have fail2ban setup on these logs, among other things, > and that breaking suddenly is _far_ from ideal. If always done it should > be on a major release, that avoids the headache of keeping (work for devs) > and detecting the existence of that option (work for the user). > > But not a must, we can also guard this with some option or the like now. yeah, we could switch to the new format *only* if the header option is set? as else, the two IPs are identical anyway, so logging the same one twice while breaking the format provides no benefit? and then maybe with 9.0 switch the format unconditionally, so that parsers/fail2ban configs only need to handle one of them going forward.. _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel