open-telemetry / opentelemetry-collector

OpenTelemetry Collector
https://opentelemetry.io
Apache License 2.0
4.51k stars 1.48k forks source link

[extension/auth] Support authentication for websockets #10864

Open BinaryFissionGames opened 3 months ago

BinaryFissionGames commented 3 months ago

Is your feature request related to a problem? Please describe. Currently, the authentication extension client interface supports HTTP and gRPC authentication. However, we would like to be able to authenticate websocket connections as well, in particular for supporting authentication for the opamp extension, which can be run using websockets.

On the surface, it seems like the HTTP RoundTripper should be applicable to websocket connections, but the most popular libraries do not take a RoundTripper.

See: github.com/gorilla/websocket, golang.org/x/net/websocket

Instead, they are limited essentially to a URL + Headers.

Describe the solution you'd like Here are a couple ways we could support authentication for websockets:

1. Try using the existing RoundTripper

This is a straight-forward "hacky" way to accomplish authentication. Basically the idea would be to implement a "base" round-tripper that modifies a struct to pass the websocket connection transparently through the "wrapper" round-tripper, for example:

type baseRoundTripper struct {
    dialer websocket.Dialer
    conn   *websocket.Conn
}

func (b baseRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
    conn, resp, err := b.dialer.Dial(req.URL.String(), req.Header)
    if err != nil {
        return resp, err
    }

    b.conn = conn
    return resp, err
}]

Here, the connection is preserved on the base RoundTripper and could be retrieved after

The issue with this is that it assumes a few things about the function:

  1. the base RoundTripper must only succeed once (since a connection is long-lived, and does not actually do a single "round-trip") - calling a second time after failure is OK
  2. the base RoundTripper must only modify the URL or Headers. If anything else of the request is modified (e.g. the body), it will be ignored.

Because of these limitations, it's possible for an authentication extension to follow the interface, but be incompatible with authenticating over a websocket. That feels undesirable to me, but I'm leaving it here as an option; It could be so uncommon for anything other than the headers to be modified that it's worth doing this more hacky solution. (See appendix, "Calls RoundTripper Once", "Modifies Other")

2. Add a method that just gets request headers

type Client interface {
    WebsocketHeaders() (http.Headers, error)
}

Here, you could imagine fetching headers for the request before making it. This approach pretty simplistic, but seems like it could work for most authentication cases (See appendix, "Uses HTTP response").

The downside here is that nothing can happen after the request, because the extension doesn't have a hook to get the http response. It seems like most of the current authentication extensions don't make use of this and always call the RoundTripper last.

3. Add a restricted "url + headers" method

You could imagine an interface like this (ignore the clumsy naming):

type HeaderRequester interface {
    DoRequestWithHeaders(url string, headers http.Header) http.Response 
}

type Client interface {
    HeaderRequester(base HeaderRequester) HeaderRequester
}

Essentially, this is the same thing as the http.RoundTripper, but the surface area of the request is reduced to just the URL + headers, which is fully compatible with the interface for dialing a websocket in all the most popular websocket libraries. In order to make this work with websockets, we'd still have the downside of only allowing the base requester to be called once.

4. Add a method for dialing a websocket directly

We could decide on directly supporting dialing for a particular websocket library. For instance:

type WebsocketDialer interface {
        DialWebsocket(url string, headers http.Header) (*websocket.Conn, *http.Response, error)
}

type Client interface {
    WebsocketDialer(base WebsocketDialer) (WebsocketDialer, error)
}

The huge downside here is that this creates a dependency on a particular library (in this case, the gorilla library). It's not unreasonable to assume that component may want to use a different library, or even that at some point we may need to switch libraries. I think that makes this option not suitable, but it's worth mentioning if we want to tie ourselves to a particular library, we get a "simpler" interface here.

5. Create a Dial interface that returns a Conn interface

Like 4, we essentially have a dial method, but instead of using concrete types we could return an interface like this:

type MessageType int

const (
// These values align with RFC6455 opcodes https://www.rfc-editor.org/rfc/rfc6455.html#section-11.8
TextMessage MessageType = 1
BinaryMessage MessageType = 2
)

type WebsocketConn interface {
    WriteMessage(ctx context.Context, msgType MessageType, msg []byte) error
    ReadMesage(ctx context.Context) (MessageType, []byte, error)
    Close(code int, reason string) error
}

type Dialer interface {
    Dial(url string, headers http.Header) (WebsocketConn, http.Response, error)
}

type Client interface {
    WebsocketDialer(base websocket.Dialer) (websocket.Dialer, error)
}

The con here is that we have to essentially define our own interface for a websocket connection. It's also on the user of this interface to create an adapter for handling websocket connections.

Appendix: Table of client auth extensions and RoundTripper use

Note: The vendor specific ones here likely have no use to a generic websocket-based component (like the opamp extension), since they are for specific cloud platforms. I've left them in the table for consideration of how different extensions using RoundTripper might be implemented.

Calls RoundTripper Once Modifies Headers Modifies URL Modifies other Uses HTTP response Vendor specific
basicauthextension + + - - - -
bearertokenauthextension + + - - - -
headerssetterextension + + - - - -
oauth2clientauthextension + + - - - -
asapauthextension + + - - - +
googleclientauthextension + + - - - +
sigv4authextension + + + - (note: reads body) - +
sumologicextension + + (note: Modifies cookies) - - + (uses response to update sticky session cookie) +

Additional context See #32762 which is tracking using existing authentication extensions with the supervisor.

jpkrohling commented 3 months ago

I see websockets in the same position as gRPC, in that a per-RPC auth call is desirable. In that sense, I'd go with option 2, which maps nicely with the pattern we have for gRPC as well.

That said, as we are trying to stabilize everything., I'd prefer this change to be made after v1 of this module.

BinaryFissionGames commented 3 months ago

That makes sense to me.