5
0
mirror of git://git.proxmox.com/git/pve-access-control.git synced 2025-01-05 09:17:53 +03:00
Commit Graph

614 Commits

Author SHA1 Message Date
Fiona Ebner
2dabf3c3ae api: user: add pattern for user keys option
While nowadays, most entries should be just 'x', there can also still
be legacy entries with 'x!u2f', 'x!yubico' and base32 encoded secrets.
For example, some users might be syncing them from LDAP.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2024-02-08 11:08:49 +01:00
Fabian Grünbichler
a53fd5d882 build: fix file list
this file is installed by a sub-dir Makefile, it does not exist in
src/PVE/API2.

the error is not fatal, but printed during build:

 install: cannot stat 'RealmSync.pm': No such file or directory

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2023-12-07 12:36:40 +01:00
Wolfgang Bumiller
ffc4e503ec bump version to 8.0.7
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
2023-11-20 12:24:32 +01:00
Fabian Grünbichler
4418b06b35 pools: record parent/subpool information
and ensure a missing intermediate pool exists at all times.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2023-11-20 10:22:45 +01:00
Fabian Grünbichler
e7224f6e30 fix #1148: allow up to three levels of pool nesting
with ACLs being inherited along the pool hierarchy.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2023-11-20 10:22:44 +01:00
Thomas Lamprecht
401e32056e bump version to 8.0.6
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-11-17 08:27:22 +01:00
Fabian Grünbichler
7b5d2abde5 acl: add missing SDN ACL paths to allowed list
else it's not actually possible to define ACLs on them, which means they are
effectively root only instead of allowing their intended permission scheme.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2023-11-08 13:09:28 +01:00
Fabian Grünbichler
3d7afd6f3e perms: fix wrong /pools entry in default set of ACL paths
/pools is not an allowed ACL path, so this would add a bogus entry into the
effective permissions in case something got propagated from /.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2023-11-08 13:07:17 +01:00
Wolfgang Bumiller
b8a52eac77 bump version to 8.0.5
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
2023-08-11 13:35:37 +02:00
Wolfgang Bumiller
d4397b514f fixup comments about config locks
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
2023-08-11 13:31:40 +02:00
Christoph Heiss
fbb1fa448f ldap: add opt-in check-connection param to perform a bind check
Removes the dreaded DN regex, instead introducing a optional
connect/bind check on creation/update, aligning it with the way PBS does
it.

Additionally, it has the benefit that instead of letting a sync fail on
the first try due to e.g. bad bind credentials, it gives the user some
direct feedback when trying to add/update a LDAP realm, if enabled.

Should be rather a corner case, but it's the easiest way for us to
accomodate and the most versatile for users needing this.

This is part of the result of a previous discussion [0], and the same
approach is already implemented for PBS [1].

[0] https://lists.proxmox.com/pipermail/pve-devel/2023-May/056839.html
[1] https://lists.proxmox.com/pipermail/pbs-devel/2023-June/006237.html

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
2023-08-11 13:31:40 +02:00
Wolfgang Bumiller
9ac31bc6d3 bump pve-common dependency to 8.0.8
required for next patch introducing the check-connection parameter for
ldap based realms

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
2023-08-11 13:30:59 +02:00
Christoph Heiss
162e68903d api: domains: fix setting only the realm authentication password
Previously, if one tried to only update e.g. the bind password for an
LDAP realm, it would fail with an rather unhelpful error message:

  # pveum realm modify ldap -password foo
  update auth server failed: no options specified

The root cause was that the `password` parameter was removed early from
the parameter object, which than would fail the check whether it is
empty or not.

Thus, additionally check if only `password` was specified and if so,
allow it.

Reported-by: Friedrich Weber <f.weber@proxmox.com>
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
2023-07-21 13:20:15 +02:00
Wolfgang Bumiller
33e4480a77 bump version to 8.0.4
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
2023-07-20 10:59:56 +02:00
Friedrich Weber
032e7d6d44 auth: tfa: fail if realm requires TFA but no challenge is generated
Before 0f3d14d6 ("auth: tfa: read tfa.cfg also if the user.cfg entry
has no "x" marker"), `user_get_tfa` failed if the realm required TFA,
but the user's `keys` attribute was empty. Since 0f3d14d6,
`user_get_tfa` fails if the realm requires TFA, but neither user.cfg
nor tfa.cfg define any second factors for that user.

However, both before and after 0f3d14d6, a realm that requires TOTP
allows a user to login without a second factor if they have at least
one configured factor in tfa.cfg and all factors are disabled -- for
example if they have only a disabled TOTP factor. This behavior is
unwanted, as users can then circumvent the realm-mandated TFA
requirement by disabling their own TOTP factor.

This happens because a user with a disabled TOTP factor in tfa.cfg
passes the check in `user_get_tfa`. Hence, `authenticate_2nd_new_do`
proceeds to call `authentication_challenge`, which does not generate a
challenge (and returns undef) because the user has no enabled factors.
Consequently, `authenticate_2nd_new_do` returns undef and allows login
without a second factor.

Note that this does not happen for realms that require Yubico TFA,
because for these realms, `authenticate_2nd_new_do` does not call
`authentication_challenge` and instead generates a challenge in any
case, regardless of whether the user has enabled Yubico factors or
not.

This patch fixes the issue by moving the check out of `user_get_tfa`,
and instead letting `authenticate_2nd_new_do` fail if the realm
requires TFA but `authentication_challenge` generates no challenge
(returns undef). This also saves a call to `api_list_user_tfa` that
was introduced in 0f3d14d6.

This patch still allows users to login with a recovery key to a realm
that requires TFA , which is the intended behavior.

Suggested-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Signed-off-by: Friedrich Weber <f.weber@proxmox.com>
2023-07-20 10:53:41 +02:00
Wolfgang Bumiller
72950c1d53 add fixme comments about pending pve-rs improvements
Alternatively we could potentially move the realm-tfa check to after
`authentication_challenge`.

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
2023-07-14 14:21:26 +02:00
Friedrich Weber
0f3d14d6be auth: tfa: read tfa.cfg also if the user.cfg entry has no "x" marker
Previously, `user_get_tfa` read the `keys` user attribute from
user.cfg to determine whether a user has second factors configured.
`keys` could contain TOTP secrets or Yubico key IDs (for realms that
require TFA), or the marker "x" to signify that second factors are
defined in tfa.cfg, in which case `user_get_tfa` would additionally
read tfa.cfg.

However, syncing an LDAP realm with `remove-vanished=properties`
erases the `keys` attribute, and thus the "x" marker (unless custom
`sync_attributes` with a mapping for `keys` are defined). This would
allow TFA-enabled users to log in without a second factor after a
realm sync. This issue was first reported in the forum [1].

To fix this issue, `user_get_tfa` now reads tfa.cfg unconditionally,
and thus independently of the value of `keys`. In other words, the "x"
marker is now irrelevant for authentication. The reasoning for this
change is that most current setups define second factors in tfa.cfg
anyway.

Special care is needed to avoid breaking realms that require TFA: In
that case, `user_get_tfa` must fail authentication if neither user.cfg
nor tfa.cfg define any second factors.

This patch changes the behavior of a hypothetical (and not officially
supported) LDAP realm setup in which `sync_attributes: keys=attr` and
`remove-vanished=properties` is used to maintain `keys` in the LDAP
directory. In such a setup, an admin could enable/disable TFA for a
user who has an enabled second factor in tfa.cfg by editing their LDAP
entry and switching between "x" and "". With this patch, TFA is always
enabled for that user.

This patch makes the "x" marker irrelevant for authentication, but PVE
still *writes* it if the user has second factors configured in
tfa.cfg. This behavior is kept for now to avoid issues in cluster
upgrade scenarios, where some nodes that still rely on the "x" marker
could allow logins without a second factor.

[1] https://forum.proxmox.com/threads/130440/

Suggested-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Signed-off-by: Friedrich Weber <f.weber@proxmox.com>
2023-07-14 13:56:55 +02:00
Thomas Lamprecht
8a856968f7 bump version to 8.0.3
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-06-21 19:45:32 +02:00
Thomas Lamprecht
b3edff39f9 drop assert_new_tfa_config_available for Proxmox VE 8
All nodes should be new enough, especially as this is understood
since pve-manager 7.0-15 and users must upgrade to 7.4 before
upgrading to Proxmox VE 8

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-06-21 19:43:52 +02:00
Thomas Lamprecht
aba03c966e pveum: list tfa: sort by user ID
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-06-21 19:43:49 +02:00
Thomas Lamprecht
c200d9fd70 pveum: list tfa: recovery keys have no descriptions
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-06-21 19:43:49 +02:00
Thomas Lamprecht
1852a92938 bump version to 8.0.2
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-06-21 18:13:58 +02:00
Thomas Lamprecht
b7ab634a20 api: tfa: add missing links for child-routes
making this more useable in things like pvesh or the HTML formatter

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-06-15 16:55:43 +02:00
Thomas Lamprecht
79c9d3b84b api: tfa: don't block tokens from viewing and list TFA entries
The `allowtoken` property is a total, unconditional block on using
API tokens on an endpoint. We reserve those only for a limited set of
security critical endpoints like changing passwords or second
factors, or creating a (cookie) ticket, which are exempt from this
limitations, so require to have limited access to them too.

Anyhow, listing and getting TFA entries for users, where the API
token has the correct permissions granted, is not critical, as the
API token cannot gain more permissions than they have from that
info, so drop the total block on those GET methods.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-06-15 09:23:35 +02:00
Thomas Lamprecht
329780350c api: users: sort groups
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-06-14 16:07:55 +02:00
Thomas Lamprecht
ebf82c7781 bump version to 8.0.1
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-06-09 16:12:05 +02:00
Thomas Lamprecht
4a7b5956ec tfa: cope with native versions in cluster version check
Reported-by: Friedrich Weber <f.weber@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-06-09 16:10:33 +02:00
Thomas Lamprecht
6004f25eda bump version to 8.0.0
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-06-09 10:14:36 +02:00
Thomas Lamprecht
00b90e7185 api: roles: forbid creatin new roles starting with "PVE" namespace
makes our reasoning when adding new top-level privileges way easier
in the future.
We already had two major upgrades with role additions where we had to
add special checks in the upgrade script and breaking changes, so
let's reserve any role starting with PVE (case-insensitive to avoid
confusion potential) and forbid creating those via API.

We might also think about letting the config parser choke on that, as
otherwise one could still create them via editing the config
manually.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-06-08 09:31:24 +02:00
Thomas Lamprecht
d6fb84bfef api: roles: cleanup imports
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-06-08 09:30:27 +02:00
Thomas Lamprecht
43aa2489fc api: roles: whitespace and indendation clean-ups
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-06-08 09:22:00 +02:00
Thomas Lamprecht
8e8023b1e9 bump version to 8.0.0~3
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-06-07 19:12:08 +02:00
Thomas Lamprecht
bd993a4cfc tests: adapt new test for admin ACL with mapping privs
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-06-07 19:12:08 +02:00
Dominik Csapak
8b5fd2e66e add privileges and paths for cluster resource mapping
uses the privileges:

Mapping.Use
Mapping.Modify
Mapping.Audit

on /mapping/{TYPE}/{id}

so that we can assign privileges on resource level

this will generate new roles (PVEMappingUser, PVEMappingAdmin,
PVEMappingAuditor)

note that every user with Permissions.Modify on '/' and propagate can add these
new roles to themselves

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
2023-06-07 18:58:33 +02:00
Alexandre Derumier
a62d78db33 add new SDN.use privilege in PVESDNUser role
Signed-off-by: Alexandre Derumier <aderumier@odiso.com>

FG: fix test
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2023-06-07 13:17:40 +02:00
Fabian Grünbichler
e1ea58c810 check_sdn_bridge: check bridge first
it's cheap, so let's use it for early returning

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2023-06-07 13:17:23 +02:00
Fabian Grünbichler
3c97bee53c check_sdn_bridge: correctly handle noerr
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2023-06-07 13:17:23 +02:00
Alexandre Derumier
a5616d5c6e rpcenvironnment: add check_sdn_bridge
check if user have access to 1 vlan of the bridge
or the bridge itself

Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
2023-06-07 13:17:23 +02:00
Alexandre Derumier
4d5b0937a3 access control: add /sdn/zones/<zone>/<vnet>/<vlan> path
Signed-off-by: Alexandre Derumier <aderumier@odiso.com>

FG: add missing /sdn/zones path

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2023-06-07 13:17:23 +02:00
Alexandre Derumier
c27eb24526 rpcenv: api permission heuristic: query Sys.Modify for root ACL-path
Ensures that we can use this in the capabilities heuristic check in
the web UI

Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
2023-06-07 12:02:24 +02:00
Thomas Lamprecht
3ef602fe95 bump version to 8.0.0~2
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-06-07 11:34:33 +02:00
Fabian Grünbichler
46bfd59dfc acls: restrict less-privileged ACL modifications
there are currently three possibilities to modify ACLs without the
'Permissions.Modify' privilege in PVE::RPCEnvironment::check_perm_modify:

    if ($path =~ m|^/storage/.+$|) {
	push @$testperms, 'Datastore.Allocate';
    } elsif ($path =~ m|^/vms/.+$|) {
	push @$testperms, 'VM.Allocate';
    } elsif ($path =~ m|^/pool/.+$|) {
	push @$testperms, 'Pool.Allocate';
    }

lock those down by only allowing the currently authenticated user to hand out a
subset of their own privileges, never more.

for example, this still allows a PVEVMAdmin to create ACLs for other
users/tokens with PVEVMUser (on '/vm/XXX'), but not with Administrator or
PVEPermAdmin.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2023-06-07 11:13:16 +02:00
Fabian Grünbichler
df619a8dc2 roles: restrict Permissions.Modify to Administrator
to reduce the chances of accidentally handing out privilege modification
privileges. the old default setup of having Permissions.Modify in PVESysAdmin
and PVEAdmin weakened the distinction between those roles and Administrator.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2023-06-07 11:13:16 +02:00
Thomas Lamprecht
27014b5908 fix module namespace of realm sync API
it isn't mounted in PVE::API2::AccessControl and it doesn't lives
anywhere in /access, so using that is just confusing.

Both, API and backend could simply move to manager, but as we already
got an api package here and it does somewhat fits into the topic lets
keep it here for now.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-06-07 09:37:47 +02:00
Thomas Lamprecht
37976acc6d api: realm sync: die early if no job options
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-06-07 09:33:16 +02:00
Thomas Lamprecht
c0210e3cf1 api: realm sync: code cleanups
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-06-07 09:32:59 +02:00
Thomas Lamprecht
51ae7bbb69 realm sync: cleanup imports
with: perlimports -i --no-padding

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-06-07 09:31:40 +02:00
Dominik Csapak
dc982c4b76 add realm-sync plugin for jobs and CRUD api for realm-sync-jobs
to be able to define automated jobs that sync ldap/ad

The jobs plugin contains special handling when no node is given, since
we only want it to run on a single node when that triggers. For that,
we save a statefile in /etc/pve/priv/jobs/ which contains the
node/time/upid of the node that runs the job. The first node that
is able to lock the realm (via cfs_lock_domain) "wins" and may
sync from the ldap.

in case a specific node was selected, this is omitted and the Jobs
handling will not let it run on other nodes anyway

the API part is our usual sectionconfig CRUD api, but specialized
for the specific type of job.

the api will be at /cluster/jobs/realm-sync
(this must be done in pve-manager)

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
 [ T: resolve merge conflict due to packaging/source split ]
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-06-07 08:01:07 +02:00
Wolfgang Bumiller
53a2b71563 api: user index: only include existing tfa lock flags
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
2023-06-06 11:19:00 +02:00
Wolfgang Bumiller
f63364a755 bump version to 8.0.0~1
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
2023-06-05 14:52:39 +02:00