Disallow None algorithm by default

[p-v-d-Veeken]

At my company we just discovered that the financial services application (NestJS backend) we are building has a critical security vulnerability because our JWT passport strategy accepted claims signed with the None algorithm. In the first place this is our fault of course, because we should've been more thorough with the implementation, also we weren't live for customers yet so really no harm done.

That being said, I think it's a bad design choice that using passport-jwt with the defaults unchanged results in an extremely unsafe authentication method. We were fortunate enough to find this vulnerability before any real harm was done, but I suspect there are a lot companies/projects/applications out there that unknowingly have this landmine in their codebase.

My suggestion is to disable the None algorithm (and any other unsafe algorithms) by default and optionally allow users to explicitly enable it.

Thoughts?

[mikenicholson]

How would you propose explicitly disabling the "None" algorithm? jsonwebtoken jumps through some hoops to guess the correct algorithm based on the provided secret, whether the token is signed etc. See here. Based on this, simply passing a list of all of the "blessed" algorithms probably won't work and I'd rather not duplicate all of that logic.

jsonwebtoken doesn't appear to have an option to explicitly disable a single algorithm, only selectively enable them.

The best approach seems like it might be to force users of passport-jwt to set algorithms to a list or explicitly set another frighteningly named option like INSECUREallowAllAlgorithms: true to get the default guessing behavior of the jsonwebtoken library with no algorithms specified.

[pixtron]

Just out of curiosity i tried to reproduce the issue of accepting unsinged tokens with the default configuration. But could not reproduce it with the default configuration. @p-v-d-Veeken maybe i'm overseeing something and you could provide a small example server and a token, how to reproduce your issue?

Findings

• if neither secretOrKey nor secretOrKeyProvider are provided or if secretOrKey is set to a non truthy value "TypeError: JwtStrategy requires a secret or key" is thrown by passport-jwt
• as soon as a truthy value is set for secretOrKey (either by secretOrKey itself or via secretOrKeyProvider), jsonwebtoken module requires the token to have a signature.
• only if secretOrKeyProvider is set to a function that sets a non truthy value for secretOrKey (eg (req, token, cb) => cb()), I could get unsigned tokens to be accepted as valid (see example below). This only works because the jsonwebtoken module falls back to none algorithm if: "no secret or key is provided" && "the token has no signature" && "options.algorithms is not set"

Conclusion

1.) To me it seems that passport-jwt already does a great job by requiring a secret or a key (and therefore disabling none algorithm by default). It seems the consumer actively needs to misconfigure the strategy in order to allow unsigned tokens. Might be that nestjs under some circumstances does force that misconfiguration? 2.) It's questionable if automatically falling back to none algorithm in jsonwebtoken is a great choice. But that would be an issue for jsonwebtoken module.

Possible improvements for passport-jwt

1.) Adding a check for non truthy secretOrKey returned by secretOrKeyProvider and throwing an error if secretOrKey is not set, might be an improvement (eventually a breaking change?). 2.) I think @mikenicholson's proposal makeing algorithms a required option and throw when it is not set, seems to be a great – tho breaking – change to me. 3.) Akward configs like INSECUREallowAllAlgorithms: true seem to be a really bad choice. none should never be allowed in a auth environment anyway.

Example server

This is the server i used to test, where an unsigned token will be accepted:

const express = require('express');
const passport = require('passport');
const app = express();

const { ExtractJwt, Strategy: JwtStrategy} = require('passport-jwt');
const opts = {
    jwtFromRequest: ExtractJwt.fromAuthHeaderAsBearerToken(),
    secretOrKeyProvider: (req, token, cb) => cb()
}

passport.use('jwt', new JwtStrategy(opts, (jwt_payload, done) => {
    console.log('Got a valid jwt payload', jwt_payload);
    done(null, jwt_payload);
}));

app.get('/', 
    passport.authenticate('jwt', {session: false}),
    function(req, res){
      res.send('welcome in');
    }
);

app.listen(4000);

Tested with this request curl http://localhost:4000/ -H "Authorization: Bearer eyJhbGciOiJub25lIiwidHlwIjoiSldUIn0.eyJmb28iOiJiYXIiLCJpYXQiOjE2NDY0OTAzMzN9."

Used modules

"dependencies": {
  "express": "4.17.3",
  "passport": "0.5.2",
  "passport-jwt": "4.0.0"
}

[Outternet]

thanks for your analysis this has helped me, the simplest migration seems to me a standard algorithm (other than none)

[pixtron]

only if secretOrKeyProvider is set to a function that sets a non truthy value for secretOrKey (eg (req, token, cb) => cb()), I could get unsigned tokens to be accepted as valid

passport-jwt@4.0.1 fixed that issue with the update to jsonwebtoken@9.0.0. Looks like this issue can be closed.

[rjmccluskey]

only if secretOrKeyProvider is set to a function that sets a non truthy value for secretOrKey (eg (req, token, cb) => cb()), I >>could get unsigned tokens to be accepted as valid

passport-jwt@4.0.1 fixed that issue with the update to jsonwebtoken@9.0.0. Looks like this issue can be closed.

Does this mean that a secret is now always required? I have a use case to use a JWT that is unsigned (None algorithm). Is there any way to accept an unsigned JWT?

[pixtron]

Does this mean that a secret is now always required? I have a use case to use a JWT that is unsigned (None algorithm). Is there any way to accept an unsigned JWT?

jsonwebtoken@9.0.0 and with that passport-jwt do still allow unsigned tokens (alg: none) if they are explicitly enabled with algorithms: ['none']. But you should reconsider your requirements. If you accept unsigned tokens, you basically do not have any authorization, as everyone can create valid unsigned tokens.