Skip to content

Commit 1535d4a

Browse files
simonLeary42simonLeary42
authored
no key sharing (#719)
1 parent 593049a commit 1535d4a

6 files changed

Lines changed: 125 additions & 19 deletions

File tree

‎resources/lib/UnityLDAP.php‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,4 +356,29 @@ public function getUsersAttributes(
356356
ksort($output);
357357
return $output;
358358
}
359+
360+
/** @return string[] */
361+
public function whoIsUsingKey(string $target_key): array
362+
{
363+
$who = [];
364+
[$target_type, $target_data, $target_comment] = tokenizeSSHKey($target_key);
365+
$attributes = $this->getAllNativeUsersAttributes(
366+
["uid", "sshPublicKey"],
367+
["sshPublicKey" => []],
368+
);
369+
foreach ($attributes as $attrs) {
370+
foreach ($attrs["sshpublickey"] as $key) {
371+
try {
372+
[$type, $data, $comment] = tokenizeSSHKey((string) $key);
373+
} catch (\Throwable) {
374+
// if someone's key is invalid, assume it doesn't match
375+
continue;
376+
}
377+
if ($type === $target_type && $data === $target_data) {
378+
array_push($who, (string) $attrs["uid"][0]);
379+
}
380+
}
381+
}
382+
return array_unique($who);
383+
}
359384
}

‎resources/lib/UnityUser.php‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -248,9 +248,6 @@ public function getMail(): string
248248
*/
249249
public function addSSHKey(string $key, bool $send_mail = true): bool
250250
{
251-
if ($this->SSHKeyExists($key)) {
252-
return false;
253-
}
254251
$this->setSSHKeys(array_merge($this->getSSHKeys(), [$key]), $send_mail);
255252
$key_info = formatSSHKeyInfoInternal($key);
256253
$this->SQL->addLog("sshkey_added", _json_encode(["uid" => $this->uid, "key" => $key_info]));

‎test/functional/SSHKeyAddTest.php‎

Lines changed: 74 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,26 +3,27 @@
33
use UnityWebPortal\lib\UnityGithub;
44
use PHPUnit\Framework\Attributes\DataProvider;
55
use PHPUnit\Framework\Attributes\AllowMockObjectsWithoutExpectations;
6+
use UnityWebPortal\lib\UnityHTTPDMessageLevel;
67

78
#[AllowMockObjectsWithoutExpectations]
89
class SSHKeyAddTest extends UnityWebPortalTestCase
910
{
1011
public static function keyProvider()
1112
{
1213
$validKey =
13-
"ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIB+XqO25MUB9x/pS04I3JQ7rMGboWyGXh0GUzkOrTi7a foobar";
14+
"ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIPUef6kU0/P0lTO5KBZq6aFVm7nBHhB85SaG4HB0nh7p foobar";
1415
$invalidKey = "foobar";
1516
return [[false, $invalidKey], [true, $validKey]];
1617
}
1718

1819
public static function keysProvider()
1920
{
2021
$validKey =
21-
"ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIB+XqO25MUB9x/pS04I3JQ7rMGboWyGXh0GUzkOrTi7a foobar";
22+
"ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIPUef6kU0/P0lTO5KBZq6aFVm7nBHhB85SaG4HB0nh7p foobar";
2223
$validKeyDuplicateDifferentComment =
23-
"ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIB+XqO25MUB9x/pS04I3JQ7rMGboWyGXh0GUzkOrTi7a foobar2";
24+
"ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIPUef6kU0/P0lTO5KBZq6aFVm7nBHhB85SaG4HB0nh7p foobar2";
2425
$validKey2 =
25-
"ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBF/dSI9/7YWeyB8wa4rEWRdeb9pQbrGxZwYFV2ulr0agXdbiJIApp0MWDYlIc9XI+4Y+cVAj66PQ2YaRz44BV+o=";
26+
"ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBF5Ossk5huH48Gdyw1nuC+1TKajZzF+83rwbFhml0b915mWzYbKqFtjFze8+4uW+xBjLmwx4e+vGiZbNR4ucm6w=";
2627
$invalidKey = "foobar";
2728
return [
2829
[0, []],
@@ -161,4 +162,73 @@ public function testAddSshKeysGithub(int $expectedKeysAdded, array $keys)
161162
callPrivateMethod($USER, "setSSHKeys", []);
162163
}
163164
}
165+
166+
public function testShareKeysBetweenUsers()
167+
{
168+
global $USER;
169+
$key =
170+
"ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIPUef6kU0/P0lTO5KBZq6aFVm7nBHhB85SaG4HB0nh7p foobar";
171+
$this->switchUser("Admin");
172+
$user1 = $USER;
173+
$this->switchUser("Blank");
174+
$user2 = $USER;
175+
$user1_keys_before = $user1->getSSHKeys();
176+
$user2_keys_before = $user2->getSSHKeys();
177+
try {
178+
$user1->addSSHKey($key);
179+
// as user2, try to add the key that user1 already added
180+
$this->http_post(
181+
__DIR__ . "/../../webroot/panel/account.php",
182+
[
183+
"form_type" => "addKey",
184+
"add_type" => "paste",
185+
"key" => $key,
186+
],
187+
do_validate_messages: false,
188+
);
189+
$this->assertMessageExists(
190+
UnityHTTPDMessageLevel::WARNING,
191+
"/.*/",
192+
"/This incident has been reported/",
193+
);
194+
$this->assertEquals($user2_keys_before, $user2->getSSHKeys());
195+
} finally {
196+
callPrivateMethod($user1, "setSSHKeys", $user1_keys_before);
197+
callPrivateMethod($user2, "setSSHKeys", $user2_keys_before);
198+
}
199+
}
200+
201+
/*
202+
while attempting to share keys between users says "this incident has been reported"
203+
you should not see this message if you add the same key to your account twice
204+
*/
205+
public function testAddDuplicateKey()
206+
{
207+
global $USER;
208+
$key =
209+
"ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIPUef6kU0/P0lTO5KBZq6aFVm7nBHhB85SaG4HB0nh7p foobar";
210+
$this->switchUser("Blank");
211+
$this->assertEmpty($USER->getSSHKeys());
212+
try {
213+
$USER->addSSHKey($key);
214+
$this->assertEquals([$key], $USER->getSSHKeys());
215+
$this->http_post(
216+
__DIR__ . "/../../webroot/panel/account.php",
217+
[
218+
"form_type" => "addKey",
219+
"add_type" => "paste",
220+
"key" => $key,
221+
],
222+
do_validate_messages: false,
223+
);
224+
$this->assertMessageExists(
225+
UnityHTTPDMessageLevel::WARNING,
226+
"/Key Already Added/",
227+
"/.*/",
228+
);
229+
$this->assertEquals([$key], $USER->getSSHKeys());
230+
} finally {
231+
callPrivateMethod($USER, "setSSHKeys", []);
232+
}
233+
}
164234
}

‎test/unit/AjaxSshValidateTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ public static function providerTestSshValidate()
1111
[false, "foobar"],
1212
[
1313
true,
14-
"ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIB+XqO25MUB9x/pS04I3JQ7rMGboWyGXh0GUzkOrTi7a",
14+
"ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIPUef6kU0/P0lTO5KBZq6aFVm7nBHhB85SaG4HB0nh7p",
1515
],
1616
];
1717
}

‎test/unit/UtilsTest.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,10 @@ public static function SSHKeyProvider()
99
global $HTTP_HEADER_TEST_INPUTS;
1010
// these key types must all be in CONFIG["ldap"]["allowed_ssh_key_types"]
1111
$validKeys = [
12-
"ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIB+XqO25MUB9x/pS04I3JQ7rMGboWyGXh0GUzkOrTi7a",
13-
"ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIB+XqO25MUB9x/pS04I3JQ7rMGboWyGXh0GUzkOrTi7a foobar",
14-
"ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIB+XqO25MUB9x/pS04I3JQ7rMGboWyGXh0GUzkOrTi7a foo bar baz",
15-
"ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBF/dSI9/7YWeyB8wa4rEWRdeb9pQbrGxZwYFV2ulr0agXdbiJIApp0MWDYlIc9XI+4Y+cVAj66PQ2YaRz44BV+o=",
12+
"ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIPUef6kU0/P0lTO5KBZq6aFVm7nBHhB85SaG4HB0nh7p",
13+
"ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIPUef6kU0/P0lTO5KBZq6aFVm7nBHhB85SaG4HB0nh7p foobar",
14+
"ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIPUef6kU0/P0lTO5KBZq6aFVm7nBHhB85SaG4HB0nh7p foo bar baz",
15+
"ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBF5Ossk5huH48Gdyw1nuC+1TKajZzF+83rwbFhml0b915mWzYbKqFtjFze8+4uW+xBjLmwx4e+vGiZbNR4ucm6w=",
1616
"ecdsa-sha2-nistp384 AAAAE2VjZHNhLXNoYTItbmlzdHAzODQAAAAIbmlzdHAzODQAAABhBOr8ZnJPs/mP/1c74P8NsiPL2pq/vKo6u0vtkgqgyZjqJJpPS5rP6EFJkT8DI0Fx9/70jvyH8wGK6tx+/gNElMlZ6P2RyHbDvL4Nh2LAEW3BQ2lbULyElP/ZeXIEQzPxng==",
1717
"ecdsa-sha2-nistp521 AAAAE2VjZHNhLXNoYTItbmlzdHA1MjEAAAAIbmlzdHA1MjEAAACFBAFmNNrz+B6exxuReTXQJzXUzJ4zB5JTuB8Xtcr79P4tk4SlA5a5ufQlsqMdPRhA76KFaLmONGF1e+vwcQWsj/MbRQE0H56tkZRNa+ch5/YI6iKSffkzpRKogl/uTP4rlpRb1vppsURRYxQ2JBzLYolj8VUV+N0sCwM+8maiOGJYuc4dlQ==",
1818
"ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQC6RjRhmaBfCN9l9qsnjplvatK/a7zb2tdbg7kDj8mWXfbC1zkELLLX/L+5hAySbm8QXPgr18CqcyV9LqK+vJ/aPHRNo3E/mp14pxp0nHpPlMzUV8ybl2uk2kBMXWRweOYfAcA5eJToHVAXJEVBvcwDI1WVG9Nfo5w1UhGSqcn4oQ==",

‎webroot/panel/account.php‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -54,14 +54,28 @@
5454
UnityHTTPD::messageError("SSH Key Not Added: $explanation", $key_short);
5555
continue;
5656
}
57-
$keyWasAdded = $USER->addSSHKey($key);
58-
if ($keyWasAdded) {
59-
$sha256_fingerprint = getSSHKeyInfo($key)[1];
60-
$stub_fingprint = substr($sha256_fingerprint, 0, 6);
61-
UnityHTTPD::messageSuccess("SSH Key Added", $stub_fingprint);
62-
} else {
63-
UnityHTTPD::messageInfo("SSH Key Not Added: Already Exists", $key_short);
57+
$already_using_this_key = $LDAP->whoIsUsingKey($key);
58+
if (count($already_using_this_key) > 0) {
59+
if ($already_using_this_key === [$USER->uid]) {
60+
UnityHTTPD::messageWarning("SSH Key Not Added: Key Already Added", $key_short);
61+
continue;
62+
} else {
63+
UnityHTTPD::errorLog(
64+
"security warning",
65+
"attempted SSH public key sharing between users",
66+
data: ["already using this key" => $already_using_this_key]
67+
);
68+
UnityHTTPD::messageWarning(
69+
"SSH Key Not Added: Another User Is Already Using This Key",
70+
"Sharing SSH keys with other users is against terms of service. This incident has been reported.",
71+
);
72+
continue;
73+
}
6474
}
75+
$USER->addSSHKey($key);
76+
$sha256_fingerprint = getSSHKeyInfo($key)[1];
77+
$stub_fingprint = substr($sha256_fingerprint, 0, 6);
78+
UnityHTTPD::messageSuccess("SSH Key Added", $stub_fingprint);
6579
}
6680
UnityHTTPD::redirect();
6781
break; /** @phpstan-ignore deadCode.unreachable */

0 commit comments

Comments
 (0)