Ticket #831 (new defect)

Opened 2 years ago

Last modified 6 months ago

Case sensitive Authentication, and Case in-sensitive Authorization.

Reported by: andrew.krock@gmail.com Assigned to: pacopablo
Priority: highest Component: AccountManagerPlugin
Severity: blocker Keywords:
Cc: trac-hacks@BUGabundo.net Trac Release: 0.9

Description

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

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.

Attachments

Change History

10/19/06 20:50:46 changed by mgood

  • keywords set to needinfo.

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.

10/19/06 20:55:55 changed by mgood

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.

10/19/06 21:07:19 changed by mgood

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.

(follow-up: ↓ 6 ) 10/19/06 22:21:06 changed by 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.

10/19/06 22:26:26 changed by andrew.krock@gmail.com

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

(in reply to: ↑ 4 ; follow-up: ↓ 7 ) 10/31/06 05:11:25 changed by ThurnerRupert

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.

(in reply to: ↑ 6 ) 11/10/06 22:49:06 changed by mgood

  • keywords deleted.

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.

(follow-up: ↓ 11 ) 11/10/06 22:57:10 changed by mgood

  • status changed from new to closed.
  • resolution set to fixed.

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

(follow-up: ↓ 10 ) 11/14/06 12:21:43 changed by ThurnerRupert

  • status changed from closed to reopened.
  • resolution deleted.

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?

(in reply to: ↑ 9 ) 11/14/06 12:25:31 changed by 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.

(in reply to: ↑ 8 ; follow-up: ↓ 12 ) 08/08/07 12:26:19 changed by 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.

(in reply to: ↑ 11 ; follow-up: ↓ 13 ) 08/08/07 12:56:48 changed by mgood

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.

(in reply to: ↑ 12 ; follow-up: ↓ 15 ) 08/19/07 04:26:16 changed by 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.

05/06/08 08:18:20 changed by trac-hacks@BUGabundo.net

  • cc set to trac-hacks@BUGabundo.net.

ccing

(in reply to: ↑ 13 ) 05/27/08 14:24:21 changed by pacopablo

  • owner changed from mgood to pacopablo.
  • status changed from reopened to new.

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


Add/Change #831 (Case sensitive Authentication, and Case in-sensitive Authorization.)




Change Properties
Action