search

georchestra / datadir

geOrchestra configuration directory for generic wars & Debian packages (eg: /etc/georchestra)
7 stars 33 forks source link

Set mapstore as proxy and update conf #414

Closed f-necas closed 3 months ago

f-necas commented 4 months ago

to allow ogcapi,csv,json and getFeature

This allows datahub to use mapstore proxy and extends default config to read more data

fvanderbiest commented 4 months ago

Altough this introduces a dependency between datahub and mapstore, I am +1 on this since :

edevosc2c commented 4 months ago

I think this change is preterm since we haven't proposed a good solution for the official CORS proxy remplacement: https://github.com/georchestra/georchestra-gateway/issues/39

I'm ok with doing temporary changes like using mapstore proxy on our infrastructure at Camptocamp, but for the community, I'm against it.

Mainly because the CORS proxy in mapstore is integrated for mapstore and there will be side effects of trying to make it work for other components that are not mapstore. Also, that's not a good thing to integrate workarounds instead of deploying stable integrations.

edevosc2c commented 4 months ago

Also, you are making the change in the master branch, so this affects deployments like ansible. So it would be great to have the input from @landryb about that.

For the docker-master branch, I'm rooting towards using an integrated CORS proxy in the docker composition once https://github.com/georchestra/georchestra-gateway/issues/39 will be solved.

landryb commented 4 months ago

i like that. mapstore has a proxy, and it's here to stay, so might aswell use it. And since afaict there are no plans to implement one in the gateway (no, adding yet another microservice just for that isn't an option), it fixes something that is broken by the s-p deprecation, so it's improving the feature parity.

fvanderbiest commented 4 months ago

Mainly because the CORS proxy in mapstore is integrated for mapstore and there will be side effects of trying to make it work for other components that are not mapstore.

None that I'm aware of.

edevosc2c commented 4 months ago

I'm highly confused on the target solution that we wanted to propose to the community. I thought in https://github.com/georchestra/georchestra-gateway/issues/39 that we wanted to propose an official CORS proxy component based on geosolutions-it/http-proxy.

I have explained in https://github.com/georchestra/georchestra-gateway/issues/39 that currently this CORS proxy is missing some critical features.

Maybe we can discuss together during like a gardening session? My feeling is that this is quite a rushed change.

Mainly because the CORS proxy in mapstore is integrated for mapstore and there will be side effects of trying to make it work for other components that are not mapstore.

None that I'm aware of.

@f-necas already had difficulties yesterday to make mapstore proxy work with datahub on geo2france.

f-necas commented 4 months ago

I selected 4 metadatas which use proxy from datahub.

I can't predict how many metadatas will be impacted but, for me, we should definitely not embed proxy in GW and/or Security Proxy.

For community, I think, Mapstore's one is the best option. Mostly working with everything, maintained and less code duplication.

For existing platforms, we should inform them about risks, impacts and security issues by staying with SP/GW proxy or by switching to Mapstore's one, and let them choose.

Then if no solution is convenient, we should indeed fork or develop another tool but it will need funding as it is an explicit need.

edevosc2c commented 4 months ago

Was proposing to set proxy_path to be commented. This way, only the user is taking the risk of a CORS proxy that does not 100% work and is missing some functionalities.

f-necas commented 3 months ago

Won't do. A dedicated proxy tool will be used.

landryb commented 3 months ago

Won't do. A dedicated proxy tool will be used.

sad. what was the rationale ? dedicated proxy tool = another jvm/microservice ?

f-necas commented 3 months ago

Yes, unless we want to keep mapstore's one, we have to do some PR to match our needs.

landryb commented 3 months ago

Yes, unless we want to keep mapstore's one, we have to do some PR to match our needs.

wouldnt that be 'better', instead of reinventing the square wheel once again, and require more resources at runtime ?

edevosc2c commented 3 months ago

Won't do. A dedicated proxy tool will be used.

sad. what was the rationale ? dedicated proxy tool = another jvm/microservice ?

For security.

As of now, the mapstore proxy is missing one critical feature, the ability to whitelist hosts

We have discovered that even though you set some restrictions for what query you can do: https://github.com/georchestra/datadir/blob/master/mapstore/proxy.properties#L25-L41. You can still reach those through the internal network, thus bypassing the geOrchestra authentication.

If we do a PR to add this functionality and this gets merged, we may not see the light of this functionality until mapstore 2024. A user could be very well be using mapstore 2022 or 2023. If we release a dedicated tool, we are sure that this functionality will be included.

On top of this, a dedicated tool is great because you can isolate it.

In Kubernetes or Docker or with IPAddressAllow in systemd, you can say that this CORS proxy can only reach the internet and nothing else.

Whereas if we are using the built-in mapstore proxy you can't easily do that, you have to let the whole mapstore process connecting to internal services like LDAP and PostgreSQL. If the CORS proxy whitelist may have some flaws discovered later on, an attacker could easily reach your internal services.

There are probably more things to do security wise. Remember that a CORS proxy is dangerous:

landryb commented 3 months ago

If we do a PR to add this functionality and this gets merged, we may not see the light of this functionality until mapstore 2024. A user could be very well be using mapstore 2022 or 2023. If we release a dedicated tool, we are sure that this functionality will be included.

nothing says that you cant run a newer http-proxy with an older mapstore version....oh well.

but i get the whole security handwaving argument ;) if you've found something that is sensible, please report it properly upstream..

edevosc2c commented 3 months ago

nothing says that you cant run a newer http-proxy with an older mapstore version....oh well.

We don't know how it is integrated with mapstore. By using a newer http-proxy library on an old mapstore version, you could very well run into dependencies clash.

Like this famous issue: https://github.com/georchestra/mapstore2-georchestra/issues/622

That's the last thing I want to worry about :smile: