add Sys.AccessNetwork privilege 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>
oidc: enforce generic URI regex for the ACR value Restrict the acr-value regex a little bit so as to align the behavior with PBS. The openid documentation says that the acr-value *should* be an URI [0]. Added a regex that loosely disallows some of the reserved URI characters specified in the RFC [1]. Values like: * "urn:mace:incommon:iap:silver" * "urn:comsolve.nl:idp:contract:rba:location" SHOULD work, but values like: * "urn:#ace:incommon:iap:silver" * "urn:"omsolve.nl:idp:contract:rba:location" should NOT work. This is related to the fix [2] for bug #5190 in PBS, but different as there we had to make the verifier more flexible, whereas here we make it stricter – mostly to have both projects aligned to avoid confusion. [0]: https://openid.net/specs/openid-connect-core-1_0.html [1]: https://www.rfc-editor.org/rfc/rfc2396.txt [2]: https://git.proxmox.com/?p=proxmox-backup.git;a=commit;h=e0222ce83c28397d493c70825e873943c1223c67 Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
api: user: limit email to 254 characters and user comment to 2048 For email the reasoning is: > In addition to restrictions on syntax, there is a length limit on > email addresses. That limit is a maximum of 64 characters (octets) > in the "local part" (before the "@") and a maximum of 255 > characters (octets) in the domain part (after the "@") for a total > length of 320 characters. However, there is a restriction in RFC > 2821 on the length of an address in MAIL and RCPT commands of 254 > characters. Since addresses that do not fit in those fields are > not normally useful, the upper limit on address lengths should > normally be considered to be 254. -- https://www.rfc-editor.org/errata_search.php?rfc=3696&eid=1690 And for user-comments, we normally show those as single line and using 2048 bytes as maximum, while also a rather arbitrary number it allows for about 2.5 times more users on a system (full name + comment can be up to 4 KiB vs 10 KiB), and we can re-raise this relatively easily again if there are somewhat reasonable complaints. Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
LDAP sync: bail if there is no schema to verify an attribute's value Should not matter for now, but better to to catch explicitly, e.g., if anybody ever adds new attributes or changes the default options names without adapting this too. Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
LDAP sync: build valid-target-attribute list on the fly to avoid coupling Build the set of valid target attributes on the fly by using the existing ldap => ours mapping. This avoids that one needs to adapt both lists when changing this, which even though it should be caught on testing, is needlessly adding friction. The is-known-target-attr check could never trigger as this was already checked in the parent before even calling the verify method, so just remove it. Rename the `verify_sync_attribute` to `verify_sync_attribute_value` to clarify that it really only checks the value of an attribute, not the attribute (key) itself. As a side-benefit, this also makes the code shorter and avoids a permanent global variable using up (a tiny amount of) space. Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
LDAP sync: improve validation of synced attributes and skip the ones not fitting our schema, while warning the user about them. Also warns the user if the specified 'sync_attributes' mapping contains entries for attributes that don't exist, e.g. 'enabled=active' (since the property on PVE side is called 'enable'). For the 'enable' property, any value coming from the server led to the user being enabled, even "0", because it is a string. This is not changed by this patch, by not trying to validate or parse a boolean. In get_users(), the username is also set in the returned hash, but without the realm. This doesn't seem to be necessary for syncing, because the username with the realm is used as a hash key and that's what's relied upon when updating the config. But the tests require it to be set, so that is not changed by this patch either. Relies on the user properties (other than username) to be standard options called 'user-XYZ'. Could be improved by moving the schema for user properties from the API module to a module that can be accessed by both API and plugin here and creating a helper for accessing it. Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
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>
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>