Skip to content

Commit 6201b37

Browse files
authored
improve ssh key validation (#187)
* improve ssh key validation * catch all throwables * trim should be done externally * phpunit expect warning * put back checks to please phpunit * add comments, try out removing digits check * remove digits check * remove use
1 parent 94f71aa commit 6201b37

File tree

4 files changed

+53
-11
lines changed

4 files changed

+53
-11
lines changed

resources/lib/UnitySite.php

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,24 @@ public static function getGithubKeys($username)
5252

5353
public static function testValidSSHKey($key_str)
5454
{
55+
// key loader still throws, these just mute warnings for phpunit
56+
// https://github.com/phpseclib/phpseclib/issues/2079
57+
if ($key_str == "") {
58+
return false;
59+
}
60+
// https://github.com/phpseclib/phpseclib/issues/2076
61+
// https://github.com/phpseclib/phpseclib/issues/2077
62+
// there are actually valid JSON keys (JWK), but I don't think anybody uses it
63+
if (!is_null(@json_decode($key_str))) {
64+
return false;
65+
}
5566
try {
5667
PublicKeyLoader::load($key_str);
5768
return true;
58-
} catch (\Exception $e) {
69+
// phpseclib should throw only NoKeyLoadedException but that is not the case
70+
// https://github.com/phpseclib/phpseclib/pull/2078
71+
// } catch (\phpseclib3\Exception\NoKeyLoadedException $e) {
72+
} catch (\Throwable $e) {
5973
return false;
6074
}
6175
}

test/unit/AjaxSshValidateTest.php

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
<?php
2+
3+
namespace UnityWebPortal\lib;
4+
5+
use PHPUnit\Framework\TestCase;
6+
use PHPUnit\Framework\Attributes\DataProvider;
7+
8+
class AjaxSshValidateTest extends TestCase
9+
{
10+
public static function providerTestSshValidate()
11+
{
12+
// sanity check only, see UnitySiteTest for more comprehensive test cases
13+
return [
14+
[false, "foobar"],
15+
// phpcs:disable
16+
[true, "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIB+XqO25MUB9x/pS04I3JQ7rMGboWyGXh0GUzkOrTi7a"],
17+
// phpcs:enable
18+
];
19+
}
20+
21+
#[DataProvider("providerTestSshValidate")]
22+
public function testSshValidate(bool $is_valid, string $pubkey)
23+
{
24+
$_SERVER["REQUEST_METHOD"] = "POST";
25+
$_POST["key"] = $pubkey;
26+
ob_start();
27+
include __DIR__ . "/../../webroot/js/ajax/ssh_validate.php";
28+
$output = ob_get_clean();
29+
if ($is_valid) {
30+
$this->assertEquals("true", $output);
31+
} else {
32+
$this->assertEquals("false", $output);
33+
}
34+
}
35+
}

test/unit/UnitySiteTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ public static function SSHKeyProvider()
1515
[false, "1"],
1616
[false, '{"key": "value"}'],
1717
[true, "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIB+XqO25MUB9x/pS04I3JQ7rMGboWyGXh0GUzkOrTi7a"],
18-
[true, " ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIB+XqO25MUB9x/pS04I3JQ7rMGboWyGXh0GUzkOrTi7a "],
18+
[false, " ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIB+XqO25MUB9x/pS04I3JQ7rMGboWyGXh0GUzkOrTi7a "],
1919
//phpcs:disable
2020
[true, "ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBJNqo8NKTfXgCsaE3ly0tDCfwFuFgJiftup0bIZnRi5bP5QgDN5BFeJfEUPSY/s/GL2hUAjkz3ytGqvadt84W7w="],
2121
[true, "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAIAQDMHfRgu2HjTAODg+1yAXeZalNrT3S0sXv7fqC9/uJW86AHU6l384TpSEoqVbl4cke8lev49ljsEg50ZppoC4fiP6+nAeBy609VWfcHBmbVDeVdLZiAh2XpNW3Fns6ecM24OPr7kdxuhV8pKTMupXYc/mEUdKTB7DiQcRWcLp8BhX14K3PuFbiprqnacoeiu1In9SLKZd2E4vg2TrhptdZuuav4WX0r2s2uUwAz+7jpXWYoXUUjfmImEg6h9ETCzKGFwHYATn879WW+28RUOIurfUUU5njnGmtVoWG1s0L7JpoJfu16ePdcPCEpn92coP8DpFw10iQwh4AsjIdEVYEYfjtxbdk5TqACkfgKo+h9G3nNXO6x8mGjhfNa6CHF+wVJ4RrJWdhBGfKog+CtD+NHDYXFbyciGT2CtGTFay182DdUg2MoXn4eSmPEqZJ+kHtJ0mwe0nKtLNwik4c/54X1rZLETauEFMKRE3/JSlAdAMm0jZNW7fadQOJHB63q/yxyCFLztLrtzhZlBH0DoGuGdHxznbKYDctcQypztP2aG4G+W1gozwNmwsJRLY3pWos377QRUOq0w3qvcw9BZwigfqixGWHbC2JUyxqoa4opez6zoDH+tjvJQ0iKv9DQrVOdh8e6NzHMlSnri3r34K5NzDIvV3uROO1yC7pbHogOS2XmO+nnJYlnINhitXe9+gcfkcEwZs14lxrxcAElJmJOSPE+uui80ZMUHd6ClemoptFE4cAhdczmkQXURbpzWguHRSWk+5/AXb5r1P7AYpaZSRFfOqy4oB/v6rKTBjplH7LYi3otykeB1PooHnUfpziOLLFq5ghVdCU8R10yE43drDRnu8dFpirxBF6AAUzQpUMbYae9BvVTPMWAyM2Wn5P9EUZ+hngvhDlyoZBoNCeqEeWN6l4KcQVPZwyg1b1PhkAtVzhss3mJQ0Xsqabp2cQvvj+Z/rfQyZKJlAiv2gKd0W+E5zTd0TqA50JZLKOtPhMsEXUqKop4H6OcJ+SDqqNWzGdnYJHYccQ7y/2IXrqBlW0gs6BX04Yx+5LusnLBKH8D2MB9kvASPKopzzcF2KFsIw0pLkEc0cVPoY5gwy05JTuKYoxzIbePgM8KV8rgQ1it442LHEAo5k/6GwVkl/6aQDCwmQV2YhBxfioyOZZLTCG95ANHaz19H7M+T4BY/d3lUD9FsFcPmY7Ikj6Ma0YMGmvgghdIvaXmCxEyIQRi+lpcjPVHV3MzELgNTRDDVkM0TFXlGSBv63XRjos0kbDNkOo2wLmTModCFuudLDGxOjYriMKdkXmU4Tc7wZSGgngZch49u9b3A6RVwxa20LUkuAXOS1EvyBqOcQ1m5RklzwPuK0FD+9qVNHPFSNpRsXbH/mljqlR8MYydDGphZW5vPmJ1RGhO8EOkQIk6bZz46Y8U4fVsvSslBX1TWczmAZ6RPA/rFg9RKAehmze5GJLa0ypVcD86ILJftBd7a+Rzx7G9liLR5HTMv+3k/cbYLiahTQ33thHK0jiB2DLa0D2tXmQEdEHR1lHlGBwLr5XK+fOFbYKAyhIt63aEL9hmdUBLDFQfjBLbVEToGonSM54diks9Nesy2wrVYe4bWCmj+TCut0cDXtgQSxiSJhoDkS2gYIEX6Rrc4ETMhqtfG7LgH2wHeVvJ+wjT/uNQ+8c9Eft4/NZfVpK1vru1A31ZGqlZKuBxnO4Cd6PiwzBk2YKMul9QpXJxKGH2X0wwqc9wYk4IoydzMaftnFI+q+ALNpU9BneMJ/FFhbP9NaHVBnQtzs9vNbsBPLEBTyZNfihuJtExRjDcTj/tRvrOHsKWC+OdtMJT1MemgHw+/zXG59BwNOfxsStEV79O+F68g5I4FeomW9fPZYw6d/xLWqPXsxigPPUcH/JXGQ3+p1L+ChOiFvhCTMciJ3+7gc1huLWWZqZtOTVxXWKMdia/ox31MqWxWiZcvHOJopum1RmR/OBwtaahSl27LmorLrr2QPRrtY+OVDBeeNQLNk9/aSzYcMWO66cju2C2Myvadfb6o8bTjw6rUJDSqlG+pvAhBhQvjxsrcPkT9BspFE+r/SPxWExKWL5djQYRit2druxBtw6Y+ylIg9CZJMTf1IBjveBUySF+gOonShB1nLmRZ9zX2hwJSXqYbGQCqzfwRWwPYSnQak0skh4J1p7OYFgZCuxkiSaEikpbuHJWAz1cpYsomXQ7l/m+F05JFfAm2LvNjVoeAblw+Tj7T/Fx7x63b1CU4Wy3L7Ho25i89fksFhsV/fBk9c3QqSkwpkiDVnvYeIhM349An75ncfBjToQT5o5Ayn0ritOUHh5NW+SS+955CFFM8ZQhxZaluWKcrvjwN/UCtBWjGTu7JpwOnHuTTj03ti2wcYsEKNpPB/Nm3kql+UbARmsq24j5foxjN6gmiKVtuho7SDMsu11UGgidKCiHAM3/o4Im53awfRJiqoouJNTisSuhxHp7b+4Z14+CUPfQcKkyYCSzrptrJhg2FO5vz1YZOuExIm3deDfcPsK5vg4LiLam6FJ3qqonXP6krCuH/crJYLTPBJn6eX3noL7TjCqiMWEtLmGj0431YbcrgG7Fy2a+VWwcB6w0nzyxbqg16AP+luuqHxfVsvP6Uyde4C7LPeB3r3GhAfuUNxnpz/bXGxbJu3+aCnbtaZMzGJ6UFBeJp8MtlmVajDnjx3oEuOGGmobTlaopHYVsQ3ySfQ=="],

webroot/js/ajax/ssh_validate.php

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,5 @@
11
<?php
22

3-
require "../../../resources/autoload.php";
3+
require_once __DIR__ . "/../../../resources/lib/UnitySite.php";
44

5-
use phpseclib3\Crypt\PublicKeyLoader;
6-
7-
try {
8-
PublicKeyLoader::load($_POST['key'], $password = false);
9-
echo "true";
10-
} catch (Exception $e) {
11-
echo "false";
12-
}
5+
echo UnityWebPortal\lib\UnitySite::testValidSSHKey($_POST["key"]) ? "true" : "false";

0 commit comments

Comments
 (0)