[Geoserver-devel] Disabling security does not work, call for code review

Christian_Mueller · May 21, 2013, 1:40pm

The related issue is here

https://jira.codehaus.org/browse/GEOS-5820

The user guide information

http://docs.geoserver.org/stable/en/user/security/disable.html

The git commit for review is on my githup repo

https://github.com/mcrmcr/geoserver-1/commit/eaf2de921028dc1e8dcb66d4547b835868b5cac0

Summary:

I have no idea about the point in time when the information in the user guide became wrong. As an example I had to patch WorkSpaceAccessLimits, a class I never touched before.

The idea is to set an HttpServletRequest attribute indicating if a request has passed a security filter chain. Disabling security on a chain results in an empty filter list, enabling security adds a mandatory persistence context filter (and others) . This mandatory filter flags the request. I added a public static method

GeoServerSecurityFilterChainProxy.isSecurityEnabledForCurrentRequest()

This method can be called from anywhere. Additionally I had to do a refactoring removing all these individual checks for “ROLE_ADMINISTRATOR”. This logic is moved to the GeoserverSecurityManager.

Enabling & Disabling for individual chains now works on the fly.

mvn clean install -Prelease runs successfully .

I would like a core developer for a review since I had to patch some classes new to me. And last not least, I need an opinion about a backport to 2.3.x.

Thanks in advance
Christian

–

DI Christian Mueller MSc (GIS), MSc (IT-Security)
OSS Open Source Solutions GmbH

Andrea_Aime4 · May 25, 2013, 9:41am

On Tue, May 21, 2013 at 3:40 PM, Christian Mueller <
christian.mueller@anonymised.com> wrote:

The related issue is here

https://jira.codehaus.org/browse/GEOS-5820

The user guide information

http://docs.geoserver.org/stable/en/user/security/disable.html

The git commit for review is on my githup repo

https://github.com/mcrmcr/geoserver-1/commit/eaf2de921028dc1e8dcb66d4547b835868b5cac0

Summary:

I have no idea about the point in time when the information in the user
guide became wrong. As an example I had to patch WorkSpaceAccessLimits, a
class I never touched before.

The idea is to set an HttpServletRequest attribute indicating if a request
has passed a security filter chain. Disabling security on a chain results
in an empty filter list, enabling security adds a mandatory persistence
context filter (and others) . This mandatory filter flags the request. I
added a public static method

GeoServerSecurityFilterChainProxy.isSecurityEnabledForCurrentRequest()

This method can be called from anywhere. Additionally I had to do a
refactoring removing all these individual checks for "ROLE_ADMINISTRATOR".
This logic is moved to the GeoserverSecurityManager.

Makes sense to me, although I don't see how the state of security, disabled
by default, gets raised to true when there is an active filter chain?

Enabling & Disabling for individual chains now works on the fly.

mvn clean install -Prelease runs successfully .

I would like a core developer for a review since I had to patch some
classes new to me. And last not least, I need an opinion about a backport
to 2.3.x.

I'd like someone else to have a look too. About an opinion to backport to
2.3.x, uh, feels risky to me, but I'm open to discussion.

Cheers
Andrea

--

GeoServer training in Milan, 6th & 7th June 2013! Visit
http://geoserver.geo-solutions.it for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

-------------------------------------------------------

Christian_Mueller · May 25, 2013, 2:32pm

Security is enabled here
https://github.com/mcrmcr/geoserver-1/blob/eaf2de921028dc1e8dcb66d4547b835868b5cac0/src/main/src/main/java/org/geoserver/security/filter/GeoServerSecurityContextPersistenceFilter.java

Look at the method “doFilter”

This filter must be the first filter on each chain protecting resources. This is assured by validation code when the filter chain is saved.

And yes, a backport is risky, on the other hand, we are talking about a broken feature. Hmmmm

Cheers
Christian

···

2013/5/25 Andrea Aime <andrea.aime@anonymised.com>

On Tue, May 21, 2013 at 3:40 PM, Christian Mueller <christian.mueller@anonymised.com> wrote:

–

DI Christian Mueller MSc (GIS), MSc (IT-Security)
OSS Open Source Solutions GmbH

The related issue is here

https://jira.codehaus.org/browse/GEOS-5820

The user guide information

http://docs.geoserver.org/stable/en/user/security/disable.html

The git commit for review is on my githup repo

https://github.com/mcrmcr/geoserver-1/commit/eaf2de921028dc1e8dcb66d4547b835868b5cac0

Summary:

I have no idea about the point in time when the information in the user guide became wrong. As an example I had to patch WorkSpaceAccessLimits, a class I never touched before.

The idea is to set an HttpServletRequest attribute indicating if a request has passed a security filter chain. Disabling security on a chain results in an empty filter list, enabling security adds a mandatory persistence context filter (and others) . This mandatory filter flags the request. I added a public static method

GeoServerSecurityFilterChainProxy.isSecurityEnabledForCurrentRequest()

This method can be called from anywhere. Additionally I had to do a refactoring removing all these individual checks for “ROLE_ADMINISTRATOR”. This logic is moved to the GeoserverSecurityManager.

Makes sense to me, although I don’t see how the state of security, disabled by default, gets raised to true when there is an active filter chain?

Enabling & Disabling for individual chains now works on the fly.

mvn clean install -Prelease runs successfully .

I would like a core developer for a review since I had to patch some classes new to me. And last not least, I need an opinion about a backport to 2.3.x.

I’d like someone else to have a look too. About an opinion to backport to 2.3.x, uh, feels risky to me, but I’m open to discussion.