Re: [Wikitech-l] PHPUnit tests now fixed

2009-08-07 Thread Platonides
Aryeh Gregor wrote:
 In that case, I think
 AdminSettings.php is certainly a good idea, so it could be readable
 only to root and not the web server. 

only by /root/?

If an attacker has read access to your AdminSettings.php he might as
well have write permissions.
He just needs to change your php files and wait until you run a
maintenance script to get mailed your /etc/shadow, or rm -rf / you.

Maintenance scripts shouldn't be run as root. OTOH that would be a good
method if you used a specific account eg. 'WikiAdmin'. Still, you might
get funny permissions when running scripts that deal with files.


___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


Re: [Wikitech-l] PHPUnit tests now fixed

2009-08-07 Thread Aryeh Gregor
On Fri, Aug 7, 2009 at 5:18 AM, Platonidesplatoni...@gmail.com wrote:
 only by /root/?

Well, or otherwise not readable by the web server, like 640
root:admins.  You're right that there's no reason to run a PHP script
as root if you only need root DB access, of course.

 Maintenance scripts shouldn't be run as root. OTOH that would be a good
 method if you used a specific account eg. 'WikiAdmin'. Still, you might
 get funny permissions when running scripts that deal with files.

Well, when I run maintenance scripts I usually don't bother sudoing to
www-data anyway, personally.  Practically none of the scripts touch
files.

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


[Wikitech-l] PHPUnit tests now fixed

2009-08-06 Thread dan nessett
I have fixed 5 bugs in /tests/ and added one feature to run-tests.php (a 
--runall option so testers can run the PHPUnit tests without using make - 
although make test still works). With these changes all of the tests in /tests/ 
now work. A unified diff patch is attached to bug ticket 20077.

I had to make an architectural decision when enhancing run-test.php with the 
--runall option. The bug ticked describes this decision and suggests two other 
ways to achieve the same objective. I chose the approach implemented by the 
patch because it required no changes to the directory structure of /tests/. 
However, I actually prefer the second possibility. So, if senior developers 
could look at the bug ticket description and give me some feedback (especially 
if they also think the second option is better), that would be great.

I also would appreciate some feedback on the following question. One of the 
tests referenced the global variables $wgDBadminname and $wgDBadminuser. When I 
ran the configuration script during Mediawiki installation on my machine, the 
LocalSettings.php file created defined the globals $wgDBname and $wgDBuser. So, 
I changed the test to use these variables rather than the 'admin' versions. 
However, I don't remember if the script gave me a choice to use the 'admin' 
versions or not. Also, if the configuration script has changed, then some 
installations may use the 'admin' versions and some may not. In either case, I 
would have to modify the bug fix to accept both types of global variable. If 
someone would fill me in, I can make any required changes to the bug fix.


  

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


Re: [Wikitech-l] PHPUnit tests now fixed

2009-08-06 Thread Happy-melon
dan nessett dness...@yahoo.com wrote in message 
news:630381.19130...@web32503.mail.mud.yahoo.com...
 I also would appreciate some feedback on the following question. One of 
 the tests referenced the global variables $wgDBadminname and 
 $wgDBadminuser. When I ran the configuration script during Mediawiki 
 installation on my machine, the LocalSettings.php file created defined the 
 globals $wgDBname and $wgDBuser. So, I changed the test to use these 
 variables rather than the 'admin' versions. However, I don't remember if 
 the script gave me a choice to use the 'admin' versions or not. Also, if 
 the configuration script has changed, then some installations may use the 
 'admin' versions and some may not. In either case, I would have to modify 
 the bug fix to accept both types of global variable. If someone would fill 
 me in, I can make any required changes to the bug fix.

I think these are to allow you to define a separate MySQL user (in 
adminsettings.php) that can be given higher privileges, as a security 
device.  IIRC this is now deprecated (cf bug18768).

--HM 



___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


Re: [Wikitech-l] PHPUnit tests now fixed

2009-08-06 Thread Chad
On Thu, Aug 6, 2009 at 12:56 PM, Happy-melonhappy-me...@live.com wrote:
 dan nessett dness...@yahoo.com wrote in message
 news:630381.19130...@web32503.mail.mud.yahoo.com...
 I also would appreciate some feedback on the following question. One of
 the tests referenced the global variables $wgDBadminname and
 $wgDBadminuser. When I ran the configuration script during Mediawiki
 installation on my machine, the LocalSettings.php file created defined the
 globals $wgDBname and $wgDBuser. So, I changed the test to use these
 variables rather than the 'admin' versions. However, I don't remember if
 the script gave me a choice to use the 'admin' versions or not. Also, if
 the configuration script has changed, then some installations may use the
 'admin' versions and some may not. In either case, I would have to modify
 the bug fix to accept both types of global variable. If someone would fill
 me in, I can make any required changes to the bug fix.

 I think these are to allow you to define a separate MySQL user (in
 adminsettings.php) that can be given higher privileges, as a security
 device.  IIRC this is now deprecated (cf bug18768).

 --HM



 ___
 Wikitech-l mailing list
 Wikitech-l@lists.wikimedia.org
 https://lists.wikimedia.org/mailman/listinfo/wikitech-l


Yes, it's been deprecated. AdminSettings will still load in maintenance
environments (commandLine.inc and Maintenance.php) if it still exists,
but it is no longer _required_ for anything. These variables can safely
be set in LocalSettings.

HM is right on what these users are for. Some (not all) maintenance
scripts require higher permissions than your normal $wgDBuser, so
$wgDBadminuser is supposed to have those privileges.

In practice, I've found that the vast majority of maintenance scripts
don't actually need this much permission. I'm trying to clean that
up over time, so we're not using a user with higher permissions when
it's not needed. (see the Maintenance constants and the getDbType()
function).

-Chad

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] PHPUnit tests now fixed

2009-08-06 Thread Aryeh Gregor
On Thu, Aug 6, 2009 at 1:05 PM, Chadinnocentkil...@gmail.com wrote:
 HM is right on what these users are for. Some (not all) maintenance
 scripts require higher permissions than your normal $wgDBuser, so
 $wgDBadminuser is supposed to have those privileges.

$wgDBuser needs to have DELETE rights on pretty much all tables, so
what's the security gain of bothering with a different user for ALTER
TABLE/CREATE TABLE/etc.?  $wgDBadminuser doesn't need to be able to
create new databases or reconfigure replication or anything, right?

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


Re: [Wikitech-l] PHPUnit tests now fixed

2009-08-06 Thread Chad
On Thu, Aug 6, 2009 at 1:20 PM, Aryeh
Gregorsimetrical+wikil...@gmail.com wrote:
 On Thu, Aug 6, 2009 at 1:05 PM, Chadinnocentkil...@gmail.com wrote:
 HM is right on what these users are for. Some (not all) maintenance
 scripts require higher permissions than your normal $wgDBuser, so
 $wgDBadminuser is supposed to have those privileges.

 $wgDBuser needs to have DELETE rights on pretty much all tables, so
 what's the security gain of bothering with a different user for ALTER
 TABLE/CREATE TABLE/etc.?  $wgDBadminuser doesn't need to be able to
 create new databases or reconfigure replication or anything, right?

 ___
 Wikitech-l mailing list
 Wikitech-l@lists.wikimedia.org
 https://lists.wikimedia.org/mailman/listinfo/wikitech-l


Depends on which maintenance script you're talking about. Update.php
certainly does, as does renameDbPrefix (just to grab one off the top of
my head). The vast majority of scripts can function just fine with normal
DB access. Some (mcc and digit2html, to name a few) don't need any
DB access at all.

-Chad

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] PHPUnit tests now fixed

2009-08-06 Thread Brion Vibber
On 8/6/09 10:30 AM, Chad wrote:
 Depends on which maintenance script you're talking about. Update.php
 certainly does, as does renameDbPrefix (just to grab one off the top of
 my head). The vast majority of scripts can function just fine with normal
 DB access. Some (mcc and digit2html, to name a few) don't need any
 DB access at all.

Generally, only updaters need create/alter/etc privs; a few script that 
do mass rebuilds of indexes will also want to do things like dropping 
and re-adding indexes -- for example rebuildTextIndex drops the fulltext 
index on the searchindex table, then re-adds it after refilling the 
table's contents.

And a few extreme maintenance ops like MySQL replication master 
switches of course will need all kinds of fun privs. ;)

-- brion

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


Re: [Wikitech-l] PHPUnit tests now fixed

2009-08-06 Thread Chad
On Thu, Aug 6, 2009 at 3:04 PM, Brion Vibberbr...@wikimedia.org wrote:
 On 8/6/09 10:30 AM, Chad wrote:
 Depends on which maintenance script you're talking about. Update.php
 certainly does, as does renameDbPrefix (just to grab one off the top of
 my head). The vast majority of scripts can function just fine with normal
 DB access. Some (mcc and digit2html, to name a few) don't need any
 DB access at all.

 Generally, only updaters need create/alter/etc privs; a few script that
 do mass rebuilds of indexes will also want to do things like dropping
 and re-adding indexes -- for example rebuildTextIndex drops the fulltext
 index on the searchindex table, then re-adds it after refilling the
 table's contents.

 And a few extreme maintenance ops like MySQL replication master
 switches of course will need all kinds of fun privs. ;)

 -- brion

 ___
 Wikitech-l mailing list
 Wikitech-l@lists.wikimedia.org
 https://lists.wikimedia.org/mailman/listinfo/wikitech-l


Right, which is what my idea behind getDbType() was (which still needs
actual implementation, it's more an idea than practice at the moment).
If we don't need root DB access, we shouldn't be using it! If we don't need
DB access at all, don't bother connecting.

-Chad

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


Re: [Wikitech-l] PHPUnit tests now fixed

2009-08-06 Thread Brion Vibber
On 8/6/09 12:10 PM, Chad wrote:
 Right, which is what my idea behind getDbType() was (which still needs
 actual implementation, it's more an idea than practice at the moment).
 If we don't need root DB access, we shouldn't be using it! If we don't need
 DB access at all, don't bother connecting.

nom nom nom...

Might be good to be able to req increased privileges on command line if 
we can't do it with default user as well.

-- brion

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l