[pbs-devel] [PATCH proxmox-backup] auth: add locking to `PbsAuthenticator` to avoid race conditions

Wolfgang Bumiller w.bumiller at proxmox.com
Mon Jun 3 11:00:51 CEST 2024


On Thu, May 23, 2024 at 01:46:47PM GMT, Gabriel Goller wrote:
> On 23.05.2024 13:25, Shannon Sterz wrote:
> > currently we don't lock the shadow file when removing or storing a
> > password. by adding locking here we avoid a situation where storing
> > and/or removing a password concurrently could lead to a race
> > condition. in this scenario it is possible that a password isn't
> > persisted or a password isn't removed. we already do this for
> > the "token.shadow" file, so just use the same mechanism here.
> > 
> > Signed-off-by: Shannon Sterz <s.sterz at proxmox.com>
> 
> Is there any reason why the store_password function does not lock the
> shadow.json file?

I'm assuming you mean this as an alternative to having a separate file
for the locking. In this case the answer is yes: We use `repalce_file()`
to store the new version to make sure the update is atomic (and a
parallel `authenticat_user` does not read a *partial* file and fail).
When you hold a lock on a file and you *replace* it, the new file is a
separate thing with no lock on it. Old threads waiting for a lock on the
old file will continue when the lock is free, new threads waiting on the
lock on the new file will be able to acquire that simultaneously.

Iow.: you can't lock something you're *replacing*.




More information about the pbs-devel mailing list