Page MenuHomePhabricator

Investigate making cas usernames case sensitive
Open, LowPublic

Description

Currently CAS users ldap as its main identity database and then value is compared in a case insensitive manner. We should investigate making this case sensitive. We could also explore using the uid value.

Event Timeline

jbond triaged this task as Medium priority.Jun 29 2020, 4:13 PM
jbond created this task.
Reedy renamed this task from Investigate making cas usernames case sensetive to Investigate making cas usernames case sensitive.Jun 29 2020, 4:57 PM

I had a closer look at various angles here:

  • CAS offers to munge LDAP search results via Ldaptive. I played around with it on the idp-test1001 and e.g. this would configure CAS to explicitly not modify the CN: It would not make a practical difference, though (see below).
cas.authn.ldap[0].searchEntryHandlers[0].type=CASE_CHANGE
cas.authn.ldap[0].searchEntryHandlers[0].caseChange.attributeValueCaseChange=NONE
cas.authn.ldap[0].searchEntryHandlers[0].caseChange.attributeNames=cn
  • The case-insensitivity of the cn attribute (what we're currently authenticating against) by CAS is the same as what is applied by OpenLDAP (Confusingly the comment in the core schema is commented out, but the cn attribute is such a fundamental internal reference and created by a bootstrap-like initial LDAP DB (as by default these days the config is stored in cn=config and schemas in cn=schema, so the "cn" attribute needs to pre-exist before schemas are stored)). In addition the LDAP schemas also ensure the uniqueness of the cn attribute across case-insensitive variations. To confirm I created two test users jmmtest1 and jmmtest2 and tried to change the "cn" of jmmtest2 to "jmmTEST1", which was correctly rejected with "Constraint violation: some attributes are not unique". As such, enforcing a specific case-variation does not plug some security issue by potentially allowing case-duplicated user attributes.
  • So if we were to change this to only allow a specific casing, we would need to update the LDAP schemas and it would lead to many other issues down the road.
  • One alternative would be to match against the "id" attribute and not "cn". I think the CN is more appropriate, as it's more visible to people and commonly referred to as the Wikimedia Developer Name already. It's what everyone else sees e.g. in Gerrit as well (while many people never log into a shell to begin with).
  • Revisiting this issue (which has been very useful to confirm assumptions we made in early planning!) came up with the topic of Icinga permissions: The permissions to downtime hosts etc. are configured with static usernames in the Icinga module. If one logs in with "foo" the user is case-normalised, but the permissions check when downtiming a host is case-sensitive. That's not ideal, but the same situation has been around since forever and also applies to mod_ldap: when I log in as "MuehlenhoFF", I can log into the UI, but setting a downtime is rejected with "Not Authorized". There's no configuration setting to configure the case handling for the Icinga authorisations, but given that the user set is small "SREs and a handful of other people", that seems fine until we migrate to a more modern alerting solution.

TLDR, I think we're all good with the status quo :-)

TLDR, I think we're all good with the status quo :-)

Thanks for the in depth analysis and i largely agree with you

Revisiting this issue (which has been very useful to confirm assumptions we made in early planning!) came up with the topic of Icinga permissions: The permissions to downtime hosts etc. are configured with static usernames in the Icinga module. If one logs in with "foo" the user is case-normalised, but the permissions check when downtiming a host is case-sensitive. That's not ideal, but the same situation has been around since forever and also applies to mod_ldap: when I log in as "MuehlenhoFF", I can log into the UI, but setting a downtime is rejected with "Not Authorized". There's no configuration setting to configure the case handling for the Icinga authorisations, but given that the user set is small "SREs and a handful of other people", that seems fine until we migrate to a more modern alerting solution.

Yes this was the initial issue i was trying to solve. When logging in with cas the REMOTE_USERvariable which is ultimately used by icinga to make authorization decisions is based on the CAS populated variable HTTP_CAS_USER. This variable maintains the case that the user eters however CAS also populates the variable HTTP_X_CAS_CN which uses the exact case present in LDAP. in an ideal world it would be good to map REMOTE_USER to HTTP_X_CAS_CN instead of HTTP_CAS_USER to remove the confusion with icinga. however this is very much a low priority task as most users have allready got use to and are aware of this quirk.

As a side note i hacked a quick debug page onto puppetboard so we can see what variables mod_auth_cas. I will moigrate this to a page on people at somepoint

jbond lowered the priority of this task from Medium to Low.Jun 30 2020, 12:38 PM

Yes this was the initial issue i was trying to solve. When logging in with cas the REMOTE_USERvariable which is ultimately used by icinga to make authorization decisions is based on the CAS populated variable HTTP_CAS_USER. This variable maintains the case that the user eters however CAS also populates the variable HTTP_X_CAS_CN which uses the exact case present in LDAP. in an ideal world it would be good to map REMOTE_USER to HTTP_X_CAS_CN instead of HTTP_CAS_USER to remove the confusion with icinga. however this is very much a low priority task as most users have allready got use to and are aware of this quirk.

Ack, from my PoV given that it's just 30ish tech-savvy users and that we're intending to replace Icinga anyway in favour of something better, we could even close it as Declined.

As a side note i hacked a quick debug page onto puppetboard so we can see what variables mod_auth_cas. I will moigrate this to a page on people at somepoint

That's awesome! We could even add this as a profile option to profile::idp::client:httpd? Might be very useful for people to provide feedback once we get reports from people who are not in SRE.

Ack, from my PoV given that it's just 30ish tech-savvy users and that we're intending to replace Icinga anyway in favour of something better, we could even close it as Declined.

I mostly agree and have set priority to low, however i want to keep this around to play with on a friday or when things are quite just in case it comes up elses where

That's awesome! We could even add this as a profile option to profile::idp::client:httpd? Might be very useful for people to provide feedback once we get reports from people who are not in SRE.

Ill have a think about his, it requires some type of server side processing to walk the [equivalent of] $_SERVER vars so it may not be that easy to have a generic solution.

Change 610299 had a related patch set uploaded (by Jbond; owner: John Bond):
[operations/puppet@production] icinga: permissions add both cases for Vgutierrez

https://gerrit.wikimedia.org/r/610299

Change 610299 merged by Jbond:
[operations/puppet@production] icinga: permissions add both cases for Vgutierrez

https://gerrit.wikimedia.org/r/610299

Change 612356 had a related patch set uploaded (by Andrew Bogott; owner: Andrew Bogott):
[operations/puppet@production] eqiad1 neutron: move to galera-hosted database

https://gerrit.wikimedia.org/r/612356

Change 612356 merged by Andrew Bogott:
[operations/puppet@production] eqiad1 neutron: move to galera-hosted database

https://gerrit.wikimedia.org/r/612356