search

svenvc / zinc

Zinc HTTP Components is an open-source Smalltalk framework to deal with the HTTP networking protocol.
MIT License
96 stars 55 forks source link

ZnClient allows untrusted HTTPS certificates #146

Open Rinzwind opened 2 weeks ago

Rinzwind commented 2 weeks ago

When using HTTPS, ZnClient seems to not require the server to have a trusted certificate. In Pharo VM pull request #816, I gave a snippet for setting up a ZnSecureServer on macOS, which can be continued as follows:

"Remove trust in the certificate authority’s certificate (needs confirmation):"
result7 := LibC resultOfCommand: 'cd /tmp && /usr/bin/security ' ,
    'remove-trusted-cert ca-cert 2>&1'.

"Get ‘/’ from the server again:"
response2 := ZnEasy get: 'https://localhost:1443'.

The last send of #get: unexpectedly still answers a ZnResponse, I would have expected it to signal an error instead as the server’s certificate is no longer trusted.

There’s a method #certificateVerificationState on ZdcPluginSSLSession. Adding connection sslSession certificateVerificationState inspect at the end of #setupTLSTo: on ZnClient shows that it does distinguish between the certificate being trusted (value 0) or not (value 1), at least for the example of this snippet. But the method is not otherwise used.

While it would be good to, when the certificates do get verified, still have an option to disable that, I would expect it to be enabled by default, as described in the last paragraph of section 4.3.4 in RFC 9110.

svenvc commented 2 weeks ago

You are absolutely right, the default should be to verify trust in server certificates as a client.

The reason this is not done is historical, though I fear nothing much has changed.

To enable this, it must be supported on all 3 major platforms. The SSL plugin is native code that is totally different between those platforms. As far as I know, it is incomplete, especially in this area.

So this is an area for improvement (but my opinion is that the plugin needs a serious rewrite, not my area of expertise).

Rinzwind commented 1 week ago

Would having a check on #certificateVerificationState with the current plugin implementations work, or are they known to have false negatives, meaning cases in which a certificate that is valid according to say Chrome or curl is not valid according to the plugin? False positives are also undesirable but not worse I guess than the current situation of not having a check at all.