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

["Laurențiu Leahu-Vlăducu" <l.leahu-vladucu@proxmox.com>]

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