Since this last one is safer and less invasive on existing code I've
merged it, and
I'm backporting it to 2.9.x in order to get this fix in before the
release Monday (to
allow a full cycle of nightly build and CITE tests).
Cheers
Andrea
On Fri, Sep 16, 2016 at 3:53 PM, Andrea Aime
<andrea.a...@geo-solutions.it <mailto:andrea.a...@geo-solutions.it>>
wrote:
Hi,
I actually also coded the "wrap the facade" path which has no
interface breaking changes:
https://github.com/geoserver/geoserver/pull/1823
<https://github.com/geoserver/geoserver/pull/1823>
The SecureCatalogFacade is not a pretty sight, but changes are
otherwise much more contained
Cheers
Andrea
On Fri, Sep 16, 2016 at 12:46 PM, Andrea Aime
<andrea.a...@geo-solutions.it
<mailto:andrea.a...@geo-solutions.it>> wrote:
Ok,
took a stab at it, pull request is here:
https://github.com/geoserver/geoserver/pull/1822
<https://github.com/geoserver/geoserver/pull/1822>
Most of the options to alter the I've explored have some
issues (including the chosen one):
1. Just adding the extra method with SortBy... breaks clients
passing null as the sortyby, because the compiler cannot
choose anymore which version to use
2. Just changing the existing method to use SortBy... still
causes issues with code currently passing null make the
implementations get a SortBy[] {null}
3. Adding a new method with a SortBy[] suffers from the same
issues as above
In the end I went for 2 taking into account that the array can
get null values in and sanitizing them out in the
implementations (as you say catalog is not
really meant to be extended).
A different approach could have been to return a
SecureCatalogFacade from SecureCatalogImpl, but the
CatalogFacade interface is huge and I did not
quite like the idea of a facade wrapping a catalog wrapping
other catalogs which ultimately wrap another facade. Imho
client code should just not
access the facade underlying the catalog, and indeed CSW is
the only exception.
So this is a fix, an important one, that breaks the Catalog
interface. But that's an interface which is not really
pluggable and/or meant to be replaced.
So, any objection to backporting? If there are objections,
could you please also report how you'd fix the security issue?
Cheers
Andrea
On Thu, Sep 15, 2016 at 11:40 AM, Niels Charlier
<ni...@scitus.be <mailto:ni...@scitus.be>> wrote:
Hi Andrea,
As I can remember, the security leak was not done on
purpose. I remember needing multiple SortBy's and being
confused by the API. This security issue is likely an
oversight on my behalf.
The difference in signature between Catalog and
CatalogFacade is not very logical/consistent. I would
correct the interface. There are only three direct
implementations (regular, secured and a decorator) in
geoserver, and I doubt that there are other
implementations with third parties, since it is rather
CatalogFacade that you must override to make your own catalog.
Kind Regards
Niels
On 14-09-16 18:30, Andrea Aime wrote:
Hi,
I've been asked to look at an issue with CSW that
apparently is ignoring the
security filters and always returning all of the layers
metadata to any user (including anonymous ones).
At first I thought the code was referring directly the
internalCatalog bean, but
no, it's a different variation of the same issue:
instead of using the catalog,
the code is extracting the CatalogFacade from it, and
then queries the facade:
https://github.com/geoserver/geoserver/blob/master/src/extension/csw/core/src/main/java/org/geoserver/csw/store/internal/CatalogStoreFeatureIterator.java#L86
<https://github.com/geoserver/geoserver/blob/master/src/extension/csw/core/src/main/java/org/geoserver/csw/store/internal/CatalogStoreFeatureIterator.java#L86>
This ends up dodging all the security, which is not
wrapped around the facade.
As far as I can tell the queries are hitting the facade
because there is a need to sort over multiple sortby in
CSW, and the methods to query are exposed as follows:
Catalog.java:
public <T extends CatalogInfo> CloseableIterator<T>
list(final Class<T> of,
final Filter filter, @Nullable Integer
offset, @Nullable Integer count,
@Nullable SortBy sortBy);
CatalogFacade.java:
/**
* @return an iterator over the catalog objects of
the requested type that match the given
* filter
*/
public <T extends CatalogInfo> CloseableIterator<T>
list(final Class<T> of,
final Filter filter, @Nullable Integer
offset, @Nullable Integer count,
@Nullable SortBy... sortOrder);
See the difference? To make sure CSW honors security, the
Catalog interface would have to be
modified to support multiple sortBy.
To avoid breaking the interface I guess I could use java
8 default methods which use getFacade().list(...),
and then just override that implementation in the
SecuredCatalog, but I'm afraid that might have non
obvious side effects in other implementations... better
be explicit and break the interface instead?
Btw, Jira already contains a report due to this issue,
but it does not picture the entire problem (complete
lack of security control in CSW):
https://osgeo-org.atlassian.net/browse/GEOS-6200
<https://osgeo-org.atlassian.net/browse/GEOS-6200>
I'm also wondering if by any chance dodging security was
done on purpose, if that's the case,
we might want to add a system setting to get the current
behavior, in case the original sponsors
of the current implementation actually need it.
Cheers
Andrea
--
==
GeoServer Professional Services from the experts! Visit
http://goo.gl/it488V for more information.
==
Ing. Andrea Aime
@geowolf
Technical Lead
GeoSolutions S.A.S.
Via di Montramito 3/A
55054 Massarosa (LU)
phone: +39 0584 962313 <tel:%2B39%200584%20962313>
fax: +39 0584 1660272 <tel:%2B39%200584%201660272>
mob: +39 339 8844549 <tel:%2B39%20%C2%A0339%208844549>
http://www.geo-solutions.it
http://twitter.com/geosolutions_it
<http://twitter.com/geosolutions_it>
*AVVERTENZE AI SENSI DEL D.Lgs. 196/2003*
Le informazioni contenute in questo messaggio di posta
elettronica e/o nel/i file/s allegato/i sono da
considerarsi strettamente riservate. Il loro utilizzo è
consentito esclusivamente al destinatario del messaggio,
per le finalità indicate nel messaggio stesso. Qualora
riceviate questo messaggio senza esserne il destinatario,
Vi preghiamo cortesemente di darcene notizia via e-mail e
di procedere alla distruzione del messaggio stesso,
cancellandolo dal Vostro sistema. Conservare il messaggio
stesso, divulgarlo anche in parte, distribuirlo ad altri
soggetti, copiarlo, od utilizzarlo per finalità diverse,
costituisce comportamento contrario ai principi dettati
dal D.Lgs. 196/2003.
The information in this message and/or attachments, is
intended solely for the attention and use of the named
addressee(s) and may be confidential or proprietary in
nature or covered by the provisions of privacy act
(Legislative Decree June, 30 2003, no.196 - Italy's New
Data Protection Code).Any use not in accord with its
purpose, any disclosure, reproduction, copying,
distribution, or either dissemination, either whole or
partial, is strictly forbidden except previous formal
approval of the named addressee(s). If you are not the
intended recipient, please contact immediately the sender
by telephone, fax or e-mail and delete the information in
this message that has been received in error. The sender
does not give any warranty or accept liability as the
content, accuracy or completeness of sent messages and
accepts no responsibility for changes made after they
were sent or for other risks which arise as a result of
e-mail transmission, viruses, etc.
-------------------------------------------------------
--
==
GeoServer Professional Services from the experts! Visit
http://goo.gl/it488V for more information.
==
Ing. Andrea Aime
@geowolf
Technical Lead
GeoSolutions S.A.S.
Via di Montramito 3/A
55054 Massarosa (LU)
phone: +39 0584 962313 <tel:%2B39%200584%20962313>
fax: +39 0584 1660272 <tel:%2B39%200584%201660272>
mob: +39 339 8844549 <tel:%2B39%20%C2%A0339%208844549>
http://www.geo-solutions.it
http://twitter.com/geosolutions_it
<http://twitter.com/geosolutions_it>
*AVVERTENZE AI SENSI DEL D.Lgs. 196/2003*
Le informazioni contenute in questo messaggio di posta
elettronica e/o nel/i file/s allegato/i sono da considerarsi
strettamente riservate. Il loro utilizzo è consentito
esclusivamente al destinatario del messaggio, per le finalità
indicate nel messaggio stesso. Qualora riceviate questo
messaggio senza esserne il destinatario, Vi preghiamo
cortesemente di darcene notizia via e-mail e di procedere alla
distruzione del messaggio stesso, cancellandolo dal Vostro
sistema. Conservare il messaggio stesso, divulgarlo anche in
parte, distribuirlo ad altri soggetti, copiarlo, od
utilizzarlo per finalità diverse, costituisce comportamento
contrario ai principi dettati dal D.Lgs. 196/2003.
The information in this message and/or attachments, is
intended solely for the attention and use of the named
addressee(s) and may be confidential or proprietary in nature
or covered by the provisions of privacy act (Legislative
Decree June, 30 2003, no.196 - Italy's New Data Protection
Code).Any use not in accord with its purpose, any disclosure,
reproduction, copying, distribution, or either dissemination,
either whole or partial, is strictly forbidden except previous
formal approval of the named addressee(s). If you are not the
intended recipient, please contact immediately the sender by
telephone, fax or e-mail and delete the information in this
message that has been received in error. The sender does not
give any warranty or accept liability as the content, accuracy
or completeness of sent messages and accepts no responsibility
for changes made after they were sent or for other risks which
arise as a result of e-mail transmission, viruses, etc.
-------------------------------------------------------
--
==
GeoServer Professional Services from the experts! Visit
http://goo.gl/it488V for more information.
==
Ing. Andrea Aime
@geowolf
Technical Lead
GeoSolutions S.A.S.
Via di Montramito 3/A
55054 Massarosa (LU)
phone: +39 0584 962313 <tel:%2B39%200584%20962313>
fax: +39 0584 1660272 <tel:%2B39%200584%201660272>
mob: +39 339 8844549 <tel:%2B39%20%C2%A0339%208844549>
http://www.geo-solutions.it
http://twitter.com/geosolutions_it
<http://twitter.com/geosolutions_it>
*AVVERTENZE AI SENSI DEL D.Lgs. 196/2003*
Le informazioni contenute in questo messaggio di posta elettronica
e/o nel/i file/s allegato/i sono da considerarsi strettamente
riservate. Il loro utilizzo è consentito esclusivamente al
destinatario del messaggio, per le finalità indicate nel messaggio
stesso. Qualora riceviate questo messaggio senza esserne il
destinatario, Vi preghiamo cortesemente di darcene notizia via
e-mail e di procedere alla distruzione del messaggio stesso,
cancellandolo dal Vostro sistema. Conservare il messaggio stesso,
divulgarlo anche in parte, distribuirlo ad altri soggetti,
copiarlo, od utilizzarlo per finalità diverse, costituisce
comportamento contrario ai principi dettati dal D.Lgs. 196/2003.
The information in this message and/or attachments, is intended
solely for the attention and use of the named addressee(s) and may
be confidential or proprietary in nature or covered by the
provisions of privacy act (Legislative Decree June, 30 2003,
no.196 - Italy's New Data Protection Code).Any use not in accord
with its purpose, any disclosure, reproduction, copying,
distribution, or either dissemination, either whole or partial, is
strictly forbidden except previous formal approval of the named
addressee(s). If you are not the intended recipient, please
contact immediately the sender by telephone, fax or e-mail and
delete the information in this message that has been received in
error. The sender does not give any warranty or accept liability
as the content, accuracy or completeness of sent messages and
accepts no responsibility for changes made after they were sent
or for other risks which arise as a result of e-mail transmission,
viruses, etc.
-------------------------------------------------------
--
==
GeoServer Professional Services from the experts! Visit
http://goo.gl/it488V for more information.
==
Ing. Andrea Aime
@geowolf
Technical Lead
GeoSolutions S.A.S.
Via di Montramito 3/A
55054 Massarosa (LU)
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549
http://www.geo-solutions.it
http://twitter.com/geosolutions_it
*AVVERTENZE AI SENSI DEL D.Lgs. 196/2003*
Le informazioni contenute in questo messaggio di posta elettronica e/o
nel/i file/s allegato/i sono da considerarsi strettamente riservate.
Il loro utilizzo è consentito esclusivamente al destinatario del
messaggio, per le finalità indicate nel messaggio stesso. Qualora
riceviate questo messaggio senza esserne il destinatario, Vi preghiamo
cortesemente di darcene notizia via e-mail e di procedere alla
distruzione del messaggio stesso, cancellandolo dal Vostro sistema.
Conservare il messaggio stesso, divulgarlo anche in parte,
distribuirlo ad altri soggetti, copiarlo, od utilizzarlo per finalità
diverse, costituisce comportamento contrario ai principi dettati dal
D.Lgs. 196/2003.
The information in this message and/or attachments, is intended solely
for the attention and use of the named addressee(s) and may be
confidential or proprietary in nature or covered by the provisions of
privacy act (Legislative Decree June, 30 2003, no.196 - Italy's New
Data Protection Code).Any use not in accord with its purpose, any
disclosure, reproduction, copying, distribution, or either
dissemination, either whole or partial, is strictly forbidden except
previous formal approval of the named addressee(s). If you are not the
intended recipient, please contact immediately the sender by
telephone, fax or e-mail and delete the information in this message
that has been received in error. The sender does not give any warranty
or accept liability as the content, accuracy or completeness of sent
messages and accepts no responsibility for changes made after they
were sent or for other risks which arise as a result of e-mail
transmission, viruses, etc.
-------------------------------------------------------
