You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@tomcat.apache.org by Francis GALIEGUE <fg...@one2team.com> on 2011/10/02 16:49:18 UTC

Request for comments: Apache-like allow/deny remote host filtering

Hello list,

Given the problems I've had with the RemoteAddrValve, I have decided
to try my hands at implementing a more sophisticated valve which acts
more like Apache's allow/deny and which takes netmasks as arguments.

Note, it has been compile tested only (in my IDE, mind), and I have
tested that it essentially works using a main() (not in the patch).
Right now I'm struggling to build the thing and figure out the
build.xml...

The patch can be seen here:

https://github.com/fge/tomcat70/commit/79e0ab03188e00163ef7f97ca84bd8a8d0815f33

Any comments are welcome!

-- 
Francis Galiegue
ONE2TEAM
Ingénieur système
Mob : +33 (0) 683 877 875
Tel : +33 (0) 178 945 552
fge@one2team.com
40 avenue Raymond Poincaré
75116 Paris

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org


Re: Request for comments: Apache-like allow/deny remote host filtering

Posted by Francis GALIEGUE <fg...@one2team.com>.
On Tue, Oct 4, 2011 at 21:02, Christopher Schultz
<ch...@christopherschultz.net> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Francis,
>
> On 10/4/2011 2:50 PM, Francis GALIEGUE wrote:
>> Patch attached. I didn't know Bugzilla would treat patches this
>> way...
>
> Can you give us a link to the bug?
>

https://issues.apache.org/bugzilla/show_bug.cgi?id=51953

[...]
>
> The good thing is that you only have to do a very simple test harness.
> Something like this:
>
> FilterConfig config = .. // mock-up the filter config
> FilterChain chain = .. // mock-up the filter chain
> Filter filter = new RemoteAddrFilter();
> filter.init(config);
>
> HttpServletRequest request = ... // mock-up request
> HttpServletResponse response = ... // mock-up response
>
> foreach(ip in test_ips) {
>  // request.setRemoteAddr(ip);
>  filter.doFilter(request, response, chain);
>
>  assertEquals(expected_status_code, response.getStatusCode());
> }
>
> Of course, you'll have to do some significant work to make sure that
> the filter chain is mocked-up correctly and that your request object
> can be programmed appropriately.
>
> Perhaps that's what you were hoping to get some help doing? :)
>

Indeed. Not right now though, I want to read some more first ;)


Thanks,
-- 
Francis Galiegue
ONE2TEAM
Ingénieur système
Mob : +33 (0) 683 877 875
Tel : +33 (0) 178 945 552
fge@one2team.com
40 avenue Raymond Poincaré
75116 Paris

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org


Re: Request for comments: Apache-like allow/deny remote host filtering

Posted by Christopher Schultz <ch...@christopherschultz.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Francis,

On 10/4/2011 2:50 PM, Francis GALIEGUE wrote:
> Patch attached. I didn't know Bugzilla would treat patches this
> way...

Can you give us a link to the bug?

> I implemented those two interfaces again, since the existing
> abstract class wouldn't fit the bill (it only tried regexes). Or
> maybe I don't understand what you actually mean?

I'll have to look at the base classes... I seem to recall a great deal
of extending and overriding in that package.

> Well, I need help precisely on the above... I have a hard time 
> figuring out how TestRemoteIP{Filter,Valve} work at all... But
> I've been only having a superficial glance at them so far.

The good thing is that you only have to do a very simple test harness.
Something like this:

FilterConfig config = .. // mock-up the filter config
FilterChain chain = .. // mock-up the filter chain
Filter filter = new RemoteAddrFilter();
filter.init(config);

HttpServletRequest request = ... // mock-up request
HttpServletResponse response = ... // mock-up response

foreach(ip in test_ips) {
  // request.setRemoteAddr(ip);
  filter.doFilter(request, response, chain);

  assertEquals(expected_status_code, response.getStatusCode());
}

Of course, you'll have to do some significant work to make sure that
the filter chain is mocked-up correctly and that your request object
can be programmed appropriately.

Perhaps that's what you were hoping to get some help doing? :)

If so, I'll keep going.

- -chris
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk6LWEgACgkQ9CaO5/Lv0PCtLQCbBH+EHvUg6GwhgcL+RefS6f11
qPoAmwRZq7YY352W77bnpndfHdl4CyG4
=vTv7
-----END PGP SIGNATURE-----

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org


Re: Request for comments: Apache-like allow/deny remote host filtering

Posted by Francis GALIEGUE <fg...@one2team.com>.
On Tue, Oct 4, 2011 at 20:43, Christopher Schultz
<ch...@christopherschultz.net> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Francis,
>
> On 10/2/2011 3:57 PM, Francis GALIEGUE wrote:
>> On Sun, Oct 2, 2011 at 19:46, Konstantin Kolinko
>> <kn...@gmail.com> wrote:
>>> 2011/10/2 Francis GALIEGUE <fg...@one2team.com>:
>>
>>>
>>> 1. If you want to submit it as a patch for Tomcat, you should
>>> attach it to a Bugzilla issue.
>>
>> OK, will do.
>
> +1
>
> Is there a way to see the code as a diff instead of complete files?
> This looks like a diff where the whole file is new.
>

Indeed. Patch attached. I didn't know Bugzilla would treat patches this way...

[...]
>
> It appears that no changes would be needed to either the Filter or the
> Valve -- only to the shared implementation that interprets the meaning
> of those settings.
>

I implemented those two interfaces again, since the existing abstract
class wouldn't fit the bill (it only tried regexes). Or maybe I don't
understand what you actually mean?

[...]
>>>
>>> You can run a single test by setting "test.entry" property in
>>> build.properties equal to the test name. Otherwise the full
>>> testsuite will be run.
>>>
>>
>> That's in the plan.
>
> I don't see jmock in the set of libs available for TC7, so you may
> have to play some games with HttpServletRequestWrapper in order to
> test this thing properly.
>
> Let us know if you need any help.
>

Well, I need help precisely on the above... I have a hard time
figuring out how TestRemoteIP{Filter,Valve} work at all... But I've
been only having a superficial glance at them so far.

Have fun!
-- 
Francis Galiegue
ONE2TEAM
Ingénieur système
Mob : +33 (0) 683 877 875
Tel : +33 (0) 178 945 552
fge@one2team.com
40 avenue Raymond Poincaré
75116 Paris

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org


Re: Request for comments: Apache-like allow/deny remote host filtering

Posted by Christopher Schultz <ch...@christopherschultz.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Francis,

On 10/2/2011 3:57 PM, Francis GALIEGUE wrote:
> On Sun, Oct 2, 2011 at 19:46, Konstantin Kolinko 
> <kn...@gmail.com> wrote:
>> 2011/10/2 Francis GALIEGUE <fg...@one2team.com>:
> 
>> 
>> 1. If you want to submit it as a patch for Tomcat, you should 
>> attach it to a Bugzilla issue.
> 
> OK, will do.

+1

Is there a way to see the code as a diff instead of complete files?
This looks like a diff where the whole file is new.

>> 4. In Tomcat 7 there are  RemoteAddrValve and RemoteAddrFilter. 
>> Both implement the same filtering, but one is implemented as a 
>> valve, another as a filter.
> 
> I hadn't seen that. Thanks for the correction!

It appears that no changes would be needed to either the Filter or the
Valve -- only to the shared implementation that interprets the meaning
of those settings.

>> 5. Tomcat 7 has tests written for JUnit. I'd be nice if your 
>> valve had such tests. To start testsuite you execute the "test" 
>> target in Tomcat's build.xml.
>> 
>> You can run a single test by setting "test.entry" property in 
>> build.properties equal to the test name. Otherwise the full 
>> testsuite will be run.
>> 
> 
> That's in the plan.

I don't see jmock in the set of libs available for TC7, so you may
have to play some games with HttpServletRequestWrapper in order to
test this thing properly.

Let us know if you need any help.

- -chris
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk6LU8sACgkQ9CaO5/Lv0PAdXQCeKgIMXbD+jvtrRcnLHQFH1JDZ
gQ8AoMIbwvUrPv936WCIYm69zAdoMXVD
=YiPA
-----END PGP SIGNATURE-----

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org


Re: Request for comments: Apache-like allow/deny remote host filtering

Posted by Francis GALIEGUE <fg...@one2team.com>.
On Sun, Oct 2, 2011 at 19:46, Konstantin Kolinko <kn...@gmail.com> wrote:
> 2011/10/2 Francis GALIEGUE <fg...@one2team.com>:

>
> 1. If you want to submit it as a patch for Tomcat, you should attach
> it to a Bugzilla issue.

OK, will do.

> 2. Coding conventions:
> http://tomcat.apache.org/getinvolved.html
> 3. ASF software is collective work. @author tags are discouraged.
>

I was just copying existing code ;)

> 4. In Tomcat 7 there are  RemoteAddrValve and RemoteAddrFilter.  Both
> implement the same filtering, but one is implemented as a valve,
> another as a filter.

I hadn't seen that. Thanks for the correction!

> 5. Tomcat 7 has tests written for JUnit. I'd be nice if your valve had
> such tests.
> To start testsuite you execute the "test" target in Tomcat's build.xml.
>
> You can run a single test by setting "test.entry" property in
> build.properties equal to the test name. Otherwise the full testsuite
> will be run.
>

That's in the plan.

Thanks for the input,
-- 
Francis Galiegue
ONE2TEAM
Ingénieur système
Mob : +33 (0) 683 877 875
Tel : +33 (0) 178 945 552
fge@one2team.com
40 avenue Raymond Poincaré
75116 Paris

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org


Re: Request for comments: Apache-like allow/deny remote host filtering

Posted by Konstantin Kolinko <kn...@gmail.com>.
2011/10/2 Francis GALIEGUE <fg...@one2team.com>:
> Hello list,
>
> Given the problems I've had with the RemoteAddrValve, I have decided
> to try my hands at implementing a more sophisticated valve which acts
> more like Apache's allow/deny and which takes netmasks as arguments.
>
> Note, it has been compile tested only (in my IDE, mind), and I have
> tested that it essentially works using a main() (not in the patch).
> Right now I'm struggling to build the thing and figure out the
> build.xml...
>
> The patch can be seen here:
>
> https://github.com/fge/tomcat70/commit/79e0ab03188e00163ef7f97ca84bd8a8d0815f33
>
> Any comments are welcome!

1. If you want to submit it as a patch for Tomcat, you should attach
it to a Bugzilla issue.
2. Coding conventions:
http://tomcat.apache.org/getinvolved.html
3. ASF software is collective work. @author tags are discouraged.

4. In Tomcat 7 there are  RemoteAddrValve and RemoteAddrFilter.  Both
implement the same filtering, but one is implemented as a valve,
another as a filter.
5. Tomcat 7 has tests written for JUnit. I'd be nice if your valve had
such tests.
To start testsuite you execute the "test" target in Tomcat's build.xml.

You can run a single test by setting "test.entry" property in
build.properties equal to the test name. Otherwise the full testsuite
will be run.

Best regards,
Konstantin Kolinko

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org