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

[Thomas Lamprecht]

On 02.12.20 13:35, Oguz Bektas wrote:
> On Wed, Dec 02, 2020 at 01:27:47PM +0100, Thomas Lamprecht wrote:
>>> 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.
> 
> of course this can happen on arbitrary files...  i don't see why it
> would add any complexity to use multiple files though (actually makes it
> simpler imo). the reasoning behind this was to avoid a single point of
> failure like i explained:
> 
> multiple files for users -> only that user is affected by broken config,
> other users can log in
> single file for all users -> all users affected if config breaks and
> nobody can log in

See that almost as anti-feature, it's actually better if such a thing happens
that it's broken for all, as then one gets admin attention and can actually
look for the underlying root cause - which at that point is probably memory or
disk corruption/failure - or where does wolfgangs serializer breaks for all in one
but not for split??


> 
> so the point wasn't to magically fix (potential) incorrect deserialization but to
> reduce breakage in case something like that happens.


like "what" happens? There's no such thing as one serialization is fine and the
other not - if you start assuming that transient error model you cannot do anything
at all anymore!

I rather have it corrupt for all files as then the admin needs to fix it and we
get notified, as some "magic" bug that only happens if it's a Tuesday and full moon.

So no I do *not* want to have user.cfg, token.cfg, shadow.json with all info in
one file, and then start to split TFA for every user, because of an error model
which just assumes whatever one wishes.

>>
>>> 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
> yes, but imo 2fa is more sensitive to bruteforcing than regular
> passwords so it would make sense to limit it by default

why is it more sensitive? I need both, so it's the same? If I get leaked shadow
and tfa, I need to break both, only one has no use - that's the idea of TFA...


>>
>>>
>>> 5.b also if recovery keys are available, limit amount of TOTP attempts
>>> for that user
>>
>> what?
>>
> 
> if a user sets up TOTP + recovery keys, then it would make sense to lock
> account in case of a lot of auth attempts with TOTP, until recovery key
> is entered (afaik this is a common mechanism). but maybe just
> notifying the user is enough as well.

and why do you place more trust onto the fixed recovery keys than another TFA
option? Which services/programs/websites do that, can you name a few examples?