Re: [pve-devel] Strategy for Active Directory and OpenID Connect groups and usernames with spaces and other special characters

["Shannon Sterz" <s.sterz@proxmox.com>]

On Wed Mar 5, 2025 at 5:10 PM CET, Laurențiu Leahu-Vlăducu wrote:
> Hello everyone,
>
> I would need some feedback on a feature that was requested multiple
> times by different users over the years. Specifically, many people have
> complained that synchronizing Active Directory groups to PVE
> partially/mostly fails due to many groups containing spaces by default:
> https://bugzilla.proxmox.com/show_bug.cgi?id=2929

-->8 snip 8<--

> Questions:
>
> 1. Do we want to allow spaces in groups and/or usernames, or should we
> prefer replacement characters (e.g. mapping space(s) to _ or some other
> character)?
>
> My take on this: we have to differentiate between groups and usernames -
> this is because usernames are also used to log in, while groups are not.
>
> In other words, having a replacement character (e.g. space to _ ) for
> groups means that the group name would be slightly different compared to
> the original, but would otherwise work. However, doing the same for
> usernames would mean that we would always need to do the same
> replacements at login in order to allow logging in both with the
> original name and with the replaced name.
>
> Another issue with doing replacements is the possibility of having
> collisions - e.g. having both "Domain Admins" and "Domain_Admins" in the
> Active Directory. Of course, we should check for such cases and prevent
> PVE from synchronizing such groups.
>
> I was wondering whether allowing spaces would mean dramatic changes to
> our code, but managed to make it work by adapting the Regex in a few
> places - so I have a (mostly) working version already. However, I am
> also aware that this change eventually has a higher potential of
> breaking existing code that assumes not having any spaces. On the other
> hand, this is also the reason why we have discussions and code reviews ;)

not sure just allowing them or replacing them is the way to go. imo, we
should escape certain characters? percent encoding as suggested by
dietmar for the config already would be one way to go. we could also
simply follow the rfc here and use "\" to escape the following
character set unconditionally (this is allowed per the rfc [1]):

' ', '"', '#', '+', ',', ';', '<', '=', '>', or '\'
(U+0020, U+0022, U+0023, U+002B, U+002C, U+003B,
U+003C, U+003D, U+003E, U+005C, respectively)

[1]: https://www.rfc-editor.org/rfc/rfc4514#section-2.4


that specific rfc triggered some memories btw. (don't mind the incorrect
first name ;) ):

https://lore.proxmox.com/pve-devel/20230517133931.148634-1-s.sterz@proxmox.com/

> 2. In case we want to allow spaces in groups and/or usernames, we also
> have to ask ourselves whether we want to allow other special characters
> as well. It is not necessarily unusual for a group or user to have
> non-ASCII characters in some parts of the world. Currently we are quite
> restrictive with group names, but allow most usernames. Note: at this
> point we also have slight inconsistencies in our Regex checks between
> Perl and Rust.

yeah, i'm not sure if this could cause problems in the long run. utf-8
has a lot of confusables [1]. i think this could become a problem with
groups, as attackers could create a group that looks just like a highly
privileged other group and possibly trick admins into giving the
"confusing" group more privileges than is intended. although, at that
point they'd have some control over the corresponding auth realm anyway,
so maybe maybe i'm a little paranoid here.

dietmar has a point that this is kind of an endless debate, though.

[1]: https://www.unicode.org/Public/security/8.0.0/confusables.txt

-->8 snip 8<--


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel