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
Unknown Object (File)
Sun, Apr 28, 4:10 AM
Unknown Object (File)
Sun, Apr 28, 4:10 AM
Unknown Object (File)
Sun, Apr 28, 4:10 AM
Unknown Object (File)
Sat, Apr 27, 4:53 PM
Unknown Object (File)
Sat, Apr 20, 6:08 PM
Unknown Object (File)
Tue, Apr 16, 7:25 PM
Unknown Object (File)
Tue, Apr 16, 4:05 AM
Unknown Object (File)
Wed, Apr 10, 4:23 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?