Added JWT tokens capability

[UXabre]

This PR adds the capability to use (a)symmetric JWTs as token for retrieving connection details. The only argument needed, is the public key or shared secret file for verification.

This way, the management of tokens to hosts becomes very manageable and quite secure.

I've also added some tests to confirm its functionality. Needless to say that the public/private keys displayed here are only for testing and mustn't be used in production

Let me know what you guys think!

[UXabre]

Hey! I noticed that 3 out of 4 builds have failed. Didn't check on python 2.6 but the others seem to work on my devenv? Could it be something does need an update on the build server? (sorry for wreaking havoc on my first day here :p)

[CendioOssman]

Looks good, but I'd like to avoid a hard requirement on PyJWT. So could you omit the change to setup.py? You can put it in test-requirements.txt instead for the tests to succeed.

As for the tests failing, there seems to be some issues with the cryptography module. Is that really needed? I don't see any imports for it.

[UXabre]

Hey @CendioOssman, thanks for reviewing! Unfortunately, the pyjwt is needed for this code to work (all testing aside), the import happens a bit further down the line: https://github.com/novnc/websockify/pull/372/files#diff-f7cac2852d62df1cca378dcc6467212bR97

How would we best proceed with that?

On your second note, regarding cryptography lib, it's needed to support all security hashes in pyjwt. So, they're kinda "married" if we want to support the entire spectrum of jwt hashes.

[ekohl]

Using with automatically closes the file ensures you don't leak file handles:

with open(self.source) as secret_file:
    secret = secret_file.read()

[ekohl]

A newline at the end is generally preferred.

[alex88]

Does this add the ability to pass just a token to the websocket connection to specify the host/port/credentials? If this is the case, would be possible to generate the token on server side, pass it to the novnc client which passes it to websocksify but in a way that the token is non readable from the client (by readable I mean the user can't just go to jwt.io and read the token host/port/credentials)?

[UXabre]

Does this add the ability to pass just a token to the websocket connection to specify the host/port/credentials? If this is the case, would be possible to generate the token on server side, pass it to the novnc client which passes it to websocksify but in a way that the token is non readable from the client (by readable I mean the user can't just go to jwt.io and read the token host/port/credentials)?

Hey @alex88 , yet, it is possible to provide a whole heap of information to the receiving end, including port and address. The credentials are not passed (as websockify basically translates from a "regular" TCP socket to a websocket, it doesn't know about the session/application/presentation layer, credentials are thus passed somewhere higher up the chain). However, in this case, I've only added support for JWS', what you seem to describe is JWE. I'd have to investigate but I'd at least would have to switch JWT library (and read up if it has no vulnerabilities on that part).

Given this, one could probably debate if then a port and address are worth keeping a secret to begin with? (credentials would be a different cake)

[CendioOssman]

People have already complained about numpy being a hard requirement, so I'm not keen on adding more. Especially requirements that will not be used by most users. So I'm afraid this will have to be an optional thing.

[ekohl]

Given this, one could probably debate if then a port and address are worth keeping a secret to begin with? (credentials would be a different cake)

Hostnames/addresses are internal details that you might not want to leak.

[UXabre]

@ekohl , given the interest of other parties as well, I'll see to include a JWE compatible library instead to support these cases

@CendioOssman, okay, what would the best way of making this optional, or do I just add it to the test-requirements.txt document? :-)

Will try and adapt then tonight :-)

[CendioOssman]

@CendioOssman, okay, what would the best way of making this optional, or do I just add it to the test-requirements.txt document? :-)

I don't think we have anything better than that, no. Possibly you could catch import errors and print something more useful.

[UXabre]

Hi Everyone! So, I've managed to add support for JWE in this PR; so all can be signed and can be additionally encrypted. I've adapted the tests to reflect this. Important note here, is that the signing key and encryption key are the same; as AFAIK it's not possible to pass more parameters to the token plugin?

I've also changed to a different library, instead of pyJWT I've switched to jwcrypto which seems to lack some elegant interface but, in the end I managed to get it to work.

Let me know what you guys think!

[CendioOssman]

The tests are failing, but otherwise this looks good to me. So get those working and then we can merge this.

[UXabre]

The tests are failing, but otherwise this looks good to me. So get those working and then we can merge this.

I noticed the tests are failing (at least for python > 2.7) because setuptools seems to be outdated? To mee, this looks more like a build infrastructure issue; no? Is this something that we could update? Or, is there a way I could enforce the upgrade via this commit?

Seems 2.6 is a different can of worms, will need to investigate what's going on there... perhaps I skip support for this feature on python <2.7?

[CendioOssman]

I noticed the tests are failing (at least for python > 2.7) because setuptools seems to be outdated? To mee, this looks more like a build infrastructure issue; no? Is this something that we could update? Or, is there a way I could enforce the upgrade via this commit?

Maybe. The setup is in the .travis.yml file. You could try playing around with updating things there.

Seems 2.6 is a different can of worms, will need to investigate what's going on there... perhaps I skip support for this feature on python <2.7?

That is fine by me.

[UXabre]

Allright, another day, another update!
Thanks to the excellent input from @CendioOssman, I've managed to make the build work, but, as expected, I had to make some exceptions for the "ancient" python versions and thus totally skipped support for 2.6.

As usual, I'd like to thank all reviewers so far! Hope this is the one!

(not sure if we should still support Python2 at all actually because it'll soon be "retired": https://pythonclock.org/ ; but that's totally outside of the scope of this PR :p)

[CendioOssman]

Awesome. Thanks for all your work here.