When removing TFA from a user via the command line, the change was not
reflected in the GUI or in the output of `pveum user list`. Both
continued to show that TFA was enabled for the user. Fixed the issue
by updating the user configuration file.
Signed-off-by: Shan Shaji <s.shaji@proxmox.com>
Currently, guest replication is guarded with Datastore.Allocate on
'/storage', which is rather surprising. One could require
Datastore.AllocateSpace on all involved storages, but having a
dedicated privilege like for other VM operations like migration and
snapshot seems to be more natural.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
The name VM.Monitor is ambiguous and makes it hard to guess what the
privilege is for. The privilege was used for two things:
1. QEMU guest agent operations, for which dedicated privileges were
introduced, see commit "add VM.GuestAgent privileges".
2. Access to the QEMU HMP monitor, where only the 'info' and 'help'
commands were usable without an additional Sys.Modify privilege.
Access to the monitor will be guarded with Sys.Audit.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Link: https://lore.proxmox.com/20250717133711.84715-3-f.ebner@proxmox.com
The privilege VM.Monitor has a very ambiguous name and is planned to
be dropped. Most of the API endpoints using it are for the QEMU guest
agent commands. Introduce dedicated, more fine-grained privileges for
those.
There is a basic VM.GuestAgent.Audit privilege for read-only,
informational commands.
There are dedicated privileges VM.GuestAgent.File{Read,Write} for
the file-{read,write} commands. There is a separate
VM.GuestAgent.FileSystemMgmt privilege for filesystem freeze, thaw
and trim.
The VM.GuestAgent.Unrestricted privilege is to allow all guest agent
operations, in particular also execution of arbitrary commands with
guest-exec.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Link: https://lore.proxmox.com/20250717133711.84715-2-f.ebner@proxmox.com
Add permission path /sdn/fabrics/{fabric_id}. There are currently only
SDN-specific permissions for the fabric itself, not the nodes. For
displaying / editing the nodes, the existing permissions Sys.Audit or
Sys.Modify on /nodes/{node} are required, because they are already
used for viewing / editing the network configuration of a node.
The node settings mostly revolve around configuring IPs and network
interfaces on that node, so we decided to stick with the permission
that is already governing that, since it would need to be checked when
editing a node anyway. Otherwise, users with access to a fabric node
could change parts of the network configuration of arbitrary
interfaces that node, circumventing the current permission checks. A
separate, SDN-specific, permission would not add much benefit because
of that.
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
Link: https://lore.proxmox.com/20250716130837.585796-35-g.goller@proxmox.com
See previous commit: its functionality has been replaced by the rust
code and it has not been used since the support for the old login API
has been dropped.
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Its functionality has been replaced by the rust TFA implementation and
the last calls to these were removed with commit cfd8636b5e ("drop
support for old login API") in 2023.
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
using the new top-level `make tidy` target, which calls perltidy via
our wrapper to enforce the desired style as closely as possible.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
See pve-common's commit 5ae1f2e ("buildsys: add tidy make target")
for details about the chosen parameters.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
The previous regex matched exactly that combination of characters,
rather than any combination of the specified ones.
Fixes: e80f840 ("openid: make groups-claim RE more restrictive")
Signed-off-by: Mira Limbeck <m.limbeck@proxmox.com>
Link: https://lore.proxmox.com/20250408113349.165831-1-m.limbeck@proxmox.com
Given that the information is only needed for case-sensitive realms,
move the read into the conditional.
No functional changes.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
Originally reported in the forum [0].
This is only a cosmetic fix and has no user-visible impact, just fixing
a code warning in the syslog. Applies only for case-insensitive realms
too, where Active Directory is the only type to support that.
When looking up a non-existing username on case-insensitive realms, it
currently returns `undef`, which then causes the following warning in
the syslog:
Use of uninitialized value $username in concatenation (.) or string at /usr/share/perl5/PVE/API2/AccessControl.pm line 303.
authentication failure; rhost=::ffff:10.0.0.1 user= msg=user name '' is too short
This now follows the logic from the common, case-sensitive path, to just
return the original, given username (which is then later on validated in
the auth chain).
No functional changes.
[0] https://forum.proxmox.com/threads/new-ad-realm-not-working-blank-username.157859/
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
Upgrading from a pre-PVE 6 version directly to a (current) PVE 8.x
version can never work in the first place, so the test is not needed
anymore.
The snippet was added by commit 3a98190 ("add postinst script") and
enabled by commit 243262f ("fix #2079: activate authkey rotation every
24 hours").
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
when creating new users or updating existing passwords this new
minimum is enforced which aligns with NIST's latest recommendations
[1].
[1]: https://pages.nist.gov/800-63-4/sp800-63b.html#passwordver
Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
Adds test cases for the GET permissions API endpoint to ensure that:
- users with the Sys.Audit perm can access any user's permissions
- users with the Sys.Audit perm can access any token's permissions
- users without the Sys.Audit perm can access their own permissions
- users without the Sys.Audit perm can access their token's permissions
- tokens with the Sys.Audit perm can access any user's permissions
- tokens without the Sys.Audit perm can access their own permissions
These tests also separate whether a token has the privilege-separated
property or not.
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
even if they lack Sys.Audit on /access - since tokens are self-service,
checking whether the ACLs work as expected should also be doable for every
user.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Tested-by: Daniel Kral <d.kral@proxmox.com>
even when specifying an explicit userid matching their own.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Tested-by: Daniel Kral <d.kral@proxmox.com>
All other groups use delete for deleting users/groups/... so it makes
sense to do so for the user token command group too.
Add an alias for the old command for backwards compatibility to avoid
that scrips and muscle memory break.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
With the PVE 8.0 major release, the scope of
non-"Permissions.Modify"-based ACL update privileges were reduced (so
that users with for example, VM.Allocate on a VM could only delegate
their own privileges, but not arbitrary other ones). that additional
logic had a wrong guard and was accidentally triggered for calls where
the user had the "Permissions.Modify" privilege on the modified ACL
path, but without propagation set.
A user with "Permissions.Modify" on a path should be able to set
arbitrary ACLs for that path, even without propagation.
Reported on the forum: https://forum.proxmox.com/threads/151032/
Fixes: 46bfd59 ("acls: restrict less-privileged ACL modifications")
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Stable sorting in user.cfg config file allows tracking changes by
checking into git or when using automation like ansible.
Signed-off-by: Daniel Krambrock <krambrock@hrz.uni-marburg.de>
Tested-by: Folge Gleumes <f.gleumes@proxmox.com>
This was reported by a user in the forum [0].
The cause was that the user-* standard options were not registered
when the sync was called from the scheduler, resulting in the
following error:
pvescheduler[2849]: skipping attribute mapping 'cn'->'comment' for user 'test@samba0' - no such standard option 'user-comment'
Fix this by simply importing the PVE::API2::User module, thus ensuring
the options get registered.
[0] https://forum.proxmox.com/threads/ldap-integration-comment-email-first-name-lastname.143490/
Fixes: cb93636 ("LDAP sync: improve validation of synced attributes")
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
Tested-by: Fiona Ebner <f.ebner@proxmox.com>
Add a new 'confirmation-password' parameter to the change-password
endpoint and require non-root users to confirm their passwords.
Doing so avoids that an attacker that has direct access to a computer
where a user is logged in to the PVE interface can change the password
of said user and thus either prolong their possibility to attack,
and/or create a denial of service situation, where the original user
cannot login into the PVE host using their old credentials.
Note that this might sound worse than it is, as for this attack to
work the attacker needs either:
- physical access to an unlocked computer that is currently logged in
to a PVE host
- having taken over such a computer already through some unrelated
vulnerability
As these required pre-conditions are pretty big implications, which
allow (temporary) access to all of the resources (including PVE ones)
that the user can control, we see this as slight improvement that
won't hurt, might protect one in some specific cases that is simply
too cheap not to do.
For now we avoid additional confirmation through a second factor,
as that is a much higher complexity without that much gain, and some
forms like (unauthenticated) button press on a WebAuthn token or the
TOTP code would be easy to circumvent in the physical access case and
in the local access case one might be able to MITM themselves too.
Reported-by: Wouter Arts <security@wth-security.nl>
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
[ TL: extend commit message ]
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
since the upcoming use case in change_password uses the returned $ruid
and the parameter is called 'confirmation-password' there
also generalize the error so it does not mention TFA
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
now called "reauth_user_for_user_modification" since we
re-authenticate the current user while they are trying to modify
their (or others') password/tfa settings
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
We have some API endpoints that can access the network from the POV of
a Proxmox VE node, like e.g., the one for downloading a template/ISO
image directly to a PVE storage from an HTTP URL, and the matching
query-url-metadata that makes this functionality much more convenient
to use in the UI. But the downside of such calls is naturally that
they basically allow to scan the whole network via HTTP URLs, and
potentially even download some image that the user should not have
access to and adding to a VM that the user controls.
Due to that we limited the exposure of those API endpoints to
Sys.Modify on / (in addition to e.g. basic storage privs) for the
initial addition of the feature, as we were not sure about user
adoption and if a separate privilege could be justified.
Since we got a handful requests like #5254 this justification is now
met, so add a 'Sys.AccessNetwork' privilege.
That name should make it clear that having that privilege will allow
access to the network and the sys(tem) prefix should underline that
it's about the host systems network. Add it such, that it will only be
available for the most powerful of our built-in special roles, namely
the Administration one, besides naturally the all-powerful root@pam
special user.
Admins can then e.g. create new roles that include Sys.AccessNetwork
and Datastore.AllocateTemplate which can then be used for allowing
automation to download images while adhering to the Least Privilege
Principle.
Buglink: https://bugzilla.proxmox.com/show_bug.cgi?id=5254
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
