s-macke / jor1k

Online OR1K Emulator running Linux
http://jor1k.com
BSD 2-Clause "Simplified" License
1.73k stars 196 forks source link

Decide on strategy to support authentication/authorization so that multiple relay providers can be used #117

Open gdm85 opened 8 years ago

gdm85 commented 8 years ago

Sorry if this is the wrong way to communicate with you guys, however please note that I have ported Benjamin's websockproxy to Go and just added a reference here:

https://github.com/s-macke/jor1k/wiki/Network-support/_compare/6946626baab0b9a0f23f25f7476499cde02b6165...de4254df4906d475b277168a45fc3e922a11864a

Hope it will be of use; in particular I think it may be useful for use with Docker containers and when persistent TAP will be supported (it's planned).

I have not submitted patches for AUTH support, however I may if you think it's an useful addition. This is how it would work:

  1. if the querystring variable authKey is specified, then send a special AUTH frame when the websocket connects

You can see a rough version of this here: https://github.com/gdm85/jor1k/commit/cfa0181e511958a4606a7d1ebc121d4708cde7b9

Just to be precise, the AUTH feature I added is optional and current go-websockproxy works as a replacement for the python version.

https://github.com/gdm85/go-websockproxy

s-macke commented 8 years ago

Hi gdm,

thanks for the Go implementation. Looks like the source code is very clean. As far as I understand the authentication query can be sent from the server, but will be not mandatory. In case this query is sent, a window should pop up in which you type the password. Is it possible, to implement it and to stay compatible with the version written in Python from @benjamincburns?

benjamincburns commented 8 years ago

If compatibility may be broken in some way just let me know how and I can coordinate a fix accordingly for the demo page.

On Tue, Apr 5, 2016, 21:20 Sebastian Macke notifications@github.com wrote:

Hi gdm,

thanks for the Go implementation. Looks like the source code is very clean. As far as I understand the authentication query can be sent from the server, but will be not mandatory. In case this query is sent, a window should pop up in which you type the password. Is it possible, to implement it and to stay compatible with the version written in Python from @benjamincburns https://github.com/benjamincburns?

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/s-macke/jor1k/issues/117#issuecomment-205725117

gdm85 commented 8 years ago

@s-macke thanks for checking this out. The server-side go-websockproxy can either require or not the initial AUTH special frame, it depends on whether it was started with a non-empty parameter passed to --auth-key. If this feature is disabled then it would ignore any such frames sent from clients and it would be 1:1 with the python version (maybe a bit more strict on the possibility of spoofing MAC addresses of other clients, but that's good).

At the moment the server is not sending clients any "auth is required" message that would enable what you described but it's rather expecting the clients to provide an auth key already.

I could indeed modify it so that instead of sending all unauthenticated traffic to /dev/null it rather first thing notifies the clients that a form of authorization is necessary before any frame is accepted. I will follow up with a branch for this.

Right now for these special frames I set the first 6 octects to 0 and the rest is an UTF8 payload (AUTH xxxxx in case of authentication); I know it's possible to exchange on websockets messages of type String instead of ArrayBuffer (as you do with pings/pongs), but in my tinkering with the Go websockets implementation I had the feeling it was simpler by just using one type of data (I might be wrong on this) so that's likely the route I would pursue also now.

@benjamincburns good point. I think we might do that because the (only) problem I see here already is that:

Probably the most sensible thing to do is to introduce a change in the python version too that goes like this:

Example of a simple payload (e.g. sent by a modified version of websockproxy in its server-side on_connect):

\0\0\0\0\0\0SERVER websockproxy\nAUTH none

AUTH none explicitly tells about the available feature.

Example of a more complex payload:

\0\0\0\0\0\0SERVER go-websockproxy\nAUTH key\nAUTH kerberos\nAUTH saml

The first field, the part right after SERVER, would be arbitrary; I am not serious about kerberos there by the way.

Afterwards we could modify jor1k to expect such a frame before actually starting jor1k emulation. This way we would know for sure when it's the moment to ask (or not) for a key and we are sure that the sending of further ethernet frames won't happen until the (potential) AUTH handshake has happened.

I think this way it would have a quite small impact on current python version and client-side jor1k. Do you guys think this approach would be desirable? On client-side jor1k would probably have a "preamble" Ethernet object that replaces itself (or its handlers) once the SERVER special frame has been processed (and eventually the key prompt has happened, if needed).

benjamincburns commented 8 years ago

I think there are likely a bunch of ways to handle auth without needing a special frame. For example, the auth token could be sent to the relay as a query string and the server could respond with 403 should this token not match.

I'm not familiar w/ standard security practises on websockets. I intend to do a bit of reading before I'm keen to settle on a particular approach.

In the mean time, I'll change the name of this issue to reflect what it is we're discussing here.

Edit: From just a cursory glance, querystring and cookie-based auth are decent options.

The most extensible thing to do might be to have each relay serve a json blob (perhaps at /capabilities, but this need not be standardized) which could indicate what URI to use to access the relay itself, and details about the auth strategies which the relay in question supports.

For example:

{
    "capabilities_version": "0.0.1",
    "name": "websockproxy",
    "relay_uri": "wss://relay.widgetry.org/relay",
    "supported_auth_schemes" : {
        "auth_frame" : false,
        "querystring_token": {
            "token_field": "auth_token"
        },
        "post_credentials_for_cookie": {
            "uri": "https://relay.widgetry.org/auth",
            "username_field": "username",
            "password_field": "password"
        }
    }
}
gdm85 commented 8 years ago

@benjamincburns as far as I know an HTTP auth request on websockets doesn't trigger any use interaction, it simply fails. As per the relay answering with a 403, I think that falls in the problem I described above: since packets are already flying to the relay by the time the onmessage handler of jor1k's Ethernet is checking for the 403, you would loose the possibility to put emulation on hold and correctly manage the user input to do an handshake.

As per your proposal to have a specific URI to describe the capabilities: I think that would work instead and we could do that already

benjamincburns commented 8 years ago

@gdm85 I understand that standard http basic and http digest auth don't work correctly for websockets, and I didn't propose these methods. Regarding a race condition between the emulator and the websocket error event handler, I wouldn't worry much about dropped Ethernet frames, especially if the race is short. Ethernet is expected to drop frames by higher level protocols, and if we want to prevent this we could easily implement cable connect detection in the emulated ethernet device to get around this.

I may not have explained myself well, so I'll dive into my example a bit.

As you likely understood, relay_uri is the URI to which jor1k should connect via its websocket.

supported_auth_schemes is intended to be an object where each field is named after a particular auth scheme, and either contains an object or a boolean value. Truthy values indicate that the relay supports the auth scheme named by the respective field. In the case where a field describing an auth scheme contains an object, the object describes some configuration information about this particular scheme.

In my not-meant-to-be-canonical examples above, auth_frame is the scheme you've discussed. It is false as websockproxy does not support this scheme (though really it doesn't support any auth scheme at the moment). Were there some configurable options for this scheme, you could indicate them as an object here. If there are no configurable options, just indicate true here.

querystring_token would simply add an auth token to the querystring to the relay_uri request, with the key portion of this named for the value indicated by token_field. In this case, if jor1k were accessed with querystring ?relay_token=12345, I'd expect jor1k to connect to the relay at wss://relay.widgetry.org/relay?auth_token=12345.

post_credentials_for_cookie was meant to describe a scheme where a username and password are POSTed to some URI (hooray CORS!) and that URI sets a secure cookie containing a session token as part of its response. Upon receipt of an HTTP 200 response from this POST, jor1k would be free to connect to the relay. The secure cookie containing the session token will be sent by the browser automatically as part of the websocket request, and the server may validate it prior to converting the request to a websocket session.

All of the above schemes require the user trusting the service hosting jor1k to not store off the user's secrets. This is a poor security model. The best standard which gets around this would be OAUTH, but IIRC (has been a while since I've read the OAUTH spec), implementing require jor1k to have some secure back-channel interaction with the relay by exposing an endpoint to which the relay can send a request containing a service-specific (revocable) auth token for the user in question. At the moment I believe jor1k is intended to work 100% as a front-end-only service, so implementing this would be a bit more effort than the above choices.

gdm85 commented 8 years ago

@benjamincburns sorry I didn't mean to say that you were proposing http auth/digest, I was just telling a case where websockets show a difference with common HTTP.

As per the /capabilities endpoint you proposed, I already said it sounds very good to me and would allow even a scenario without the lossy ethernet frame drops. The cable connect/disconnect also sounds really nice if it is ever planned/implemented.

Maybe I was not explicit enough, thanks for the detailed explanation (I am sure other readers will benefit from it too): I am willing to implement it as you said and it could definitely work with POST+cookies. I also understand that there could be better implementations, but honestly a /capabilities endpoint and POST with cookie sounds already very fair to me.

So I guess I shall go on and implement this in the Go version? At least as a prototype branch

benjamincburns commented 8 years ago

No worries, and I apologize if I seemed offended. I find it's best in conversations like this to be a bit more explicit and deliberate than I normally would be just to make sure my point comes across.

Regarding implementing, I'm a bit biased since I proposed the scheme, so I defer to @s-macke as he will ultimately need to maintain it on the jor1k side. Otherwise I'm certainly keen.

On Thu, Apr 7, 2016, 19:34 gdm85 notifications@github.com wrote:

@benjamincburns https://github.com/benjamincburns sorry I didn't mean to say that you were proposing http auth/digest, I was just telling a case where websockets show a difference with common HTTP.

As per the /capabilities endpoint you proposed, I already said it sounds very good to me and would allow even a scenario without the lossy ethernet frame drops. The cable connect/disconnect also sounds really nice if it is ever planned/implemented.

Maybe I was not explicit enough, thanks for the detailed explanation (I am sure other readers will benefit from it too): I am willing to implement it as you said and it could definitely work with POST+cookies. I also understand that there could be better implementations, but honestly a /capabilities endpoint and POST with cookie sounds already very fair to me.

So I guess I shall go on and implement this in the Go version? At least as a prototype branch

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/s-macke/jor1k/issues/117#issuecomment-206739188

gdm85 commented 8 years ago

@benjamincburns no worries from my side either :) agree, let's hear what @s-macke says.

It looks good and sound to me