-
Notifications
You must be signed in to change notification settings - Fork 57
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
ZnClient allows untrusted HTTPS certificates #146
Comments
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). |
Would having a check on |
I tested for false negatives by modifying url := 'https://github.com/chromium/chromium/raw/main/net/http/transport_security_state_static.json'.
contents := (ZnEasy get: url) contents.
nonCommentLines := contents lines reject: [ :line | line matchesRegex: '[[:space:]]*//.*' ].
entries := (STONJSON fromString: (String lf join: nonCommentLines)) at: 'entries'.
hosts := entries select: [ :entry |
(#('test' 'public-suffix' 'public-suffix-requested') includes: (entry at: 'policy')) not ]
thenCollect: [ :entry | entry at: 'name' ].
randomHosts := (1 to: 100) collect: [ :i | hosts atRandom ].
results := Dictionary newFromKeys: randomHosts andValues: (randomHosts collect: [ :host |
([ ZnConnectionTimeout value: 2 during: [ ZnEasy head: 'https://' , host ] ]
on: Error do: [ :error | error ]) ]).
failedHosts := (results select: [ :result | result isKindOf: ZnCertificateVerificationFailed ]) keys.
curlResults := Dictionary newFromKeys: failedHosts andValues: (failedHosts collect: [ :host |
LibC resultOfCommand: 'curl --silent --show-error --head https://' , host , ' 2>&1' ]). On Ubuntu I had to execute the following first because, as far as I understand, the default directory path is compiled into the OpenSSL library and the path in the library included with the Pharo VM is not the path compiled into Ubuntu’s library: OSEnvironment current at: 'SSL_CERT_DIR' put: '/usr/lib/ssl/certs' I would suggest adding the check on |
@svenvc: I was wondering whether you took a look at my previous comment? In particular: the suggestion to add a check on |
Yes I already had a look and yes I want to try this, at least the first step of it. |
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:
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. Addingconnection 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.
The text was updated successfully, but these errors were encountered: