Bug#665696: gosa-sync breaks on passwords containing spaces
Steven Chamberlain a écrit, le 27/03/2012 01:54: Hi, On 26/03/12 10:05, Petter Reinholdtsen wrote: The fix for gosa.conf is not upgradable, so we need to come up with a better idea. The fix won't work. Using quotes in gosa.conf is no good if the %userPassword substitution could contain double quotes. yes the patch to gosa.conf I had first sent has to be reversed if GOsa is upgraded to escape userPassword (in functions.inc). With such an escaped %userPassword the variable can be sent to the gosa-sync script untampered, then the only thing to do is make sure gosa-sync handles it correctly : re-quote it to be used in kadmin, because kadmin only uses double quotes. Without that, it is possible, and fairly easy, for a user to exploit %userPassword to send any command to kadmin, run as root, which is a pretty big vulnerability at the moment. That's why I had send that patch to gosa-sync, which is the only thing to patch once GOsa's functions.inc is upgraded. --- /usr/share/debian-edu-config/tools/gosa-sync.orig 2012-03-25 09:28:32.0 +0200 +++ /usr/share/debian-edu-config/tools/gosa-sync2012-03-26 15:34:13.0 +0200 @@ -28,9 +28,10 @@ $USERPASSWORD EOF IAM=`ldapwhoami -x -Z -y $TMPFILE -D $USERDN 2/dev/null || true` +EUSERPASSWORD=`cat $TMPFILE | sed -e 's///g'` # escapes because kadmin need to use double quotes if [ $IAM = dn:$USERDN ] ; then cat $TMPFILE EOF -change_password -pw $USERPASSWORD $USERID +change_password -pw $EUSERPASSWORD $USERID EOF cat $TMPFILE | kadmin.local 21 | logger -t gosa-sync -p notice logger -t gosa-sync -p notice Kerberos password for \'$USERID\' changed.
Bug#665696: gosa-sync breaks on passwords containing spaces
[Samuel Krempp] yes the patch to gosa.conf I had first sent has to be reversed if GOsa is upgraded to escape userPassword (in functions.inc). OK. Then I believe we should patch gosa instead to fix it properly and completely, and get a fix into squeeze. For r1 we should probably provide our own patched package, until a update make it into squeeze proper. I undid the gosa.conf change and uploaded to squeeze-test, along with other fixes. This also get rid of the conffile question for those upgrading in the future. That's why I had send that patch to gosa-sync, which is the only thing to patch once GOsa's functions.inc is upgraded. Thank you. Applied. -- Happy hacking Petter Reinholdtsen -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#665696: gosa-sync breaks on passwords containing spaces
tags 665696 + security clone 665696 -1 reassign -1 gosa retitle -1 gosa: unescaped arguments used on a command line found -1 gosa/2.6.11-3 found -1 gosa/2.6.11-3+squeeze1 fixed -1 gosa/2.7.3-1 tags -1 + squeeze fixed-upstream blocks 665696 by -1 thanks Hi! So, the problem here was that %userPassword, or similar string substitutions into command lines specified in gosa.conf, are not escaped; and adding quotes to the gosa.conf file cannot properly escape them either. On 27/03/12 10:27, Petter Reinholdtsen wrote: [Samuel Krempp] [...] to escape userPassword (in functions.inc). OK. Then I believe we should patch gosa instead to fix it properly and completely, and get a fix into squeeze. For r1 we should probably provide our own patched package, until a update make it into squeeze proper. I was going to suggest we chase this upstream, but then I noticed: * gosa 2.6.12 - Escaped command line arguments in some locations - Updated password handling and hooks, allows sepcial chars in passwords - Added lock/unlock events for users $ grep -nR %userP gosa-core-2.6.13/ gosa-core-2.6.13/include/functions.inc:3075:$command= preg_replace(/%userPassword/, escapeshellarg($password), $command); $ ls -al gosa-core-2.6.13/include/functions.inc -rw-r--r-- 1 steven steven 104887 Dec 14 2010 gosa-core-2.6.13/include/functions.inc They already fixed this in the 2.6 series (back in December), and apparently made similar fixes in other places too? In my opinion the fixes of 2.6.12 want to go into Debian s-p-u. Maybe even to security if it could be an issue outside of Debian Edu; fortunately I think the 'sudo' command line in gosa.conf was something specific to Debian Edu and that other GOsa users are not at such a risk by default. Regards, -- Steven Chamberlain ste...@pyro.eu.org -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#665696: gosa-sync breaks on passwords containing spaces
The fix for gosa.conf is not upgradable, so we need to come up with a better idea. When upgrading squeeze-test to the new version of debian-edu-config with the new gosa.conf file, a conffile question is asked and both options (keeping the old or upgrading to the new file) are wrong. The old file have the password quoting issue and the correct LDAP password, the new file have a fix for the password quoting issue but lack the correct LDAP password. If I pick the old one the security issue is still present. If I pick the new one, gosa stop working completely. Anyone got an idea how to fix this for upgrades? -- Happy hacking Petter Reinholdtsen -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#665696: gosa-sync breaks on passwords containing spaces
Petter Reinholdtsen a écrit, le 26/03/2012 11:05: The fix for gosa.conf is not upgradable, so we need to come up with a better idea. When upgrading squeeze-test to the new version of debian-edu-config with the new gosa.conf file, a conffile question is asked and both options (keeping the old or upgrading to the new file) are wrong. The old file have the password quoting issue and the correct LDAP password, the new file have a fix for the password quoting issue but lack the correct LDAP password. Personnally here I didn't take the time to upgrade GOsa, fearing other issues. But I did fix /usr/share/gosa/include/functions.inc with escapeshellarg($password), and then modified gosa-sync that needs specific escaping for kadmin : --- /usr/share/debian-edu-config/tools/gosa-sync.orig 2012-03-25 09:28:32.0 +0200 +++ /usr/share/debian-edu-config/tools/gosa-sync2012-03-26 15:34:13.0 +0200 @@ -28,9 +28,10 @@ $USERPASSWORD EOF IAM=`ldapwhoami -x -Z -y $TMPFILE -D $USERDN 2/dev/null || true` +EUSERPASSWORD=`cat $TMPFILE | sed -e 's///g'` # escapes because kadmin need to use double quotes if [ $IAM = dn:$USERDN ] ; then cat $TMPFILE EOF -change_password -pw $USERPASSWORD $USERID +change_password -pw $EUSERPASSWORD $USERID EOF cat $TMPFILE | kadmin.local 21 | logger -t gosa-sync -p notice logger -t gosa-sync -p notice Kerberos password for \'$USERID\' changed. And I verified it to handle spaces, double and single quotes, and backslashes. It only breaks on double backslashes, but that's at the PHP level replacing \\ with \, and does not lead to vulnerability AFAICT - it just means that password wont work. Is that good with you ? -- Samuel Krempp -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#665696: gosa-sync breaks on passwords containing spaces
Hi, On 26/03/12 10:05, Petter Reinholdtsen wrote: The fix for gosa.conf is not upgradable, so we need to come up with a better idea. The fix won't work. Using quotes in gosa.conf is no good if the %userPassword substitution could contain double quotes. As Samuel said, the correct fix is for GOsa to use escapeshellarg(), and while there I see no reason not to do the same for all the others, like %uid or %homeDirectory in case GOsa ever forgets to sanitise them (coding defensively in case of a bug elsewhere). After doing escapeshellarg(), the quotes in gosa.conf actually have to be removed, or else you are double-quoting and would get extra quotes (single) included within the password. Regards, -- Steven Chamberlain ste...@pyro.eu.org -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#665696: gosa-sync breaks on passwords containing spaces
package: debian-edu-config severity: important version: squeeze/r0 spaces need adequate quoting of the password variable in both gosa-sync and gosa.conf. It is also very likely a security hazard in letting the user-supplied password string unquoted in those two files, whence severity=important. following patch just adds the quoting, and was verified to fix the issue. -- Samuel Krempp --- /etc/gosa/gosa.conf.befSK 2012-03-25 09:45:33.0 +0200 +++ /etc/gosa/gosa.conf 2012-03-25 09:50:10.0 +0200 @@ -44,7 +44,7 @@ plugin acl=users/phoneAccount:self class=phoneAccount/ plugin acl=users/nagiosAccount:self class=nagiosAccount/ plugin acl=users/scalixAccount:self class=scalixAccount/ - plugin acl=users/password:self class=password postmodify=USERPASSWORD=%userPassword /usr/bin/sudo /usr/share/debian-edu-config/tools/gosa-sync %dn/ + plugin acl=users/password:self class=password postmodify=USERPASSWORD=quot;%userPasswordquot; /usr/bin/sudo /usr/share/debian-edu-config/tools/gosa-sync %dn/ /section !-- Section to enable administrative services -- --- /usr/share/debian-edu-config/tools/gosa-sync.orig 2012-03-25 09:28:32.0 +0200 +++ /usr/share/debian-edu-config/tools/gosa-sync2012-03-25 09:56:04.0 +0200 @@ -15,7 +15,6 @@ ## principal's one. RETVAL=0 - USERDN=$1 USERID=`echo $USERDN | sed s/^uid=\([^,]*\),.*$/\1/` @@ -30,7 +29,7 @@ IAM=`ldapwhoami -x -Z -y $TMPFILE -D $USERDN 2/dev/null || true` if [ $IAM = dn:$USERDN ] ; then cat $TMPFILE EOF -change_password -pw $USERPASSWORD $USERID +change_password -pw $USERPASSWORD $USERID EOF cat $TMPFILE | kadmin.local 21 | logger -t gosa-sync -p notice logger -t gosa-sync -p notice Kerberos password for \'$USERID\' changed.
Bug#665696: gosa-sync breaks on passwords containing spaces
tags 665696 + pending thanks [Samuel Krempp] following patch just adds the quoting, and was verified to fix the issue. Thank you. I have commited the fix to svn. -- Happy hacking Petter Reinholdtsen -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#665696: gosa-sync breaks on passwords containing spaces
Petter Reinholdtsen a écrit, le 25/03/2012 10:45: tags 665696 + pending thanks [Samuel Krempp] following patch just adds the quoting, and was verified to fix the issue. Thank you. I have commited the fix to svn. the issue remains for other special characters, at least quotes. But the only way to really solve the issue is in GOsa functions.inc : $command= preg_replace(/%userPassword/, $password, $command); $password should be properly escaped here otherwise there is no way to write a safe command-line using %userPassword. The proper solution seems to be http://php.net/manual/en/function.escapeshellarg.php once the script parameters are properly escaped in php, there should be no need for quoting in gosa.conf, and this patch might have to be reversed. I see GOsa devs noticed the security issue 19 months ago : https://oss.gonicus.de/labs/gosa/ticket/1026 Additionally the script parameter are not escaped right now, somebody could do nasty thing with it. I will have a look at this too. How serious is knowingly leaving such a vulnerability, with easy fix, open for 19 months ? -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#665696: gosa-sync breaks on passwords containing spaces
Samuel Krempp a écrit, le 25/03/2012 11:41: I see GOsa devs noticed the security issue 19 months ago : https://oss.gonicus.de/labs/gosa/ticket/1026 Additionally the script parameter are not escaped right now, somebody could do nasty thing with it. I will have a look at this too. How serious is knowingly leaving such a vulnerability, with easy fix, open for 19 months ? Sorry, did not check before posting, the issue was indeed fixed 19 months ago in GOsa trunk, I shouldn't send emails with one hand while playing with my kids with the other : https://oss.gonicus.de/labs/gosa/changeset/19467 It's been present in releases since GOsa's 2.6.12, so SkoleLinux should upgrade. It's rather important to prevent malicious students to execute arbitrary commands as www-data, and hopefully there isn't any change that breaks skolelinux : https://oss.gonicus.de/labs/gosa/changeset?old_path=%2Ftags%2F2.6.12old=20607new_path=%2Ftags%2F2.6.11new=20520 Once GOsa version is updated and %userPassword is properly escaped, my patch will likely have to reversed. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
