PHP-Open-Source-Saver / jwt-auth

🔐 JSON Web Token Authentication for Laravel & Lumen
MIT License
729 stars 113 forks source link

An issue when updating to "lcobucci/jwt"@4.2.0 #177

Closed kevinpurwito closed 2 years ago

kevinpurwito commented 2 years ago

Subject of the issue

There is an issue running an app after updating to "lcobucci/jwt"@4.2.0 It is working fine with "lcobucci/jwt"@4.1.5

Your environment:

Q A
Bug? yes
New Feature? no
Framework Laravel
Framework version 9.25.1
Package version 1.4.2
PHP version 8.1.x

Steps to reproduce

Expected behaviour

No errors

Actual behaviour

An error showed:

Lcobucci\JWT\Signer\InvalidKeyProvided 

  Key cannot be empty

  at vendor/lcobucci/jwt/src/Signer/InvalidKeyProvided.php:34
     30▕     }
     31▕ 
     32▕     public static function cannotBeEmpty(): self
     33▕     {
  ➜  34▕         return new self('Key cannot be empty');
     35▕     }
aprokopenko commented 2 years ago

I created the issue on lcobucci/jwt too, they answered, that this package has to be updated:

https://github.com/lcobucci/jwt/issues/877

I suggest setting fixed dependencies in composer, because there are often problems with lcobucci/jwt packages

kevinpurwito commented 2 years ago

Hi, yes that's what I did. For now I set the dependency to lcobucci/jwt@4.1.5

Thanks for creating issue there too. Hope it could be resolved smoothly.

lcobucci commented 2 years ago

Hey there 👋

I'm sorry this package got affected by the behavioural BC-breaks we introduce to enforce the recommended security levels.

I really hoped that people would follow the RFCs despite my implementation or catch this on the development branch, avoiding all the mess. I've got a longer response here to explain reasons but I'm happy to clarify things further and help out here.

I suggest setting fixed dependencies in composer

If you want, you may create a patch release of this package to declare incompatibility with >= v4.2 until issues are resolved. It's definitely not pretty but it might contain issue reporting on this lib.

because there are often problems with lcobucci/jwt packages

I'm terribly sorry that you feel this way, I was under the impression that this was the first issue we caused (at least I think it is the first reported issue on my repo).

Nevertheless, it would be great to work together to prevent issues in the future. Well, at least to be informed on the rare occasion of BC-breaks.


I see that php artisan jwt:secret does set a 64 bytes long key, which should be more than enough for HS256. @kevinpurwito are you sure that the .env file is being loaded correctly? Do you have the full stacktrace?

lcobucci commented 2 years ago

@kevinpurwito not trying to scare you but already scaring you... if your system is working fine with lcobucci/jwt@4.1.5 and throwing that particular error on lcobucci/jwt@4.2.0, it's possible that you have an empty key being used to sign and verify tokens.

That means that anyone can forge tokens that would pass the basic validation, which is kind of a big security vulnerability of your app (and you were completely clueless about it until v4.2.x alerted you).

Here's a token issued with an empty key: https://jwt.io/#debugger-io?token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxIiwiaWF0IjoxNTE2MjM5MDIyfQ.4F0nNGzczOw0hjzIOpNVxiy78KWxLVCuo4Q4PSTi9kQ (if you clear the key field, you'll see the signature verification passing). I don't know the internals of this package buuuut if only uses sub to fetch the user, that might suffice to forge a request using the user ID 1.

To confirm, you may also put any token issued by your system into https://jwt.io and clear out the key field to see if the signature is verified.

Please check your environment ASAP to make sure you're not vulnerable and take immediate actions if you are 😄

kevinpurwito commented 2 years ago

Hi @lcobucci

It's happening in my CI environment so I don't think my production environment is in trouble yet! (You are scaring me though haha)

I'll try to dig into the CI information a bit more to give more context.

kevinpurwito commented 2 years ago

Ok, I found the issue. So the problem is because I set

JWT_SECRET=

in the CI .env, and then running the

php artisan jwt:secret --force

command.

I assumed that the JWT_SECRET will be updated in the .env, from being empty, to a random 64 chars.

But in reality it's not.

I fixed the issue by adding the 64 random chars to JWT_SECRET entry to my CI .env, so that I won't need to use the php artisan jwt:secret command in the CI step.

So the problem perhaps lies with the artisan command or my own wrong assumption, not the JWT package.

kevinpurwito commented 2 years ago

Thank you for your help & clarification @lcobucci

Keep doing your great work. Cheers!

Closing this issue now

lcobucci commented 2 years ago

Phew, good outcome then @kevinpurwito! I'm relieved to know it was isolated to the CI environment 👍

aprokopenko commented 2 years ago

Hey, I still think there is a problem.

The command php artisan jwt:secret do not replace .env. It has a bug!

The code is:

 file_put_contents(
                    $filepath,
                    str_replace(
                        "/{$key}=.*/",
                        "{$key}={$value}",
                        file_get_contents($filepath)
                    )
                );

as you can see, the search string is a regexp, but str_replace doesn't allow regexps as arguments, I updated this line to preg_replace - and it works now!