reverseproxy: Mask the WS close message when we're the client

[francislavoie]

While testing a NodeJS app of mine, I noticed that since our changes to the proxy to send a WebSocket close frame, when Caddy would close first, we would see RangeError: Invalid WebSocket frame: MASK must be set in our logs from https://github.com/websockets/ws.

After some investigation, it looks like when we're the "client" (i.e. the upstream half of the proxy, whereas for the downstream half we act as the "server"), we need to mask the message according to https://www.rfc-editor.org/rfc/rfc6455#section-5.3.

I copied the maskKey and maskBytes code from https://github.com/websockets/ws as noted in code comments.

Needs some testing to make sure this still works correctly.

[francislavoie]

Tested it with my NodeJS app, seems to work! I see both the client and server closing the connection when I stop Caddy, and the error went away on the server.

[climba03003]

I just notice the same on my server. It crashed the whole application and require hard restart to resume the upstream.

[francislavoie]

@climba03003 could you try out a build with this PR to confirm the fix works for you too?

What made you find this issue, is this just an insane coincidence?? I only just discovered this problem a few hours ago.

[climba03003]

What made you find this issue, is this just an insane coincidence??

I am migrating my company from nginx to caddy in this few days. I notice it from one of my reporter that the application is down without any reason (e.g. no restart of server, no code changes).
So, I search a bit today and see this fix.

could you try out a build with this PR to confirm the fix works for you too?

I am using container for the application, it will be a bit hard to use a custom build.

[francislavoie]

I am using container for the application, it will be a bit hard to use a custom build.

No, it should be super easy, actually.

FROM caddy:2.6.2-builder AS builder

# TODO: Temporarily using a commit from https://github.com/caddyserver/caddy/pull/5199
# until that pull request is merged.
RUN xcaddy build 775e352b6feb6f85f2066a8933d3e0e9751b32a0

FROM caddy:2.6.2-alpine
COPY --from=builder /usr/bin/caddy /usr/bin/caddy

[climba03003]

Good to know that, I will report back after the reporter test.

[climba03003]

Seems it is not fixed in my end. I might need to patch the outside caddy as well.

stateDiagram
  [*] --> Caddy(2.6.2)
  state Container {
    Caddy(775e352b6feb6f85f2066a8933d3e0e9751b32a0) --> Node.JS
    Node.JS --> [*]
  }
  Caddy(2.6.2) --> Container

Loading

[francislavoie]

Probably. It depends which one is reloaded or restarted, as that's the one the close frame will be sent from.

[climba03003]

After running of 3 days, it does solve the issue. Thanks.

[francislavoie]

Thanks!

We'll merge as soon as Matt has a chance to give this a look.

[mholt]

LGTM. Thanks for figuring this out, Francis! 😁