labstack/echo

Lack of Origin Header Results in 403 Even When AllowOrigins is set to "*"

Jyosua opened this issue · 4 comments

Jyosua commented

Issue Description

The default CORS configuration for echo sets AllowOrigins to the wildcard, "*", which should allow requests to be made regardless of origin. However, when we were switching over from x/net/websocket to nhooyr/websocket on the client side, we noticed that if the client doesn't send an Origin header, the request will be denied with a 403.

We discovered this because while x/net/websocket requires an origin be set when calling its Dial function, nhooyr/websocket has no such requirement.

I also opened a related documentation, etc. issue on the websocket library here

Checklist

  • Dependencies installed
  • No typos
  • Searched existing issues and docs

Expected behaviour

If AllowOrigins is set to *, then the absence of the Origin header will not result in a 403 status code.

Actual behaviour

Lack of an Origin header will always result in a 403 even if any origin is allowed.

Steps to reproduce

  1. Start the server
  2. Run the client code

Working code to debug

Server

func hello(c echo.Context) error {
	websocket.Handler(func(ws *websocket.Conn) {
		defer ws.Close()
		for {
			// Write
			err := websocket.Message.Send(ws, "Hello, Client!")
			if err != nil {
				c.Logger().Error(err)
			}

			// Read
			msg := ""
			err = websocket.Message.Receive(ws, &msg)
			if err != nil {
				c.Logger().Error(err)
			}
			fmt.Printf("%s\n", msg)
		}
	}).ServeHTTP(c.Response(), c.Request())
	return nil
}

func main() {
	e := echo.New()
	e.Use(middleware.Logger())
	e.Use(middleware.Recover())
	e.Static("/", "../public")
	e.GET("/ws", hello)
	e.Logger.Fatal(e.Start(":3000"))
}

Client

ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()

c, _, err := websocket.Dial(ctx, "ws://localhost:3000/ws", nil)
if err != nil {
	// ...
}
defer c.CloseNow()

err = wsjson.Write(ctx, c, "hi")
if err != nil {
	// ...
}

c.Close(websocket.StatusNormalClosure, "")

Version/commit

v4.12.0

Jyosua commented

Issue #2534 may be the same underlying problem, but I'm unsure.

aldas commented

I can not test this ATM but looking at CORS middleware code (https://github.com/labstack/echo/blob/master/middleware/cors.go) I can not find line where 403 is being returned. Do you mean 203?

It would be nice if you could copy/paste example of that handshake request here with headers etc.

aldas commented

I think your problem is with x/net/websocket library and with this https://github.com/golang/net/blob/765c7e89b3bdd76bfc210acddd3ca73931eb8d1d/websocket/server.go#L101 handshake method and that 403 originates from here https://github.com/golang/net/blob/765c7e89b3bdd76bfc210acddd3ca73931eb8d1d/websocket/server.go#L33

there is a comment there

// Handler is a simple interface to a WebSocket browser client.
// It checks if Origin header is valid URL by default.
// You might want to verify websocket.Conn.Config().Origin in the func.
// If you use Server instead of Handler, you could call websocket.Origin and
// check the origin in your Handshake func. So, if you want to accept
// non-browser clients, which do not send an Origin header, set a
// Server.Handshake that does not check the origin.
Jyosua commented

I think your problem is with x/net/websocket library and with this https://github.com/golang/net/blob/765c7e89b3bdd76bfc210acddd3ca73931eb8d1d/websocket/server.go#L101 handshake method and that 403 originates from here https://github.com/golang/net/blob/765c7e89b3bdd76bfc210acddd3ca73931eb8d1d/websocket/server.go#L33

there is a comment there

// Handler is a simple interface to a WebSocket browser client.
// It checks if Origin header is valid URL by default.
// You might want to verify websocket.Conn.Config().Origin in the func.
// If you use Server instead of Handler, you could call websocket.Origin and
// check the origin in your Handshake func. So, if you want to accept
// non-browser clients, which do not send an Origin header, set a
// Server.Handshake that does not check the origin.

Yeah, you are correct, this does seem to be the source of the issue. Thank you!