From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <t.lamprecht@proxmox.com> 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)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id DE58076C9D for <pve-devel@lists.proxmox.com>; Wed, 20 Oct 2021 16:12:34 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id D0E0E12F48 for <pve-devel@lists.proxmox.com>; Wed, 20 Oct 2021 16:12:34 +0200 (CEST) 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)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 5637C12F3A for <pve-devel@lists.proxmox.com>; Wed, 20 Oct 2021 16:12:34 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 27D5045CDB for <pve-devel@lists.proxmox.com>; Wed, 20 Oct 2021 16:12:34 +0200 (CEST) Message-ID: <18c23b5b-6eb4-f1db-c507-f5b072a988d2@proxmox.com> Date: Wed, 20 Oct 2021 16:12:33 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:94.0) Gecko/20100101 Thunderbird/94.0 Content-Language: en-US To: Oguz Bektas <o.bektas@proxmox.com>, Dominik Csapak <d.csapak@proxmox.com>, Proxmox VE development discussion <pve-devel@lists.proxmox.com> References: <20211011105704.760773-1-o.bektas@proxmox.com> <20211011105704.760773-2-o.bektas@proxmox.com> <39f67e0b-143e-93f6-fd96-7b208b86a3ae@proxmox.com> <YXAHZKkSs38FtP/G@gaia> From: Thomas Lamprecht <t.lamprecht@proxmox.com> In-Reply-To: <YXAHZKkSs38FtP/G@gaia> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 1.332 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -2.267 Looks like a legit reply (A) 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 v4 firewall 1/2] implement fail2ban backend and API X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com> List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe> List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/> List-Post: <mailto:pve-devel@lists.proxmox.com> List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help> List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe> X-List-Received-Date: Wed, 20 Oct 2021 14:12:34 -0000 On 20.10.21 14:11, Oguz Bektas wrote: > On Tue, Oct 19, 2021 at 03:43:49PM +0200, Dominik Csapak wrote: >> while the code looks ok IMHO, i have some general questions: >> * does it really make sense to hard depend on fail2ban? >> could it not also make sense to have it as 'recommends' or 'suggests'? >> setting enabled to 1 could then check if its installed and >> raise an error >> >> * if we do not plan to add more fail2ban options in our config, >> i would rather see a combined fail2ban option (propertystring?) >> that would go into the general host firewall options >> >> that way we would not have to c&p the whole config parsing/setting api >> and could have a single new option line in the gui instead >> of a whole new panel with only 3 options (i think the majority of our >> users will not use fail2ban) > >> >> does that make sense to you? >> > > well if we hide it like that inside the menu, then surely nobody will > use it ;) > > separate panel makes it more visible IMO. it shouldn't be hidden 3 > layers deep in the options menu (Host -> Firewall -> Options -> Fail2ban > -> Enable) for such a simple feature, i think a lot more people would > use it if it's placed in a visible location (Host -> Fail2ban -> > Enable). if you really insist on putting it in the firewall options menu > then i'll have to insist for it to be installed & enabled by default :) With that arguing every option would need to be its own panel placed at the top-level.. PVE's interface is complex as is, sensible grouping simple one time option actually helps in UX and to find stuff, documentation can ensure features are found, they provide a central searchable place for that, after all.