From 3b42f93654babfd1919726663945a249a6e2bdf4 Mon Sep 17 00:00:00 2001 From: Austin Kregel <5355937+austinkregel@users.noreply.github.com> Date: Wed, 5 Jun 2024 03:38:31 -0400 Subject: [PATCH] Update ssh key validation to accept other common standards (#228) --- app/ValidationRules/SshKeyRule.php | 30 +++------- tests/Feature/ServerKeysTest.php | 93 ++++++++++++++++++++++++++++++ tests/Feature/SshKeysTest.php | 93 ++++++++++++++++++++++++++++++ tests/TestCase.php | 4 ++ 4 files changed, 198 insertions(+), 22 deletions(-) diff --git a/app/ValidationRules/SshKeyRule.php b/app/ValidationRules/SshKeyRule.php index f84a717..6f74dfc 100755 --- a/app/ValidationRules/SshKeyRule.php +++ b/app/ValidationRules/SshKeyRule.php @@ -3,6 +3,8 @@ namespace App\ValidationRules; use Illuminate\Contracts\Validation\Rule; +use phpseclib3\Crypt\PublicKeyLoader; +use phpseclib3\Exception\NoKeyLoadedException; class SshKeyRule implements Rule { @@ -15,29 +17,13 @@ class SshKeyRule implements Rule */ public function passes($attribute, $value) { - $key_parts = explode(' ', $value, 3); - if (count($key_parts) < 2) { - return false; - } - if (count($key_parts) > 3) { - return false; - } - $algorithm = $key_parts[0]; - $key = $key_parts[1]; - if (! in_array($algorithm, ['ssh-rsa', 'ssh-dss'])) { - return false; - } - $key_base64_decoded = base64_decode($key, true); - if ($key_base64_decoded == false) { - return false; - } - $check = base64_decode(substr($key, 0, 16)); - $check = preg_replace("/[^\w\-]/", '', $check); - if ((string) $check !== (string) $algorithm) { - return false; - } + try { + PublicKeyLoader::load($value); - return true; + return true; + } catch (NoKeyLoadedException $e) { + return false; + } } /** diff --git a/tests/Feature/ServerKeysTest.php b/tests/Feature/ServerKeysTest.php index e4f6c86..49c0a0f 100644 --- a/tests/Feature/ServerKeysTest.php +++ b/tests/Feature/ServerKeysTest.php @@ -93,4 +93,97 @@ public function test_add_existing_key() 'status' => SshKeyStatus::ADDED, ]); } + + /** + * @dataProvider ssh_key_data_provider + */ + public function test_create_ssh_key_handles_invalid_or_partial_keys(array $postBody, bool $expectedToSucceed): void + { + $this->actingAs($this->user); + + // Some existing amount of seed data to make the test more realistic + SshKey::factory()->create([ + 'user_id' => $this->user->id, + 'name' => 'My first key', + 'public_key' => 'public-key-content', + ]); + + $response = $this->post(route('servers.ssh-keys.store', $this->server), $postBody); + + if ($expectedToSucceed) { + $response->assertSessionDoesntHaveErrors(); + } else { + $response->assertSessionHasErrors('public_key', 'Invalid key'); + } + } + + public static function ssh_key_data_provider(): array + { + return [ + [ + [ + 'name' => 'My first key', + // Key Already exists + 'public_key' => 'public-key-content', + ], + self::EXPECT_FAILURE, + ], + [ + [ + 'name' => 'My first key', + // RSA type + 'public_key' => 'ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAklOUpkDHrfHY17SbrmTIpNLTGK9Tjom/BWDSUGPl+nafzlHDTYW7hdI4yZ5ew18JH4JW9jbhUFrviQzM7xlELEVf4h9lFX5QVkbPppSwg0cda3Pbv7kOdJ/MTyBlWXFCR+HAo3FXRitBqxiX1nKhXpHAZsMciLq8V6RjsNAQwdsdMFvSlVK/7XAt3FaoJoAsncM1Q9x5+3V0Ww68/eIFmb1zuUFljQJKprrX88XypNDvjYNby6vw/Pb0rwert/EnmZ+AW4OZPnTPI89ZPmVMLuayrD2cE86Z/il8b+gw3r3+1nKatmIkjn2so1d01QraTlMqVSsbxNrRFi9wrf+M7Q== test@test.local', + ], + self::EXPECT_SUCCESS, + ], + [ + [ + 'name' => 'My first key', + // DSS + 'public_key' => 'ssh-dss AAAAB3NzaC1kc3MAAACBAPLQ1v111G0vuWwurCJdg0rt2C8OwWW88sR3sCS+CPjvqPyRtFOiJLqnO0/J/tIlCZVHN0VWgKN19jOiMy2Kx2mMZoAJ0z6AGxQMoTB0eqBeYYfAfxL9KwVw6EV7QWXTu5/EDbl+6k60EQ9EXOIsnhTQsok2Ki52Td0TUxfA3Vy7AAAAFQDx8Fi0pWPoJDn7jUqog7L748iBowAAAIADrgFxfpQuujYgxS2RXX8euxgrfa1KMQNT2kXQ3781L7ihS5EjyFy03K2pV0DSQo2NeFSJv9rtJNXfCDoAofUhgugZkMWUAv8ebZsy6SxWAocjw6A5ED1YkvA03eNUEaSm9vb8ts8m1wJme6Urx41Yh9c2YVXB6yJ7T1jaeZrhbQAAAIEA3hp8vZeImWol/tTm4LE1FXkqU7cMo863MAs5gdqRhJ5Xnwvsbx1WSVajJY78e8s/Yd4GhBAJpNjAVfep0CqbfJeNKV/D6oCKCfVikKefRw4GIREtAfkijlh/WOkwmWE0VcZRQk1sIizYtqIwtghvMvzDRSGFMC3l6hF5AHYyrSg= test@test.local', + ], + self::EXPECT_SUCCESS, + ], + [ + [ + 'name' => 'My first key', + // ECDSA type + 'public_key' => 'ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBBY0xeD9/iTVZZO/qVFRjAwtNmC0zHVWqY4Q7nmB4nGvfpHhlze+rEpxXwNWW5olIcAjAXJO+gQCa4iNV6UYDp8= test@test.local', + ], + self::EXPECT_SUCCESS, + ], + [ + [ + 'name' => 'My first key', + // ED25519 type + 'public_key' => 'ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIIxUhYjgZEXKYnlerxHYyL/e0HZ4C9t3WCpQ/LCPQZMp test@test.local', + ], + self::EXPECT_SUCCESS, + ], + [ + [ + 'name' => 'My first key', + // Partial ED25519 type + 'public_key' => 'ssh-afeef AAAAC3NzaC1lZDI1NTE5AAAAIIxUhYjgZEXKYnlerxHYyL/e0HZ4C9t3WCpQ/LCPQZMp test@test.local', + ], + self::EXPECT_FAILURE, + ], + [ + [ + 'name' => 'My first key', + // Partial ED25519 type + 'public_key' => 'ssh-ed25519 AAAAC3NzAffefIIIAAAAAAAIIxUhYjgZEXKYnlerxHYyL/e0HZ4C9t3WCpQ/LCPQZMp test@test.local', + ], + self::EXPECT_FAILURE, + ], + [ + [ + 'name' => 'My first key', + // Partial DSS type, close but not quite + 'public_key' => 'ssh-dss AAAAB3NzaC1kc3MAAACBAPLQ1v111G0vuWwurCJdga830asW88sR3sCS+CPjvqPyRtFOiJLqnO0/J/tIlCZVHN0VWgKN19jOiMy2Kx2mMZoAJ0z6AGxQMoTB0eqBeYYfAfxL9KwVw6EV7QWXTu5/EDbl+6k60EQ9EXOIsnhTQsok2Ki52Td0TUxfA3Vy7AAAAFQDx8Fi0pWPoJDn7jUqog7L748iBowAAAIADrgFxfpQuujYgxS2RXX8euxgrfa1KMQNT2kXQ3781L7ihS5EjyFy03K2pV0DSQo2NeFSJv9rtJNXfCDoAofUhgugZkMWUAv8ebZsy6SxWAocjw6A5ED1YkvA03eNUEaSm9vb8ts8m1wJme6Urx41Yh9c2YVXB6yJ7T1jaeZrhbQAAAIEA3hp8vZeImWol/tTm4LE1FXkqU7cMo863MAs5gdqRhJ5Xnwvsbx1WSVajJYa103s/Yd4GhBAJpNjAVfep0CqbfJeNKV/D6oCKCfVikKefRw4GIREtAfkijlh/WOkwmWE0VcZRQk1sIizYtqIwtghvMvzDRSGFMC3l6hF5AHYyrSg= test@test.local', + ], + self::EXPECT_FAILURE, + ], + ]; + } } diff --git a/tests/Feature/SshKeysTest.php b/tests/Feature/SshKeysTest.php index 81b1fd6..81956b7 100644 --- a/tests/Feature/SshKeysTest.php +++ b/tests/Feature/SshKeysTest.php @@ -48,4 +48,97 @@ public function test_delete_key(): void 'id' => $key->id, ]); } + + /** + * @dataProvider ssh_key_data_provider + */ + public function test_create_ssh_key_handles_invalid_or_partial_keys(array $postBody, bool $expectedToSucceed): void + { + $this->actingAs($this->user); + + // Some existing amount of seed data to make the test more realistic + SshKey::factory()->create([ + 'user_id' => $this->user->id, + 'name' => 'My first key', + 'public_key' => 'public-key-content', + ]); + + $response = $this->post(route('settings.ssh-keys.add'), $postBody); + + if ($expectedToSucceed) { + $response->assertSessionDoesntHaveErrors(); + } else { + $response->assertSessionHasErrors('public_key', 'Invalid key'); + } + } + + public static function ssh_key_data_provider(): array + { + return [ + [ + [ + 'name' => 'My first key', + // Key Already exists + 'public_key' => 'public-key-content', + ], + self::EXPECT_FAILURE, + ], + [ + [ + 'name' => 'My first key', + // RSA type + 'public_key' => 'ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAklOUpkDHrfHY17SbrmTIpNLTGK9Tjom/BWDSUGPl+nafzlHDTYW7hdI4yZ5ew18JH4JW9jbhUFrviQzM7xlELEVf4h9lFX5QVkbPppSwg0cda3Pbv7kOdJ/MTyBlWXFCR+HAo3FXRitBqxiX1nKhXpHAZsMciLq8V6RjsNAQwdsdMFvSlVK/7XAt3FaoJoAsncM1Q9x5+3V0Ww68/eIFmb1zuUFljQJKprrX88XypNDvjYNby6vw/Pb0rwert/EnmZ+AW4OZPnTPI89ZPmVMLuayrD2cE86Z/il8b+gw3r3+1nKatmIkjn2so1d01QraTlMqVSsbxNrRFi9wrf+M7Q== test@test.local', + ], + self::EXPECT_SUCCESS, + ], + [ + [ + 'name' => 'My first key', + // DSS + 'public_key' => 'ssh-dss AAAAB3NzaC1kc3MAAACBAPLQ1v111G0vuWwurCJdg0rt2C8OwWW88sR3sCS+CPjvqPyRtFOiJLqnO0/J/tIlCZVHN0VWgKN19jOiMy2Kx2mMZoAJ0z6AGxQMoTB0eqBeYYfAfxL9KwVw6EV7QWXTu5/EDbl+6k60EQ9EXOIsnhTQsok2Ki52Td0TUxfA3Vy7AAAAFQDx8Fi0pWPoJDn7jUqog7L748iBowAAAIADrgFxfpQuujYgxS2RXX8euxgrfa1KMQNT2kXQ3781L7ihS5EjyFy03K2pV0DSQo2NeFSJv9rtJNXfCDoAofUhgugZkMWUAv8ebZsy6SxWAocjw6A5ED1YkvA03eNUEaSm9vb8ts8m1wJme6Urx41Yh9c2YVXB6yJ7T1jaeZrhbQAAAIEA3hp8vZeImWol/tTm4LE1FXkqU7cMo863MAs5gdqRhJ5Xnwvsbx1WSVajJY78e8s/Yd4GhBAJpNjAVfep0CqbfJeNKV/D6oCKCfVikKefRw4GIREtAfkijlh/WOkwmWE0VcZRQk1sIizYtqIwtghvMvzDRSGFMC3l6hF5AHYyrSg= test@test.local', + ], + self::EXPECT_SUCCESS, + ], + [ + [ + 'name' => 'My first key', + // ECDSA type + 'public_key' => 'ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBBY0xeD9/iTVZZO/qVFRjAwtNmC0zHVWqY4Q7nmB4nGvfpHhlze+rEpxXwNWW5olIcAjAXJO+gQCa4iNV6UYDp8= test@test.local', + ], + self::EXPECT_SUCCESS, + ], + [ + [ + 'name' => 'My first key', + // ED25519 type + 'public_key' => 'ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIIxUhYjgZEXKYnlerxHYyL/e0HZ4C9t3WCpQ/LCPQZMp test@test.local', + ], + self::EXPECT_SUCCESS, + ], + [ + [ + 'name' => 'My first key', + // Partial ED25519 type + 'public_key' => 'ssh-afeef AAAAC3NzaC1lZDI1NTE5AAAAIIxUhYjgZEXKYnlerxHYyL/e0HZ4C9t3WCpQ/LCPQZMp test@test.local', + ], + self::EXPECT_FAILURE, + ], + [ + [ + 'name' => 'My first key', + // Partial ED25519 type + 'public_key' => 'ssh-ed25519 AAAAC3NzAffefIIIAAAAAAAIIxUhYjgZEXKYnlerxHYyL/e0HZ4C9t3WCpQ/LCPQZMp test@test.local', + ], + self::EXPECT_FAILURE, + ], + [ + [ + 'name' => 'My first key', + // Partial DSS type, close but not quite + 'public_key' => 'ssh-dss AAAAB3NzaC1kc3MAAACBAPLQ1v111G0vuWwurCJdga830asW88sR3sCS+CPjvqPyRtFOiJLqnO0/J/tIlCZVHN0VWgKN19jOiMy2Kx2mMZoAJ0z6AGxQMoTB0eqBeYYfAfxL9KwVw6EV7QWXTu5/EDbl+6k60EQ9EXOIsnhTQsok2Ki52Td0TUxfA3Vy7AAAAFQDx8Fi0pWPoJDn7jUqog7L748iBowAAAIADrgFxfpQuujYgxS2RXX8euxgrfa1KMQNT2kXQ3781L7ihS5EjyFy03K2pV0DSQo2NeFSJv9rtJNXfCDoAofUhgugZkMWUAv8ebZsy6SxWAocjw6A5ED1YkvA03eNUEaSm9vb8ts8m1wJme6Urx41Yh9c2YVXB6yJ7T1jaeZrhbQAAAIEA3hp8vZeImWol/tTm4LE1FXkqU7cMo863MAs5gdqRhJ5Xnwvsbx1WSVajJYa103s/Yd4GhBAJpNjAVfep0CqbfJeNKV/D6oCKCfVikKefRw4GIREtAfkijlh/WOkwmWE0VcZRQk1sIizYtqIwtghvMvzDRSGFMC3l6hF5AHYyrSg= test@test.local', + ], + self::EXPECT_FAILURE, + ], + ]; + } } diff --git a/tests/TestCase.php b/tests/TestCase.php index 1fb04ef..ed2d85e 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -24,6 +24,10 @@ abstract class TestCase extends BaseTestCase protected Site $site; + public const EXPECT_SUCCESS = true; + + public const EXPECT_FAILURE = false; + public function setUp(): void { parent::setUp();