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.

Sign up for GitHub

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

Jump to bottom

net.nim CVerifyPeerUseEnvVars does not work #19246

Closed
ragingpastry opened this issue Dec 12, 2021 · 1 comment · Fixed by #19247
Closed

net.nim CVerifyPeerUseEnvVars does not work #19246

ragingpastry opened this issue Dec 12, 2021 · 1 comment · Fixed by #19247

Comments

@ragingpastry
Copy link
Contributor

ragingpastry commented Dec 12, 2021 •
edited
Loading

net.nim's newContext proc does not pass along the value of verifymode to scanSSLCertificates. scanSSLCertificates has an optional parameter which controls whether or not it looks at the environment variables SSL_CERT_DIR and SSL_CERT_FILE, however scanSSLCertificates is never called with this parameter set. This means that newContext doesn't actually look at environment variables SSL_CERT_DIR or SSL_CERT_FILE.

I believe this is a simple fix and I can submit a PR for this. I may need some assistance with how to best accomplish this in the code though.

Example

First we try without a certificate

import net
import httpclient

var client = newHttpClient(sslContext = newContext(verifyMode = CVerifyPeerUseEnvVars))
echo client.getContent("https://self-signed.badssl.com")

## nim r -d:ssl test_ssl.nim

Now we download the certificate and store it in a file

echo | openssl s_client -showcerts -connect self-signed.badssl.com:443 2>/dev/null | openssl x509 -outform pem  > /tmp/test.crt
# Test to make sure it works
curl --cacert /tmp/test.crt https://self-signed.badssl.com

Now we try to point to this file with our environment variable

export SSL_CERT_FILE=/tmp/test.crt
curl https://self-signed.badssl.com # This works! That means openSSL knows about our environment variable
nim r -d:ssl test_ssl.nim # this does not work =(

Current Output

root@b95ab39b616e:/build/fixture/test-tap-repo# ./test_ssl
/build/fixture/test-tap-repo/test_ssl.nim(5) test_ssl
/nim/lib/pure/httpclient.nim(1188) getContent
/nim/lib/pure/httpclient.nim(1183) get
/nim/lib/pure/httpclient.nim(1110) request
/nim/lib/pure/httpclient.nim(1011) requestAux
/nim/lib/pure/httpclient.nim(911) newConnection
/nim/lib/pure/net.nim(825) wrapConnectedSocket
/nim/lib/pure/net.nim(937) socketError
/nim/lib/pure/net.nim(540) raiseSSLError
[[reraised from:
/build/fixture/test-tap-repo/test_ssl.nim(5) test_ssl
/nim/lib/pure/httpclient.nim(1188) getContent
/nim/lib/pure/httpclient.nim(1183) get
/nim/lib/pure/httpclient.nim(1110) request
/nim/lib/pure/httpclient.nim(1011) requestAux
/nim/lib/pure/httpclient.nim(915) newConnection
]]
Error: unhandled exception: error:1416F086:SSL routines:tls_process_server_certificate:certificate verify failed [SslError]

Expected Output

<html>
<head>
  <meta charset="utf-8">
  <meta name="viewport" content="width=device-width, initial-scale=1">
  <link rel="shortcut icon" href="/icons/favicon-red.ico"/>
  <link rel="apple-touch-icon" href="/icons/icon-red.png"/>
  <title>self-signed.badssl.com</title>
  <link rel="stylesheet" href="/style.css">
  <style>body { background: red; }</style>
</head>
<body>
<div id="content">
  <h1 style="font-size: 12vw;">
    self-signed.<br>badssl.com
  </h1>
</div>

</body>
</html>

Possible Solution

Submitted a pull request here: #19247

Additional Information

This was introduced with this merge (I think) 5b85444

root@b95ab39b616e:/build/fixture/test-tap-repo# nim -v
Nim Compiler Version 1.6.0 [Linux: amd64]
Compiled at 2021-10-19
Copyright (c) 2006-2021 by Andreas Rumpf

git hash: 727c6378d2464090564dbcd9bc8b9ac648467e38
active boot switches: -d:release -d:danger
@ragingpastry ragingpastry mentioned this issue Dec 12, 2021
Merged
@ragingpastry
Copy link
Contributor Author

ragingpastry commented Dec 13, 2021 •
edited
Loading

Added some tests to this. Please let me know if you'd like to see additional / different testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: fixes bug in CVerifyPeerUseEnvVars ragingpastry/Nim
1 participant
@ragingpastry