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

websock client hangs when connecting to co-hosting server that routes with SNI when NoVerifyServerName is set #134

Open
iffy opened this issue Jan 22, 2023 · 7 comments

Comments

@iffy
Copy link

iffy commented Jan 22, 2023

I don't know yet if this is an issue with nim-websock, chronos, bearssl or fly.io, but I'm running a websockets server on fly.io and my websock client hangs when trying to connect to it. I'll leave the server up so people can test it, but I'll also include instructions for making a new one for posterity's sake. I would love any help anyone can spare -- I'm running out of things I know how to debug :)

Here is my client script, which attempts to connect to 3 different servers:

## Try connecting to various servers to see if they work
import std/strformat
import std/uri

import chronos
import chronos/apps/http/httpclient
import chronicles
import websock/websock


const flyAppHostname = "quiet-shape-2200.fly.dev"

template maybe(x: untyped): untyped =
  try: x
  except: echo "FAILED: ", getCurrentExceptionMsg()

proc string(x: seq[byte]): string =
  for c in x:
    result.add(chr(c))

proc httpGet(uri: string) {.async.} =
  echo "\nhttpGet ", uri
  let session = HttpSessionRef.new()
  let resp = await session.fetch(uri.parseUri)
  echo "  status: ", $resp.status
  echo "  resp: ", resp.data.string

proc sendAndReceive(uri: string) {.async.} =
  echo "\nsendAndReceive ", uri
  let ws = await WebSocket.connect(
    parseUri(uri),
    flags = {TLSFlags.NoVerifyHost, TLSFlags.NoVerifyServerName},
    # hostName = hostname,
  )
  echo "  sending ..."
  await ws.send("hello, world!")
  echo "  sent"
  let resp = await ws.recvMsg()
  echo "  got response: ", resp.string

proc main {.async.} =
  # an echo websockets server
  maybe await sendAndReceive("wss://ws.postman-echo.com/raw")
  # Not a websockets server
  maybe await wait(sendAndReceive("wss://www.google.com/"), 3.seconds)
  # HTTP 
  maybe await httpGet(&"https://{flyAppHostname}/")
  maybe await wait(sendAndReceive(&"wss://{flyAppHostname}/ws"), 3.seconds)

waitFor main()

When I run that I see the following output:

sendAndReceive wss://ws.postman-echo.com/raw
  sending ...
  sent
  got response: hello, world!

sendAndReceive wss://www.google.com/
FAILED: Server did not reply with a websocket upgrade: Header code: 400 Header reason: Bad Request Address: xxx.xxx.xxx.xxx:443

httpGet https://quiet-shape-2200.fly.dev/
  status: 200
  resp: Hello, world!

sendAndReceive wss://quiet-shape-2200.fly.dev/ws
FAILED: Timeout exceeded!
  • The websockets connection to wss://ws.postman-echo.com/raw succeeded.
  • The websockets connection to wss://www.google.com/ failed to upgrade to WS, which means it successfully completed the TLS handshake
  • The HTTP GET request to my fly-hosted app succeeded. This means the chronos HTTP client works.
  • the websockets connection to my fly-hosted app times out.

Debugging I've done

Inserting lots of debug statements, I think I've determined that it's hanging on this await item.future line: https://github.com/status-im/nim-chronos/blob/40143f8798e49e036179fbe066b0b32138ecfd85/chronos/streams/asyncstream.nim#L940

From what I can tell, the HTTP headers never get sent. This leads me to believe it's a TLS problem, not a websockets problem.

I've compared openssl s_client output for ws.postman-echo.com and my fly app with the following commands:

openssl s_client -connect quiet-shape-2200.fly.dev:443 -showcerts
openssl s_client -connect quiet-shape-2200.fly.dev:443 -showcerts -tls1_2
openssl s_client -connect ws.postman-echo.com:443 -showcerts

By default, fly uses TLS 1.3, but BearSSL doesn't support that, so I think it's trying to fallback to TLS 1.2, though I don't know if it succeeds.

Certificate property ws.postman-echo.com fly.io app
Peer signing digest SHA512 SHA256
Peer signature type RSA ECDSA
Server Temp Key ECDH, P-256, 256 bits X25519, 253 bits
Cipher ECDHE-RSA-AES128-GCM-SHA256 ECDHE-ECDSA-AES256-GCM-SHA384

Is it the cipher that's not supported? My reading of BearSSL docs makes me think ECDHE-ECDSA should work.


Run your own

If you want to run the whole thing yourself, here's all the code: https://gist.github.com/iffy/f2a4bcd78af9d8ae8e71f583b310e410

After installing flyctl and authenticating this should do it:

git clone https://gist.github.com/f2a4bcd78af9d8ae8e71f583b310e410.git websock-fly-issue
cd websock-fly-issue
fly launch
# follow the prompts
# edit testclient.nim to change the flyAppHostname to your new app hostname
nim c -r testclient.nim
@Menduist
Copy link
Contributor

Removing flags = {TLSFlags.NoVerifyHost, TLSFlags.NoVerifyServerName}, seems to make it work, that is a limitation of our SSL library unfortunately (can't remember if it's tracked somewhere)

@iffy
Copy link
Author

iffy commented Jan 23, 2023

You beat me to it! I just discovered it's the NoVerifyServerName flag alone that causes it to fail. Seems to work with NoVerifyHost. Looks like the former is for SNI verification and the latter for classic hostname verification. I may be able to work around that, but I also might dig some more into why that flag fails, or at least produce a PR that makes it fail rather than hang.

@Menduist
Copy link
Contributor

bearssl doesn't send the ServerName to the server when using NoVerifyServerName, but some servers uses the SNI to know who is to actual target of the request. Fixing it would require adding a specific flag upstream to disable server name verification while still allowing to send it.

We shouldn't hang, though

@iffy
Copy link
Author

iffy commented Jan 23, 2023

Okay, thanks for looking into this. Yes, here's the part in BearSSL where it says what you've just said: https://www.bearssl.org/api1.html#reset-and-sni Disabling verification of the SNI name also disables sending the name in ClientHello. You can't do only one or the other.

If you're familiar enough with the code to know how to make it not hang, I'll defer to you. Otherwise, if I get some time I'll submit a PR. I'm changing the title of this issue to match the issue.

@iffy iffy changed the title websock client hangs when connecting to fly.io-hosted websockets server websock client hangs when connecting to co-hosting server that routes with SNI when NoVerifyServerName is set Jan 23, 2023
@Menduist
Copy link
Contributor

It's hanging here:

await client.stream.writer.write(headerString)

We should be doing something similar to:

hlenfut = stream.readUntil(
addr buffer[0], MaxHttpHeadersSize, sep = HeaderSep)
ores = await withTimeout(hlenfut, HttpHeadersTimeout)
if not ores:
raise newException(HttpError,
"Timeout expired while receiving headers")

(and maybe that shows a bug in the write somewhere, would need to investigate deeper)
Feel free to open a PR :)

@Menduist
Copy link
Contributor

Found where the bearssl issue was tracked : status-im/nim-chronos#313

@dryajov
Copy link
Member

dryajov commented Mar 2, 2023

It's hanging here:

await client.stream.writer.write(headerString)

Should still have a timeout, otherwise it will wait until the server closes the connection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants