Re: [pbs-devel] [RFC backup 0/6] Two factor authentication

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

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:
> 
> 1. increase the length of recovery codes for bruteforce mitigation
> 
> most websites use 12-16 characters for the length of recovery keys.

makes sense

> 
> 2. do not store recovery codes in cleartext (hash them instead, we thought
> 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

> 
> 3. don't store all the tfa information in a single json file.
> 

makes no sense to me, any reason you mention below can happen to arbitrary
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 logins)
> - 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)
> 
> better is to at least create a file for each user:
> /etc/proxmox-backup/tfa/<username>.json or similar
> 
> 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.

> 
> 4. html/js injection in the description field on gui (fixed on staff
> repo already)
> 

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. Notification
can only work if the user has setup a mail in the first place - but yes, sou

> 
> 5.b also if recovery keys are available, limit amount of TOTP attempts
> for that user

what?