assert valid JWK #2078
assert valid JWK #2078
Conversation
|
manually testing: <?php
ini_set("display_errors", 0);
require "./vendor/autoload.php";
\phpseclib3\Crypt\PublicKeyLoader::load('12345');
<?php
ini_set("display_errors", 0);
require "./vendor/autoload.php";
\phpseclib3\Crypt\PublicKeyLoader::load('{"foo": "bar"}');before: $ (php ./test-jwk-int.php; echo ""; php ./test-jwk-json.php) 2>&1 | sed -E "s|$PWD|...|g"
PHP Warning: Attempt to read property "keys" on int in .../vendor/phpseclib/phpseclib/phpseclib/Crypt/Common/Formats/Keys/JWK.php on line 53
PHP Fatal error: Uncaught TypeError: count(): Argument #1 ($value) must be of type Countable|array, null given in .../vendor/phpseclib/phpseclib/phpseclib/Crypt/Common/Formats/Keys/JWK.php:53
Stack trace:
#0 .../vendor/phpseclib/phpseclib/phpseclib/Crypt/Common/Formats/Keys/JWK.php(53): count(NULL)
#1 .../vendor/phpseclib/phpseclib/phpseclib/Crypt/EC/Formats/Keys/JWK.php(46): phpseclib3\Crypt\Common\Formats\Keys\JWK::load(12345, false)
#2 .../vendor/phpseclib/phpseclib/phpseclib/Crypt/Common/AsymmetricKey.php(150): phpseclib3\Crypt\EC\Formats\Keys\JWK::load('12345', false)
#3 .../vendor/phpseclib/phpseclib/phpseclib/Crypt/PublicKeyLoader.php(39): phpseclib3\Crypt\Common\AsymmetricKey::load('12345', false)
#4 .../test-jwk-int.php(4): phpseclib3\Crypt\PublicKeyLoader::load('12345')
#5 {main}
thrown in .../vendor/phpseclib/phpseclib/phpseclib/Crypt/Common/Formats/Keys/JWK.php on line 53
PHP Warning: Undefined property: stdClass::$keys in .../vendor/phpseclib/phpseclib/phpseclib/Crypt/Common/Formats/Keys/JWK.php on line 53
PHP Fatal error: Uncaught TypeError: count(): Argument #1 ($value) must be of type Countable|array, null given in .../vendor/phpseclib/phpseclib/phpseclib/Crypt/Common/Formats/Keys/JWK.php:53
Stack trace:
#0 .../vendor/phpseclib/phpseclib/phpseclib/Crypt/Common/Formats/Keys/JWK.php(53): count(NULL)
#1 .../vendor/phpseclib/phpseclib/phpseclib/Crypt/EC/Formats/Keys/JWK.php(46): phpseclib3\Crypt\Common\Formats\Keys\JWK::load(Object(stdClass), false)
#2 .../vendor/phpseclib/phpseclib/phpseclib/Crypt/Common/AsymmetricKey.php(150): phpseclib3\Crypt\EC\Formats\Keys\JWK::load('{"foo": "bar"}', false)
#3 .../vendor/phpseclib/phpseclib/phpseclib/Crypt/PublicKeyLoader.php(39): phpseclib3\Crypt\Common\AsymmetricKey::load('{"foo": "bar"}', false)
#4 .../test-jwk-json.php(4): phpseclib3\Crypt\PublicKeyLoader::load('{"foo": "bar"}')
#5 {main}
thrown in .../vendor/phpseclib/phpseclib/phpseclib/Crypt/Common/Formats/Keys/JWK.php on line 53after: $ (php ./test-jwk-int.php; echo ""; php ./test-jwk-json.php) 2>&1 | sed -E "s|$PWD|...|g"
PHP Fatal error: Uncaught phpseclib3\Exception\NoKeyLoadedException: Unable to read key in .../vendor/phpseclib/phpseclib/phpseclib/Crypt/PublicKeyLoader.php:63
Stack trace:
#0 .../test-jwk-int.php(4): phpseclib3\Crypt\PublicKeyLoader::load('12345')
#1 {main}
thrown in .../vendor/phpseclib/phpseclib/phpseclib/Crypt/PublicKeyLoader.php on line 63
PHP Fatal error: Uncaught phpseclib3\Exception\NoKeyLoadedException: Unable to read key in .../vendor/phpseclib/phpseclib/phpseclib/Crypt/PublicKeyLoader.php:63
Stack trace:
#0 .../test-jwk-json.php(4): phpseclib3\Crypt\PublicKeyLoader::load(false)
#1 {main}
thrown in .../vendor/phpseclib/phpseclib/phpseclib/Crypt/PublicKeyLoader.php on line 63 |
|
There's no reason this can't be in the 3.0 branch. To that end I thought I'd squash all your commits into one and then cherry-pick that commit into the 3.0 branch. Doing that required a few merge conflicts be resolved and when I did that here's what I got: https://github.com/terrafrost/phpseclib/tree/bad-jwk2 The problem is that the unit tests are now failing: https://github.com/terrafrost/phpseclib/actions/runs/14579735654/job/40893600946
This doesn't make any sense to me. When I run the code outside of a unit test an exception is thrown as I'd expect: <?php
include('vendor/autoload.php');
use phpseclib3\Crypt\PublicKeyLoader;
$key = 'a';
PublicKeyLoader::load($key);At this point I'm half tempted to just remove the stupid test. I just wish I knew why it wasn't working on Github Actions... |
|
I figured out the issue. The test vectors for Ed448 in https://tools.ietf.org/html/rfc8032#section-7.4 don't use keys that conform to any of the formats phpseclib supports so, for the unit tests, specifically, I have custom key loaders. This also serves to test custom key format support. The problem is that these unit test only key loaders were kinda treating the malformed keys added by your unit test as valid keys. To fix that I clamped down on the conditions under which these unit test only key loaders run: Maybe the ability to unregister already loaded key formats might be useful idk. Another issue: all the unit tests that use data providers (as your test does) are failing with |
|
I ran into this "too few arguments" problem when I was setting up PHPUnit for the 1st time because what I read online was to use |
|
Seems like a reasonable assumption. phpseclib v3 is, in some ways, harder
to develop for than the master branch because it supports so many more PHP
versions than the master branch does, but targeting the master branch with
a feature that can provide immediate benefit means that it’ll prob be years
before that feature ever sees the light of day because the big feature that
v4 will introduce is nowhere near being completed.
Anyway, I’ll take a look this evening - gotta head into the office now!
…On Tue, Apr 22, 2025 at 8:51 AM simonLeary42 ***@***.***> wrote:
I ran into this "too few arguments" problem when I was setting up PHPUnit
for the 1st time because what I read online was to use @dataProvider in a
docBlock, but in newer versions that doesn't work, instead you have to use
#[DataProvider("providerFuncName")]. Could that be it?
—
Reply to this email directly, view it on GitHub
<#2078 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABULSVLMEGUVKJEPVCGZ7L22ZCM3AVCNFSM6AAAAAB3O3CMMKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQMRRGQYDGNZXGA>
.
You are receiving this because you commented.Message ID:
***@***.***>
*simonLeary42* left a comment (phpseclib/phpseclib#2078)
<#2078 (comment)>
I ran into this "too few arguments" problem when I was setting up PHPUnit
for the 1st time because what I read online was to use @dataProvider in a
docBlock, but in newer versions that doesn't work, instead you have to use
#[DataProvider("providerFuncName")]. Could that be it?
—
Reply to this email directly, view it on GitHub
<#2078 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABULSVLMEGUVKJEPVCGZ7L22ZCM3AVCNFSM6AAAAAB3O3CMMKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQMRRGQYDGNZXGA>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
|
I've fixed the unit test issue in the 3.0 branch and have merged the cherry-picked squashed commit into master. You were right with the attributes. phpseclib 3.0 uses what Sebastian Bergmann calls the death star version constraint whereas the master branch doesn't (the master branch uses brianiumparatest ^6.6, which in turn, uses a specific version of phpunit). I can't comment on the virtues of that vs using phpunit directly but as far as using the death star version constraint is concerned... phpseclib 3.0 would have to deal with PHPUnit's changes sooner or later, esp given how long phpseclib versions are supported. Like maybe PHPUnit 9 or 10 currently work with PHP 8.4 (for example) but will they work with PHP 9.0? idk but it's my intention that phpseclib 3.0 work with PHP 9.0 so I need to be prepared for the possibility that they won't and if I wait to fix issues when I'm trying to get the newest version of PHPUnit to run because the version I had been using doesn't work at all on it then there are going to be a lot more changes I'd have to deal with whereas if I deal with them now it's less changes I need to deal with. |
fixes #2076, #2077
JWK::loadHelperdoes not do the necessary assertions on the result ofjson_decode. It assumes that if the key is valid JSON, then it must become an object and it must have thekeysproperty. Because of these bad assumptions, there is an uncaught TypeError. AsymmetricKey catches any exceptions and throws NoKeyLoadedException, but errors are not exceptions.JWK::loadHelper@throwsto docblock forPublicKeyLoader::load.