5
\$\begingroup\$

In my app I'm using JSON Web Tokens for authentication purposes, using the pyjwt library. Instead of using static keys and/or worrying about key distribution, the server generates a public/private key pair upon startup itself and just keeps it in memory. The keys will be reset and thereby all existing tokens invalidated when the server restarts, which is fine for the intended use case. The key pair is generated using pycrypto.

The question is: am I generating and using the keys correctly, or is there any flaw which can lead to an exploit? Below is the general class which generates and validates all JWTs in my app:

from typing import NamedTuple
from datetime import datetime
import jwt
from Crypto.PublicKey import RSA

KeyPair = NamedTuple('KeyPair', [('public', str), ('private', str)])

class JWT:
    def __init__(self, keypair: KeyPair=None, algorithm: str='RS256'):
        self.keypair = keypair or self.generate_keypair()
        self.algorithm = algorithm

    @staticmethod
    def generate_keypair() -> KeyPair:
        key = RSA.generate(2048)
        return KeyPair(public=key.publickey().exportKey('PEM').decode('ascii'),
                       private=key.exportKey('PEM').decode('ascii'))

    def generate_token(self, payload: dict) -> str:
        payload.update(dict(iat=datetime.utcnow()))
        return jwt.encode(payload, key=self.keypair.private, algorithm=self.algorithm).decode('ascii')

    def decode_payload(self, token: str, **kwargs) -> dict:
        try:
            return jwt.decode(token, self.keypair.public, algorithms=[self.algorithm], **kwargs)
        except jwt.exceptions.InvalidTokenError as e:
            raise InvalidAuthenticationToken
\$\endgroup\$
  • 2
    \$\begingroup\$ This sounds like a really great idea until you consider load balancing. Such a setup like this would require clients to have session affinity otherwise all of their tokens would be invalid. This removes one of the great benefits of JWT: Statelessness. \$\endgroup\$ – Dan Pantry Aug 5 '16 at 8:33
  • \$\begingroup\$ Valid point, but that's not a concern at the moment. The service is structured as a bunch of micro services on a message broker, which means load balancing can be achieved by distributing the services among machines if necessary. Due to the nature of the service there's a strong need for a central state manager anyway, so some parts of it cannot be distributed across nodes anyway at the moment. If absolutely necessary this micro service can be put on its own super beefy machine away from others to achieve something like load balancing/reducing individual server load. \$\endgroup\$ – deceze Aug 5 '16 at 8:37
  • \$\begingroup\$ The main goal of using JWTs is to keep other services stateless and reduce space complexity. If load balancing ever becomes an issue, I can think about distributing the keys among a bunch of JWT instances. \$\endgroup\$ – deceze Aug 5 '16 at 8:39
  • 1
    \$\begingroup\$ That, again, sounds really dangerous... say you want to take one server down for maintenance, when you bring it back up, without significant effort, it will have a different key to the old one, to the point where you might as well just use the filesystem and find some other mechanism for invalidating tokens IMO \$\endgroup\$ – Dan Pantry Aug 5 '16 at 8:53
  • 1
    \$\begingroup\$ Since the instances are talking over a central message broker (pub/sub/RPC) (they have to, it's in the nature of the service), it would actually be fairly easy to distribute the key: make an RPC request to see if there are other instances online which already have a key, otherwise generate one and make it available to others. – Again, that part of it is not a concern at the moment, the service is way, way, waaaay in the green in terms of resource consumption. But your point is well taken and considered. \$\endgroup\$ – deceze Aug 5 '16 at 8:58
3
\$\begingroup\$

There isn't too much to see here because the key generation simply relies on RSA.generate(2048), but I wonder why you would need this code as it is exceedingly shallow.

Regenerating key pairs for signing at startup is utter nonsense because a key pair is next to useless if the public key isn't trusted by the receiving party.

Exporting the private key as unprotected PEM is also very dangerous. The method would fail if hardware support (a HSM) is ever used.

The inclusion of payload.update(dict(iat=datetime.utcnow())) is undocumented. Although it may be expected for webtokens, a caller will want to know that this is performed. Undocumented side effects are a sure fire way of breaking the principle of least surprise.

Passing kwargs** without any validation doesn't give me a very warm feeling inside.

Slightly less of an issue maybe, but an RSA 2048 bit key provides a key strength of only 112 bits or so. You may want to consider at least 3072 bits.

This code is clearly related only to webtokens and RSA keys, even with a specific key size. So allowing an argument algorithm: str='RS256' may just allow a user to get the object instance in an invalid state, and not much more.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Can you elaborate on the unprotected PEM HSM problem? \$\endgroup\$ – deceze Mar 1 at 13:06
  • \$\begingroup\$ Well, basically you try and protect the private key to the best of your capabilities. Outputting a PEM string that is likely also exported (why would you otherwise) means that it is written to memory and to disk. A HSM or smart card can be used to protect a key value so that you can only use it on the device (a "token") itself, and commonly only after authentication. In that case PEM export (of the private key) would of course fail. \$\endgroup\$ – Maarten Bodewes Mar 1 at 15:37
  • \$\begingroup\$ In my use case the key isn’t actually exported at all; I guess the major flaw in this use is using asymmetric encryption when symmetric encryption would do just fine. \$\endgroup\$ – deceze Mar 1 at 15:57
  • \$\begingroup\$ That's OK though, better than the other way around :) \$\endgroup\$ – Maarten Bodewes Mar 1 at 16:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.