Page MenuHomePhabricator

LDAPAuthentication2 allows login with invalid password
Closed, ResolvedPublic

Description

From security@

Mediawiki v1.31.5 with extensions LDAPProvider v1.0.1 and
LDAPAuthentication2  v1.0.0

With $LDAPAuthentication2AllowLocalLogin = true; and choosing the local
database as domain in the login screen,

I can log in with a username from LDAP or the local database with nearly
any random Password.

This problem is reported by another user here :

https://www.mediawiki.org/wiki/Extension_talk:LDAPAuthentication2

Best,

Henrik Schmidt

Details

Related Gerrit Patches:
mediawiki/extensions/LDAPAuthentication2 : REL1_32Fix local login issue
mediawiki/extensions/LDAPAuthentication2 : REL1_33Fix local login issue
mediawiki/extensions/LDAPAuthentication2 : REL1_34Fix local login issue

Event Timeline

Reedy created this task.Dec 10 2019, 12:55 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 10 2019, 12:55 PM

Subscribing @Hschmidt-ti as the reporter

Thanks for reporting. I will have a look at it ASAP.

sbassett triaged this task as High priority.Dec 10 2019, 3:57 PM
sbassett moved this task from Backlog / Other to External (Non-WMF) Issues on the acl*security board.
sbassett added a subscriber: sbassett.

I know this isn't official, but at least it does not appear to be widely used.

https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/LDAPAuthentication2/+/556196/

This patch does not work. I can't login with a valid password now -> "Fatal error authenticating user."

@Hschmidt-ti There is a new version of the patch available. It's working for me, but I'd appreciate your confirmation.

I get an internal error:

[9a90659b7fbf5e72a26e011b] /llocs/Special:PluggableAuthLogin Error from line 158 of /var/www/html/mediawiki_hbs-1.31.5/extensions/LDAPAuthentication2/src/PluggableAuth.php: Call to undefined method MediaWiki\MediaWikiServices::getPasswordFactory()
Backtrace:
#0 /var/www/html/mediawiki_hbs-1.31.5/extensions/LDAPAuthentication2/src/PluggableAuth.php(60): MediaWiki\Extension\LDAPAuthentication2\PluggableAuth->checkLocalPassword(string, string)
#1 /var/www/html/mediawiki_hbs-1.31.5/extensions/PluggableAuth/includes/PluggableAuthLogin.php(31): MediaWiki\Extension\LDAPAuthentication2\PluggableAuth->authenticate(NULL, string, NULL, NULL, NULL)
#2 /var/www/html/mediawiki_hbs-1.31.5/includes/specialpage/SpecialPage.php(565): PluggableAuthLogin->execute(NULL)
#3 /var/www/html/mediawiki_hbs-1.31.5/includes/specialpage/SpecialPageFactory.php(568): SpecialPage->run(NULL)
#4 /var/www/html/mediawiki_hbs-1.31.5/includes/MediaWiki.php(288): SpecialPageFactory::executePath(Title, RequestContext)
#5 /var/www/html/mediawiki_hbs-1.31.5/includes/MediaWiki.php(861): MediaWiki->performRequest()
#6 /var/www/html/mediawiki_hbs-1.31.5/includes/MediaWiki.php(524): MediaWiki->main()
#7 /var/www/html/mediawiki_hbs-1.31.5/index.php(42): MediaWiki->run()
#8 {main}

Oh. Seems I have developed against MediaWiki 1.33. Sorry for that. Here is another patch: https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/LDAPAuthentication2/+/557021/ ATTENTION: This is a different URL than the previous one. Looks like we need different implementations for MW 1.31 LTS and upcoming MW 1.35 LTS

It is working as expected now. Thanks.

Osnard closed this task as Resolved.Dec 17 2019, 6:57 AM
Osnard claimed this task.

Great! Thank you for your help!

@Osnard @Hschmidt-ti - I'd like to make this task public if we can, but I wanted to confirm that the gerrit patch was deployed to any MediaWiki systems that either of you care about first. I'd also like to include this task within T234983 given that we're trying to better communicate security issues like this to the community. Thanks.

Reedy added a comment.Dec 17 2019, 6:41 PM

Something looks a little odd with the branches

It's been backported to REL1_31 as an LTS, which is fine, but according to https://gerrit.wikimedia.org/r/#/admin/projects/mediawiki/extensions/LDAPAuthentication2,branches there are only REL1_27, REL1_31 and master in gerrit

https://github.com/wikimedia/mediawiki-extensions-LDAPAuthentication2/branches has REL1_33 and REL1_34 too

Not sure where REL1_32 has gone from both, and why REL1_33 and REL1_34 are missing from gerrit

@sbassett - Thanks for caring. I have applied the patch to our systems.

Something looks a little odd with the branches
It's been backported to REL1_31 as an LTS, which is fine, but according to https://gerrit.wikimedia.org/r/#/admin/projects/mediawiki/extensions/LDAPAuthentication2,branches there are only REL1_27, REL1_31 and master in gerrit
https://github.com/wikimedia/mediawiki-extensions-LDAPAuthentication2/branches has REL1_33 and REL1_34 too
Not sure where REL1_32 has gone from both, and why REL1_33 and REL1_34 are missing from gerrit

I imagine they were manually deleted for some reason, unhelpfully. I've manually re-created REL1_33 and REL1_34 in gerrit.

The branch point of REL1_32 in core (1b24617001ce) maps to 7faf6a3c73f1aa1140de110e725321ee4b309806 as the candidate commit for REL1_32; anyone object to me fixing it to that?

The branch point of REL1_32 in core (1b24617001ce) maps to 7faf6a3c73f1aa1140de110e725321ee4b309806 as the candidate commit for REL1_32; anyone object to me fixing it to that?

No objection here.

Done. I've cherry-picked and merged (plus some other commits to make them clean) to REL1_34, and REL1_33. Pulling to REL1_32 requires git cherry-pick -x 41199293856019472e03fab935ce1b9e38e56655 6830fd88ddef972b31b3de5407f58a97802372da 41199293856019472e03fab935ce1b9e38e56655 6830fd88ddef972b31b3de5407f58a97802372da 0440ed9341ff9385c62007df3f474748cd74999a 8120386acd63477be082bc49b28d0aff76aab90f 8120386acd63477be082bc49b28d0aff76aab90f 4d0f30657b979cb0935a65d8c1d9f83838a7f581 4fb9e439f2610731ba2f505c148bf9141fcfa3c8 which seems a bit aggressive? Pushed as https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/LDAPAuthentication2/+/559475 and down for consideration.

Thanks for your work on this @Jdforrester-WMF! If I understand correctly from @Osnard, they only support LTS releases so as a matter of course remove the non-LTS branches to prevent folks from relying on unsupported/untested branches. This does not align completely with either the master or release branch compatibility policies [0]. The automatic release branch generation causes similar issues with the master compatibility policy. I'm not sure what an ideal resolution for this would be, but I'm noting this here for informational purposes since @Osnard is unavailable to comment right now.

[0] https://www.mediawiki.org/wiki/Compatibility#mediawiki_extensions

The extension's infobox says "release branches" re: compatibility. Should that be updated?

sbassett changed the visibility from "Custom Policy" to "Public (No Login Required)".Dec 19 2019, 6:37 PM

@sbassett The template allows only for values rel or master in the compatibility policy field. I only support LTS release branches. Usually I remove all other branched. Might have forgotten that, so thanks for cherry-picking to the other branches, @Jdforrester-WMF , but I'll probably remove these in the near future.

Change 559475 abandoned by Robert Vogel:
Fix local login issue

Reason:
Not supported release branch

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

Reedy added a comment.Dec 20 2019, 2:47 PM

@sbassett The template allows only for values rel or master in the compatibility policy field. I only support LTS release branches. Usually I remove all other branched. Might have forgotten that, so thanks for cherry-picking to the other branches, @Jdforrester-WMF , but I'll probably remove these in the near future.

Filed T241243: Update {{Extension}} to accept compatibility policy = lts so we can more accurate reflect this support policy in the docs

@sbassett The template allows only for values rel or master in the compatibility policy field. I only support LTS release branches. Usually I remove all other branched. Might have forgotten that, so thanks for cherry-picking to the other branches, @Jdforrester-WMF , but I'll probably remove these in the near future.

I'm not sure deleting the branches does what you think it does. It doesn't say "hey, this extension's maintainers are telling you not to use this branch", it says "hey, complain to James/Lego/etc. that the extension distributor/git infrastructure has flaked out". :-(

Osnard added a comment.Jan 7 2020, 7:06 AM

I am sorry to hear that. Of course I don't want to cause any support requests or complains to the infrastructure-team. But I am using the LTS version of MediaWiki in any installation. Therefore all my extensions are compatible to that version, not to any other stable or even current master. To me it was logical to only keep branches that are actually maintained.

So the mw.org Extension template was updated in T241243. And LDAPAuthentication2's info box was changed to reflect that it only supports LTS versions. I understand that isn't anywhere close to a foolproof solution, but it's a place to quickly point anyone who has questions regarding "missing" release branches.