BTBurke / caddy-jwt

JWT middleware for the Caddy server
MIT License
114 stars 39 forks source link

Supporting RSA-SHA256 #3

Closed akramer closed 7 years ago

akramer commented 8 years ago

Wonderful middleware! It would be even better with public key algorithm support. I can give it a shot and send a PR in a week or two.

BTBurke commented 8 years ago

Oh you're right, I hard coded the algorithm based on my usage. Implementing RS256 is supported by the underlying library this is using for validation so it shouldn't be a tough change.

Is it correct to assume that Caddy would still have prior knowledge of the public key it's supposed to use to validate the token such that it can be stored in JWT_SECRET for all requests? If not, then it becomes slightly more complicated because it would have to support both discovering the public key and having a configuration parameter in the Caddyfile that would specify the signing algorithm, otherwise it would be vulnerable to the attack discovered last year by altering the "alg" header.

akramer commented 8 years ago

My assumption was that the key would be available via an environment variable like JWT_PUBLIC_KEY. You've already nicely broken out the validation function so it could switch which variable it uses based on which algorithm is passed to the function.

BTBurke commented 8 years ago

Shouldn't be too tricky. I think as long as we use two different environment variables and warn people in the docs not to put their public key in JWT_SECRET it should be ok. The problem is if they mistakenly put their public key in JWT_SECRET, an attacker can forge a token that would be accepted by only knowing the public key. They would use the public key to sign a new token and specify HS256 instead of RS256 in the token header. The JWT middleware currently would have no way of knowing that it's expecting only RS256 tokens and so would use the public key as the secret in HS256. That would be a serious vulnerability.

Another avenue is to modify the Caddyfile to force everyone to specify which algorithm they are using. This would make it more secure but also a bit harder to parse and for people to configure.

It could look something like this:

jwt alg hs256
jwt /path1

or get rid of the one line configs and have everything look like

jwt {
   alg hs256
   path /path1
   allow user someone
}
akramer commented 8 years ago

The cert needs to be in PEM format for the JWT library. It should be safe to verify that JWT_SECRET doesn't start with "-----BEGIN CERTIFICATE" and error out if it does.

Is that a gross hack? I can't tell, but it should keep the configs simple and still allow use of both hmac and rsa safely.

BTBurke commented 8 years ago

I'm still working on this. It's actually a bit harder than I thought it would be to fit it into the current auth flow.

coolaj86 commented 8 years ago

Shouldn't there be a way to specify the public key used for validation?

Or maybe there is and I'm just missing it. I would expect it to be a key in the jwt object in the Caddyfile

BTBurke commented 8 years ago

There will be once I finish the RSA support. Right now the current middleware supports HS256/384/512.

The RSA version will use JWT_PUBLIC_KEY for validation when it encounters an RSA-SHA encrypted token.

It's not ready for prime time yet but I'm going to work on it this weekend.

On Apr 20, 2016, at 9:04 AM, AJ ONeal notifications@github.com wrote:

Shouldn't there be a way to specify the public key used for validation?

— You are receiving this because you commented. Reply to this email directly or view it on GitHub

namsral commented 8 years ago

How about using the kid header parameter documented in RFC 7515 to look up the key and its corresponding algorithm to support multiple algorithms and solve the signature verification method attack.

Caddyfile example:

jwt {
    path [path]

    # key [id] [type] [env*]
    key a oct KEY_A
    key b oct KEY_B
    key c rsa KEY_C
    key "d e" ec KEY_DE
}

*environment variable containing passphrase or pem encoded public key

Related JOSE header which matches the first key parameter in the Caddyfile:

{
    "kid": "a",
    "alg": "hs256"
}
BTBurke commented 8 years ago

I think it becomes a little complicated to handle so many possible key combinations in the Caddy file. The way I was going about it now was to just declare two possible ENV states, you're either using the JWT_SECRET or JWT_PUBLIC_KEY and it returns 401 if either both are set or the public key doesn't look like PEM format. By determining the configuration based on the environment variables you're only in RSA mode in the event that you have one public key set and no conflicting information. From then on it will assert that the "alg" header is always RS family to prevent the attack.

I think once you get to the point where you need to be able to rotate keys, use multiple algorithms, invalidate keys, etc you're beyond what Caddy was designed for and in the custom middleware realm in your downstream app.

Using ENV at least lets you change keys from the outside should you need to immediately invalidate the public key without reloading Caddy.

This isn't a huge change, I just haven't had time or inclination to work on it lately since I'm not using the RS algorithms.

namsral commented 8 years ago

I concur, supporting more complex JWT configurations wouldn't fit the Caddy mindset.

lsjostro commented 7 years ago

PR #15 adds RSA support.