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

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

[Laurențiu Leahu-Vlăducu]

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

Unfortunately, most groups created by Windows by default contain spaces. 
This effectively means that most groups cannot be synchronized to PVE by 
default without further (manual) changes. While this can be fixed by 
removing the spaces manually, this is much harder to do if the AD domain 
already exists and is used since a longer time.

Furthermore, our documentation states that spaces should be allowed as 
long as they are not at the beginning or end: 
https://pve.proxmox.com/pve-docs/chapter-pveum.html#pveum_ldap_reserved_characters

At this point I would also like to cite some LDAP specifications on this 
matter:
- RFC 2253 (obsolete): https://www.rfc-editor.org/rfc/rfc2253#section-2.4
- RFC 4514 (newer, obsoletes RFC 2253): 
https://www.rfc-editor.org/rfc/rfc4514#section-2.4

Not long ago, someone has sent a patch series trying to fix a similar 
issue with OpenID Connect: 
https://lore.proxmox.com/pve-devel/26f0c8f2-57f0-4064-a87d-0de9f65e316f@proxmox.com/T/#m23bfbefdfd66d7dde80b3fe4e4033a6b6d8ce3de

I experimented around with the best strategy for going forward. Ideally, 
our strategy should apply for both Active Directory and OpenID Connect 
to ensure consistency. Also, while the Active Directory issue is more 
about the groups, this equally applies to usernames (but is less 
time-critical due to usernames not containing spaces by default).

I would like to ask for your feedback on what's the best strategy for 
going forward, also regarding backwards compatibility and future major 
versions.

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 ;)


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.


3. If we also want to allow using special characters, we have to think 
about the encoding we use for user.cfg. Currently, we're not doing any 
conversions, meaning that Perl could write the strings to user.cfg as 
they are (e.g. as UTF-8), but would read them without any conversions, 
treating the text as Latin-1.

I have already started a discussion on UTF-8 in our config files, so for 
more details on how Perl handles encodings, look here: 
https://lore.proxmox.com/pve-devel/082d3fe0-9c6c-494d-9ec3-f64645cd7a53@proxmox.com/T/#t


4. We also have to think about how we want to handle upgrades after such 
a change, especially regarding clusters. I'm specifically talking about 
the short period of time when upgrading a cluster to a new version, 
where not all nodes are on the same version at the same time (e.g. for a 
few minutes). A possibility would be to already implement the changes as 
part of PVE 8.4, meaning that the code could handle it but we would 
disable it by default, while making it available beginning with PVE 9.0.


Thank you in advance for your input!


Laurențiu


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

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

[Dietmar Maurer]

> 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 feeling is that we need to allow all characters -  else this will be an endless issue ...

> 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.

see above

> 3. If we also want to allow using special characters, we have to think 
> about the encoding we use for user.cfg. Currently, we're not doing any 
> conversions, meaning that Perl could write the strings to user.cfg as 
> they are (e.g. as UTF-8), but would read them without any conversions, 
> treating the text as Latin-1.
> 
> I have already started a discussion on UTF-8 in our config files, so for 
> more details on how Perl handles encodings, look here: 
> https://lore.proxmox.com/pve-devel/082d3fe0-9c6c-494d-9ec3-f64645cd7a53@proxmox.com/T/#t

I would use url encoding for that.

> 4. We also have to think about how we want to handle upgrades after such 
> a change, especially regarding clusters. I'm specifically talking about 
> the short period of time when upgrading a cluster to a new version, 
> where not all nodes are on the same version at the same time (e.g. for a 
> few minutes). A possibility would be to already implement the changes as 
> part of PVE 8.4, meaning that the code could handle it but we would 
> disable it by default, while making it available beginning with PVE 9.0.

yes, something like that.


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

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

[Shannon Sterz]

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