You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lenya.apache.org by Jörn Nettingsmeier <po...@uni-duisburg.de> on 2006/06/05 00:11:19 UTC

[patch] modify admin.changePassword usecase to default to current user id

hi *!


when i tried to use the currently "hidden" admin.changePassword usecase,
i realized it is quite hard to use, because it will throw a very nasty
exception if one does not also set the userId parameter. this is
unintuitive imho: if userId is not set explicitly, it should default to
changing the password of the currently active user.

the attached patch tries to accomplish this. could you please review it?
it seems to me that my way to get the currently active user id is
overcomplicated... there must be a shortcut somewhere, but since i could
not find any documentation about what information is available in the
context of a usecase, i basically grepped for static methods in the api
and worked my way up :'(


best,

jörn



-- 
"Open source takes the bullshit out of software."
	- Charles Ferguson on TechnologyReview.com

--
Jörn Nettingsmeier, EDV-Administrator
Institut für Politikwissenschaft
Universität Duisburg-Essen, Standort Duisburg
Mail: pol-admin@uni-due.de, Telefon: 0203/379-2736

Re: grave security issue in trunk!!!

Posted by Andreas Hartmann <an...@apache.org>.
Joern Nettingsmeier wrote:
> Jörn Nettingsmeier wrote:
>> i just realized we have a huuuuge security hole that affects every lenya
>> 1.4 installation:
>>
>> * checkOldPassword is set by the (potentially hostile) client.
>> * the java code does not check that only admins may change passwords for
>> other user-ids than the currently logged-in one.
>>
>> ergo any user can change the passwords of arbitrary other users,
>> including admins. instant dos and privilege escalation, remotely
>> exploitable. not nice at all.
> <..>
>> i will try to fix this on friday if no one gets to do it tomorrow.
> 
> hi guys! before i try to implement it, please comment on this new policy
> for changing passwords:
> 
> /**
>  * Usecase to change a user's password.
>  */
> public class UserPassword extends AccessControlUsecase {
> /*
>     If the optional parameter "userId" is not set, we assume that the
>     password of the currently logged in user is to be changed.
> 
>     Non-privileged users (i.e. those not belonging to the "admin" group)
>     cannot set userId to another user, i.e. they can only ever change
>     their own passwords.
> 
>     Privileged users (those in the admin group) can set userId, and thus
>     change the passwords of other users.
> 
>     All users will have to provide their own password again when they
>     try to change passwords, regardless of the fact that they are
>     already authenticated.
>     This is to protect users who leave their session unattended "for
>     just a second" from having their passwords changed by passers-by.
> 
>     a formerly existing parameter "checkPassword", which could be
>     used to override this password check, has been abolished for
>     security reasons.
> */
> 
> 
> 
> as you see, this adds a very special meaning to the group "admin". is
> that appropriate and in keeping with current usage? if so, i want to
> make sure that this is sufficiently documented. where are the current
> semantics of the admin group defined? (if the answer is "in the source
> code somewhere", bzzzt, zero points.) what would be the best place to
> define security semantics authoritatively?

IMO the cleanest way would be:

- changing passwords without entering the old password should be a
   different usecase (it could be handled by the same class + JX template)

- you set the permissions in usecase-policies.xconf

This way, we don't need a group with special semantics.

An alternative would be to make the "admin" group configurable for the
UserPassword usecase, but I'd prefer the first solution.

WDYT?

-- Andreas


-- 
Andreas Hartmann
Wyona Inc.  -   Open Source Content Management   -   Apache Lenya
http://www.wyona.com                      http://lenya.apache.org
andreas.hartmann@wyona.com                     andreas@apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
For additional commands, e-mail: dev-help@lenya.apache.org


Re: grave security issue in trunk!!!

Posted by Joern Nettingsmeier <po...@uni-due.de>.
Andreas Hartmann wrote:
> Doug Chestnut wrote:
>> Currently usecases get restrictive with policies in
>> usecase-policies.xml.  Should we make usecase-policies.xml be
>> permissive instead (only allow usecase execution if a policy exists
>> and the policy is met).  This would force us to think about policies
>> when creating new functionality.
> 
> +1

**2 :-D


-- 
"Án nýrra verka, án nútimans, hættir fortíðin að vekja áhuga."
"Without new works, without the present the past will cease to be of
interest."
        - Ásmundur Sveinsson (1893-1982)

--
Jörn Nettingsmeier, EDV-Administrator
Institut für Politikwissenschaft
Universität Duisburg-Essen, Standort Duisburg
Mail: pol-admin@uni-due.de, Telefon: 0203/379-2736


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
For additional commands, e-mail: dev-help@lenya.apache.org


Re: grave security issue in trunk!!!

Posted by Andreas Hartmann <an...@apache.org>.
Doug Chestnut wrote:
> Currently usecases get restrictive with policies in 
> usecase-policies.xml.  Should we make usecase-policies.xml be permissive 
> instead (only allow usecase execution if a policy exists and the policy 
> is met).  This would force us to think about policies when creating new 
> functionality.

+1

-- Andreas

> 
> --Doug
> 
> Andreas Hartmann wrote:
>> Doug Chestnut wrote:
>>
>>> Hi Joern,
>>> The first thing to change would be to add
>>>     <usecase id="admin.changePassword">
>>>         <role id="admin"/>
>>>     </usecase>
>>>
>>> to your (and our default pubs) pubs usecase-policies.xml
>>
>>
>> Exactly - and we could declare two different usecases so that one can
>> be permitted only for admins and the other one (with entering the old
>> password) for all users (see my other reply).
>>
>> -- Andreas
>>
>>>
>>> --Doug
>>>
>>> Joern Nettingsmeier wrote:
>>>
>>>> Jörn Nettingsmeier wrote:
>>>>
>>>>> i just realized we have a huuuuge security hole that affects every 
>>>>> lenya
>>>>> 1.4 installation:
>>>>>
>>>>> * checkOldPassword is set by the (potentially hostile) client.
>>>>> * the java code does not check that only admins may change 
>>>>> passwords for
>>>>> other user-ids than the currently logged-in one.
>>>>>
>>>>> ergo any user can change the passwords of arbitrary other users,
>>>>> including admins. instant dos and privilege escalation, remotely
>>>>> exploitable. not nice at all.
>>>>
>>>>
>>>> <..>
>>>>
>>>>> i will try to fix this on friday if no one gets to do it tomorrow.
>>>>
>>>>
>>>>
>>>> hi guys! before i try to implement it, please comment on this new 
>>>> policy
>>>> for changing passwords:
>>>>
>>>> /**
>>>>  * Usecase to change a user's password.
>>>>  */
>>>> public class UserPassword extends AccessControlUsecase {
>>>> /*
>>>>     If the optional parameter "userId" is not set, we assume that the
>>>>     password of the currently logged in user is to be changed.
>>>>
>>>>     Non-privileged users (i.e. those not belonging to the "admin" 
>>>> group)
>>>>     cannot set userId to another user, i.e. they can only ever change
>>>>     their own passwords.
>>>>
>>>>     Privileged users (those in the admin group) can set userId, and 
>>>> thus
>>>>     change the passwords of other users.
>>>>
>>>>     All users will have to provide their own password again when they
>>>>     try to change passwords, regardless of the fact that they are
>>>>     already authenticated.
>>>>     This is to protect users who leave their session unattended "for
>>>>     just a second" from having their passwords changed by passers-by.
>>>>
>>>>     a formerly existing parameter "checkPassword", which could be
>>>>     used to override this password check, has been abolished for
>>>>     security reasons.
>>>> */
>>>>
>>>>
>>>>
>>>> as you see, this adds a very special meaning to the group "admin". is
>>>> that appropriate and in keeping with current usage? if so, i want to
>>>> make sure that this is sufficiently documented. where are the current
>>>> semantics of the admin group defined? (if the answer is "in the source
>>>> code somewhere", bzzzt, zero points.) what would be the best place to
>>>> define security semantics authoritatively?
>>>>
>>>>
>>>> regards,
>>>>
>>>> jörn
>>>>
>>>>
>>>>
>>
>>


-- 
Andreas Hartmann
Wyona Inc.  -   Open Source Content Management   -   Apache Lenya
http://www.wyona.com                      http://lenya.apache.org
andreas.hartmann@wyona.com                     andreas@apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
For additional commands, e-mail: dev-help@lenya.apache.org


Re: grave security issue in trunk!!!

Posted by Andreas Hartmann <an...@apache.org>.
Andreas Hartmann wrote:
> Doug Chestnut wrote:
>> Currently usecases get restrictive with policies in 
>> usecase-policies.xml.  Should we make usecase-policies.xml be 
>> permissive instead (only allow usecase execution if a policy exists 
>> and the policy is met).  This would force us to think about policies 
>> when creating new functionality.
> 
> Doug, would you mind filing a bug for this so that the idea doesn't
> get lost? Thanks a lot!

http://issues.apache.org/bugzilla/show_bug.cgi?id=39797

-- Andreas

-- 
Andreas Hartmann
Wyona Inc.  -   Open Source Content Management   -   Apache Lenya
http://www.wyona.com                      http://lenya.apache.org
andreas.hartmann@wyona.com                     andreas@apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
For additional commands, e-mail: dev-help@lenya.apache.org


Re: grave security issue in trunk!!!

Posted by Jörn Nettingsmeier <po...@uni-duisburg.de>.
Andreas Hartmann wrote:
> Doug Chestnut wrote:
>> Currently usecases get restrictive with policies in 
>> usecase-policies.xml.  Should we make usecase-policies.xml be 
>> permissive instead (only allow usecase execution if a policy exists 
>> and the policy is met).  This would force us to think about policies 
>> when creating new functionality.
> 
> Doug, would you mind filing a bug for this so that the idea doesn't
> get lost? Thanks a lot!

very good idea.

-- 
"Open source takes the bullshit out of software."
	- Charles Ferguson on TechnologyReview.com

--
Jörn Nettingsmeier, EDV-Administrator
Institut für Politikwissenschaft
Universität Duisburg-Essen, Standort Duisburg
Mail: pol-admin@uni-due.de, Telefon: 0203/379-2736

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
For additional commands, e-mail: dev-help@lenya.apache.org


Re: grave security issue in trunk!!!

Posted by Andreas Hartmann <an...@apache.org>.
Doug Chestnut wrote:
> Currently usecases get restrictive with policies in 
> usecase-policies.xml.  Should we make usecase-policies.xml be permissive 
> instead (only allow usecase execution if a policy exists and the policy 
> is met).  This would force us to think about policies when creating new 
> functionality.

Doug, would you mind filing a bug for this so that the idea doesn't
get lost? Thanks a lot!

-- Andreas


-- 
Andreas Hartmann
Wyona Inc.  -   Open Source Content Management   -   Apache Lenya
http://www.wyona.com                      http://lenya.apache.org
andreas.hartmann@wyona.com                     andreas@apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
For additional commands, e-mail: dev-help@lenya.apache.org


Re: grave security issue in trunk!!!

Posted by Doug Chestnut <dh...@virginia.edu>.
Currently usecases get restrictive with policies in 
usecase-policies.xml.  Should we make usecase-policies.xml be permissive 
instead (only allow usecase execution if a policy exists and the policy 
is met).  This would force us to think about policies when creating new 
functionality.

--Doug

Andreas Hartmann wrote:
> Doug Chestnut wrote:
> 
>> Hi Joern,
>> The first thing to change would be to add
>>     <usecase id="admin.changePassword">
>>         <role id="admin"/>
>>     </usecase>
>>
>> to your (and our default pubs) pubs usecase-policies.xml
> 
> 
> Exactly - and we could declare two different usecases so that one can
> be permitted only for admins and the other one (with entering the old
> password) for all users (see my other reply).
> 
> -- Andreas
> 
>>
>> --Doug
>>
>> Joern Nettingsmeier wrote:
>>
>>> Jörn Nettingsmeier wrote:
>>>
>>>> i just realized we have a huuuuge security hole that affects every 
>>>> lenya
>>>> 1.4 installation:
>>>>
>>>> * checkOldPassword is set by the (potentially hostile) client.
>>>> * the java code does not check that only admins may change passwords 
>>>> for
>>>> other user-ids than the currently logged-in one.
>>>>
>>>> ergo any user can change the passwords of arbitrary other users,
>>>> including admins. instant dos and privilege escalation, remotely
>>>> exploitable. not nice at all.
>>>
>>>
>>> <..>
>>>
>>>> i will try to fix this on friday if no one gets to do it tomorrow.
>>>
>>>
>>>
>>> hi guys! before i try to implement it, please comment on this new policy
>>> for changing passwords:
>>>
>>> /**
>>>  * Usecase to change a user's password.
>>>  */
>>> public class UserPassword extends AccessControlUsecase {
>>> /*
>>>     If the optional parameter "userId" is not set, we assume that the
>>>     password of the currently logged in user is to be changed.
>>>
>>>     Non-privileged users (i.e. those not belonging to the "admin" group)
>>>     cannot set userId to another user, i.e. they can only ever change
>>>     their own passwords.
>>>
>>>     Privileged users (those in the admin group) can set userId, and thus
>>>     change the passwords of other users.
>>>
>>>     All users will have to provide their own password again when they
>>>     try to change passwords, regardless of the fact that they are
>>>     already authenticated.
>>>     This is to protect users who leave their session unattended "for
>>>     just a second" from having their passwords changed by passers-by.
>>>
>>>     a formerly existing parameter "checkPassword", which could be
>>>     used to override this password check, has been abolished for
>>>     security reasons.
>>> */
>>>
>>>
>>>
>>> as you see, this adds a very special meaning to the group "admin". is
>>> that appropriate and in keeping with current usage? if so, i want to
>>> make sure that this is sufficiently documented. where are the current
>>> semantics of the admin group defined? (if the answer is "in the source
>>> code somewhere", bzzzt, zero points.) what would be the best place to
>>> define security semantics authoritatively?
>>>
>>>
>>> regards,
>>>
>>> jörn
>>>
>>>
>>>
> 
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
For additional commands, e-mail: dev-help@lenya.apache.org


Re: grave security issue in trunk!!!

Posted by Jörn Nettingsmeier <po...@uni-duisburg.de>.
Andreas Hartmann wrote:

> Thanks for the quick answer and sorry for that I'm rushing,

votes++ for rushing :)




-- 
"Open source takes the bullshit out of software."
	- Charles Ferguson on TechnologyReview.com

--
Jörn Nettingsmeier, EDV-Administrator
Institut für Politikwissenschaft
Universität Duisburg-Essen, Standort Duisburg
Mail: pol-admin@uni-due.de, Telefon: 0203/379-2736

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
For additional commands, e-mail: dev-help@lenya.apache.org


Re: grave security issue in trunk!!!

Posted by Andreas Hartmann <an...@apache.org>.
Jörn Nettingsmeier wrote:
> Andreas Hartmann wrote:
>> Joern Nettingsmeier wrote:
>>> Andreas Hartmann wrote:
>>>> Doug Chestnut wrote:
>>>>> Hi Joern,
>>>>> The first thing to change would be to add
>>>>>     <usecase id="admin.changePassword">
>>>>>         <role id="admin"/>
>>>>>     </usecase>
>>>>>
>>>>> to your (and our default pubs) pubs usecase-policies.xml
>>>> Exactly - and we could declare two different usecases so that one can
>>>> be permitted only for admins and the other one (with entering the old
>>>> password) for all users (see my other reply).
>>>>
>>>
>>> yes, that sounds nice. i'll try to get my head around it tonight...
>>
>> Jörn, did you already find a solution?
>> I thought about it too yesterday night and made some minimal changes,
>> but I wanted to wait for your ideas before checking them in.
>> Should I go ahead, or are you also working on this issue?
> 
> i'm still at it. the thing i'm stuck with atm is that i don't understand 
> the mapping from usecase names in usecases-policy.xml to the actual 
> usecases. in the file, there are no prefixes, and in real life, almost 
> all usecases are prefixed "site.usecaseName" or "admin.usecaseName".
> ?

The content of usecase-policies.xml is still legacy from Lenya 1.2.
The names just weren't updated yet.


> and somebody seems to have anticipated the problem and stopped halfway 
> through: there are policies for userChangePasswordUser and 
> userChangePasswordAdmin, but i think there's no actual code for those 
> usecases...

Right, that are still the old 1.2 names of the usecases.

> if you want to commit your changes, that's fine, i'm still in the 
> process of understanding how usecase policies are done.

OK, I'll do some more testing and check it in as soon as it works.

Thanks for the quick answer and sorry for that I'm rushing,
but I would like to see this issue solved as quickly as possible :/

-- Andreas


-- 
Andreas Hartmann
Wyona Inc.  -   Open Source Content Management   -   Apache Lenya
http://www.wyona.com                      http://lenya.apache.org
andreas.hartmann@wyona.com                     andreas@apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
For additional commands, e-mail: dev-help@lenya.apache.org


Re: grave security issue in trunk!!!

Posted by Andreas Hartmann <an...@apache.org>.
Jörn Nettingsmeier wrote:
> Andreas Hartmann wrote:
>> Joern Nettingsmeier wrote:
>>> Andreas Hartmann wrote:
>>>> Doug Chestnut wrote:
>>>>> Hi Joern,
>>>>> The first thing to change would be to add
>>>>>     <usecase id="admin.changePassword">
>>>>>         <role id="admin"/>
>>>>>     </usecase>
>>>>>
>>>>> to your (and our default pubs) pubs usecase-policies.xml
>>>> Exactly - and we could declare two different usecases so that one can
>>>> be permitted only for admins and the other one (with entering the old
>>>> password) for all users (see my other reply).
>>>>
>>>
>>> yes, that sounds nice. i'll try to get my head around it tonight...
>>
>> Jörn, did you already find a solution?
>> I thought about it too yesterday night and made some minimal changes,
>> but I wanted to wait for your ideas before checking them in.
>> Should I go ahead, or are you also working on this issue?
> 
> i'm still at it. the thing i'm stuck with atm is that i don't understand 
> the mapping from usecase names in usecases-policy.xml to the actual 
> usecases. in the file, there are no prefixes, and in real life, almost 
> all usecases are prefixed "site.usecaseName" or "admin.usecaseName".
> ?
> 
> and somebody seems to have anticipated the problem and stopped halfway 
> through: there are policies for userChangePasswordUser and 
> userChangePasswordAdmin, but i think there's no actual code for those 
> usecases...
> 
> if you want to commit your changes, that's fine, i'm still in the 
> process of understanding how usecase policies are done.

OK, I added a new class UsecasePasswordWithCheck to avoid that the
password check option is set by a parameter. The code became also a
bit cleaner this way.

-- Andreas


-- 
Andreas Hartmann
Wyona Inc.  -   Open Source Content Management   -   Apache Lenya
http://www.wyona.com                      http://lenya.apache.org
andreas.hartmann@wyona.com                     andreas@apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
For additional commands, e-mail: dev-help@lenya.apache.org


Re: grave security issue in trunk!!!

Posted by Jörn Nettingsmeier <po...@uni-duisburg.de>.
Andreas Hartmann wrote:
> Joern Nettingsmeier wrote:
>> Andreas Hartmann wrote:
>>> Doug Chestnut wrote:
>>>> Hi Joern,
>>>> The first thing to change would be to add
>>>>     <usecase id="admin.changePassword">
>>>>         <role id="admin"/>
>>>>     </usecase>
>>>>
>>>> to your (and our default pubs) pubs usecase-policies.xml
>>> Exactly - and we could declare two different usecases so that one can
>>> be permitted only for admins and the other one (with entering the old
>>> password) for all users (see my other reply).
>>>
>>
>> yes, that sounds nice. i'll try to get my head around it tonight...
> 
> Jörn, did you already find a solution?
> I thought about it too yesterday night and made some minimal changes,
> but I wanted to wait for your ideas before checking them in.
> Should I go ahead, or are you also working on this issue?

i'm still at it. the thing i'm stuck with atm is that i don't understand 
the mapping from usecase names in usecases-policy.xml to the actual 
usecases. in the file, there are no prefixes, and in real life, almost 
all usecases are prefixed "site.usecaseName" or "admin.usecaseName".
?

and somebody seems to have anticipated the problem and stopped halfway 
through: there are policies for userChangePasswordUser and 
userChangePasswordAdmin, but i think there's no actual code for those 
usecases...

if you want to commit your changes, that's fine, i'm still in the 
process of understanding how usecase policies are done.


regards,

jörn



-- 
"Open source takes the bullshit out of software."
	- Charles Ferguson on TechnologyReview.com

--
Jörn Nettingsmeier, EDV-Administrator
Institut für Politikwissenschaft
Universität Duisburg-Essen, Standort Duisburg
Mail: pol-admin@uni-due.de, Telefon: 0203/379-2736

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
For additional commands, e-mail: dev-help@lenya.apache.org


Re: grave security issue in trunk!!!

Posted by Andreas Hartmann <an...@apache.org>.
Joern Nettingsmeier wrote:
> Andreas Hartmann wrote:
>> Doug Chestnut wrote:
>>> Hi Joern,
>>> The first thing to change would be to add
>>>     <usecase id="admin.changePassword">
>>>         <role id="admin"/>
>>>     </usecase>
>>>
>>> to your (and our default pubs) pubs usecase-policies.xml
>> Exactly - and we could declare two different usecases so that one can
>> be permitted only for admins and the other one (with entering the old
>> password) for all users (see my other reply).
>>
> 
> yes, that sounds nice. i'll try to get my head around it tonight...

Jörn, did you already find a solution?
I thought about it too yesterday night and made some minimal changes,
but I wanted to wait for your ideas before checking them in.
Should I go ahead, or are you also working on this issue?

-- Andreas


-- 
Andreas Hartmann
Wyona Inc.  -   Open Source Content Management   -   Apache Lenya
http://www.wyona.com                      http://lenya.apache.org
andreas.hartmann@wyona.com                     andreas@apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
For additional commands, e-mail: dev-help@lenya.apache.org


Re: grave security issue in trunk!!!

Posted by Joern Nettingsmeier <po...@uni-due.de>.
Andreas Hartmann wrote:
> Doug Chestnut wrote:
>> Hi Joern,
>> The first thing to change would be to add
>>     <usecase id="admin.changePassword">
>>         <role id="admin"/>
>>     </usecase>
>>
>> to your (and our default pubs) pubs usecase-policies.xml
> 
> Exactly - and we could declare two different usecases so that one can
> be permitted only for admins and the other one (with entering the old
> password) for all users (see my other reply).
> 

yes, that sounds nice. i'll try to get my head around it tonight...


-- 
"Án nýrra verka, án nútimans, hættir fortíðin að vekja áhuga."
"Without new works, without the present the past will cease to be of
interest."
        - Ásmundur Sveinsson (1893-1982)

--
Jörn Nettingsmeier, EDV-Administrator
Institut für Politikwissenschaft
Universität Duisburg-Essen, Standort Duisburg
Mail: pol-admin@uni-due.de, Telefon: 0203/379-2736


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
For additional commands, e-mail: dev-help@lenya.apache.org


Re: grave security issue in trunk!!!

Posted by Andreas Hartmann <an...@apache.org>.
Doug Chestnut wrote:
> Hi Joern,
> The first thing to change would be to add
>     <usecase id="admin.changePassword">
>         <role id="admin"/>
>     </usecase>
> 
> to your (and our default pubs) pubs usecase-policies.xml

Exactly - and we could declare two different usecases so that one can
be permitted only for admins and the other one (with entering the old
password) for all users (see my other reply).

-- Andreas

> 
> --Doug
> 
> Joern Nettingsmeier wrote:
>> Jörn Nettingsmeier wrote:
>>
>>> i just realized we have a huuuuge security hole that affects every lenya
>>> 1.4 installation:
>>>
>>> * checkOldPassword is set by the (potentially hostile) client.
>>> * the java code does not check that only admins may change passwords for
>>> other user-ids than the currently logged-in one.
>>>
>>> ergo any user can change the passwords of arbitrary other users,
>>> including admins. instant dos and privilege escalation, remotely
>>> exploitable. not nice at all.
>>
>> <..>
>>
>>> i will try to fix this on friday if no one gets to do it tomorrow.
>>
>>
>> hi guys! before i try to implement it, please comment on this new policy
>> for changing passwords:
>>
>> /**
>>  * Usecase to change a user's password.
>>  */
>> public class UserPassword extends AccessControlUsecase {
>> /*
>>     If the optional parameter "userId" is not set, we assume that the
>>     password of the currently logged in user is to be changed.
>>
>>     Non-privileged users (i.e. those not belonging to the "admin" group)
>>     cannot set userId to another user, i.e. they can only ever change
>>     their own passwords.
>>
>>     Privileged users (those in the admin group) can set userId, and thus
>>     change the passwords of other users.
>>
>>     All users will have to provide their own password again when they
>>     try to change passwords, regardless of the fact that they are
>>     already authenticated.
>>     This is to protect users who leave their session unattended "for
>>     just a second" from having their passwords changed by passers-by.
>>
>>     a formerly existing parameter "checkPassword", which could be
>>     used to override this password check, has been abolished for
>>     security reasons.
>> */
>>
>>
>>
>> as you see, this adds a very special meaning to the group "admin". is
>> that appropriate and in keeping with current usage? if so, i want to
>> make sure that this is sufficiently documented. where are the current
>> semantics of the admin group defined? (if the answer is "in the source
>> code somewhere", bzzzt, zero points.) what would be the best place to
>> define security semantics authoritatively?
>>
>>
>> regards,
>>
>> jörn
>>
>>
>>


-- 
Andreas Hartmann
Wyona Inc.  -   Open Source Content Management   -   Apache Lenya
http://www.wyona.com                      http://lenya.apache.org
andreas.hartmann@wyona.com                     andreas@apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
For additional commands, e-mail: dev-help@lenya.apache.org


Re: grave security issue in trunk!!!

Posted by Doug Chestnut <dh...@virginia.edu>.
Hi Joern,
The first thing to change would be to add
     <usecase id="admin.changePassword">
         <role id="admin"/>
     </usecase>

to your (and our default pubs) pubs usecase-policies.xml

--Doug

Joern Nettingsmeier wrote:
> Jörn Nettingsmeier wrote:
> 
>>i just realized we have a huuuuge security hole that affects every lenya
>>1.4 installation:
>>
>>* checkOldPassword is set by the (potentially hostile) client.
>>* the java code does not check that only admins may change passwords for
>>other user-ids than the currently logged-in one.
>>
>>ergo any user can change the passwords of arbitrary other users,
>>including admins. instant dos and privilege escalation, remotely
>>exploitable. not nice at all.
> 
> <..>
> 
>>i will try to fix this on friday if no one gets to do it tomorrow.
> 
> 
> hi guys! before i try to implement it, please comment on this new policy
> for changing passwords:
> 
> /**
>  * Usecase to change a user's password.
>  */
> public class UserPassword extends AccessControlUsecase {
> /*
>     If the optional parameter "userId" is not set, we assume that the
>     password of the currently logged in user is to be changed.
> 
>     Non-privileged users (i.e. those not belonging to the "admin" group)
>     cannot set userId to another user, i.e. they can only ever change
>     their own passwords.
> 
>     Privileged users (those in the admin group) can set userId, and thus
>     change the passwords of other users.
> 
>     All users will have to provide their own password again when they
>     try to change passwords, regardless of the fact that they are
>     already authenticated.
>     This is to protect users who leave their session unattended "for
>     just a second" from having their passwords changed by passers-by.
> 
>     a formerly existing parameter "checkPassword", which could be
>     used to override this password check, has been abolished for
>     security reasons.
> */
> 
> 
> 
> as you see, this adds a very special meaning to the group "admin". is
> that appropriate and in keeping with current usage? if so, i want to
> make sure that this is sufficiently documented. where are the current
> semantics of the admin group defined? (if the answer is "in the source
> code somewhere", bzzzt, zero points.) what would be the best place to
> define security semantics authoritatively?
> 
> 
> regards,
> 
> jörn
> 
> 
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
For additional commands, e-mail: dev-help@lenya.apache.org


Re: grave security issue in trunk!!!

Posted by Joern Nettingsmeier <po...@uni-due.de>.
Jörn Nettingsmeier wrote:
> i just realized we have a huuuuge security hole that affects every lenya
> 1.4 installation:
> 
> * checkOldPassword is set by the (potentially hostile) client.
> * the java code does not check that only admins may change passwords for
> other user-ids than the currently logged-in one.
> 
> ergo any user can change the passwords of arbitrary other users,
> including admins. instant dos and privilege escalation, remotely
> exploitable. not nice at all.
<..>
> i will try to fix this on friday if no one gets to do it tomorrow.

hi guys! before i try to implement it, please comment on this new policy
for changing passwords:

/**
 * Usecase to change a user's password.
 */
public class UserPassword extends AccessControlUsecase {
/*
    If the optional parameter "userId" is not set, we assume that the
    password of the currently logged in user is to be changed.

    Non-privileged users (i.e. those not belonging to the "admin" group)
    cannot set userId to another user, i.e. they can only ever change
    their own passwords.

    Privileged users (those in the admin group) can set userId, and thus
    change the passwords of other users.

    All users will have to provide their own password again when they
    try to change passwords, regardless of the fact that they are
    already authenticated.
    This is to protect users who leave their session unattended "for
    just a second" from having their passwords changed by passers-by.

    a formerly existing parameter "checkPassword", which could be
    used to override this password check, has been abolished for
    security reasons.
*/



as you see, this adds a very special meaning to the group "admin". is
that appropriate and in keeping with current usage? if so, i want to
make sure that this is sufficiently documented. where are the current
semantics of the admin group defined? (if the answer is "in the source
code somewhere", bzzzt, zero points.) what would be the best place to
define security semantics authoritatively?


regards,

jörn



-- 
"Án nýrra verka, án nútimans, hættir fortíðin að vekja áhuga."
"Without new works, without the present the past will cease to be of
interest."
        - Ásmundur Sveinsson (1893-1982)

--
Jörn Nettingsmeier, EDV-Administrator
Institut für Politikwissenschaft
Universität Duisburg-Essen, Standort Duisburg
Mail: pol-admin@uni-due.de, Telefon: 0203/379-2736



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
For additional commands, e-mail: dev-help@lenya.apache.org


grave security issue in trunk!!!

Posted by Jörn Nettingsmeier <po...@uni-duisburg.de>.
Jörn Nettingsmeier wrote:
> hi *!
> 
> 
> when i tried to use the currently "hidden" admin.changePassword usecase,
> i realized it is quite hard to use, because it will throw a very nasty
> exception if one does not also set the userId parameter. this is
> unintuitive imho: if userId is not set explicitly, it should default to
> changing the password of the currently active user.
> 
> the attached patch tries to accomplish this. could you please review it?

i just realized we have a huuuuge security hole that affects every lenya 
1.4 installation:

* checkOldPassword is set by the (potentially hostile) client.
* the java code does not check that only admins may change passwords for
other user-ids than the currently logged-in one.

ergo any user can change the passwords of arbitrary other users,
including admins. instant dos and privilege escalation, remotely
exploitable. not nice at all.

to reproduce the problem, do the following:

1. in the default publication, log in as lenya/levi.
2. create a guest account with limited rights.
3. log out.
4. log in as guest.
5. append the following to the url in your browser:
?lenya.usecase=admin.changePassword&userId=lenya&checkOldPassword=0
(the last part is not even necessary, since checkOldPassword defaults to
null)
6. change the password of user lenya.

the fact that the changePassword usecase is not accessible via the menu
does nothing to alleviate the problem, it merely means that this option
is only available to the bad guys. :(


i will try to fix this on friday if no one gets to do it tomorrow.

bed now.


regards,

jörn









-- 
"Open source takes the bullshit out of software."
	- Charles Ferguson on TechnologyReview.com

--
Jörn Nettingsmeier, EDV-Administrator
Institut für Politikwissenschaft
Universität Duisburg-Essen, Standort Duisburg
Mail: pol-admin@uni-due.de, Telefon: 0203/379-2736


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
For additional commands, e-mail: dev-help@lenya.apache.org