Skip to content

Commit b172250

Browse files
MsprgMsprg
authored
Feature/revoke access on decommisioning (#1)
* Marking server as decommisioned now revokes existing access to users * Refactor sync.php to remove code duplicity --------- Co-authored-by: Msprg <[email protected]>
1 parent 8ae494e commit b172250

File tree

2 files changed

+238
-81
lines changed

2 files changed

+238
-81
lines changed

‎scripts/sync.php‎

Lines changed: 235 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@
100100

101101
$pending_syncs = array();
102102
foreach($servers as $server) {
103-
if($server->key_management != 'keys') {
103+
if($server->key_management != 'keys' && $server->key_management != 'decommissioned') {
104104
continue;
105105
}
106106
$pending_syncs[$server->hostname] = $server;
@@ -153,6 +153,146 @@ function show_help() {
153153
<?php
154154
}
155155

156+
/**
157+
* Establish an SSH connection while handling hanging connection attempts uniformly.
158+
*/
159+
function connect_ssh_with_timeout_handling($server, $hostname, callable $on_timeout, callable $on_connect_failure) {
160+
// ssh2 sometimes hangs on connect; use SIGTERM from wrapper timeout to bail out cleanly
161+
declare(ticks = 1);
162+
pcntl_signal(SIGTERM, function($signal) use($server, $hostname, $on_timeout) {
163+
echo date('c')." {$hostname}: SSH connection timed out.\n";
164+
$on_timeout();
165+
exit(0);
166+
});
167+
168+
echo date('c')." {$hostname}: Attempting to connect.\n";
169+
try {
170+
$connection = $server->connect_ssh();
171+
} catch (SSHException $e) {
172+
$reason = describe_oneline($e);
173+
echo date('c')." {$hostname}: $reason\n";
174+
$on_connect_failure($reason);
175+
return null;
176+
}
177+
178+
// From this point on, catch SIGTERM and ignore. SIGINT or SIGKILL is required to stop, so timeout wrapper won't
179+
// cause a partial sync/decommission
180+
pcntl_signal(SIGTERM, SIG_IGN);
181+
182+
return $connection;
183+
}
184+
185+
function decommission_server($id, $preview = false) {
186+
global $server_dir;
187+
188+
$keydir = '/var/local/keys-sync';
189+
$server = $server_dir->get_server_by_id($id);
190+
$hostname = $server->hostname;
191+
echo date('c')." {$hostname}: Server is decommissioned, removing all keys (preserving keys-sync access).\n";
192+
193+
if($preview) {
194+
echo date('c')." {$hostname}: [PREVIEW] Would remove all key files from {$keydir}/ except 'keys-sync' and '.hostnames'\n";
195+
return;
196+
}
197+
198+
$connection = connect_ssh_with_timeout_handling(
199+
$server,
200+
$hostname,
201+
function() use ($server) {
202+
$server->sync_report('sync failure', 'SSH connection timed out during decommission');
203+
$server->reschedule_sync_request();
204+
},
205+
function($reason) use ($server) {
206+
$server->sync_report('sync failure', 'Failed to connect during decommission: '.$reason);
207+
$server->reschedule_sync_request();
208+
}
209+
);
210+
if (is_null($connection)) {
211+
return;
212+
}
213+
214+
$cleanup_errors = 0;
215+
$removed_count = 0;
216+
217+
// First verify the directory exists and is accessible (don't suppress errors)
218+
try {
219+
$connection->exec('test -d '.escapeshellarg($keydir).' && test -r '.escapeshellarg($keydir));
220+
} catch (SSHException $e) {
221+
$cleanup_errors++;
222+
echo date('c')." {$hostname}: Cannot access key directory: ".describe_oneline($e)."\n";
223+
$server->sync_report('sync failure', 'Cannot access key directory during decommission: '.describe_oneline($e));
224+
$server->reschedule_sync_request();
225+
return;
226+
}
227+
228+
// Get list of all files in the key directory (don't suppress errors)
229+
try {
230+
// Try sha1sum first without suppressing stderr
231+
$output = $connection->exec('/usr/bin/sha1sum '.escapeshellarg($keydir).'/* 2>&1');
232+
$entries = explode("\n", $output);
233+
$files_to_check = array();
234+
foreach($entries as $entry) {
235+
// Check for error messages
236+
if(strpos($entry, 'No such file') !== false || strpos($entry, 'Permission denied') !== false) {
237+
// sha1sum failed due to access issues, try ls instead
238+
break;
239+
}
240+
if(preg_match('|^([0-9a-f]{40}) '.preg_quote($keydir, '|').'/(.*)$|', $entry, $matches)) {
241+
$files_to_check[] = $matches[2];
242+
}
243+
}
244+
245+
// If sha1sum didn't work or found no files, try ls directly (don't suppress errors)
246+
if(empty($files_to_check)) {
247+
$output = $connection->exec('ls -1 '.escapeshellarg($keydir).' 2>&1');
248+
// Check if ls output contains error messages
249+
if(strpos($output, 'No such file') !== false || strpos($output, 'Permission denied') !== false) {
250+
throw new SSHException("Failed to list files in key directory: {$output}");
251+
}
252+
$files_to_check = array_filter(explode("\n", trim($output)), function($f) { return trim($f) !== ''; });
253+
}
254+
255+
// Remove all files except keys-sync and .hostnames
256+
foreach($files_to_check as $file) {
257+
$file = trim($file);
258+
if($file == '' || $file == 'keys-sync' || $file == '.hostnames') {
259+
continue;
260+
}
261+
try {
262+
$connection->unlink("$keydir/$file");
263+
echo date('c')." {$hostname}: Removed key file: {$file}\n";
264+
$removed_count++;
265+
} catch (SSHException $e) {
266+
$cleanup_errors++;
267+
echo date('c')." {$hostname}: Couldn't remove key file {$file}: ".describe_oneline($e)."\n";
268+
}
269+
}
270+
271+
if($removed_count == 0) {
272+
echo date('c')." {$hostname}: No key files found to remove (directory is empty or contains only protected files)\n";
273+
}
274+
} catch (SSHException $e) {
275+
$cleanup_errors++;
276+
echo date('c')." {$hostname}: Error listing key files: ".describe_oneline($e)."\n";
277+
}
278+
279+
// Update status
280+
if($cleanup_errors > 0) {
281+
$server->sync_report('sync failure', 'Failed to remove '.$cleanup_errors.' key file'.($cleanup_errors == 1 ? '' : 's').' during decommission');
282+
$server->reschedule_sync_request();
283+
} else {
284+
$server->sync_report('sync success', 'Decommissioned: removed '.$removed_count.' key file'.($removed_count == 1 ? '' : 's').' (keys-sync access preserved)');
285+
}
286+
287+
try {
288+
$server->update_status_file($connection);
289+
} catch (SSHException $e) {
290+
// Ignore status file update errors during decommission
291+
}
292+
293+
echo date('c')." {$hostname}: Decommission completed\n";
294+
}
295+
156296
function sync_server($id, $only_username = null, $preview = false) {
157297
global $config;
158298
global $server_dir;
@@ -170,6 +310,13 @@ function sync_server($id, $only_username = null, $preview = false) {
170310
$server = $server_dir->get_server_by_id($id);
171311
$hostname = $server->hostname;
172312
echo date('c')." {$hostname}: Preparing sync.\n";
313+
314+
// Handle decommissioned servers: remove all keys except keys-sync
315+
if($server->key_management == 'decommissioned') {
316+
decommission_server($id, $preview);
317+
return;
318+
}
319+
173320
if($server->key_management != 'keys') return;
174321
$accounts = $server->list_accounts();
175322
$keyfiles = array();
@@ -237,36 +384,24 @@ function sync_server($id, $only_username = null, $preview = false) {
237384
return;
238385
}
239386

240-
// This is working around deficiencies in the ssh2 library. In some cases, ssh connection attempts will fail, and
241-
// the socket timeout of 60 seconds is somehow not triggered. Script execution timeout is also not triggered.
242-
// Reproducing this problem is not easy - dropping packets to port 22 is not sufficient (it will timeout correctly).
243-
// To workaround, we wrap calls to this script with 'timeout' shell command, and from this point on until we have
244-
// established a connection, catch SIGTERM and report server sync failure if received
245-
declare(ticks = 1);
246-
pcntl_signal(SIGTERM, function($signal) use($server, $hostname, $keyfiles) {
247-
echo date('c')." {$hostname}: SSH connection timed out.\n";
248-
$server->sync_report('sync failure', 'SSH connection timed out');
249-
$server->reschedule_sync_request();
250-
report_all_accounts_failed($keyfiles);
251-
exit(0);
252-
});
253-
254-
echo date('c')." {$hostname}: Attempting to connect.\n";
255-
try {
256-
$connection = $server->connect_ssh();
257-
} catch (SSHException $e) {
258-
$reason = describe_oneline($e);
259-
echo date('c')." {$hostname}: $reason\n";
260-
$server->sync_report('sync failure', $reason);
261-
$server->reschedule_sync_request();
262-
report_all_accounts_failed($keyfiles);
387+
$connection = connect_ssh_with_timeout_handling(
388+
$server,
389+
$hostname,
390+
function() use ($server, $keyfiles) {
391+
$server->sync_report('sync failure', 'SSH connection timed out');
392+
$server->reschedule_sync_request();
393+
report_all_accounts_failed($keyfiles);
394+
},
395+
function($reason) use ($server, $keyfiles) {
396+
$server->sync_report('sync failure', $reason);
397+
$server->reschedule_sync_request();
398+
report_all_accounts_failed($keyfiles);
399+
}
400+
);
401+
if (is_null($connection)) {
263402
return;
264403
}
265404

266-
// From this point on, catch SIGTERM and ignore. SIGINT or SIGKILL is required to stop, so timeout wrapper won't
267-
// cause a partial sync
268-
pcntl_signal(SIGTERM, SIG_IGN);
269-
270405
$account_errors = 0;
271406
$cleanup_errors = 0;
272407

@@ -359,6 +494,44 @@ function sync_server($id, $only_username = null, $preview = false) {
359494
echo date('c')." {$hostname}: Sync finished\n";
360495
}
361496

497+
function append_user_keys($keyfile, $entity, $prefix, $account_name, $hostname, $comment, $grant_details = null) {
498+
if ($comment == 1) {
499+
$keyfile .= "# {$entity->uid}";
500+
if (!is_null($grant_details)) {
501+
$keyfile .= " {$grant_details}";
502+
}
503+
$keyfile .= "\n";
504+
}
505+
if($entity->active) {
506+
$keys = $entity->list_public_keys($account_name, $hostname, false);
507+
foreach($keys as $key) {
508+
$keyfile .= $prefix.$key->export_userkey_with_fixed_comment($entity, $comment)."\n";
509+
}
510+
} elseif ($comment == 1) {
511+
$keyfile .= "# Account disabled\n";
512+
}
513+
return $keyfile;
514+
}
515+
516+
function append_serveraccount_keys($keyfile, $entity, $prefix, $account_name, $hostname, $comment, $grant_details = null) {
517+
if ($comment == 1) {
518+
$keyfile .= "# {$entity->name}@{$entity->server->hostname}";
519+
if (!is_null($grant_details)) {
520+
$keyfile .= " {$grant_details}";
521+
}
522+
$keyfile .= "\n";
523+
}
524+
if($entity->server->key_management != 'decommissioned') {
525+
$keys = $entity->list_public_keys($account_name, $hostname, false);
526+
foreach($keys as $key) {
527+
$keyfile .= $prefix.$key->export_serverkey_with_fixed_comment($entity, $comment)."\n";
528+
}
529+
} elseif ($comment == 1) {
530+
$keyfile .= "# Decommissioned server\n";
531+
}
532+
return $keyfile;
533+
}
534+
362535
function get_keys($access_rules, $account_name, $hostname, $comment) {
363536
$keyfile = '';
364537
foreach($access_rules as $access) {
@@ -373,34 +546,26 @@ function get_keys($access_rules, $account_name, $hostname, $comment) {
373546
if($prefix !== '') $prefix .= ' ';
374547
switch(get_class($entity)) {
375548
case 'User':
376-
if ($comment == 1) {
377-
$keyfile .= "# {$entity->uid}";
378-
$keyfile .= " granted access by {$access->granted_by->uid} on {$grant_date_full}";
379-
$keyfile .= "\n";
380-
}
381-
if($entity->active) {
382-
$keys = $entity->list_public_keys($account_name, $hostname, false);
383-
foreach($keys as $key) {
384-
$keyfile .= $prefix.$key->export_userkey_with_fixed_comment($entity, $comment)."\n";
385-
}
386-
} elseif ($comment == 1) {
387-
$keyfile .= "# Account disabled\n";
388-
}
549+
$keyfile = append_user_keys(
550+
$keyfile,
551+
$entity,
552+
$prefix,
553+
$account_name,
554+
$hostname,
555+
$comment,
556+
"granted access by {$access->granted_by->uid} on {$grant_date_full}"
557+
);
389558
break;
390559
case 'ServerAccount':
391-
if ($comment == 1) {
392-
$keyfile .= "# {$entity->name}@{$entity->server->hostname}";
393-
$keyfile .= " granted access by {$access->granted_by->uid} on {$grant_date_full}";
394-
$keyfile .= "\n";
395-
}
396-
if($entity->server->key_management != 'decommissioned') {
397-
$keys = $entity->list_public_keys($account_name, $hostname, false);
398-
foreach($keys as $key) {
399-
$keyfile .= $prefix.$key->export_serverkey_with_fixed_comment($entity, $comment)."\n";
400-
}
401-
} elseif ($comment == 1) {
402-
$keyfile .= "# Decommissioned server\n";
403-
}
560+
$keyfile = append_serveraccount_keys(
561+
$keyfile,
562+
$entity,
563+
$prefix,
564+
$account_name,
565+
$hostname,
566+
$comment,
567+
"granted access by {$access->granted_by->uid} on {$grant_date_full}"
568+
);
404569
break;
405570
case 'Group':
406571
// Recurse!
@@ -432,32 +597,24 @@ function get_group_keys($entities, $account_name, $hostname, $prefix, &$seen, $c
432597
foreach($entities as $entity) {
433598
switch(get_class($entity)) {
434599
case 'User':
435-
if ($comment == 1) {
436-
$keyfile .= "# {$entity->uid}";
437-
$keyfile .= "\n";
438-
}
439-
if($entity->active) {
440-
$keys = $entity->list_public_keys($account_name, $hostname, false);
441-
foreach($keys as $key) {
442-
$keyfile .= $prefix.$key->export_userkey_with_fixed_comment($entity, $comment)."\n";
443-
}
444-
} elseif ($comment == 1) {
445-
$keyfile .= "# Account disabled\n";
446-
}
600+
$keyfile = append_user_keys(
601+
$keyfile,
602+
$entity,
603+
$prefix,
604+
$account_name,
605+
$hostname,
606+
$comment
607+
);
447608
break;
448609
case 'ServerAccount':
449-
if ($comment == 1) {
450-
$keyfile .= "# {$entity->name}@{$entity->server->hostname}";
451-
$keyfile .= "\n";
452-
}
453-
if($entity->server->key_management != 'decommissioned') {
454-
$keys = $entity->list_public_keys($account_name, $hostname, false);
455-
foreach($keys as $key) {
456-
$keyfile .= $prefix.$key->export_serverkey_with_fixed_comment($entity, $comment)."\n";
457-
}
458-
} elseif ($comment == 1) {
459-
$keyfile .= "# Decommissioned server\n";
460-
}
610+
$keyfile = append_serveraccount_keys(
611+
$keyfile,
612+
$entity,
613+
$prefix,
614+
$account_name,
615+
$hostname,
616+
$comment
617+
);
461618
break;
462619
case 'Group':
463620
// Recurse!

‎templates/server.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -316,9 +316,9 @@
316316
</label>
317317
</div>
318318
<div class="radio">
319-
<label>
319+
<label class="text-warning">
320320
<input type="radio" name="key_management" value="decommissioned"<?php if($this->get('server')->key_management == 'decommissioned') out(' checked') ?>>
321-
Disabled - server has been decommissioned
321+
Disabled - server has been decommissioned (remove all user access keys)
322322
</label>
323323
</div>
324324
</div>
@@ -410,7 +410,7 @@
410410
case 'keys': out('SSH keys managed and synced by SSH Key Authority'); break;
411411
case 'none': out('Disabled - server has no key management'); break;
412412
case 'other': out('Disabled - SSH keys managed by another system'); break;
413-
case 'decommissioned': out('Disabled - server has been decommissioned'); break;
413+
case 'decommissioned': out('Disabled - server has been decommissioned (remove all user access keys)'); break;
414414
}
415415
?>
416416
</dd>

0 commit comments

Comments
 (0)