You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@struts.apache.org by "bphillips@ku.edu" <bp...@ku.edu> on 2013/07/23 21:26:26 UTC
Add to ParameterNameAware JavaDoc Warning About Using?
I'm working on issue https://issues.apache.org/jira/browse/WW-4137
Though we don't need to change the source code to address the reported issue
I think we may want to change the JavaDoc for the ParameterNameAware
interface.
The JavaDoc for the ParametersInterceptor -
http://struts.apache.org/release/2.3.x/xwork-core/apidocs/com/opensymphony/xwork2/interceptor/ParametersInterceptor.html
has this warning:
Using ParameterNameAware could be dangerous as @{link
ParameterNameAware#acceptableParameterName(String)} takes precedence over
ParametersInterceptor which means if ParametersInterceptor excluded given
parameter name you can accept it with
What I understand the above to mean (after reviewing the source code of the
ParametersInterceptor) is that if my Action class implements
ParameterNameAware then the acceptableParameterName method I define in my
action is the only reference for filtering which parameters and values
should be added to the Value stack. So using ParameterNameAware does away
with the additional protection provided by the ParametersInterceptor.
If my understanding is correct should I add the warning to the
ParamenterNameAware interface's JavaDoc?
Bruce
--
View this message in context: http://struts.1045723.n5.nabble.com/Add-to-ParameterNameAware-JavaDoc-Warning-About-Using-tp5713285.html
Sent from the Struts - Dev mailing list archive at Nabble.com.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: Add to ParameterNameAware JavaDoc Warning About Using?
Posted by nick <ni...@gmail.com>.
Hi,
I would like to add something to this discussion since i'm the guy who
have reported *WW-4154*
Why && is obvious rather than ||
We have an Abstract Base Action class which contains some white listed and
black list parameter names, using isAcceptableParam we are handling it.
We are not handling or included any default struts2 black listed param name,
so we are leaving to strust2 to handle it. So in ParameterInterceptor first
strust2 default check will takes place (if successful &&) then
*isAcceptableParam* will check. Here there is no chance of checking same
parameter twice.
So according to me Strust2 default check is as equally important as custom
param check.
The problem with || is, if struts2 default check returns true then it will
never touch custom param check.
--
View this message in context: http://struts.1045723.n5.nabble.com/Add-to-ParameterNameAware-JavaDoc-Warning-About-Using-tp5713285p5713569.html
Sent from the Struts - Dev mailing list archive at Nabble.com.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: Add to ParameterNameAware JavaDoc Warning About Using?
Posted by Lukasz Lenart <lu...@apache.org>.
Just for reference, the old behaviour restored with incoming 2.3.16 version
https://cwiki.apache.org/confluence/display/WW/Version+Notes+2.3.16
2013/9/23 nick <ni...@gmail.com>:
> Any updates please?
>
>
>
> --
> View this message in context: http://struts.1045723.n5.nabble.com/Add-to-ParameterNameAware-JavaDoc-Warning-About-Using-tp5713285p5714001.html
> Sent from the Struts - Dev mailing list archive at Nabble.com.
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> For additional commands, e-mail: dev-help@struts.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: Add to ParameterNameAware JavaDoc Warning About Using?
Posted by nick <ni...@gmail.com>.
Any updates please?
--
View this message in context: http://struts.1045723.n5.nabble.com/Add-to-ParameterNameAware-JavaDoc-Warning-About-Using-tp5713285p5714001.html
Sent from the Struts - Dev mailing list archive at Nabble.com.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: Add to ParameterNameAware JavaDoc Warning About Using?
Posted by nick <ni...@gmail.com>.
But i guess its always good to give first preference to strust2 core security
check (ie acceptableName check first) as long as struts2 OGNL is vulnerable.
Users of strust2 framework should not bypass these default check by mistake
by excluding it in acceptableParams.
So i hope the following code will be subtle
*acceptableName(name) && (parameterNameAware == null ||
parameterNameAware.acceptableParameterName(name));*
So the check will be strong from security point of view.
1) acceptableName(name)
Default check will make sue nobody bypassing strust2 default check knowingly
or unknowingly.
2) &&
This makes Strust2 security check stronger so that user can't bypass
3) (parameterNameAware == null ||
parameterNameAware.acceptableParameterName(name))
If strust2 default check accepted the parameter
then
If this action is not ParameterNameAware then return true
else if this action is ParameterNameAware then call
acceptableParameterName(name),
so that the user will understand he can't bypass strust2 default due to
strust2 security vulnerability, and he will have the control over some
custom parameter he want to bypass (except all default struts2 params).
--
View this message in context: http://struts.1045723.n5.nabble.com/Add-to-ParameterNameAware-JavaDoc-Warning-About-Using-tp5713285p5713672.html
Sent from the Struts - Dev mailing list archive at Nabble.com.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: Add to ParameterNameAware JavaDoc Warning About Using?
Posted by Lukasz Lenart <lu...@apache.org>.
There is another option - flipping order of checking parameter name.
In that case user filter will be first. But precedence can occur in
other way - user will accept the parameter, but interceptor will throw
it away - which can be good indicator for users they are doing
something wrong.
return (parameterNameAware != null &&
parameterNameAware.acceptableParameterName(name)) ||
acceptableName(name);
Regards
--
Łukasz
+ 48 606 323 122 http://www.lenart.org.pl/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: Add to ParameterNameAware JavaDoc Warning About Using?
Posted by nick <ni...@gmail.com>.
Hi,
I would like to add something to this discussion since i'm the guy who
have reported WW-4154 <https://issues.apache.org/jira/browse/WW-4154>
Why && is obvious rather than ||
We have an Abstract Base Action class which contains some white listed and
black list parameter names, using *isAcceptableParam* we are handling it.
We are not handling or included any default struts2 black listed param name,
so we are leaving to strust2 to handle it. So in ParameterInterceptor first
strust2 default check will takes place (if successful &&) then
isAcceptableParam will check. Here there is no chance of checking same
parameter twice.
So according to me Strust2 default check is as equally important as custom
param check.
The problem with || is, if struts2 default check returns true then it will
never touch custom param check.
.
--
View this message in context: http://struts.1045723.n5.nabble.com/Add-to-ParameterNameAware-JavaDoc-Warning-About-Using-tp5713285p5713568.html
Sent from the Struts - Dev mailing list archive at Nabble.com.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: Add to ParameterNameAware JavaDoc Warning About Using?
Posted by Lukasz Lenart <lu...@apache.org>.
2013/8/8 Lukasz Lenart <lu...@apache.org>:
>> For me, if I want all unfiltered parameters, I would get rid of the filter
>> -- not use the interface. That's my logic.
>
> hm...
I thought a bit about that and I'm still not sure what is the best
solution. From one side using "&&" allows filter out parameters per
action and leave other actions to accept all the parameters - which is
simple and more obvious for users. Using "||" allows introduce a
global policy how to handle parameters and gives freedom how to relax
that global restrictions.
With "&&" in place there is also another disadvantage when global
policy is in place - parameter must be accepted in two places - in
interceptor and in action.
Regards
--
Łukasz
+ 48 606 323 122 http://www.lenart.org.pl/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: Add to ParameterNameAware JavaDoc Warning About Using?
Posted by Lukasz Lenart <lu...@apache.org>.
2013/8/7 Paul Benedict <pb...@apache.org>:
> Side note... My suggestion reminds me of an issue I had when I first
> learned Struts 2. I wished (and still wish) there was a way to specify the
> <interceptor> tag with an "exclude" or "disabled" attribute. I never liked
> the idea of having to rebuild my stack when all I wanted to do was take
> away an interceptor (or a few) from my default.
Could you register this Improvement in JIRA?
> For me, if I want all unfiltered parameters, I would get rid of the filter
> -- not use the interface. That's my logic.
hm...
Regards
--
Łukasz
+ 48 606 323 122 http://www.lenart.org.pl/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: Add to ParameterNameAware JavaDoc Warning About Using?
Posted by Paul Benedict <pb...@apache.org>.
Side note... My suggestion reminds me of an issue I had when I first
learned Struts 2. I wished (and still wish) there was a way to specify the
<interceptor> tag with an "exclude" or "disabled" attribute. I never liked
the idea of having to rebuild my stack when all I wanted to do was take
away an interceptor (or a few) from my default.
For me, if I want all unfiltered parameters, I would get rid of the filter
-- not use the interface. That's my logic.
On Wed, Aug 7, 2013 at 9:28 AM, Lukasz Lenart <lu...@apache.org>wrote:
> 2013/8/7 Paul Benedict <pb...@apache.org>:
> > If you need unfiltered access, wouldn't the "correct" answer be to remove
> > the filter on the action?
>
> I'm not sure if I get you right, but as I understand it's like that
> right now - you cannot enable filtering on action without enabling
> first filtering in interceptor.
>
> I see one disadvantage right now - it's more complicated to understand
> for users. With "&&" in place, user can simple implement
> ParameterNameAware and enable filtering per action - which is fine but
> with time and additional "filters per actions" is going to be
> complicated to understand what and why.
>
>
> Regards
> --
> Łukasz
> + 48 606 323 122 http://www.lenart.org.pl/
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> For additional commands, e-mail: dev-help@struts.apache.org
>
>
--
Cheers,
Paul
Re: Add to ParameterNameAware JavaDoc Warning About Using?
Posted by Lukasz Lenart <lu...@apache.org>.
2013/8/7 Paul Benedict <pb...@apache.org>:
> If you need unfiltered access, wouldn't the "correct" answer be to remove
> the filter on the action?
I'm not sure if I get you right, but as I understand it's like that
right now - you cannot enable filtering on action without enabling
first filtering in interceptor.
I see one disadvantage right now - it's more complicated to understand
for users. With "&&" in place, user can simple implement
ParameterNameAware and enable filtering per action - which is fine but
with time and additional "filters per actions" is going to be
complicated to understand what and why.
Regards
--
Łukasz
+ 48 606 323 122 http://www.lenart.org.pl/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: Add to ParameterNameAware JavaDoc Warning About Using?
Posted by Paul Benedict <pb...@apache.org>.
If you need unfiltered access, wouldn't the "correct" answer be to remove
the filter on the action?
On Wed, Aug 7, 2013 at 4:18 AM, Lukasz Lenart <lu...@apache.org>wrote:
> return acceptableName(name) || (parameterNameAware != null &&
> parameterNameAware.acceptableParameterName(name)
> ^^
> this part was changed from && to || - ParametersInterceptor, line 348
>
>
> Regards
> --
> Łukasz
> + 48 606 323 122 http://www.lenart.org.pl/
>
> 2013/8/6 Paul Benedict <pb...@apache.org>:
> > Can you clarify what changed in 2.3.7?
> >
> >
> > On Tue, Aug 6, 2013 at 3:01 AM, Lukasz Lenart <lukaszlenart@apache.org
> >wrote:
> >
> >> 2013/8/2 Paul Benedict <pb...@apache.org>:
> >> > If we changed ParameterNameAware interceptor so that it adheres to the
> >> > filtering of the interceptor, then I think we did the right thing.
> That
> >> > should be how the two work together. I would add how the two interact
> to
> >> > BOTH javadoc headers.
> >>
> >> I'm not sure if I get you right - the behaviour of these two changed
> >> sometime ago (as from 2.3.7 as I can recall). And my doubt is if it
> >> was a good move. Right now is clearer for me what are the
> >> responsibilities of these two. But maybe I'm wrong?
> >>
> >>
> >> Regards
> >> --
> >> Łukasz
> >> + 48 606 323 122 http://www.lenart.org.pl/
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> >> For additional commands, e-mail: dev-help@struts.apache.org
> >>
> >>
> >
> >
> > --
> > Cheers,
> > Paul
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> For additional commands, e-mail: dev-help@struts.apache.org
>
>
--
Cheers,
Paul
Re: Add to ParameterNameAware JavaDoc Warning About Using?
Posted by Lukasz Lenart <lu...@apache.org>.
return acceptableName(name) || (parameterNameAware != null &&
parameterNameAware.acceptableParameterName(name)
^^
this part was changed from && to || - ParametersInterceptor, line 348
Regards
--
Łukasz
+ 48 606 323 122 http://www.lenart.org.pl/
2013/8/6 Paul Benedict <pb...@apache.org>:
> Can you clarify what changed in 2.3.7?
>
>
> On Tue, Aug 6, 2013 at 3:01 AM, Lukasz Lenart <lu...@apache.org>wrote:
>
>> 2013/8/2 Paul Benedict <pb...@apache.org>:
>> > If we changed ParameterNameAware interceptor so that it adheres to the
>> > filtering of the interceptor, then I think we did the right thing. That
>> > should be how the two work together. I would add how the two interact to
>> > BOTH javadoc headers.
>>
>> I'm not sure if I get you right - the behaviour of these two changed
>> sometime ago (as from 2.3.7 as I can recall). And my doubt is if it
>> was a good move. Right now is clearer for me what are the
>> responsibilities of these two. But maybe I'm wrong?
>>
>>
>> Regards
>> --
>> Łukasz
>> + 48 606 323 122 http://www.lenart.org.pl/
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
>> For additional commands, e-mail: dev-help@struts.apache.org
>>
>>
>
>
> --
> Cheers,
> Paul
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: Add to ParameterNameAware JavaDoc Warning About Using?
Posted by Paul Benedict <pb...@apache.org>.
Can you clarify what changed in 2.3.7?
On Tue, Aug 6, 2013 at 3:01 AM, Lukasz Lenart <lu...@apache.org>wrote:
> 2013/8/2 Paul Benedict <pb...@apache.org>:
> > If we changed ParameterNameAware interceptor so that it adheres to the
> > filtering of the interceptor, then I think we did the right thing. That
> > should be how the two work together. I would add how the two interact to
> > BOTH javadoc headers.
>
> I'm not sure if I get you right - the behaviour of these two changed
> sometime ago (as from 2.3.7 as I can recall). And my doubt is if it
> was a good move. Right now is clearer for me what are the
> responsibilities of these two. But maybe I'm wrong?
>
>
> Regards
> --
> Łukasz
> + 48 606 323 122 http://www.lenart.org.pl/
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> For additional commands, e-mail: dev-help@struts.apache.org
>
>
--
Cheers,
Paul
Re: Add to ParameterNameAware JavaDoc Warning About Using?
Posted by Lukasz Lenart <lu...@apache.org>.
2013/8/2 Paul Benedict <pb...@apache.org>:
> If we changed ParameterNameAware interceptor so that it adheres to the
> filtering of the interceptor, then I think we did the right thing. That
> should be how the two work together. I would add how the two interact to
> BOTH javadoc headers.
I'm not sure if I get you right - the behaviour of these two changed
sometime ago (as from 2.3.7 as I can recall). And my doubt is if it
was a good move. Right now is clearer for me what are the
responsibilities of these two. But maybe I'm wrong?
Regards
--
Łukasz
+ 48 606 323 122 http://www.lenart.org.pl/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: Add to ParameterNameAware JavaDoc Warning About Using?
Posted by Paul Benedict <pb...@apache.org>.
If we changed ParameterNameAware interceptor so that it adheres to the
filtering of the interceptor, then I think we did the right thing. That
should be how the two work together. I would add how the two interact to
BOTH javadoc headers.
On Fri, Aug 2, 2013 at 9:47 AM, bphillips@ku.edu <bp...@ku.edu> wrote:
> I think in the applications that use ParameterNameAware there are two use
> cases - use it to specify a "blacklist" of parameter names that Struts
> should not try to process or use it to specify a "whitelist" of parameter
> names that Struts should allow.
>
> If we change the OR to AND then the people who have used ParameterNameAware
> to create a whitelist may have to change their code if they are returning
> false in method acceptableParameterName if the parameter name is not one of
> their whitelist ones. Otherwise all parameters not in their whitelist will
> not be processed by Struts 2.
>
> Also what if we change the OR to AND and someone has whitelisted a
> parameter
> name in the acceptableParameterName method but the ParametersInterceptor
> method
> acceptableName returns false--with the AND logic the user's whitelisted
> parameter name will be ignored.
>
> I also think that most people implementing ParameterNameAware DO NOT
> understand the implications of using it given the changes made recently.
> Our documentation and examples are not clear about how and why to use the
> ParameterNameAware interface - we've buried a warning about using
> ParameterNameAware deep in the JavaDoc (which most people do not read).
>
> I also think we need a larger discussion about whether or not
> ParameterNameAware interface a good idea going forward. Is there a valid
> use case for making ParameterInterceptor less restrictive for which
> parameters are processed?
>
> I'm very new to improving the Struts 2 code so please take my comments with
> a grain of salt.
>
> Bruce
>
>
>
>
> --
> View this message in context:
> http://struts.1045723.n5.nabble.com/Add-to-ParameterNameAware-JavaDoc-Warning-About-Using-tp5713285p5713406.html
> Sent from the Struts - Dev mailing list archive at Nabble.com.
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> For additional commands, e-mail: dev-help@struts.apache.org
>
>
--
Cheers,
Paul
Re: Add to ParameterNameAware JavaDoc Warning About Using?
Posted by "bphillips@ku.edu" <bp...@ku.edu>.
I think in the applications that use ParameterNameAware there are two use
cases - use it to specify a "blacklist" of parameter names that Struts
should not try to process or use it to specify a "whitelist" of parameter
names that Struts should allow.
If we change the OR to AND then the people who have used ParameterNameAware
to create a whitelist may have to change their code if they are returning
false in method acceptableParameterName if the parameter name is not one of
their whitelist ones. Otherwise all parameters not in their whitelist will
not be processed by Struts 2.
Also what if we change the OR to AND and someone has whitelisted a parameter
name in the acceptableParameterName method but the ParametersInterceptor
method
acceptableName returns false--with the AND logic the user's whitelisted
parameter name will be ignored.
I also think that most people implementing ParameterNameAware DO NOT
understand the implications of using it given the changes made recently.
Our documentation and examples are not clear about how and why to use the
ParameterNameAware interface - we've buried a warning about using
ParameterNameAware deep in the JavaDoc (which most people do not read).
I also think we need a larger discussion about whether or not
ParameterNameAware interface a good idea going forward. Is there a valid
use case for making ParameterInterceptor less restrictive for which
parameters are processed?
I'm very new to improving the Struts 2 code so please take my comments with
a grain of salt.
Bruce
--
View this message in context: http://struts.1045723.n5.nabble.com/Add-to-ParameterNameAware-JavaDoc-Warning-About-Using-tp5713285p5713406.html
Sent from the Struts - Dev mailing list archive at Nabble.com.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: Add to ParameterNameAware JavaDoc Warning About Using?
Posted by Lukasz Lenart <lu...@apache.org>.
2013/8/1 Paul Benedict <pb...@apache.org>:
> Where in the life cycle does ParameterNameAware get executed? Is it before
> any filter?
That's the whole logic:
protected boolean isAcceptableParameter(String name, Object action) {
ParameterNameAware parameterNameAware = (action instanceof
ParameterNameAware) ? (ParameterNameAware) action : null;
return acceptableName(name) || (parameterNameAware != null &&
parameterNameAware.acceptableParameterName(name));
}
so first acceptParams and excludeParams are examined and then if param
name isn't accepted ParameterNameAware is called. To change the
behaviour just replace || with && and the ParameterNameAware must at
the same time accept param name.
Regards
--
Łukasz
+ 48 606 323 122 http://www.lenart.org.pl/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: Add to ParameterNameAware JavaDoc Warning About Using?
Posted by Paul Benedict <pb...@apache.org>.
Where in the life cycle does ParameterNameAware get executed? Is it before
any filter?
On Thu, Aug 1, 2013 at 9:34 AM, Paul Benedict <pb...@apache.org> wrote:
> First question I asked myself is, what's the purpose of providing any
> filter if you can circumvent it? There's an obvious contradiction of use
> cases between these two. I am actively thinking about what is the right
> solution.
>
>
> On Thu, Aug 1, 2013 at 7:00 AM, Lukasz Lenart <lu...@apache.org>wrote:
>
>> The problem returns as boomerang :\
>>
>> I would like hear your opinion - should I (we) revert to the behaviour
>> as suggested in the issue or stay as is?
>>
>> Right now is clear how to split responsibility: ParametersInterceptor
>> is the root of security, where ParameterNameAware can be used to relax
>> it, when the behaviour will be reverted both will play the same role.
>>
>>
>> https://issues.apache.org/jira/browse/WW-4154?focusedCommentId=13726313&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13726313
>>
>>
>> Thanks in advance
>> --
>> Łukasz
>> + 48 606 323 122 http://www.lenart.org.pl/
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
>> For additional commands, e-mail: dev-help@struts.apache.org
>>
>>
>
>
> --
> Cheers,
> Paul
>
--
Cheers,
Paul
Re: Add to ParameterNameAware JavaDoc Warning About Using?
Posted by Paul Benedict <pb...@apache.org>.
First question I asked myself is, what's the purpose of providing any
filter if you can circumvent it? There's an obvious contradiction of use
cases between these two. I am actively thinking about what is the right
solution.
On Thu, Aug 1, 2013 at 7:00 AM, Lukasz Lenart <lu...@apache.org>wrote:
> The problem returns as boomerang :\
>
> I would like hear your opinion - should I (we) revert to the behaviour
> as suggested in the issue or stay as is?
>
> Right now is clear how to split responsibility: ParametersInterceptor
> is the root of security, where ParameterNameAware can be used to relax
> it, when the behaviour will be reverted both will play the same role.
>
>
> https://issues.apache.org/jira/browse/WW-4154?focusedCommentId=13726313&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13726313
>
>
> Thanks in advance
> --
> Łukasz
> + 48 606 323 122 http://www.lenart.org.pl/
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> For additional commands, e-mail: dev-help@struts.apache.org
>
>
--
Cheers,
Paul
Re: Add to ParameterNameAware JavaDoc Warning About Using?
Posted by Lukasz Lenart <lu...@apache.org>.
The problem returns as boomerang :\
I would like hear your opinion - should I (we) revert to the behaviour
as suggested in the issue or stay as is?
Right now is clear how to split responsibility: ParametersInterceptor
is the root of security, where ParameterNameAware can be used to relax
it, when the behaviour will be reverted both will play the same role.
https://issues.apache.org/jira/browse/WW-4154?focusedCommentId=13726313&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13726313
Thanks in advance
--
Łukasz
+ 48 606 323 122 http://www.lenart.org.pl/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: Add to ParameterNameAware JavaDoc Warning About Using?
Posted by "bphillips@ku.edu" <bp...@ku.edu>.
For now I added the following to the JavaDoc for the ParameterNameAware
interface so that it matches the JavaDoc for ParametersInterceptor:
* Using {@link ParameterNameAware} could be dangerous as {@link
ParameterNameAware#acceptableParameterName(String)} takes precedence
* over {@link ParametersInterceptor} which means if ParametersInterceptor
excluded given parameter name you can accept it with
* {@link ParameterNameAware#acceptableParameterName(String)}.
*
* The best idea is to define very tight restrictions with
ParametersInterceptor and relax them per action with
* {@link ParameterNameAware#acceptableParameterName(String)}
--
View this message in context: http://struts.1045723.n5.nabble.com/Add-to-ParameterNameAware-JavaDoc-Warning-About-Using-tp5713285p5713331.html
Sent from the Struts - Dev mailing list archive at Nabble.com.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: Add to ParameterNameAware JavaDoc Warning About Using?
Posted by "bphillips@ku.edu" <bp...@ku.edu>.
Paul and Luskaz - I appreciate the feedback but I'm a little confused.
Should I add the warning about using the ParameterNameAware interface that
is in the JavaDoc for the ParametersInterceptor to the JavaDoc for the
ParameterNameAware interface?
Or do we need to make some code changes in how ParametersInterceptor deals
with Action classes that implement the ParameterNameAware interface?
I did review the complete setParameters method of class
ParametersInterceptor. Being new to the Struts 2 source code it's a bit
hard for me to follow the logic of that method. It looks like the method is
checking in two places to see if the Action implements the
ParameterNameAware interface.
Lines 306-312 where if the Action implements the ParameterNameAware
interface a new PropertyJudge object is created for the
MemberValueStackAccess object - the new PropertyJudge calls the
acceptableParameterName method of the Action class. The
MemberAccessValueStack has a set of acceptable parameters and the
PropertyJudge object.
Lines 346-347 in method isAcceptableParameter where if the Action class
implements ParameterNameAware then the method calls the
acceptableParameterName method of the Action class to determine if the
parameter name is acceptable:
Note line 347:
return acceptableName(name) || (parameterNameAware != null &&
parameterNameAware.acceptableParameterName(name));
If the Action class implements ParameterNameAware and the
acceptableParameterName method returns true then even if
acceptableName(name) returns false the value returned will be true due to
the OR
The method isAcceptableParameter name method is called by line 280 in the
setParameters method as part of putting entries in the acceptableParameters
map.
If I'm not understanding how ParametersInterceptor is handling the use case
where the Action class implements ParameterNameAware please let me know. I
reviewed the code and the JavaDoc for both but being new to the Struts 2
source code I can easily have missed something or mis-understood.
Bottom line is I just want to ensure that we let users of our API know the
effect of implementing the ParameterNameAware interface and the best
practice of how to use that interface given the changes made starting in
version 2.3.7. If users who are using the ParameterNameAware interface to
setup a "blacklist" of acceptable parameter but have the
acceptableParameterName method return true if the parameter name is not in
the "blacklist" (as the person who reported issue 4137 was doing) are
opening themselves up to a security vulnerability then we should let users
know that.
Sorry for the long-winded reply.
Bruce
--
View this message in context: http://struts.1045723.n5.nabble.com/Add-to-ParameterNameAware-JavaDoc-Warning-About-Using-tp5713285p5713292.html
Sent from the Struts - Dev mailing list archive at Nabble.com.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: Add to ParameterNameAware JavaDoc Warning About Using?
Posted by Paul Benedict <pb...@apache.org>.
I don't know about making this a "warning" as more of a note in each class:
ParameterNameAware and ParametersInterceptor.
On Wed, Jul 24, 2013 at 1:55 AM, Lukasz Lenart <lu...@apache.org>wrote:
> There is one more thing, please check lines 306-312 of
> ParametersInterceptor - we have two layers where parameter must be
> accepted:
> 1. ParametersInterceptor
> 2. MemberAccessValueStack
>
> and both must behave in the same way in case of ParameterNameAware.
>
> And ParameterNameAware should be used to relax restrictions set by
> acceptParams/excludeParams in ParametersInterceptor - like Paul said -
> a workaround.
>
>
> Regards
> --
> Łukasz
> + 48 606 323 122 http://www.lenart.org.pl/
>
> 2013/7/24 Paul Benedict <pb...@apache.org>:
> > Was the interface intended to workaround the interceptor? I find the
> > dueling unexpected.
> > On Jul 23, 2013 2:26 PM, "bphillips@ku.edu" <bp...@ku.edu> wrote:
> >
> >> I'm working on issue https://issues.apache.org/jira/browse/WW-4137
> >>
> >> Though we don't need to change the source code to address the reported
> >> issue
> >> I think we may want to change the JavaDoc for the ParameterNameAware
> >> interface.
> >>
> >> The JavaDoc for the ParametersInterceptor -
> >>
> >>
> http://struts.apache.org/release/2.3.x/xwork-core/apidocs/com/opensymphony/xwork2/interceptor/ParametersInterceptor.html
> >> has this warning:
> >>
> >> Using ParameterNameAware could be dangerous as @{link
> >> ParameterNameAware#acceptableParameterName(String)} takes precedence
> over
> >> ParametersInterceptor which means if ParametersInterceptor excluded
> given
> >> parameter name you can accept it with
> >>
> >> What I understand the above to mean (after reviewing the source code of
> the
> >> ParametersInterceptor) is that if my Action class implements
> >> ParameterNameAware then the acceptableParameterName method I define in
> my
> >> action is the only reference for filtering which parameters and values
> >> should be added to the Value stack. So using ParameterNameAware does
> away
> >> with the additional protection provided by the ParametersInterceptor.
> >>
> >> If my understanding is correct should I add the warning to the
> >> ParamenterNameAware interface's JavaDoc?
> >>
> >> Bruce
> >>
> >>
> >>
> >> --
> >> View this message in context:
> >>
> http://struts.1045723.n5.nabble.com/Add-to-ParameterNameAware-JavaDoc-Warning-About-Using-tp5713285.html
> >> Sent from the Struts - Dev mailing list archive at Nabble.com.
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> >> For additional commands, e-mail: dev-help@struts.apache.org
> >>
> >>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> For additional commands, e-mail: dev-help@struts.apache.org
>
>
--
Cheers,
Paul
Re: Add to ParameterNameAware JavaDoc Warning About Using?
Posted by Lukasz Lenart <lu...@apache.org>.
There is one more thing, please check lines 306-312 of
ParametersInterceptor - we have two layers where parameter must be
accepted:
1. ParametersInterceptor
2. MemberAccessValueStack
and both must behave in the same way in case of ParameterNameAware.
And ParameterNameAware should be used to relax restrictions set by
acceptParams/excludeParams in ParametersInterceptor - like Paul said -
a workaround.
Regards
--
Łukasz
+ 48 606 323 122 http://www.lenart.org.pl/
2013/7/24 Paul Benedict <pb...@apache.org>:
> Was the interface intended to workaround the interceptor? I find the
> dueling unexpected.
> On Jul 23, 2013 2:26 PM, "bphillips@ku.edu" <bp...@ku.edu> wrote:
>
>> I'm working on issue https://issues.apache.org/jira/browse/WW-4137
>>
>> Though we don't need to change the source code to address the reported
>> issue
>> I think we may want to change the JavaDoc for the ParameterNameAware
>> interface.
>>
>> The JavaDoc for the ParametersInterceptor -
>>
>> http://struts.apache.org/release/2.3.x/xwork-core/apidocs/com/opensymphony/xwork2/interceptor/ParametersInterceptor.html
>> has this warning:
>>
>> Using ParameterNameAware could be dangerous as @{link
>> ParameterNameAware#acceptableParameterName(String)} takes precedence over
>> ParametersInterceptor which means if ParametersInterceptor excluded given
>> parameter name you can accept it with
>>
>> What I understand the above to mean (after reviewing the source code of the
>> ParametersInterceptor) is that if my Action class implements
>> ParameterNameAware then the acceptableParameterName method I define in my
>> action is the only reference for filtering which parameters and values
>> should be added to the Value stack. So using ParameterNameAware does away
>> with the additional protection provided by the ParametersInterceptor.
>>
>> If my understanding is correct should I add the warning to the
>> ParamenterNameAware interface's JavaDoc?
>>
>> Bruce
>>
>>
>>
>> --
>> View this message in context:
>> http://struts.1045723.n5.nabble.com/Add-to-ParameterNameAware-JavaDoc-Warning-About-Using-tp5713285.html
>> Sent from the Struts - Dev mailing list archive at Nabble.com.
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
>> For additional commands, e-mail: dev-help@struts.apache.org
>>
>>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: Add to ParameterNameAware JavaDoc Warning About Using?
Posted by Paul Benedict <pb...@apache.org>.
Was the interface intended to workaround the interceptor? I find the
dueling unexpected.
On Jul 23, 2013 2:26 PM, "bphillips@ku.edu" <bp...@ku.edu> wrote:
> I'm working on issue https://issues.apache.org/jira/browse/WW-4137
>
> Though we don't need to change the source code to address the reported
> issue
> I think we may want to change the JavaDoc for the ParameterNameAware
> interface.
>
> The JavaDoc for the ParametersInterceptor -
>
> http://struts.apache.org/release/2.3.x/xwork-core/apidocs/com/opensymphony/xwork2/interceptor/ParametersInterceptor.html
> has this warning:
>
> Using ParameterNameAware could be dangerous as @{link
> ParameterNameAware#acceptableParameterName(String)} takes precedence over
> ParametersInterceptor which means if ParametersInterceptor excluded given
> parameter name you can accept it with
>
> What I understand the above to mean (after reviewing the source code of the
> ParametersInterceptor) is that if my Action class implements
> ParameterNameAware then the acceptableParameterName method I define in my
> action is the only reference for filtering which parameters and values
> should be added to the Value stack. So using ParameterNameAware does away
> with the additional protection provided by the ParametersInterceptor.
>
> If my understanding is correct should I add the warning to the
> ParamenterNameAware interface's JavaDoc?
>
> Bruce
>
>
>
> --
> View this message in context:
> http://struts.1045723.n5.nabble.com/Add-to-ParameterNameAware-JavaDoc-Warning-About-Using-tp5713285.html
> Sent from the Struts - Dev mailing list archive at Nabble.com.
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> For additional commands, e-mail: dev-help@struts.apache.org
>
>