getindata / flink-http-connector

Http Connector for Apache Flink. Provides sources and sinks for Datastream , Table and SQL APIs.
Apache License 2.0
149 stars 42 forks source link

Public certificates should not have to be supplied as they should be picked up from the jvm #119

Open davidradl opened 3 weeks ago

davidradl commented 3 weeks ago

During https://github.com/getindata/flink-http-connector/issues/91 I was testing against a rest end point. The rest call was successful if issued with curl, but failed the ssl handshake in this connector. The reason it fails is https://github.com/getindata/flink-http-connector/blob/05d3b474f403ff68ab6e7abe396833f1ebb8d060/src/main/java/com/getindata/connectors/http/internal/security/SecurityContext.java#L68. When no certs are supplied it creates a strange SSLContext.

If we do not supply an SSLContext on this line then the public certs are picked up.

I suggest we either change the default behaviour in the absence of supplied certs to not supply an SSLContext or if there is some reason to have this SSLContext then introduce a flag use_public_certs to toggle this behaviour.

kristoffSC commented 2 weeks ago

Hi yeah, that is a good finding. The reason why the SSL context is created regardless whether custom certs are defined or not was -> to make code simpler I guess, where "simpler" is probably subjective :)

We can:

  1. not to create SSL context if custom cert is not defined
  2. create it always but use Java's default, build in key store.

I personally would try with option 2, but Im ok with option 1 also.