Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SNOW-990764: Allow disabling certificate validation #1585

Open
vladimirtiukhtin opened this issue Dec 13, 2023 · 7 comments
Open

SNOW-990764: Allow disabling certificate validation #1585

vladimirtiukhtin opened this issue Dec 13, 2023 · 7 comments
Assignees
Labels
feature status-triage_done Initial triage done, will be further handled by the driver team

Comments

@vladimirtiukhtin
Copy link

What is the current behavior?

Google Looker allows a use of SSH tunnelling, but it hardcodes localhost as snowflake hostname. It unsurprisingly leads to SSL validation error

What is the desired behavior?

With some parameter disable certificate validation on JDBC level

How would this improve snowflake-jdbc?

It will work with Google Looker SSH tunnelling

@github-actions github-actions bot changed the title Allow disabling certificate validation SNOW-990764: Allow disabling certificate validation Dec 13, 2023
@ChrisCollinsIBM
Copy link

ChrisCollinsIBM commented Feb 6, 2024

What I would rather see to be able to pass in a variable in the connection string to specify the TrustManager Implementation rather than explicitly using https://github.com/snowflakedb/snowflake-jdbc/blob/master/src/main/java/net/snowflake/client/core/SFTrustManager.java exclusively.

Then if an end user wants to implement something custom (whether it be better or worse) then they can.

The Microsoft SQL Server JDBC Driver supports this method and it allows full flexibility for the end user to implement a trustManager that does what they need.

This could also be (possibly better) handled by allowing a parameter to specify the className of the SSLSocketFactory to be used which allows users to customer the SSLContext itself (TLS versions, cipher suites, etc) as well as the Trustmanager.

[Thread-1447]    javax.net.ssl.SSLHandshakeException: com.ibm.jsse2.util.j: No trusted certificate found
[Thread-1447]    at com.ibm.jsse2.g.a(g.java:52)
[Thread-1447]    at com.ibm.jsse2.bb.a(bb.java:59)
[Thread-1447]    at com.ibm.jsse2.bb.a(bb.java:132)
[Thread-1447]    at com.ibm.jsse2.bb.a(bb.java:237)
[Thread-1447]    at com.ibm.jsse2.z$c.a(z$c.java:196)
[Thread-1447]    at com.ibm.jsse2.z$c.a(z$c.java:87)
[Thread-1447]    at com.ibm.jsse2.z$c.consume(z$c.java:83)
[Thread-1447]    at com.ibm.jsse2.p.consume(p.java:89)
[Thread-1447]    at com.ibm.jsse2.aa.a(aa.java:133)
[Thread-1447]    at com.ibm.jsse2.aa.a(aa.java:200)
[Thread-1447]    at com.ibm.jsse2.bb.a(bb.java:87)
[Thread-1447]    at com.ibm.jsse2.a0.a(a0.java:40)
[Thread-1447]    at com.ibm.jsse2.bj.b(bj.java:188)
[Thread-1447]    at com.ibm.jsse2.bj.f(bj.java:115)
[Thread-1447]    at com.ibm.jsse2.bj.a(bj.java:386)
[Thread-1447]    at com.ibm.jsse2.bj.startHandshake(bj.java:431)
[Thread-1447]    at net.snowflake.client.jdbc.internal.apache.http.conn.ssl.SSLConnectionSocketFactory.createLayeredSocket(SSLConnectionSocketFactory.java:436)
[Thread-1447]    at net.snowflake.client.jdbc.internal.apache.http.conn.ssl.SSLConnectionSocketFactory.connectSocket(SSLConnectionSocketFactory.java:384)
[Thread-1447]    at net.snowflake.client.jdbc.internal.apache.http.impl.conn.DefaultHttpClientConnectionOperator.connect(DefaultHttpClientConnectionOperator.java:142)
[Thread-1447]    at net.snowflake.client.jdbc.internal.apache.http.impl.conn.PoolingHttpClientConnectionManager.connect(PoolingHttpClientConnectionManager.java:376)
[Thread-1447]    at net.snowflake.client.jdbc.internal.apache.http.impl.execchain.MainClientExec.establishRoute(MainClientExec.java:393)
[Thread-1447]    at net.snowflake.client.jdbc.internal.apache.http.impl.execchain.MainClientExec.execute(MainClientExec.java:236)
[Thread-1447]    at net.snowflake.client.jdbc.internal.apache.http.impl.execchain.ProtocolExec.execute(ProtocolExec.java:186)
[Thread-1447]    at net.snowflake.client.jdbc.internal.apache.http.impl.execchain.RetryExec.execute(RetryExec.java:89)
[Thread-1447]    at net.snowflake.client.jdbc.internal.apache.http.impl.execchain.RedirectExec.execute(RedirectExec.java:110)
[Thread-1447]    at net.snowflake.client.jdbc.internal.apache.http.impl.client.InternalHttpClient.doExecute(InternalHttpClient.java:185)
[Thread-1447]    at net.snowflake.client.jdbc.internal.apache.http.impl.client.CloseableHttpClient.execute(CloseableHttpClient.java:83)
[Thread-1447]    at net.snowflake.client.jdbc.internal.apache.http.impl.client.CloseableHttpClient.execute(CloseableHttpClient.java:108)
[Thread-1447]    at net.snowflake.client.jdbc.RestRequest.execute(RestRequest.java:225)

Something local to the code being executed would be preferred as we don't want to have to change Default SSL Factories JVM-wide for a single connection object in a large application.

For what it's worth I was able to get my application to connect using insecureMode=true in the connection string, not because I want to bypass OSCP checking, but because there's code in the SFTrustManager class

that initializes by getting the list of trust mangers and using the first one which happens to be the one I want in my environment.

Not ideal, but gets us going for now.

@sfc-gh-dszmolka sfc-gh-dszmolka added the status-triage_needed This is a new issue, and initial triage is needed label Apr 27, 2024
@sfc-gh-dszmolka
Copy link
Contributor

thank you for raising this enhancement request, we'll consider for future plans. Of course if there's also a PR within the possibilities, that would be more than welcome.

for the time being, insecuremode should be hopefully sufficient to move forward, considering if you want to disable certificate validation altogether, then you don't want OCSP validation either which the above parameter apparently conveniently does.

@sfc-gh-dszmolka sfc-gh-dszmolka added status-triage_done Initial triage done, will be further handled by the driver team and removed status-triage_needed This is a new issue, and initial triage is needed labels Apr 30, 2024
@Hakky54
Copy link

Hakky54 commented May 26, 2024

Hi guys, I am jumping into this discussion as I think I have a solution for this use case. There is an easier workaround without the need of changing snowflake-jdbc at all. I had a similar challenge while using MySQL jdbc java connector and it is basically bypassing the ssl configuration initialized by the jdbc library and supplying your own custom one. I created a library to make it easily reusable, see here: https://github.com/Hakky54/sslcontext-kickstart

Here is an example with MySQL jdbc which should be pretty similar to snowflake jdbc https://github.com/Hakky54/java-tutorials/tree/main/bypassing-overruling-ssl-configuration

The option is explained in detail here https://github.com/Hakky54/sslcontext-kickstart?tab=readme-ov-file#global-ssl-configuration

@ChrisCollinsIBM
Copy link

Hi @Hakky54, I'll have to have a deeper look but I'm guessing the Security.insertProviderAt method you're calling is temporarily making this SSLContext essentially the default for the whole JVM temporarily?

If so then it's definitely not thread safe as anything else happening in the JVM while this is going on will get this SSLContext.

Most users need something settable for the specific connection only that won't affect things elsewhere in the JVM.

If I've misread the behaviour or your proposed solution please clarify, thanks!

@Hakky54
Copy link

Hakky54 commented May 27, 2024

It is possible to configure it globally as a default for the whole JVM, but that won't be our intention I think as we want to narrow down to the scope of only snowflake-jdbc ssl configuration. For this kind of use case I would recommend to just only insert the security provider just before creating the jdbc connection and removing it after establishing the connection. I made it thread safe though but if you think I missed something, please feel free to point out so I can improve.

By the way, I think this method is a last resort if the implementation takes to long or never happens at all at snowflake. I came up with this last resort trick while using MySQL jdbc driver. The code base was not being updated that frequently anymore, so feature requests implementations could take forever

@ChrisCollinsIBM
Copy link

ChrisCollinsIBM commented May 27, 2024

So after this line is executed https://github.com/Hakky54/java-tutorials/blob/main/bypassing-overruling-ssl-configuration/src/main/java/nl/altindag/ssl/App.java#L35 wouldn't any thread elsewhere get this SSLContext as well until you remove it at the end? This essentially makes the temporary "default" one JVM-wide doesn't it?

If that's the case then it's most definitely not thread safe as it affects other connections elsewhere in the JVM.

A solution like that might work for some users in a small isolated JVM but I don't think it satisfies the use case in a production environment.

If I'm incorrect on how this would behave I'd certainly be interested in any clarification and a multi threading test of some kind to prove it!

@Hakky54
Copy link

Hakky54 commented May 27, 2024

So after this line is executed https://github.com/Hakky54/java-tutorials/blob/main/bypassing-overruling-ssl-configuration/src/main/java/nl/altindag/ssl/App.java#L35 wouldn't any thread elsewhere get this SSLContext as well until you remove it at the end? This essentially makes the temporary "default" one JVM-wide doesn't it?

Technically yes, but in a real-world use case it should not happen like that as an SSLContext is not that often initialized. Normally during the startup of a server an SSLContext is being initialized. If you need to make https calls to other services with an http client then you need another one. That one is also initialized once during the initialization of the http client. And the last one will be in this case for a jdbc connection.So at most 3 during the whole lifecycle I would say. And I don't think they will be all created at the same time and therefor it won't cause an issue, but it is up to you to give it a try. If you are planning to give it a try, it would be nice to get your feedback on this topic

If I'm incorrect on how this would behave I'd certainly be interested in any clarification and a multi threading test of some kind to prove it!

This option is available for quite some time, I haven't received any bug reports from the community yet though...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature status-triage_done Initial triage done, will be further handled by the driver team
Projects
None yet
Development

No branches or pull requests

5 participants