influxdb-cxx
influxdb-cxx copied to clipboard
offa
Add option to allow Curl to not verify self signed certs
In some instances, the influxdb host has HTTPS set using self signed certs (test environments). Without the following option set, curl will complain about the self signed cert and the query will fail:
Alow option in HTTP.cxx to set: curl_easy_setopt(handle, CURLOPT_SSL_VERIFYPEER, 0L)
The transport doesn't expose any Http related API. While I see the use case, making it too easy to disable security settings may motivate users to ignore certs instead of actually fixing them.
On the other hand, could supporting something like CURLOPT_CAINFO solve your use case? Eg. setting an env to your cert path.
Perhaps an option to InfluxDBFactory::get()?
Self signed certs are often temporary for testing purposes where https is mandatory. Production servers use real certs and would pass the cert check.
Perhaps an option to InfluxDBFactory::get()?
The factory should do only one thing and not leak any transport details. Spontaneously I'd prefer an env providing a cert file path so related transports (https) can pick it up, but others aren't involved.
Update: Having a look at libcpr, not a single on/off parameter but a generic parameter class (key-value pairs) that's translated to the transport options could be a good option actually. :+1:
What would fit your use case better (or maybe both!?)?
Production servers use real certs and would pass the cert check.
Once it's off, it's kept disabled too often unfortunately (even if it's a no-brainer to enable it). But anyway, that's another story …
I would also like to see an option to disable SSL verification. Currently, I have to hack the source code to make this work (by adding session.SetVerifySsl(false); to HTTP::HTTP().
I agree with @offa that directly exposing such an option in the factory makes it a bit too easy to circumvent SSL verification.
How about an option on InfluxDB, e.g. InfluxDB.setSslVerify(bool)? This requires an explicit and additional call, i.e., something that doesn't happen "accidentally". This call would in turn call session.SetVerifySsl(false) if the transport is indeed HTTP(S).
I see, put it on the feature list.
While the extra step sounds good to me, I'm not quite sure if InfluxDB is the right class – it shouldn't take care of transport options.
Fair point - where would you put it instead? Into some additional, optional Options object passed into the factory's ::Get() call? Or some static config that needs to be set before calling the factory? Or a flag on the factory itself?
The main problem I see with setting an SSL verify flag in general is that it only applies to HTTPS transport, which isn't specified explicitly but rather through the database URL, so an "impedance mismatch" of sorts will be almost unavoidable.
Fair point - where would you put it instead? Into some additional, optional Options object passed into the factory's ::Get() call? Or some static config that needs to be set before calling the factory? Or a flag on the factory itself?
Using a builder might be an option too, or adding a dedicated interface for options (eg. setter to transports). The connection string is not an option for these kind of settings though.
The main problem I see with setting an SSL verify flag in general is that it only applies to HTTPS transport, which isn't specified explicitly but rather through the database URL, so an "impedance mismatch" of sorts will be almost unavoidable
That's a good point. The gap between HTTP and the other transports (udp, tcp, unix sockets) is growing. While the former has full feature support, the latter provide only a very limited feature set. At some point it might become reasonable to differentiate between both types – even more as the HTTP options are getting more.
I have outlined a possible API in #216. What do you think?
/cc @kasparthommen @ItsGarand
@kasparthommen @ItsGarand could you have a look at #220? It introduces a new transport configuration which provides support for this option.
@offa Apologies for not getting back earlier - I saw your first ping already. I'm totally swamped with work atm so won't have time to check until the new year I'm afraid, but it's on my list!