api: list datastore: avoid iterating over NS for priv check, use AclTree
Make the assumption that if a user has any privilege that would make
an NS and (parts) of its content visible they also should be able to
know about the datastore and very basic errors on lookup (path
existence and maintenance mode) even if that NS doesn't even exists
(yet), as they could, e.g., make or view a backup and find out
anyway.
This avoids iterating over parts of the whole datastore folder tree
on disk, doing a priv check on each, swapping IO to virtual in memory
checks on info we got available already anyway, is always a good idea
after all
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
config: any_priv_below: plural name & switch to slice of &str for path
s/any_priv_below/any_privs_below/ for consistency and switch from a
single &str for the path param to the slice-ref string variant, as
that allows to use it more often without allocation.
Also allow passing the whole path as single &str element in the slice
by splitting each component on '/' like we do in other parts
nowadays. Note though that we need to omit the leading slash then.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Stefan Sterz [Fri, 3 Jun 2022 15:32:24 +0000 (17:32 +0200)]
pbs-config: acl-tree: add any_priv_below
`any_priv_below()` checks if a given AuthId has any given privileges
on a sub-tree of the AclTree. to do so, it first takes into account
propagating privileges on the path itself and then uses a depth-first
search to check if any of the provided privileges are set on any
node of the sub-tree pointed to by the path.
Signed-off-by: Stefan Sterz <s.sterz@proxmox.com> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
datastore: swap ConfigVersionCache with digest for change detection
We got the digest available anyway, and it's only 16 bytes more to
save (compared to last_generation and the recently removed last_time,
both being 64 bit = 8 bytes each)
Side benefit, we detect config changes made manually (e.g., `vim
datacenter.cfg`) immediately.
Note that we could restructure the maintenance mode checking to only
be done after checking if there's a cached datastore, in which case
using the generation could make sense to decide if we need to re-load
it again before blindly loading the config anyway. As that's not only
some (not exactly hard but not really trivial like a typo fix either)
restructuring work but also means we'd lose the "detect manual
changes" again I'd rather keep using the digest.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
to avoid the problematic open fresh datastore with fresh chunkstore
with, and that's the actual problematic part, fresh process locker.
As the latter uses posix record locks which are pretty dangreous as
they operate on a path level (not FD level) and thus closing any file
opened (even if it wasn't opened for locking at all) drops all active
locks on the same file on completely unrelated file descriptors -.-
Also, no operation wasn't exactly correct for this thing in the first
place, but we cannot use Operation::Lookup either, as we're currently
indeed using a rather stupid-simple way and *are* reading.
So until we optimize this to allow querying the AclTree if there's
any priv XYZ below a path, use the Operation::Read.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Dominik Csapak [Thu, 2 Jun 2022 14:27:44 +0000 (16:27 +0200)]
datastore: lookup: reuse ChunkStore on stale datastore re-open
When re-opening a datastore due to the cached entry being stale
(config change) but also if the last re-open was >60s ago). On
datastore open the chunk store was also re-opened, which in turn
creates a new ProcessLocker, loosing any existing shared lock which
can cause conflicts between long running (24h+) backups and GC.
To fix this, reuse the existing ChunkStore, and thus its
ProcessLocker, when creating a up-to-date datastore instance on
lookup, since only the datastore config should be reloaded. This is
fine as the ChunkStore path is not updatable over our API.
This was always a potential issue but got exposed in practice by
commit 118deb4db8e709b02704bc66c0551bfa7e4369ed which introduced the
unconditional "re-open after 60s" mechanism.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
[ T: reword commit message a bit and reference commit that made the
issue much more likely ] Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
along with the rest of tokio/futures/hyper/openssl being updated - this
is the only one we explicitly depend on that had a non-compatible
version number.
Thomas Lamprecht [Tue, 31 May 2022 05:03:29 +0000 (07:03 +0200)]
ui: system config: improve bottom margins and scroll behavior
setting scrollable on the parent tab panel makes not much sense and
will always add a scroll bar that can scroll a few pixels, even if
there's enough space.
Rather set it to true (= auto) in the actual panels that hold the
content.
Also set a bottom margin so that users can see the "end" of the panel
at the bottom, otherwise it looked like it had a start and sides, but
no bottom.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Mon, 30 May 2022 13:15:39 +0000 (15:15 +0200)]
ui: prune jobs: avoid duplicate params through nested input panels
input panel collect all form fields below them, so nesting two
input panels needs a bit of special care to avoid that each of the
panels adds the data of the deeper nested ones, resulting in
duplicate parameters that the backend then chokes one.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Mon, 30 May 2022 09:11:56 +0000 (11:11 +0200)]
ui: rework prune job view/edit
Fix missing load on initial view, re-use the prune input panel for
editing and avoid using a tab panel for a single tab, rework also
some columns widths and various other small parts-
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
This endpoint only lists all accessible namespace, and one doesn't
necessarily needs to have permissions on the parent itself just to
have OK ACLs on deeper down NS.
So, drop the upfront check on parent but explicitly avoid leaking if
a NS exists or not, i.e., only do so if they got access on the parent
NS.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Thu, 26 May 2022 11:36:12 +0000 (13:36 +0200)]
api: status: include empty entry for stores with ns-only privs
I.e., for those that only got permissions on a sub namespace and
those that onlöy got BACKUP_READ, as both they could just list and
count themselves too after all, so not exactly secret info.
The UI needs some adaptions to cope with gc-stats and usage being
optional, will be done in a next commit.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
it includes the path, which might be helpful when users are switching to
using namespaces. datastore and namespace lookup happens after, so this
doesn't leak anything.
instead of doing a manual lookup and check - this changes the returned
error slightly since check_privs will include the checked ACL path, but
that is okay here, checks are before we even lookup the namespace/store,
so no chance to leak anything.
priv checks: use priv_to_priv_names and include path
where appropriate. these should never leak anything sensitive, as we
check privs before checking existence or existence is already known at
that point via other privileges.
acl: fix handling of sub-components containing '/'
previously with an ACL for the path "/foo/bar" without propagation and a
check for `&["foo", "bar/baz"] this code would return the ACL (roles)
for "/foo/bar" for the path "/foo/bar/baz".
Thomas Lamprecht [Thu, 19 May 2022 11:31:06 +0000 (13:31 +0200)]
prune datastore: support max-depth and improve priv checks
use the relatively new variant of ListAccessibleBackupGroups to also
allow pruning the groups that one doesn't own but has the respective
privileges on their namespace level.
This was previously handled by the API endpoint itself, which was ok
as long as only one level was looked at.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
projects / proxmox-backup.git / log