Skip to content

cast keyset generation console arguments to int and remove dangerous …#339

Merged
Spomky merged 1 commit into
web-token:v3.0from
lucasgranberg:v3.0
Apr 14, 2022
Merged

cast keyset generation console arguments to int and remove dangerous …#339
Spomky merged 1 commit into
web-token:v3.0from
lucasgranberg:v3.0

Conversation

@lucasgranberg
Copy link
Copy Markdown
Contributor

…defaults

Q A
Branch?
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #338
License MIT

Console commands for generating keysets uses is_int() which always fails. These changes uses casts instead and removes dangerous defaults for size. More information in the issue.

@Spomky
Copy link
Copy Markdown
Member

Spomky commented Apr 14, 2022

Hi @lucasgranberg,

Many thanks for your PR.
I am not sure to clearly understand the added value here. These sections of code were changed few time ago to prevent issues with int casting.

@lucasgranberg
Copy link
Copy Markdown
Contributor Author

The problem is that the value will always be a string so it will always generate just one key in the keyset with the keysize of 1. Is there something I am missing?

@lucasgranberg
Copy link
Copy Markdown
Contributor Author

This is what I get at least

bin/console keyset:generate:ec --alg ES256 --use sig --random_id 10 P-256
{"keys":[{"kid":"Xh-Lbf7cuNP2txj2AFyo0IdPunsjvNLgnOvC01y99Eo=","use":"sig","alg":"ES256","kty":"EC","crv":"P-256","d":"GR2E-5cXt5BMGkISSx_LMBGf3WE3XVBMx2ADvPcf2Yc","x":"34w0Nmihvy-PH4zfqxfkiSN5xHLMSmOzwu4Dclo111M","y":"b6qzij4PJzqWB-sMx2BLwkOwhdzIUljTE-jgBQ9aLJo"}]}

@lucasgranberg
Copy link
Copy Markdown
Contributor Author

with bin/console keyset:generate:rsa 3 512 I get "invalid keysize" as size is "1". so not that dangerous after all..
but still not working

@lucasgranberg
Copy link
Copy Markdown
Contributor Author

/var/www/symfony # ./jose.phar keyset:generate:oct 5 128

In JWKFactory.php line 77:
                     
  Invalid key size.  
                     

keyset:generate:oct [-u|--use [USE]] [-a|--alg [ALG]] [--random_id] [--] <quantity> <size>

I just confirmed the same bug is in the phar file also

@Spomky Spomky merged commit 633e88a into web-token:v3.0 Apr 14, 2022
@lucasgranberg
Copy link
Copy Markdown
Contributor Author

okay now I understand why int casting is a problem

GitHub Actions / PHP 8.1 Test on ubuntu-latest
src/Component/Console/EcKeysetGeneratorCommand.php#L31
Cannot cast mixed to int.

The getArgument should always return a string in this case but I guess I should have used https://www.php.net/manual/en/function.intval.php to get rid of the warning.

@Spomky
Copy link
Copy Markdown
Member

Spomky commented Apr 14, 2022

You are absolutely right!
I have just merged and added some tests to prevent regressions.
Have no worries about the errors reparted by PHPStan, these ones can be ignored.
The PHAR is also up to date: https://github.com/web-token/jwt-app/releases/tag/v3.0.4

@Spomky
Copy link
Copy Markdown
Member

Spomky commented Apr 14, 2022

And many thanks for your support! I really appreciate it 🥇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unable to generate more than one key in keyset with console command

2 participants