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) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 835B0604F0 for <pbs-devel@lists.proxmox.com>; Wed, 2 Dec 2020 13:27:49 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 7122F1B486 for <pbs-devel@lists.proxmox.com>; Wed, 2 Dec 2020 13:27:49 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [212.186.127.180]) (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 id E54361B47B for <pbs-devel@lists.proxmox.com>; Wed, 2 Dec 2020 13:27:48 +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 B2CAF4472F for <pbs-devel@lists.proxmox.com>; Wed, 2 Dec 2020 13:27:48 +0100 (CET) To: Proxmox Backup Server development discussion <pbs-devel@lists.proxmox.com>, Oguz Bektas <o.bektas@proxmox.com> References: <20201119145608.16866-1-w.bumiller@proxmox.com> <20201202105650.GA7591@gaia.proxmox.com> From: Thomas Lamprecht <t.lamprecht@proxmox.com> Message-ID: <4c361a22-5caa-db5e-66b9-046638048fd5@proxmox.com> Date: Wed, 2 Dec 2020 13:27:47 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:84.0) Gecko/20100101 Thunderbird/84.0 MIME-Version: 1.0 In-Reply-To: <20201202105650.GA7591@gaia.proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Language: en-US Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL -0.076 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.001 Looks like a legit reply (A) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pbs-devel] [RFC backup 0/6] Two factor authentication X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion <pbs-devel.lists.proxmox.com> List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>, <mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe> List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/> List-Post: <mailto:pbs-devel@lists.proxmox.com> List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help> List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>, <mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe> X-List-Received-Date: Wed, 02 Dec 2020 12:27:49 -0000 Hi, thanks for taking a look, some comments regarding your feedback. On 02.12.20 11:56, Oguz Bektas wrote: > we talked with wolfgang off-list about some issues, so here are > some recommendations for the next version: >=20 > 1. increase the length of recovery codes for bruteforce mitigation >=20 > most websites use 12-16 characters for the length of recovery keys. makes sense >=20 > 2. do not store recovery codes in cleartext (hash them instead, we thou= ght > hmac-sha256 is fine). the reason being that recovery codes can bypass > other tfa methods so they shouldn't be visible make sense, would expect them to be hashed >=20 > 3. don't store all the tfa information in a single json file. >=20 makes no sense to me, any reason you mention below can happen to arbitrar= y files, so just adds complexity while not gaining anything. > current version uses a single /etc/proxmox-backup/tfa.json file > which holds all the tfa info for all the users. this is a single point > of failure because: > - file can be corrupted, causing tfa to break for everyone (no more log= ins) > - file could get deleted, disabling/bypassing 2fa for everyone > - file could get leaked in a backup etc., giving everyone's tfa secrets= > and/or recovery keys to attackers (bypass everything) >=20 > better is to at least create a file for each user: > /etc/proxmox-backup/tfa/<username>.json or similar >=20 > this way the damage is contained if for example the config breaks > because of incorrect deserialization etc. Why would deserialisation be incorrect for one single file but magically works if multiple files? Makes no sense. >=20 > 4. html/js injection in the description field on gui (fixed on staff > repo already) >=20 Yeah, as always, Ext.String.htmlEncode is your friend ;) > 5. notify user if more than X failed tfa attempts (password is already > compromised at this point, so it's important to notify) and block IP > for certain amount of time (fail2ban?) we do not setup fail2ban but any admin can already if wished. Notificatio= n can only work if the user has setup a mail in the first place - but yes, = sou >=20 > 5.b also if recovery keys are available, limit amount of TOTP attempts > for that user what?