You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by mg...@apache.org on 2016/09/20 20:08:36 UTC

wicket git commit: WICKET-6245 Open up CsrfPreventionRequestCycleListener for extension

Repository: wicket
Updated Branches:
  refs/heads/master c819c6c4c -> 247619ab1


WICKET-6245 Open up CsrfPreventionRequestCycleListener for extension

Wrap a debug logiing in LOG.isDebugEnabled()


Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/247619ab
Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/247619ab
Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/247619ab

Branch: refs/heads/master
Commit: 247619ab176c64acc3d07adcc45725e019e11a62
Parents: c819c6c
Author: Martin Tzvetanov Grigorov <mg...@apache.org>
Authored: Tue Sep 20 22:07:37 2016 +0200
Committer: Martin Tzvetanov Grigorov <mg...@apache.org>
Committed: Tue Sep 20 22:07:37 2016 +0200

----------------------------------------------------------------------
 .../protocol/http/CsrfPreventionRequestCycleListener.java    | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/wicket/blob/247619ab/wicket-core/src/main/java/org/apache/wicket/protocol/http/CsrfPreventionRequestCycleListener.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/main/java/org/apache/wicket/protocol/http/CsrfPreventionRequestCycleListener.java b/wicket-core/src/main/java/org/apache/wicket/protocol/http/CsrfPreventionRequestCycleListener.java
index ce03862..e6b61dc 100644
--- a/wicket-core/src/main/java/org/apache/wicket/protocol/http/CsrfPreventionRequestCycleListener.java
+++ b/wicket-core/src/main/java/org/apache/wicket/protocol/http/CsrfPreventionRequestCycleListener.java
@@ -27,7 +27,6 @@ import javax.servlet.http.HttpServletRequest;
 import org.apache.wicket.RestartResponseException;
 import org.apache.wicket.core.request.handler.IPageRequestHandler;
 import org.apache.wicket.core.request.handler.RenderPageRequestHandler;
-import org.apache.wicket.protocol.http.WebApplication;
 import org.apache.wicket.request.IRequestHandler;
 import org.apache.wicket.request.IRequestHandlerDelegate;
 import org.apache.wicket.request.component.IRequestablePage;
@@ -358,8 +357,11 @@ public class CsrfPreventionRequestCycleListener extends AbstractRequestCycleList
 			}
 			else
 			{
-				log.debug("Targeted page {} was opted out of the CSRF origin checks, allowed",
-					targetedPage.getClass().getName());
+				if (log.isDebugEnabled())
+				{
+					log.debug("Targeted page {} was opted out of the CSRF origin checks, allowed",
+							targetedPage.getClass().getName());
+				}
 				allowHandler(containerRequest, sourceUri, targetedPage);
 			}
 		}


Re: wicket git commit: WICKET-6245 Open up CsrfPreventionRequestCycleListener for extension

Posted by Emond Papegaaij <em...@topicus.nl>.
Hi,

Actually, there are 3. With the current configuration, 2 of those methods will 
never be called. You need to explicitly allow or suppress certain actions. 
Given the nature of the three methods, I think they all need a different 
style:

For 'allow', I suggest to change the message of 'allow' to "Allowing Cross-
site request, request URL: {}, Origin: {}" and keep the level at INFO.

For 'suppress', change the message to "Suppressing a Cross-site action, 
request URL: {}, Origin: {}" and change the level to WARN.

For 'abort' change the message to "Aborting a Cross-site request, request URL: 
{}, Origin: {}" and change the level to WARN or ERROR.

What do you think?

Best regards,
Emond

On dinsdag 20 september 2016 22:11:54 CEST you wrote:
> Hi,
> 
> There are two log.info() calls starting with "Possible CSRF attack..."
> which IMO should be with level WARN.
> Or the chance of false positives is bigger ?
> 
> 
> Martin Grigorov
> Wicket Training and Consulting
> https://twitter.com/mtgrigorov
> 
> On Tue, Sep 20, 2016 at 10:08 PM, <mg...@apache.org> wrote:
> > Repository: wicket
> > 
> > Updated Branches:
> >   refs/heads/master c819c6c4c -> 247619ab1
> > 
> > WICKET-6245 Open up CsrfPreventionRequestCycleListener for extension
> > 
> > Wrap a debug logiing in LOG.isDebugEnabled()
> > 
> > 
> > Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
> > Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/247619ab
> > Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/247619ab
> > Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/247619ab
> > 
> > Branch: refs/heads/master
> > Commit: 247619ab176c64acc3d07adcc45725e019e11a62
> > Parents: c819c6c
> > Author: Martin Tzvetanov Grigorov <mg...@apache.org>
> > Authored: Tue Sep 20 22:07:37 2016 +0200
> > Committer: Martin Tzvetanov Grigorov <mg...@apache.org>
> > Committed: Tue Sep 20 22:07:37 2016 +0200
> > 
> > ----------------------------------------------------------------------
> > 
> >  .../protocol/http/CsrfPreventionRequestCycleListener.java    | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> > 
> > ----------------------------------------------------------------------
> > 
> > 
> > http://git-wip-us.apache.org/repos/asf/wicket/blob/
> > 247619ab/wicket-core/src/main/java/org/apache/wicket/protocol/http/
> > CsrfPreventionRequestCycleListener.java
> > ----------------------------------------------------------------------
> > diff --git a/wicket-core/src/main/java/org/apache/wicket/protocol/http/
> > CsrfPreventionRequestCycleListener.java b/wicket-core/src/main/java/
> > org/apache/wicket/protocol/http/CsrfPreventionRequestCycleListener.java
> > index ce03862..e6b61dc 100644
> > --- a/wicket-core/src/main/java/org/apache/wicket/protocol/http/
> > CsrfPreventionRequestCycleListener.java
> > +++ b/wicket-core/src/main/java/org/apache/wicket/protocol/http/
> > CsrfPreventionRequestCycleListener.java
> > @@ -27,7 +27,6 @@ import javax.servlet.http.HttpServletRequest;
> > 
> >  import org.apache.wicket.RestartResponseException;
> >  import org.apache.wicket.core.request.handler.IPageRequestHandler;
> >  import org.apache.wicket.core.request.handler.RenderPageRequestHandler;
> > 
> > -import org.apache.wicket.protocol.http.WebApplication;
> > 
> >  import org.apache.wicket.request.IRequestHandler;
> >  import org.apache.wicket.request.IRequestHandlerDelegate;
> >  import org.apache.wicket.request.component.IRequestablePage;
> > 
> > @@ -358,8 +357,11 @@ public class CsrfPreventionRequestCycleListener
> > extends AbstractRequestCycleList
> > 
> >                         }
> >                         else
> >                         {
> > 
> > -                               log.debug("Targeted page {} was opted out
> > of the CSRF origin checks, allowed",
> > -                                       targetedPage.getClass().
> > getName());
> > +                               if (log.isDebugEnabled())
> > +                               {
> > +                                       log.debug("Targeted page {} was
> > opted out of the CSRF origin checks, allowed",
> > +
> > 
> >  targetedPage.getClass().getName());
> > 
> > +                               }
> > 
> >                                 allowHandler(containerRequest, sourceUri,
> > 
> > targetedPage);
> > 
> >                         }
> >                 
> >                 }



Re: wicket git commit: WICKET-6245 Open up CsrfPreventionRequestCycleListener for extension

Posted by Martijn Dashorst <ma...@gmail.com>.
The false positives occur much more often.

Martijn


On Tue, Sep 20, 2016 at 10:11 PM, Martin Grigorov <mg...@apache.org> wrote:
> Hi,
>
> There are two log.info() calls starting with "Possible CSRF attack..." which
> IMO should be with level WARN.
> Or the chance of false positives is bigger ?
>
>
> Martin Grigorov
> Wicket Training and Consulting
> https://twitter.com/mtgrigorov
>
> On Tue, Sep 20, 2016 at 10:08 PM, <mg...@apache.org> wrote:
>>
>> Repository: wicket
>> Updated Branches:
>>   refs/heads/master c819c6c4c -> 247619ab1
>>
>>
>> WICKET-6245 Open up CsrfPreventionRequestCycleListener for extension
>>
>> Wrap a debug logiing in LOG.isDebugEnabled()
>>
>>
>> Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
>> Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/247619ab
>> Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/247619ab
>> Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/247619ab
>>
>> Branch: refs/heads/master
>> Commit: 247619ab176c64acc3d07adcc45725e019e11a62
>> Parents: c819c6c
>> Author: Martin Tzvetanov Grigorov <mg...@apache.org>
>> Authored: Tue Sep 20 22:07:37 2016 +0200
>> Committer: Martin Tzvetanov Grigorov <mg...@apache.org>
>> Committed: Tue Sep 20 22:07:37 2016 +0200
>>
>> ----------------------------------------------------------------------
>>  .../protocol/http/CsrfPreventionRequestCycleListener.java    | 8 +++++---
>>  1 file changed, 5 insertions(+), 3 deletions(-)
>> ----------------------------------------------------------------------
>>
>>
>>
>> http://git-wip-us.apache.org/repos/asf/wicket/blob/247619ab/wicket-core/src/main/java/org/apache/wicket/protocol/http/CsrfPreventionRequestCycleListener.java
>> ----------------------------------------------------------------------
>> diff --git
>> a/wicket-core/src/main/java/org/apache/wicket/protocol/http/CsrfPreventionRequestCycleListener.java
>> b/wicket-core/src/main/java/org/apache/wicket/protocol/http/CsrfPreventionRequestCycleListener.java
>> index ce03862..e6b61dc 100644
>> ---
>> a/wicket-core/src/main/java/org/apache/wicket/protocol/http/CsrfPreventionRequestCycleListener.java
>> +++
>> b/wicket-core/src/main/java/org/apache/wicket/protocol/http/CsrfPreventionRequestCycleListener.java
>> @@ -27,7 +27,6 @@ import javax.servlet.http.HttpServletRequest;
>>  import org.apache.wicket.RestartResponseException;
>>  import org.apache.wicket.core.request.handler.IPageRequestHandler;
>>  import org.apache.wicket.core.request.handler.RenderPageRequestHandler;
>> -import org.apache.wicket.protocol.http.WebApplication;
>>  import org.apache.wicket.request.IRequestHandler;
>>  import org.apache.wicket.request.IRequestHandlerDelegate;
>>  import org.apache.wicket.request.component.IRequestablePage;
>> @@ -358,8 +357,11 @@ public class CsrfPreventionRequestCycleListener
>> extends AbstractRequestCycleList
>>                         }
>>                         else
>>                         {
>> -                               log.debug("Targeted page {} was opted out
>> of the CSRF origin checks, allowed",
>> -
>> targetedPage.getClass().getName());
>> +                               if (log.isDebugEnabled())
>> +                               {
>> +                                       log.debug("Targeted page {} was
>> opted out of the CSRF origin checks, allowed",
>> +
>> targetedPage.getClass().getName());
>> +                               }
>>                                 allowHandler(containerRequest, sourceUri,
>> targetedPage);
>>                         }
>>                 }
>>
>



-- 
Become a Wicket expert, learn from the best: http://wicketinaction.com

Re: wicket git commit: WICKET-6245 Open up CsrfPreventionRequestCycleListener for extension

Posted by Martin Grigorov <mg...@apache.org>.
Hi,

There are two log.info() calls starting with "Possible CSRF attack..."
which IMO should be with level WARN.
Or the chance of false positives is bigger ?


Martin Grigorov
Wicket Training and Consulting
https://twitter.com/mtgrigorov

On Tue, Sep 20, 2016 at 10:08 PM, <mg...@apache.org> wrote:

> Repository: wicket
> Updated Branches:
>   refs/heads/master c819c6c4c -> 247619ab1
>
>
> WICKET-6245 Open up CsrfPreventionRequestCycleListener for extension
>
> Wrap a debug logiing in LOG.isDebugEnabled()
>
>
> Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
> Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/247619ab
> Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/247619ab
> Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/247619ab
>
> Branch: refs/heads/master
> Commit: 247619ab176c64acc3d07adcc45725e019e11a62
> Parents: c819c6c
> Author: Martin Tzvetanov Grigorov <mg...@apache.org>
> Authored: Tue Sep 20 22:07:37 2016 +0200
> Committer: Martin Tzvetanov Grigorov <mg...@apache.org>
> Committed: Tue Sep 20 22:07:37 2016 +0200
>
> ----------------------------------------------------------------------
>  .../protocol/http/CsrfPreventionRequestCycleListener.java    | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/wicket/blob/
> 247619ab/wicket-core/src/main/java/org/apache/wicket/protocol/http/
> CsrfPreventionRequestCycleListener.java
> ----------------------------------------------------------------------
> diff --git a/wicket-core/src/main/java/org/apache/wicket/protocol/http/
> CsrfPreventionRequestCycleListener.java b/wicket-core/src/main/java/
> org/apache/wicket/protocol/http/CsrfPreventionRequestCycleListener.java
> index ce03862..e6b61dc 100644
> --- a/wicket-core/src/main/java/org/apache/wicket/protocol/http/
> CsrfPreventionRequestCycleListener.java
> +++ b/wicket-core/src/main/java/org/apache/wicket/protocol/http/
> CsrfPreventionRequestCycleListener.java
> @@ -27,7 +27,6 @@ import javax.servlet.http.HttpServletRequest;
>  import org.apache.wicket.RestartResponseException;
>  import org.apache.wicket.core.request.handler.IPageRequestHandler;
>  import org.apache.wicket.core.request.handler.RenderPageRequestHandler;
> -import org.apache.wicket.protocol.http.WebApplication;
>  import org.apache.wicket.request.IRequestHandler;
>  import org.apache.wicket.request.IRequestHandlerDelegate;
>  import org.apache.wicket.request.component.IRequestablePage;
> @@ -358,8 +357,11 @@ public class CsrfPreventionRequestCycleListener
> extends AbstractRequestCycleList
>                         }
>                         else
>                         {
> -                               log.debug("Targeted page {} was opted out
> of the CSRF origin checks, allowed",
> -                                       targetedPage.getClass().
> getName());
> +                               if (log.isDebugEnabled())
> +                               {
> +                                       log.debug("Targeted page {} was
> opted out of the CSRF origin checks, allowed",
> +
>  targetedPage.getClass().getName());
> +                               }
>                                 allowHandler(containerRequest, sourceUri,
> targetedPage);
>                         }
>                 }
>
>