Page MenuHomePhabricator

Support text-based private key credentials in DrydockSSHCommandInterface
ClosedPublic

Authored by hach-que on Dec 4 2013, 9:58 PM.
Tags
None
Referenced Files
F19073858: D7700.id.diff
Mon, Dec 1, 3:25 AM
F19068778: D7700.diff
Sun, Nov 30, 12:37 PM
F18851671: D7700.id.diff
Oct 31 2025, 2:56 AM
F18849888: D7700.diff
Oct 30 2025, 12:54 PM
F18846443: D7700.id17394.diff
Oct 29 2025, 6:55 PM
F18846441: D7700.id17392.diff
Oct 29 2025, 6:55 PM
F18841510: D7700.diff
Oct 28 2025, 9:39 AM
F18788222: D7700.id17393.diff
Oct 15 2025, 3:36 AM

Details

Summary

This updates DrydockSSHCommandInterface to correctly hold open the private key credentials for the life of the interface so that remote commands will execute correctly with a text-based private key.

Test Plan

Created a text-based private key, created a resource based on it and leased against it.

Diff Detail

Branch
migrate-execution
Lint
Lint Passed
Unit
No Test Coverage

Event Timeline

Hmm, I'm surprised this works. The PassphraseSSHKey should pass out of scope immediately, and the keyfileEnvelope doesn't actually retain a reference to the TempFile.

I would expect to store the PhassphraseSSHKey object instead, and then call:

$this->passphraseSSHKey->getKeyfileEnvelope(),
$this->passphraseSSHKey->getUsernameEnvelope(),

You can use -i %P (instead of -i %s) and -l %P to avoid opening the envelopes. This will obscure the tempfile path and username in stack traces, as well (this isn't really too important, but is a little cleaner). Login is probably reasonable to consider non-senstive here, so opening that envelope might be more useful for debugging.

Does that make sense? Possibly I'm just crazy or this code is broken though, I would not expect this to work as written.

src/applications/drydock/interface/command/DrydockSSHCommandInterface.php
8

Can this be private?

hach-que updated this revision to Unknown Object (????).Dec 4 2013, 10:12 PM

Updated based on @epriestley's suggestions and it still works after changing it. I did make username go via %P, but that's just because PassphraseSSHKey doesn't have getUsername and I didn't feel like opening the envelope.

I get results consistent with this not actually working on a simplified local test case:

>>> orbital ~/devtools/phabricator $ cat test.php 
<?php

require_once 'scripts/__init_script__.php';

$key = PassphraseSSHKey::loadFromPHID('PHID-CDTL-4zdkvwq2pkw4kn5gkqqw', PhabricatorUser::getOmnipotentUser());
$keyfile_envelope = $key->getKeyfileEnvelope();
unset($key);

echo Filesystem::readFile($keyfile_envelope->openEnvelope());
>>> orbital ~/devtools/phabricator $ php -f test.php 
[2013-12-04 14:11:41] EXCEPTION: (FilesystemException) Filesystem entity `/private/var/folders/8k/c3vkmjy5335gcxdzxkhwq82w0000gn/T/30zi4f9nd4isg404/passphrase-ssh-key' does not exist. at [/INSECURE/devtools/libphutil/src/filesystem/Filesystem.php:965]
  #0 Filesystem::assertExists(/private/var/folders/8k/c3vkmjy5335gcxdzxkhwq82w0000gn/T/30zi4f9nd4isg404/passphrase-ssh-key) called at [/INSECURE/devtools/libphutil/src/filesystem/Filesystem.php:38]
  #1 Filesystem::readFile(/var/folders/8k/c3vkmjy5335gcxdzxkhwq82w0000gn/T/30zi4f9nd4isg404/passphrase-ssh-key) called at [/INSECURE/devtools/phabricator/test.php:9]

Removing he line unset($key) works:

>>> orbital ~/devtools/phabricator $ php -f test.php 
-----BEGIN RSA PRIVATE KEY-----
...

...so maybe I'm not crazy?