[patch] Case sensitive Authentication, but Case in-sensitive Authorization.

[andrew.krock@…]

I recently had an issue with my trac install and one of my programers. I am useing the following plugins:

• WebAdmin (from the trac website)
• TracAccountManager? (from trac-hacks)

My new programmer complained that he did not have the adequate permissions that he should have, so I created a test account named "test", and added it to the same permission-groups as that programmers account. All the permissions were set properly, the problem came from the fact that his account name contained uppercase letters. Creating an account called "TEST" and not enabling any permissions, gave me all the permissions assigned to the acount "test". To the login system "TEST" and "test" are completely different, however to the authorization (permission) system "TEST" and "test" are the exact same accounts, and furthermore will only apply the permissions set to the account "test" to both accounts when logged in.

This is a pretty big security hole in the AccountManagerPlugin System, as anyone can register an account using any combination of uppercase letters for any of your users or even permission groups.

This is pretty severe, I don't wish to have to add permissions for every combination of my admin accounts just to prevent anyone from hacking into my trac.

[Matt Good]

What is the value of "ignore_auth_case" in your trac.ini? The default is "false" in which case the Trac permission system will be case-sensitive, since this is normal for common authentication setups. I believe that option was added due to browsers being inconsistent about casing with NTLM authentication. So, if you've set it to "true" please set it back to "false" so that Trac uses it's normal case-sensitive behavior.

[Matt Good]

If this is just a case of accidentally enabling "ignore_auth_case" I can update the registration component to disable registration and log that the setting needs turned off to prevent possible security issues.

[Matt Good]

In #155 I checked that new users couldn't register an account that received any existing permissions beyond what all authenticated users had. It looks like my patch for that didn't take into account the possibility of "ignore_auth_case". So, I could account for it there, but it does seem safer to simply enforce case-sensitive user accounts.

[andrew.krock@…]

your right, it was ignore_auth_case set to true. Sorry, I didn't think of that, you might wish to patch it to either enforce ignore_auth_case to be disabled or to not allow users to register names w/ different case if auth_case is enabled... To protect idiots like myself from doing things like this.

[andrew.krock@…]

Yup changed ignore_auth_case, and once again im all safe, secure, and cozy.

[rupert thurner]

Replying to andrew.krock@gmail.com:

your right, it was ignore_auth_case set to true. Sorry, I didn't think of that, you might wish to patch it to either enforce ignore_auth_case to be disabled or to not allow users to register names w/ different case if auth_case is enabled... To protect idiots like myself from doing things like this.

this would be great! we have case insensitive set, and the users regularly fall over that by registering a different case user and then wondering why login is not possible.

[Matt Good]

Replying to ThurnerRupert:

this would be great! we have case insensitive set, and the users regularly fall over that by registering a different case user and then wondering why login is not possible.

All the authentication methods supported by the AccountManager are case sensitive, so there's no reason to enable this setting, and as Andrew pointed out it may actually lead to vulnerabilities. The patch I'm going to commit will disable registration until ignore_auth_case has been disabled.

[Matt Good]

(In [1535]) disable registration if ignore_auth_case is true to prevent permission hijacking (fixes #831)

[rupert thurner]

we use this setting to switch later on to a company login system which is case insensitive. it should not be that a plugin invalidates a valid trac setting, isn't it? why the registration module just does not respect the ignore_auth_case setting and makes the username lowercase on registration, like the rest of trac is doing?

[anonymous]

Replying to ThurnerRupert:

we use this setting to switch later on to a company login system which is case insensitive. it should not be that a plugin invalidates a valid trac setting, isn't it? why the registration module just does not respect the ignore_auth_case setting and makes the username lowercase on registration, like the rest of trac is doing?

and login maybe too.

[anonymous]

Replying to mgood:

(In [1535]) disable registration if ignore_auth_case is true to prevent permission hijacking (fixes #831)

I'm sorry, but this is bad behavior for a plugin to take. Trac's case-insensitivity setting should not make portions of this plugin not function. This plugin should, as other posters have mentioned, simply lowercase the username as Trac does in all other instances when case-insensitivity is enable. This would simplify everything and, frankly, is not difficult to implement.

[Matt Good]

Replying to anonymous:

I'm sorry, but this is bad behavior for a plugin to take. Trac's case-insensitivity setting should not make portions of this plugin not function.

Not if that feature poses a security risk.

This plugin should, as other posters have mentioned, simply lowercase the username as Trac does in all other instances when case-insensitivity is enable. This would simplify everything and, frankly, is not difficult to implement.

No, the security hole still exists if the names are simply converted to lowercase. This would be ok if you were starting with a new password file, but there could be problems if you have existing users registered with mixed-case names. The password store backends provided with this plugin are all case-sensitive, so I don't think it's wise to mix them with the ignore_auth_case setting. If you'd like to provide a patch that makes sure that ignore_auth_case is handled in a safe way I'll be glad to look at it, but for right now I'd rather disable this than leave open a potential security hole.

[anonymous]

Replying to mgood:

This plugin should, as other posters have mentioned, simply lowercase the username as Trac does in all other instances when case-insensitivity is enable. This would simplify everything and, frankly, is not difficult to implement.

No, the security hole still exists if the names are simply converted to lowercase. This would be ok if you were starting with a new password file, but there could be problems if you have existing users registered with mixed-case names. The password store backends provided with this plugin are all case-sensitive, so I don't think it's wise to mix them with the ignore_auth_case setting. If you'd like to provide a patch that makes sure that ignore_auth_case is handled in a safe way I'll be glad to look at it, but for right now I'd rather disable this than leave open a potential security hole.

the case is much more simple:

• if a user feel its a security hole, he does not set ignore_auth_case, or converts all the usernames to lowercase, or lets re-register everybody.
• if a user sets ignore_auth_case, he expects it to work throughout trac.
• so this is "​separation of concerns". authmanager plugin is not responsible for this, but trac.
• the password stores have to be case sensitive. otherwise it would be a real security problem.

[trac-hacks@…]

ccing

[John Hampton]

Replying to anonymous:

I'm of the same mind as anonymous. Having the registration module be disabled if you have ignore_auth_case on is stupid. If my users backend into any type of MS system, case-insensitive auth is expected. Also, if account manager simply lowercases all of users then there is no security hole as you won't be able to create multiple accounts with differing case.

As far as the existing users issue, I think documentation and a bit of manual intervention is acceptable.

I'll whip up a patch when I get a chance. If I don't hear any objections, I'll commit it. Please speak-up if you feel that this is an undesirable change

[anonymous2]

I have the same problem:

If a user logs in with "test" everything works as it is supposed to.

If he is using "Test" instead he gets logged in and even can see/edit the perfernces of "test" but he doesn't have his rights. So for example the WIKI page returns an error about no rights.

ignore_auth_case is set to "false".

So the auth function from this plugin seems to ignore the case and lets the user in, but trac itself doesn't give him the rights.

[Sebastian Krysmanski]

Patch against r5836 and Trac 0.11.4

[Sebastian Krysmanski]

I've added a patch to fix this problem. (For some reasons the Trac's patch view doesn't display the changes to api.py; but they're there if you download the original format).

[anonymous]

does the patch also apply for 0.11.5?

[Steffen Hoffmann]

Replying to anonymous2:

I have the same problem:

If a user logs in with "test" everything works as it is supposed to.

If he is using "Test" instead he gets logged in and even can see/edit the perfernces of "test" but he doesn't have his rights. So for example the WIKI page returns an error about no rights.

ignore_auth_case is set to "false".

So the auth function from this plugin seems to ignore the case and lets the user in, but trac itself doesn't give him the rights.

Did anyone check with Trac 0.12? Always been on 0.12 I can't reproduce this behavior right now. Could it be, this issue is limited to Trac <= 0.11 somehow?

[Steffen Hoffmann]

Replying to manski:

I've added a patch to fix this problem. (For some reasons the Trac's patch view doesn't display the changes to api.py; but they're there if you download the original format).

I've prepared a slightly modified version of your patch for local testing. If all goes well, I'll commit it before the weekend.

Thank you for another great contribution to a highly disputed issue, while others did just raise concerns.

BTW, I feel sad about missing Trac-compliance in such a critical area, but this is at least partially due to ineffective discussion by really capable developers, and I feel we have an exceptionally negative example within this ticket. Wake-up guys, as a matter of fact this can't survive here as a 'blocker' for 4(!) years now, if you're serious about anything else you said here.

[Steffen Hoffmann]

(In [9286]) AccountManagerPlugin: Allow for sane registration and username handling even with case-insensitive authentication, closes #831.

Finally allow using the Trac option ignore_auth_case = true.

[Steffen Hoffmann]

(In [10521]) AccountManagerPlugin: Compare to unchanged user list, refs #831.

Initial implementation by changeset [4638] led to false-positives and DOS, because the password check has always been done against authentication stores without changing their username casing.

[Mitar]

OK, I must say that this is the most unbelievable behavior. Why would authentication be case sensitive in the first place? Authentication should be case preserving (username is in the case the user registered it) but case insensitive (any case combination used for login should map to only one user). It is crazy that tEST nad Test are two users.

And authorization should then be linked to this one user (it can internally use lower case as a database key, if this is needed, or just preserving the case).

Of course how to do that when Trac itself mangles with names is problematic. But maybe we could simply do not allow registration with the same username when an user with the same username (but case insensitively) already exist.

And when login, we could try to get original case from the password backend and authenticate with that (or ask password backend itself to authenticate in the case-insensitive manner). But at the end the case should be that of the registered user, even if the user has specified different case when login.

[Mitar]

Patch which allows case preservation, but still case insensitive authentication, against r10250

[Mitar]

I have attached a patch against a bit old revision. I have extended API a bit so that password store can return proper username when user logs in.

Also I found two bugs:

• False value returned from password stores did not break the traversal of password stores, but was handled the same as None, this is in conflict with documentation
• In _create_user function, name and email was stored into session with values from request, not with values which were checked, for example through EmailVerificationModule